diff --git a/AGENTS.md b/AGENTS.md index 343e33c..f353cde 100644 --- a/AGENTS.md +++ b/AGENTS.md @@ -30,7 +30,7 @@ Separate Python companion script in `scripts/ios-app-import/` (stdlib-only, Pyth ## Architecture -This is a Next.js 16 App Router app (TypeScript, React 19) backed by a single local SQLite file. All scraping, parsing, diffing, and AI calls happen server-side inside API routes that import helpers from `lib/`. +This is a Next.js 16 App Router app (TypeScript, React 19) backed by a single local SQLite file. All scraping, parsing, diffing, and AI calls happen server-side inside API routes that import helpers from `lib/`. End-to-end workflow diagrams (system map, import/delete/sync/wayback flows, gate chain) with known weak points marked live in [docs/ARCHITECTURE.md](docs/ARCHITECTURE.md). ### Data flow (the core loop) diff --git a/README.md b/README.md index 31890b7..06992b1 100644 --- a/README.md +++ b/README.md @@ -64,6 +64,7 @@ Full documentation lives at - [User guide](https://privacytracker-docs.privacykey.org/quickstart) — how to import apps, read privacy labels, set up alerts - [AI provider setup](https://privacytracker-docs.privacykey.org/quickstart) — bring your own OpenAI / Anthropic / local model - [Architecture](https://privacytracker-docs.privacykey.org/develop/architecture) — for developers and contributors +- [Architecture & workflows (in-repo)](docs/ARCHITECTURE.md) — end-to-end diagrams of every process, with weak points marked and an improvement backlog - [Security](https://privacytracker-docs.privacykey.org/security) — how to report a vulnerability ## License diff --git a/app/api/device-actions/backup/route.ts b/app/api/device-actions/backup/route.ts index 0c06c30..6620d4e 100644 --- a/app/api/device-actions/backup/route.ts +++ b/app/api/device-actions/backup/route.ts @@ -15,7 +15,7 @@ */ import { type NextRequest, NextResponse } from "next/server"; -import { recordBackup } from "@/lib/device-actions"; +import { normalizeEcid, recordBackup } from "@/lib/device-actions"; import { getActiveFocus } from "@/lib/feature-flag-storage"; import { readBoundedJson } from "@/lib/security"; @@ -44,8 +44,15 @@ export async function POST(request: NextRequest) { return NextResponse.json({ error: "Invalid JSON" }, { status: 400 }); } - if (!body.ecid || typeof body.ecid !== "string") { - return NextResponse.json({ error: "ecid is required" }, { status: 400 }); + if ( + !body.ecid || + typeof body.ecid !== "string" || + !normalizeEcid(body.ecid) + ) { + return NextResponse.json( + { error: "a valid ecid is required" }, + { status: 400 } + ); } if (!body.path || typeof body.path !== "string") { return NextResponse.json({ error: "path is required" }, { status: 400 }); diff --git a/app/api/device-actions/uninstall/route.ts b/app/api/device-actions/uninstall/route.ts index 252c378..d8124d9 100644 --- a/app/api/device-actions/uninstall/route.ts +++ b/app/api/device-actions/uninstall/route.ts @@ -6,16 +6,21 @@ * `run_cfgutil_remove_app`. This endpoint: * * 1. Re-runs the gate check server-side (audience + flag + backup - * freshness) so a malicious page can't bypass the webview's - * gating by hand-crafting an invoke. Returns 403 with a - * structured `{ reason }` body when refused so the wizard can - * render the right copy. + * freshness) before writing any audit row, so a hand-crafted API + * call can't stamp legitimate-looking rows for a gated-off + * configuration. Returns 403 with a structured `{ reason }` body + * when refused so the wizard can render the right copy. Note the + * limits of this check: the destructive call itself is a Tauri + * command that never passes through this server — the control + * that actually stops a compromised webview is the native + * Touch ID prompt inside `run_cfgutil_remove_app`. * 2. Writes a `cfgutil_uninstall` activity row regardless of * success — failures are as important to log as successes. * * The endpoint is GET-able too: `GET /api/device-actions/uninstall?ecid=…` * returns the gate result without committing anything. The wizard - * uses this to decide whether to render the uninstall buttons at all. + * calls this as a fail-closed pre-flight at the top of its bulk + * uninstall loop, before the first removal fires. */ import { type NextRequest, NextResponse } from "next/server"; diff --git a/app/components/ReviewRecommendationsView.tsx b/app/components/ReviewRecommendationsView.tsx index 98b5574..c059a7a 100644 --- a/app/components/ReviewRecommendationsView.tsx +++ b/app/components/ReviewRecommendationsView.tsx @@ -121,6 +121,19 @@ interface Props { */ type Step = "review" | "compare" | "action" | "backup" | "act"; +/** + * Maps `DeviceActionGate` denial reasons (see lib/device-actions.ts) to + * `review_rec.act.*` message keys for the pre-flight banner. Unknown + * reasons fall back to `gate_denied_generic` in the caller — fail + * closed with an honest "refused" message rather than guessing. + */ +const GATE_DENIAL_KEYS: Record = { + audience: "gate_denied_audience", + backup_missing: "gate_denied_backup", + backup_stale: "gate_denied_backup", + flag: "gate_denied_flag", +}; + interface BackupState { device: ConnectedDevice | null; error: string | null; @@ -223,6 +236,14 @@ export default function ReviewRecommendationsView({ const [uninstallStates, setUninstallStates] = useState< Record >({}); + /** + * True when the backup itself succeeded but persisting its stamp + * (POST /api/device-actions/backup) failed. The act step's pre-flight + * gate reads the server-side stamp, so an unrecorded backup will be + * treated as missing — this flag surfaces that mismatch on the + * backup step instead of letting the act step refuse "mysteriously". + */ + const [backupRecordFailed, setBackupRecordFailed] = useState(false); /** * Per-row free-text "replacing with" memo — captured during the * Compare step. Stored in component state only (not persisted) so @@ -368,6 +389,20 @@ export default function ReviewRecommendationsView({ null | "list" | "final" | "executing" >(null); const [bulkConfirmText, setBulkConfirmText] = useState(""); + /** + * Set when the pre-flight gate check (GET /api/device-actions/ + * uninstall) refuses the run or can't be reached. The bulk loop is + * aborted before any cfgutil call fires; this renders as an alert + * banner on the act step. + */ + const [bulkGateError, setBulkGateError] = useState(null); + /** + * Count of activity-log writes (POST /api/device-actions/uninstall) + * that failed during the last bulk run. The removals themselves + * already ran — this only drives an "audit trail is incomplete" + * warning so the user knows the log can't be trusted for this run. + */ + const [recordingFailures, setRecordingFailures] = useState(0); // Modal focus management for the two bulk dialogs — declared here, after // `bulkModal`, since the hook's `open` reads it. const bulkListCardRef = useModalFocus({ @@ -616,6 +651,7 @@ export default function ReviewRecommendationsView({ return; } const device = devices.find((d) => d.ecid === selectedEcid) ?? null; + setBackupRecordFailed(false); setBackup({ status: "running", device, @@ -635,8 +671,9 @@ export default function ReviewRecommendationsView({ }); return; } + let recorded = false; try { - await fetch("/api/device-actions/backup", { + const res = await fetch("/api/device-actions/backup", { method: "POST", headers: { "Content-Type": "application/json" }, body: JSON.stringify({ @@ -646,9 +683,17 @@ export default function ReviewRecommendationsView({ deviceName: device?.name ?? null, }), }); + recorded = res.ok; + if (!res.ok) { + console.warn("[review] backup record refused:", res.status); + } } catch (e) { console.warn("[review] failed to record backup:", e); } + // The backup itself succeeded — a failed recording downgrades to a + // warning, not a failed step. Without the stamp the act step's + // pre-flight will refuse the backed-up path, so tell the user now. + setBackupRecordFailed(!recorded); setBackup({ status: "done", device, @@ -669,8 +714,9 @@ export default function ReviewRecommendationsView({ [row.id]: { status: "running", error: null }, })); const result = await removeAppViaCfgutil(selectedEcid, row.bundleId); + let recorded = false; try { - await fetch("/api/device-actions/uninstall", { + const res = await fetch("/api/device-actions/uninstall", { method: "POST", headers: { "Content-Type": "application/json" }, body: JSON.stringify({ @@ -683,9 +729,18 @@ export default function ReviewRecommendationsView({ acknowledgeNoBackup, }), }); + recorded = res.ok; + if (!res.ok) { + console.warn("[review] uninstall record refused:", res.status); + } } catch (e) { console.warn("[review] failed to record uninstall outcome:", e); } + if (!recorded) { + // The removal already ran — losing the audit row is a + // warning-level problem, surfaced once per bulk run. + setRecordingFailures((n) => n + 1); + } setUninstallStates((prev) => ({ ...prev, [row.id]: result.ok @@ -701,21 +756,54 @@ export default function ReviewRecommendationsView({ ); /** - * Sequential bulk-uninstall runner. Iterates the queue, calling - * `runUninstall` per row. Errors don't abort the batch — the per-app - * state surfaces ✓/✕/spinner inline, and the user sees a summary - * once the loop finishes. Apps that lack a bundle ID are skipped - * (cfgutil needs one) and surface as `error`. + * Sequential bulk-uninstall runner. Pre-flights the server-side gate + * (audience + flag + backup freshness) BEFORE the first cfgutil call + * and aborts fail-closed when it refuses or can't be reached — the + * per-row POSTs after each removal only *record* outcomes, so this + * is the one point where the server can still stop the run. Then it + * iterates the queue, calling `runUninstall` per row. Errors don't + * abort the batch — the per-app state surfaces ✓/✕/spinner inline, + * and the user sees a summary once the loop finishes. Apps that lack + * a bundle ID are skipped (cfgutil needs one) and surface as `error`. * - * The `acknowledgeNoBackup` flag flows through to every per-app - * request so the server-side gate can allow the bypass uniformly - * across the batch. + * The `acknowledgeNoBackup` flag flows through the pre-flight and + * every per-app request so the server-side gate can allow the bypass + * uniformly across the batch. */ const runBulkUninstall = useCallback( async (acknowledgeNoBackup: boolean) => { if (!selectedEcid) { return; } + setBulkGateError(null); + setRecordingFailures(0); + try { + const qs = new URLSearchParams({ ecid: selectedEcid }); + if (acknowledgeNoBackup) { + qs.set("acknowledgeNoBackup", "1"); + } + const res = await fetch(`/api/device-actions/uninstall?${qs}`, { + cache: "no-store", + }); + const gate = res.ok + ? ((await res.json()) as { allowed?: boolean; reason?: string }) + : null; + if (gate?.allowed !== true) { + const key = gate + ? (GATE_DENIAL_KEYS[gate.reason ?? ""] ?? "gate_denied_generic") + : "gate_unreachable"; + setBulkGateError(tAct(key)); + setBulkModal(null); + setBulkConfirmText(""); + return; + } + } catch (e) { + console.warn("[review] gate pre-flight failed:", e); + setBulkGateError(tAct("gate_unreachable")); + setBulkModal(null); + setBulkConfirmText(""); + return; + } setBulkModal("executing"); for (const row of uninstallQueue) { if (!row.bundleId) { @@ -1725,6 +1813,11 @@ export default function ReviewRecommendationsView({ })}

)} + {backup.status === "done" && backupRecordFailed && ( +

+ ⚠ {tBackup("record_failed")} +

+ )} {backup.status === "error" && (

{backup.error} @@ -1745,7 +1838,8 @@ export default function ReviewRecommendationsView({ on file so the user understands which Modal 2 variant they'll see. Fresh ✓ → reassuring; missing / stale ⚠ → warning. Drives no other behaviour here; the actual gate - decision is made server-side when the bulk loop runs. */} + decision is the server-side pre-flight at the top of + runBulkUninstall, before any removal fires. */} {uninstallQueue.length > 0 && (

)} + {/* Pre-flight refusal / audit-trail warnings. The gate error + means NOTHING was removed; the recording warning means + removals ran but one or more activity rows failed to + persist. Both are aria-live so screen-reader users hear + the outcome of a Delete click that didn't open the + executing state. */} + {bulkGateError && ( +

+ {bulkGateError} +

+ )} + {recordingFailures > 0 && ( +

+ ⚠ {tAct("record_failures", { count: recordingFailures })} +

+ )} + {uninstallQueue.length === 0 ? (

{tAct("empty")}

) : ( @@ -1878,6 +1989,7 @@ export default function ReviewRecommendationsView({ disabled={bulkModal === "executing"} onClick={() => { setBulkConfirmText(""); + setBulkGateError(null); setBulkModal("list"); }} type="button" diff --git a/docs/ARCHITECTURE.md b/docs/ARCHITECTURE.md new file mode 100644 index 0000000..41233b3 --- /dev/null +++ b/docs/ARCHITECTURE.md @@ -0,0 +1,295 @@ +# Architecture & workflows, end to end + +Every process the app runs — from typing an app name to deleting one off a plugged-in +iPhone — drawn as flow diagrams across the runtimes, with known weak points marked where +they live. Companion to the prose in [AGENTS.md](../AGENTS.md) and the hosted docs at +[privacytracker-docs.privacykey.org](https://privacytracker-docs.privacykey.org/develop/architecture). + +**How to read the markers.** `⚠ §N·M` = open finding, `✅ §N·M` = fixed. Every marker is a +row in the [improvement backlog](#7--improvement-backlog) at the bottom. When you fix one, +update its row and the diagram label in the same PR. + +*Findings audited against source on 2026-07-06 (branch `feat/eager-shannon-b87c90`).* + +--- + +## 0 · System map + +The desktop app is a Tauri shell that boots a private Next.js server (the "sidecar") on a +random localhost port, then points its webview at it. Everything privacy-critical happens +on this machine: scraping, diffing, AI calls, and the SQLite database. The Rust shell is +the only piece that can touch a connected iPhone. The web/Docker build is the same server +without the shell column. + +```mermaid +flowchart LR + subgraph mac["This Mac · Tauri desktop app"] + shell["Rust shell (src-tauri/)
tray · deep links · usb_watcher
cfgutil bridge · Touch ID gate
ACL: 14 allowed commands"] + webview["Webview (Next.js UI)
dashboard · onboarding wizard
review-and-act wizard · TaskCenter"] + sidecar["Node sidecar (Next server)
app/api/* routes → lib/*
9 boot timers · 3 bulk runners"] + db[("SQLite data/privacy.db
WAL · synchronous better-sqlite3
apps → types → categories")] + end + phone["iPhone / iPad over USB
list · installedApps · backup · remove-app"] + apple["Apple
iTunes Search API · App Store pages"] + archive["archive.org
availability · replay · Save Page Now"] + ai["AI provider (optional)
OpenAI / Anthropic / local"] + + shell -->|"spawns · reveals window when /api/apps responds"| sidecar + webview <-->|"HTTP 127.0.0.1:<port>"| sidecar + webview -.->|"invoke() IPC · ACL + Touch ID"| shell + shell -.->|"cfgutil subprocess"| phone + sidecar --> db + sidecar -.-> apple + sidecar -.-> archive + sidecar -.-> ai +``` + +Boot handshake: `sidecar::boot()` binds `127.0.0.1:0` for a free port, spawns Node with +`PORT`/`PRIVACYTRACKER_DATA_DIR`, polls `GET /api/apps` (≤60s), then navigates the webview +and reveals the window (optionally behind a Touch ID unlock). Files: +`src-tauri/src/main.rs`, `src-tauri/src/sidecar.rs`. + +--- + +## 1 · Add & track an app (the core loop) + +The pipeline every other flow feeds into. A name becomes an App Store URL, the URL becomes +parsed privacy labels, and every re-sync diffs against the previous snapshot to produce +the change timeline and notifications. Re-syncs run the same path with `resync=true`. +Files: `lib/scraper.ts`, `lib/changelog.ts`, `lib/privacy-policy.ts`. + +```mermaid +sequenceDiagram + autonumber + participant UI as Webview UI + participant API as Sidecar API + participant LIB as lib/ pipeline + participant DB as SQLite + participant EXT as Apple / AI + + UI->>API: POST /api/search — names · bundleIds · country + API->>EXT: iTunes Search API + EXT-->>UI: candidates — user picks the right match + UI->>API: POST /api/scrape — urls · resync flag + API->>LIB: fetchAndParseApp(url) + LIB->>EXT: GET App Store page HTML + Note over LIB: ⚠ §1·1 parse fallback chain
shelfMapping → privacyHeader → shelves → shoebox + LIB->>LIB: capture previousSnapshot BEFORE the write + LIB->>DB: saveToDb tx — apps → privacy_types → privacy_categories + LIB->>DB: buildSnapshot → diffSnapshots → saveSnapshot + DB-->>DB: notification row · changeCount bump + Note over LIB: ⚠ §1·2 policy fetch + hash —
regenerate summary_json only when hash changed + LIB->>EXT: AI summarisation (optional, chunked for local models) + DB-->>UI: timeline · bell · pending-changes dot +``` + +--- + +## 2 · Import your apps from a device (cfgutil) + +Onboarding (`OnboardWizard.tsx`, five steps: choose method → import & reconcile → confirm +matches → import progress → policy summaries) accepts four sources: screenshots (OCR), +CSV/TXT upload, manual typing, and — on macOS with Apple Configurator — a live `cfgutil` +export. The cfgutil path is **read-only against the device**. There is no scheduled device +re-sync: a USB plug-in event (IOKit watcher → `DeviceConnectedToast`) re-opens this flow +on demand. Files: `src-tauri/src/cfgutil.rs`, `lib/desktop.ts`, `src-tauri/src/usb_watcher.rs`. + +```mermaid +sequenceDiagram + autonumber + participant WIZ as Onboard wizard + participant SH as Rust shell + participant PH as iPhone (USB) + participant API as Sidecar API + participant AP as Apple + + WIZ->>SH: invoke check_cfgutil — PATH + app-bundle probes, cached 5 min + WIZ->>SH: poll list_connected_devices (5s while on the step) + SH->>PH: cfgutil list · get name/model + PH-->>WIZ: devices (ECID · name · iOS version) + WIZ->>SH: invoke run_cfgutil_export(ecid) + SH->>PH: cfgutil get installedApps (90s cap, read-only) + PH-->>WIZ: rows — displayName · bundleIdentifier · version + WIZ->>WIZ: step 2 — dedupe by bundleId, diff-preview + WIZ->>API: POST /api/search with bundleIds + API->>AP: iTunes lookup — canonical track per bundle id + WIZ->>WIZ: step 3 — name-search fallback for unlisted/sideloaded + Note over API: ⚠ §2·1 step 4 — scrape each URL (§1 flow) ·
Apple 429s park rows in the import queue (60s drain) + WIZ->>API: step 5 — optional policy summaries (flag-gated) +``` + +Non-Mac alternative: `scripts/ios-app-import/export_ios_apps.py` (stdlib-only) produces a +`.txt`/`.csv` the user feeds back into the CSV path (⚠ §2·2 — manual round-trip). + +--- + +## 3 · Back up, then delete apps off the phone + +The only destructive flow in the product, so it runs the deepest gate stack: audience must +be `self`, an off-by-default Developer Options flag, a fresh backup (≤24h) or an explicit +typed acknowledgement, two confirm modals, a server-side pre-flight, and finally a native +Touch ID prompt per app that JavaScript cannot bypass. One cfgutil call per app — there is +deliberately no batch primitive. Files: `app/components/ReviewRecommendationsView.tsx`, +`lib/device-actions.ts`, `app/api/device-actions/*`, `src-tauri/src/cfgutil.rs`, +`src-tauri/src/touch_id.rs`. + +```mermaid +sequenceDiagram + autonumber + participant WIZ as Review wizard + participant API as Sidecar (gates + audit) + participant SH as Rust shell + participant PH as iPhone (USB) + + Note over WIZ: ◆ entry gate — audience=self ∧ flag on ∧ desktop build + WIZ->>WIZ: steps 1–3 — own "uninstall" verdicts only ·
imported recommendations never execute + WIZ->>WIZ: step 4 — pick device · warn when ECID ≠ app's source device + WIZ->>SH: invoke run_cfgutil_backup(ecid, destDir) + Note over SH: ⚠ §3·1 --backup-output unverified on real cfgutil
⚠ §3·2 success = exit code only, no on-disk check
⚠ §3·4 dest allowlist is lexical (symlinks not resolved) + SH->>PH: cfgutil backup (300s ceiling) + WIZ->>API: POST /api/device-actions/backup + Note over API: ✅ §3·6 normalizeEcid (0x-prefixed ECIDs) → stamp + activity row + WIZ->>WIZ: step 5 — "Delete N apps" → modal 1 (list) → modal 2 (type DELETE) + Note over WIZ: ⚠ §3·5 modal variant keys off session-local backup state + WIZ->>API: GET gate pre-flight — audience ∧ flag ∧ backup ≤24h (or acknowledged) + Note over API: ✅ §3·7 pre-flight BEFORE first removal, fail closed + loop one app at a time + WIZ->>SH: invoke run_cfgutil_remove_app(ecid, bundleId) + Note over SH: ◆ Touch ID / password per app — native LAContext,
JS cannot bypass · fails closed without biometrics+password + SH->>PH: cfgutil remove-app (45s timeout) + Note over PH: ⚠ §3·3 timed-out child is orphaned, not killed —
removal may still complete after a reported failure + WIZ->>API: POST record outcome — cfgutil_uninstall row (ok/error + ack flag) + end +``` + +--- + +## 4 · Background jobs & crash-safe resume + +Server boot (`instrumentation.ts`) arms nine timers. Three bulk runners (App Store sync, +Wayback import, policy sync) share one crash-safety pattern: a mutex key plus a state blob +in `app_settings`, rewritten at every app boundary — a process kill loses at most one +app's work, and boot-time healers resume or clear what's left. + +| When | What | Then every | +| --- | --- | --- | +| t=0 | watchdog · error ring · diagnostics · feature-flag migration · clear `import_queue_running`/`health_check_running` stale locks | — | +| +8s / +10s / +12s | resume healers: wayback → sync → policy (resume pending run, or clear a stale mutex) | on boot | +| +15s | scheduler tick — `getSchedulerStatus().isDue` (daily/weekly/manual) → `runBulkSync` | 30 min | +| +20s | import-queue drain (rows parked by onboarding 429s) | 60 s | +| +25s | update check (GitHub, 24h response cache) | 6 h | +| +35s | whole-DB backup snapshots (`lib/backup.ts`, signed JSON — distinct from device backups) | 30 min | +| +60s | health check — PASSIVE WAL checkpoint, clear provably-dead locks, report-only memory/orphan checks | 24 h | + +The resume healers are staggered *before* the 60s health check so a freshly-resumed run is +never mistaken for a dead lock. + +```mermaid +flowchart TD + acquire["◆ acquire mutex
sync_running · wayback_import_running · policy_sync_running"] + blob["state blob in app_settings
runId · queue · totals · initiator"] + work["process app N
blob rewritten at every app boundary"] + r429["429 from Apple (sync)
bail + partial activity row
clear state cleanly"] + done["clean finish
clear blob + mutex · summary row"] + crash["process killed mid-run
blob + mutex survive on disk"] + heal["◆ boot resume healers (+8/10/12s)
resume run · or clear stale lock"] + resume["runner restarts, initiator: resume
per-target dedup skips done work"] + ui["TaskCenter polls /api/tasks/active every 4s
resumed-run pill · per-job progress GETs"] + + acquire --> blob --> work + work -->|"⚠ §4·1 whole run restarts next tick"| r429 + work -->|all apps done| done + work -.->|kill -9 / power loss| crash + crash --> heal --> resume --> work + resume -.->|"⚠ §4·2 three separate pollers"| ui +``` + +Apple 429 handling is deliberate: an expected, recoverable condition clears state cleanly +(unlike a crash) so the next 30-minute tick retries fresh. + +--- + +## 5 · Wayback: back-filling label history to 2021 + +Reconstructs an app's privacy-label history from archive.org — one target per quarter back +to Q1 2021 plus an "install anchor" at `apps.firstSeen`, so the since-install diff has a +real baseline. Read-only against the archive except one Save-Page-Now request per app when +a quarter has no usable capture. Files: `lib/historical-import.ts`, `lib/wayback-bulk-runner.ts`. + +```mermaid +flowchart TD + entry["per-app or bulk entry
POST import-history · import-all (NDJSON stream)"] + targets["computeHistoricalTargets
quarters to 2021-Q1 + anchor at firstSeen"] + avail["archive.org availability API
closest capture per target"] + walk{"◆ tolerance walk
±14/28/42d probes
drop if >45d drift"} + spn["⚠ §5·1 no capture anywhere →
Save-Page-Now for the live page
once per app per run"] + fetch["fetch replay (id_ URL)
clean original HTML"] + parse["parse — shoebox extractor for old
Ember pages · modern chain for 2025+"] + pipe["same §1 pipeline
source='wayback' · backdated scrapedAt
no changeCount bump"] + tl["timeline: purple wayback rows
'Matches live sync' badge · since-install baseline"] + + entry --> targets --> avail --> walk + walk -->|miss| spn + walk -->|hit| fetch --> parse --> pipe --> tl +``` + +--- + +## 6 · The gate chain every surface answers to + +Whether any card, step, or destructive action exists at all is resolved through one +layered chain — later stages override earlier ones, user override always wins. The §3 flow +adds two hard gates on top (backup freshness, Touch ID) that no flag can soften. + +```mermaid +flowchart LR + hd["hard default
HARD_DEFAULTS"] --> aud["audience
self / loved_one / guardian"] + aud --> goal["goals
monitor · cleanup · minimal"] + goal --> a11y["accessibility
modifier bundle"] + a11y --> rt["runtime
Tauri-only off on web"] + rt --> dep["dependency
flags requiring flags"] + dep --> ovr(["user override — final word"]) +``` + +Kill-switch: `flag.devopts.feature_flag_system.enabled=off` collapses everything to hard +defaults without a code rollback. The delete flow's `flag.devopts.cfgutil_uninstall` +defaults to **off**, and the audience gate is enforced in code — flipping the flag on +under `guardian` still shows nothing. Modules: `lib/feature-flag-rules.ts`, +`lib/feature-flags*.ts` (see AGENTS.md for the five-module split). + +--- + +## 7 · Improvement backlog + +Point-in-time (2026-07-06). Refs match the `⚠`/`✅` markers in the diagrams above. Prune or +flip rows as they land, and update the diagram label in the same PR. + +| Ref | Area | Status | Finding → candidate improvement | Effort | +| --- | --- | --- | --- | --- | +| §3·1 | Device backup | **open · high** | `cfgutil backup --backup-output` appears in no public cfgutil docs (canonical: `backup` takes no options, writes to MobileSync). Verify `cfgutil help backup` on a Mac with Configurator; if rejected, run plain `backup` and resolve the real path via `list-backups`. | S–M | +| §3·2 | Device backup | **open · high** | Backup success is exit-code only. Verify on disk (dir non-empty / `Manifest.db`) before stamping; never record a fallback path that wasn't observed. | S | +| §3·3 | Device delete | open | `run_with_timeout` orphans the cfgutil child on timeout — "failed" can silently become "succeeded later". Kill the process group, or re-check installed state after timeout and correct the audit row. | S | +| §3·4 | Device backup | open | `resolve_backup_dest` claims symlink protection but checks lexically. Canonicalise the existing ancestor, or fix the comment. | S | +| §3·5 | Delete UX / audit | open | Final modal's backup variant + acknowledge flag key off session-local state. Drive both from the server stamp (GET gate) so audit rows stop over-reporting "no backup acknowledged". Spec ready: [docs/specs/3-5-server-stamp-backup-variant.md](specs/3-5-server-stamp-backup-variant.md). | S | +| §3·6 | Device actions | ✅ fixed | ECID normalisation (`0x`-prefixed) across stamp store, gate, and routes; pinned by tests with real-format ECIDs. | — | +| §3·7 | Device actions | ✅ fixed | Server gate pre-flights before the first removal (fail closed); recording failures surface in the UI. | — | +| §1·1 | Scraper | open | No parser canary. Add fixture tests against recorded App Store HTML + an alert/activity row when a scrape parses zero privacy types for an app that previously had them. | M | +| §1·2 | AI summaries | idea | Summarisation silently degrades without a provider; chunking for local models is heuristic. Consider a visible "summary stale/unavailable" state. | S | +| §2·1 / §4·1 | Rate limiting | open | On 429 the bulk sync abandons the run and restarts the whole fleet next tick. Resume from the state blob's cursor instead; consider shared per-app backoff with the import queue. | M | +| §2·2 | Cross-platform import | idea | Python export needs a manual round-trip. Drag-drop hint or watch-folder hand-off. | M | +| §4·2 | Polling | idea | Three pollers (TaskCenter 4s, notification watcher, per-job GETs) → one SSE stream from the sidecar. | L | +| §5·1 | Wayback | idea | Track quarters skipped for lack of captures and offer "retry skipped" once Save-Page-Now requests have had time to land. | S–M | + +Suggested order: §3·1 and §3·2 first (they decide whether "we back up before deleting" is +true at all), then §1·1 (protects the core product), then §3·3/§3·4/§3·5 as one small +hardening PR, then the rate-limit resume (§2·1/§4·1). + +--- + +## Keeping this document honest + +- Diagrams are Mermaid — edit them in place; GitHub renders them natively. +- The backlog is point-in-time by design. When a finding is fixed, flip its row to ✅ and + update the matching `⚠` label in the diagram in the same PR (or delete both once stale). +- Timings (boot delays, poll intervals, timeouts) were read from source on the date above; + if you change one in code, grep this file for the old value. diff --git a/docs/specs/3-5-server-stamp-backup-variant.md b/docs/specs/3-5-server-stamp-backup-variant.md new file mode 100644 index 0000000..eda129a --- /dev/null +++ b/docs/specs/3-5-server-stamp-backup-variant.md @@ -0,0 +1,168 @@ +# Spec §3·5 — Drive the delete-confirm modal's backup state from the server stamp + +**Backlog ref:** §3·5 in [docs/ARCHITECTURE.md](../ARCHITECTURE.md#7--improvement-backlog) · +**Size:** S (~150–250 LOC including tests) · **Area:** device actions (TypeScript only — no Rust) +· **Good first issue:** yes — single component + one new pure helper + tests, no hardware needed. + +Read [AGENTS.md](../../AGENTS.md) first (commands, conventions), and skim +[docs/ARCHITECTURE.md §3](../ARCHITECTURE.md#3--back-up-then-delete-apps-off-the-phone) +for the flow this touches. + +--- + +## Background + +The review-and-act wizard (`app/components/ReviewRecommendationsView.tsx`) deletes apps off +a connected iPhone via cfgutil. Before running the bulk delete it shows two modals; the +second ("type DELETE") has two copy variants: + +- **fresh-backup variant** — reassuring, shows device name + backup time; +- **no-backup variant** — louder "at your own risk" wording. + +Which variant renders — and whether the per-app recording POSTs carry +`acknowledgeNoBackup: true` — is currently decided by **session-local component state**: + +```ts +// ReviewRecommendationsView.tsx — Modal 2 confirm button +const acknowledgeNoBackup = backup.status !== "done"; +``` + +`backup.status` only becomes `"done"` when a backup ran **in this mount of the wizard**. +The server, meanwhile, keeps its own durable stamp per device +(`cfgutil_last_backup_` in `app_settings`, written by +`POST /api/device-actions/backup`, read by `checkUninstallGate()` in +`lib/device-actions.ts` with a 24h freshness window, `BACKUP_FRESHNESS_WINDOW_MS`). + +The two sources of truth disagree in real scenarios: + +1. **Backed up 2 hours ago, then reopened the wizard.** Server stamp is fresh; local state + is `idle`. The user sees the scary no-backup modal and the audit log records + `acknowledgedNoBackup: true` — over-reporting risk they didn't actually take. +2. **Backup succeeded but its recording POST failed.** Local state says `done`; the server + has no stamp. The modal shows the reassuring variant, then the pre-flight gate + (`GET /api/device-actions/uninstall`) refuses `backup_missing` — correct but confusing, + because the modal just promised a backup existed. + +The fix: make the **server stamp** the single source of truth for the modal variant, the +act-step banner, and the `acknowledgeNoBackup` flag. Session-local `backup.status` remains +the source of truth only for the backup step's own progress UI (running / done / error). + +## Current behaviour — where to look + +All in `app/components/ReviewRecommendationsView.tsx` (find by searching for the quoted +strings; line numbers drift): + +| Spot | Search for | Current driver | +| --- | --- | --- | +| Act-step banner (✓ fresh / ⚠ missing) | `review-rec-backup-status` | `backup.status === "done"` | +| Modal 2 copy variant | `bulk-final-title` | `backup.status === "done" && backup.finishedAt` | +| Acknowledge flag | `backup.status !== "done"` | local state | +| Pre-flight gate (do NOT change) | `gate pre-flight` in `runBulkUninstall` | server GET, fail closed | + +Server side: + +- `GET /api/device-actions/uninstall?ecid=…` (`app/api/device-actions/uninstall/route.ts`) + returns `checkUninstallGate(ecid)`: `{ allowed: true }` or + `{ allowed: false, reason: "audience" | "flag" | "backup_missing" | "backup_stale", … }`. +- `getLastBackup(ecid)` (`lib/device-actions.ts`) returns + `{ finishedAt: number, path: string } | null`. ECIDs are normalised (`normalizeEcid`) + so cfgutil's `0x…` spellings round-trip. + +## Desired behaviour + +1. **Extend the GET response (additive).** `GET /api/device-actions/uninstall?ecid=…` + additionally returns `lastBackup: { finishedAt, path } | null` alongside the existing + gate fields. Additive only — existing fields and the POST contract must not change. + +2. **New pure helper + shared types.** Create `lib/device-actions-shared.ts` (no + `"server-only"` import — the existing server module `lib/device-actions.ts` cannot be + imported from client components; follow the `lib/changelog-types.ts` precedent for + client-safe shared types). Export: + + ```ts + export type UninstallGateResponse = { + allowed?: boolean; + reason?: "audience" | "flag" | "backup_missing" | "backup_stale"; + lastBackup?: { finishedAt: number; path: string } | null; + }; + + export type ServerBackupState = + | { kind: "fresh"; finishedAt: number } + | { kind: "not_fresh" }; // missing, stale, unreadable, or fetch failed + + export function deriveServerBackupState( + gate: UninstallGateResponse | null + ): ServerBackupState; + ``` + + Rules: `allowed: true` **with** a `lastBackup` → `fresh` (use its `finishedAt`); + `allowed: true` **without** `lastBackup` (acknowledge path / unexpected) → `not_fresh`; + any denial, `null`, or malformed input → `not_fresh`. Never claim a backup exists that + the server didn't report — conservative by construction. + +3. **Wizard queries the stamp at the right moments.** In + `ReviewRecommendationsView.tsx`, fetch + `GET /api/device-actions/uninstall?ecid=` with `cache: "no-store"`: + - when the act step becomes active with a selected ECID (one fetch per entry, in a + `useEffect` keyed on `[step, selectedEcid]` — mirror the existing device-polling + effect's cancellation pattern); + - after `runBackup` completes successfully (so the banner flips to ✓ without a remount). + Store the derived `ServerBackupState` in component state. Do **not** fetch on every + render and do **not** touch the existing pre-flight inside `runBulkUninstall` — that + stays exactly as is (it is the execution gate; this work is display/audit semantics). + +4. **Three consumers switch to the derived state.** + - Act-step banner: ✓ variant when `fresh` (time from `finishedAt`), ⚠ otherwise. + - Modal 2: reassuring variant when `fresh` — reuse the existing + `confirm_modal.final_body` copy; when the local `backup.device` name is unknown + (cross-session case) the existing `fallback_device_name` string covers `{device}`. + No new i18n keys are expected; if you do add any, add them to **both** + `locales/en.json` and `locales/zh.json` and run `pnpm lint:i18n`. + - Confirm button: `const acknowledgeNoBackup = serverBackupState.kind !== "fresh";` + The backup step's own status text (running/done/error + saved-path line) stays on + local state. + +5. **Conservative on failure.** If the GET fails or returns junk, behave as `not_fresh` + (scary variant, `acknowledgeNoBackup: true`). This is deliberately the opposite + direction of the pre-flight (which fails **closed** by refusing) — here nothing is + blocked, we just refuse to *reassure*. + +## Tests (node:test — see `tests/app/uninstall-gate.test.ts` for style) + +New file `tests/app/device-actions-shared.test.ts` covering `deriveServerBackupState`: +`allowed+lastBackup → fresh` (finishedAt passed through) · `allowed without lastBackup → +not_fresh` · each denial reason → `not_fresh` · `null` / `{}` / garbage → `not_fresh`. + +Extend `tests/app/uninstall-gate.test.ts` (or a small new route-shaped test) to pin the +additive GET payload: after `recordBackup(...)`, the route module's GET handler for that +ECID includes `lastBackup.finishedAt`; for an unknown ECID `lastBackup` is `null`. + +## Acceptance criteria + +- [ ] `pnpm test`, `pnpm typecheck`, `pnpm lint`, `pnpm lint:i18n` all pass. +- [ ] GET response is a strict superset of today's; POST semantics untouched. +- [ ] `runBulkUninstall`'s pre-flight block is byte-identical (no behaviour change). +- [ ] With a fresh server stamp and a **freshly mounted** wizard: banner shows ✓, Modal 2 + shows the reassuring variant, recording POSTs carry `acknowledgeNoBackup: false`. +- [ ] With no/stale stamp (even right after a local backup whose recording POST failed): + ⚠ banner, at-your-own-risk variant, `acknowledgeNoBackup: true`. +- [ ] Helper lives in a client-safe module; no client import of `"server-only"` code + (this fails the build — see the five-module flag split in AGENTS.md for why). +- [ ] No polling loops added; at most one gate fetch per act-step entry + one after a + completed backup. + +## Out of scope + +Backup verification on disk (§3·2), the `--backup-output` question (§3·1), Rust changes +(§3·3/§3·4), batching or copy redesign, and anything touching `run_cfgutil_remove_app`. + +## Pitfalls + +- `lib/device-actions.ts` is `"server-only"` — importing it from the component is the + first thing that will fail the build. Hence the shared module. +- Biome (`pnpm lint`) enforces repo style; `useExhaustiveDependencies` is off, and the + file uses eslint-disable-style comments for stable `t*` translators — mimic neighbours. +- The wizard already has `bulkGateError` / pre-flight state — don't conflate it with the + new display state; they answer different questions ("may I run?" vs "should I reassure?"). +- `matches` in tests: use a real-format ECID (`0x9118908BB6027`) at least once — the + normalisation path is load-bearing (see tests already pinning it). diff --git a/lib/device-actions.ts b/lib/device-actions.ts index 82d0f36..f5d214a 100644 --- a/lib/device-actions.ts +++ b/lib/device-actions.ts @@ -30,16 +30,29 @@ export const BACKUP_FRESHNESS_WINDOW_MS = 24 * 60 * 60 * 1000; const SETTINGS_BACKUP_PREFIX = "cfgutil_last_backup_"; /** - * Apple ECIDs are hex strings (typically 12-20 chars). Validate at every - * TS-side entry point even though the Rust command also char-allowlists, - * so a stray caller can't synthesise a key like - * `cfgutil_last_backup_flag.devopts.cfgutil_uninstall` via string + * Canonicalise an ECID for use in a settings key, or return null when + * the value isn't a plausible ECID. + * + * cfgutil prints ECIDs as `0x`-prefixed hex — its JSON `Output` + * dictionaries are keyed by strings like `0x9118908BB6027`, and that + * exact spelling is what the webview passes through from + * `list_connected_devices`. Strip the prefix and upper-case the hex + * body so a stamp written under any spelling (`0x9118908bb6027`, + * `9118908BB6027`, …) reads back under every other. + * + * Validation still matters even though the Rust commands also + * char-allowlist: a stray caller must not be able to synthesise a key + * like `cfgutil_last_backup_flag.devopts.cfgutil_uninstall` via string * concatenation and collide with another setting key. Defence in depth * — if the Rust validator changes or another TS entry point is added, * this still keeps the namespace unambiguous. */ -function isValidEcid(value: string): boolean { - return /^[A-Fa-f0-9]{8,24}$/.test(value); +export function normalizeEcid(value: string): string | null { + const body = value.trim().replace(/^0[xX]/, ""); + if (!/^[A-Fa-f0-9]{8,24}$/.test(body)) { + return null; + } + return body.toUpperCase(); } interface BackupStamp { @@ -120,10 +133,11 @@ export function checkUninstallGate( /** Most recent backup stamp for the given ECID, or null. */ export function getLastBackup(ecid: string): BackupStamp | null { - if (!isValidEcid(ecid)) { + const normalized = normalizeEcid(ecid); + if (!normalized) { return null; } - const key = SETTINGS_BACKUP_PREFIX + ecid; + const key = SETTINGS_BACKUP_PREFIX + normalized; const raw = getSetting(key, ""); if (!raw) { return null; @@ -149,10 +163,11 @@ export function recordBackup(opts: { finishedAt: number; deviceName: string | null; }): void { - if (!isValidEcid(opts.ecid)) { + const normalized = normalizeEcid(opts.ecid); + if (!normalized) { throw new Error(`recordBackup: invalid ECID ${opts.ecid}`); } - const key = SETTINGS_BACKUP_PREFIX + opts.ecid; + const key = SETTINGS_BACKUP_PREFIX + normalized; const stamp: BackupStamp = { finishedAt: opts.finishedAt, path: opts.path, diff --git a/locales/en.json b/locales/en.json index 2ad05e4..025e019 100644 --- a/locales/en.json +++ b/locales/en.json @@ -180,7 +180,8 @@ "skip_backup_title": "Continue without a backup. We'll ask you to type DELETE again before any app is removed.", "mismatch_heading": "Heads up — this might not be the right device", "mismatch_body": "{count, plural, one {# of the apps you're about to delete wasn't imported from this device. cfgutil will refuse to remove an app that isn't installed.} other {# of the apps you're about to delete weren't imported from this device. cfgutil will refuse to remove apps that aren't installed.}}", - "mismatch_more": "…and {count} more" + "mismatch_more": "…and {count} more", + "record_failed": "The backup completed, but saving its record failed. The delete step won't see this backup and may refuse or ask for the no-backup confirmation." }, "act": { "heading": "Remove apps from your phone", @@ -203,7 +204,13 @@ "backup_ok": "Backup of {device} taken at {time}", "backup_missing_warn": "No recent backup of this device.", "backup_missing_action": "Back up first", - "default_device_name": "the connected device" + "default_device_name": "the connected device", + "gate_denied_audience": "The safety check refused this run, so nothing was deleted: app removal is only available when your focus is set to just you.", + "gate_denied_flag": "The safety check refused this run, so nothing was deleted: the \"Remove apps from your phone\" option is switched off in Developer Options.", + "gate_denied_backup": "The safety check refused this run, so nothing was deleted: it has no record of a fresh backup (under 24 hours old) for this device. Run a backup, then try again.", + "gate_denied_generic": "The safety check refused this run, so nothing was deleted.", + "gate_unreachable": "Couldn't reach the safety check, so nothing was deleted. Check that privacytracker's local service is running and try again.", + "record_failures": "{count, plural, one {# removal} other {# removals}} couldn't be written to the activity log — the apps were still processed, but the audit trail for this run is incomplete." }, "saved_notes": { "heading": "Your notes", diff --git a/locales/zh.json b/locales/zh.json index 79ac72d..b63ccd9 100644 --- a/locales/zh.json +++ b/locales/zh.json @@ -180,7 +180,8 @@ "skip_backup_title": "不备份直接继续。在任何应用被移除之前，我们仍会要求你再次输入 DELETE。", "mismatch_heading": "提示 — 这可能不是正确的设备", "mismatch_body": "{count, plural, one {你即将删除的应用中有 # 个不是从该设备导入的。cfgutil 不会移除未安装的应用。} other {你即将删除的应用中有 # 个不是从该设备导入的。cfgutil 不会移除未安装的应用。}}", - "mismatch_more": "…还有 {count} 个" + "mismatch_more": "…还有 {count} 个", + "record_failed": "备份已完成，但保存备份记录失败。删除步骤将看不到此备份，可能会拒绝执行或要求进行\"无备份\"确认。" }, "act": { "heading": "从手机移除应用", @@ -203,7 +204,13 @@ "backup_ok": "已于 {time} 备份 {device}", "backup_missing_warn": "该设备没有最近的备份。", "backup_missing_action": "先备份", - "default_device_name": "已连接的设备" + "default_device_name": "已连接的设备", + "gate_denied_audience": "安全检查拒绝了本次操作，未删除任何应用：只有当你的关注对象设置为\"自己\"时才能移除应用。", + "gate_denied_flag": "安全检查拒绝了本次操作，未删除任何应用：开发者选项中的\"从手机移除应用\"开关未开启。", + "gate_denied_backup": "安全检查拒绝了本次操作，未删除任何应用：没有该设备 24 小时内的备份记录。请先运行备份，然后重试。", + "gate_denied_generic": "安全检查拒绝了本次操作，未删除任何应用。", + "gate_unreachable": "无法连接安全检查服务，因此未删除任何应用。请确认 privacytracker 本地服务正在运行，然后重试。", + "record_failures": "{count, plural, one {# 项移除操作} other {# 项移除操作}}未能写入活动日志 — 应用仍已处理，但本次运行的审计记录不完整。" }, "saved_notes": { "heading": "你的备注", diff --git a/tests/app/uninstall-gate.test.ts b/tests/app/uninstall-gate.test.ts index 6266c80..9fd95a2 100644 --- a/tests/app/uninstall-gate.test.ts +++ b/tests/app/uninstall-gate.test.ts @@ -14,7 +14,12 @@ import assert from "node:assert/strict"; import test from "node:test"; -import { checkUninstallGate } from "../../lib/device-actions"; +import { + checkUninstallGate, + getLastBackup, + normalizeEcid, + recordBackup, +} from "../../lib/device-actions"; import { createDevice, getDeviceEcidsForApps, @@ -113,6 +118,62 @@ test("a stale backup (>24h) is denied unless acknowledgeNoBackup overrides", () assert.equal(overrideGate.allowed, true); }); +// ─── ECID normalisation ─────────────────────────────────────────── +// cfgutil keys its JSON `Output` by `0x`-prefixed hex ECIDs (e.g. +// `0x9118908BB6027`), and the webview passes that spelling through +// verbatim. The stamp store must accept it — and read back the same +// stamp under any prefix/case spelling of the same ECID. + +const RAW_CFGUTIL_ECID = "0x9118908BB6027"; + +test("recordBackup + gate accept cfgutil's 0x-prefixed ECIDs", () => { + beforeEach(); + recordBackup({ + ecid: RAW_CFGUTIL_ECID, + path: "/tmp/backup", + finishedAt: Date.now() - 60_000, + deviceName: "Test iPhone", + }); + const gate = checkUninstallGate(RAW_CFGUTIL_ECID); + assert.equal(gate.allowed, true); +}); + +test("a stamp reads back regardless of 0x prefix or hex case", () => { + beforeEach(); + recordBackup({ + ecid: "0x9118908bb6027", + path: "/tmp/backup", + finishedAt: Date.now() - 60_000, + deviceName: null, + }); + assert.notEqual(getLastBackup("9118908BB6027"), null); + assert.equal(checkUninstallGate("0X9118908BB6027").allowed, true); +}); + +test("normalizeEcid canonicalises valid spellings and rejects the rest", () => { + assert.equal(normalizeEcid("0x9118908BB6027"), "9118908BB6027"); + assert.equal(normalizeEcid("0X9118908bb6027"), "9118908BB6027"); + assert.equal(normalizeEcid(" ABCDEF1234567890 "), "ABCDEF1234567890"); + // Settings-key injection shapes and non-hex garbage must not pass. + assert.equal(normalizeEcid("flag.devopts.cfgutil_uninstall"), null); + assert.equal(normalizeEcid("0x"), null); + assert.equal(normalizeEcid(""), null); + assert.equal(normalizeEcid("zzzz11112222"), null); + assert.equal(normalizeEcid("0x123"), null); // hex body too short +}); + +test("recordBackup throws on a malformed ECID", () => { + beforeEach(); + assert.throws(() => + recordBackup({ + ecid: "not-an-ecid", + path: "/tmp/backup", + finishedAt: Date.now(), + deviceName: null, + }) + ); +}); + // ─── getDeviceEcidsForApps ──────────────────────────────────────── test("getDeviceEcidsForApps returns ECIDs only for apps with cfgutil-imported devices", () => {