feat(ui): guide macOS users to grant Full Disk Access when files are denied - #216
Merged
Merged
Conversation
…denied A TCC denial is invisible from the user's side: the backup "succeeds", some files just never reach Drive. Add a root-mounted dismissible banner that latches on the first local.permission_denied activity row, counts DISTINCT files (a denial is permanent, so the same file is re-reported every cycle, unbounded), and offers a one-click deep link to the Full Disk Access pane. The deep link needs an explicit opener capability scope: opener:default only permits mailto/tel/http/https, so a custom scheme would throw at runtime while every mocked test still passed. Document in README and DESIGN s5.3.3 that APFS snapshots do NOT bypass TCC, and that an unsigned binary's TCC grant can silently invalidate on update because macOS binds the grant to the code signature.
Contributor
Coverage
Gate: passed - no coverage regression (epsilon 0.1 pp). |
This was referenced Jul 29, 2026
pmaxhogan
added a commit
that referenced
this pull request
Jul 29, 2026
Targets `main`. Rebased onto `baaf7bd` (post-#200/#207/#211/#201/#216/#213), so the `BackendKind` enum, the `build_store` match, the wizard's credentials step and the locale file carry ALL the destinations - no side was taken wholesale. It is a sibling of #207 and deliberately mirrors its structure, config handling and test layout. #213 (rclone) added no `BackendKind` variant, so it merged clean. A new crate, `crates/driven-localfs`, implements `RemoteStore` against a plain directory tree: USB sticks, external SSDs, NAS shares, and fast local restore drills. No migration is needed - `accounts.backend_kind` / `backend_config_json` from migration `0013` already cover it. ## Metadata: one sidecar per object, and why A filesystem has nowhere to put `app_properties`. Three options were on the table: - **Extended attributes** - the natural fit on APFS/ext4/NTFS, and completely absent on exFAT and FAT32, which is how a USB backup stick is actually formatted. A backend whose identity vocabulary evaporates on the most common removable format is worse than no backend. Rejected. - **One index file per destination** - one file to corrupt, one file to lock, and a guaranteed desync on any crash between writing an object and updating the index. The same argument `driven-s3` makes against a side-index. Rejected. - **One sidecar per object**, at `<dir>/.driven-meta/<stored-name>.json`. Chosen. Deriving the sidecar path from the object's own filename makes a lookup by object id a single `open` (no scan, no index), gives the sidecar namespace the data namespace's uniqueness for free, and keeps the destination folder looking like a plain mirror of the user's files - which is most of the point of backing up to a folder you own. Sidecar names are hard-guarded: `sidecar_path` REFUSES any `stored` that is not a single path component (separator, `..`, NUL, empty), rather than sanitizing it - a name that is not a plain component means a corrupt `file_state` row, and quietly rewriting it would annotate the wrong object. That is the sidecar-side twin of the guard `layout::path_for_id` applies to the data path, and it is what closes the `rust/path-injection` surface CodeQL flagged on the first CI run. **Commit ordering is load-bearing: data first, sidecar second, in both directions.** A crash between the two leaves either an object with a stale annotation (the pending op replays and re-commits both) or a dangling sidecar (inert, swept by the next write). The opposite ordering would leave a LIVE data file with no annotation, which `list_source_object_ids` cannot see - so the remote-existence audit would call a live object dead and re-upload it beside itself forever. Every reader is therefore driven by the DATA file and joins the sidecar onto it, never the reverse. ## Content hashes `RemoteEntry.md5` is the same app-level md5 the executor computes over the exact bytes sent (SPEC s8). Nothing new was invented. There is no server here to return that digest, so returning the in-memory one would make the executor's post-upload check compare a value against itself and silently disable corruption detection for every file. Instead every write: streams into a temp file (hashing as it goes), `F_FULLFSYNC`es it, atomically renames it over the target, syncs the directory entry, then **re-opens the committed file and hashes it back off the destination** - and returns THAT digest. On macOS the verify handle sets `F_NOCACHE` so the read reaches the device rather than the page cache. Honest limitation: on platforms without a cache-bypass hint the re-read proves correct assembly and naming rather than physical residency. `metadata()` reports the sidecar's digest only when the sidecar's recorded size matches the file's, so the brief post-crash window where a sidecar is out of step with its data reports "unknown" rather than a lie. ## Durability Nothing is ever written into a live object's file: temp file in the target's own directory (same directory, because `rename` is only atomic within one filesystem), `F_FULLFSYNC` (a plain `fsync` on macOS returns before the DRIVE flushes its own write cache - exactly the window that matters when a stick is yanked), atomic rename, then a directory `fsync` so the entry pointing at the new inode is durable too. ## Filename encoding Percent-encoding of the offending UTF-8 bytes, applied **unconditionally** rather than per-destination: - `%` (the escape introducer, so the transform is invertible) - the Windows/FAT reserved punctuation `/ \ : * ? " < > |` - ASCII control bytes `0x00-0x1F` and `0x7F` - a TRAILING `.` or space (Windows silently strips both, colliding `report.` with `report`) - the whole-name specials `.` and `..` - an MS-DOS DEVICE name (`CON`, `PRN`, `AUX`, `NUL`, `COM1`-`COM9`, `LPT1`-`LPT9`), matched the way Windows matches them: on the stem, after trailing dots and spaces are stripped, case-insensitively - so `CON`, `nul.txt`, `COM1.log` and `CON ` are all escaped. This is not a cosmetic rejection: on Windows `NUL.txt` RESOLVES TO A DEVICE, so the write succeeds and the bytes are discarded - the same class of silent loss the destination marker exists to prevent. Escaped unconditionally (not `cfg(windows)`) so a backup written on macOS containing `CON.txt` stays restorable on Windows - anything that would shadow a Driven control name, or a macOS AppleDouble shadow (see below) - over-long names truncate on a safe boundary (never splitting an escape) and gain a deterministic `~<digest>` tail Non-ASCII bytes pass through untouched. Encoding unconditionally costs a little cosmetic ugliness on an APFS destination and buys a property that matters much more for a backup: the layout is destination-INDEPENDENT, so a backup folder can be copied from an APFS disk onto a FAT32 stick, or restored from either, with no renaming and no re-upload. ## Case-insensitive collisions `Notes/Foo.txt` and `Notes/foo.txt` are two distinct source files that want one destination filename on exFAT, FAT32 or a default APFS volume. The naive answer destroys one of them while every later backup reports success. The probe deliberately **asks the destination**: it opens `<dir>/.driven-meta/<encoded>.json` and reads the ORIGINAL name recorded there. That inherits the destination filesystem's own equivalence relation for free - case folding, Unicode normalization, any locale-specific folding the driver applies - without Driven shipping a single table, and detects exactly the collisions that filesystem would actually have caused. On collision the name gets a deterministic `~<digest>` tail. Concurrency: the executor uploads several files from one source in parallel, and the probe cannot see a sidecar that has not been committed yet. `NameClaims` closes that - the probe and the claim happen together under one lock, and an in-flight claim counts as an owner exactly like a committed sidecar. **What it does not cover:** another PROCESS writing into the same destination folder. Driven runs single-instance and a destination folder is Driven's to manage, so the in-process guarantee is the whole guarantee; no primitive that works on FAT32 would fix the cross-process case (`O_EXCL` alone cannot distinguish "someone else's live claim" from "an orphan left by a crash"). ## Removable-media realities - **The destination-identity marker is the most important safety property here.** `root.exists()` is not the check: an unmounted NAS mount point is an ordinary empty directory, so existence alone would let Driven write a whole backup onto the boot disk underneath the mount, where it vanishes on the next remount while `file_state` still calls every file synced. That is total, silent backup loss. Account creation stamps (or ADOPTS) a `.driven-destination.json` carrying a UUID; every operation re-reads it, and absent-or-different means `drive.dest_folder_missing` with nothing written. It also catches "a different stick mounted at the same path". - errno mapping: `ENOSPC`/`EDQUOT` -> `StorageQuota` (the destination is full; the executor already pauses the account and resumes when space appears - `local.disk_full` stays what it always was, the restore-target disk); `EACCES`/`EPERM`/`EROFS` -> `DestFolderPermissionDenied`; `EIO`/`ESTALE`/`ETIMEDOUT`/`ENODEV`/... -> `Network` (retryable, for a flapping NAS or USB bridge); `EFBIG` -> fatal with a message naming FAT32 and exFAT. - **One `EIO` is never promoted to `DestFolderMissing`**: "the drive is gone" is decided by exactly one check (the marker), so a bad sector cannot stop the whole account with "reconnect your drive". - Abandoned resumable-upload temp files are swept at store construction, using the trait's own 6-day session window as the cutoff, so they cannot fill a small stick. ## Trash A plain filesystem has none, and Driven does not simulate one by moving objects into a hidden folder: nothing would ever empty it, so a backup destination would grow without bound and fill the drive it lives on. `trash` is a permanent delete, identical to `delete_permanent`, exactly as the S3 backend does - and the setup UI says so rather than pretending otherwise. ## Config + UI - `LocalFsConfig { root, destinationId }` in `accounts.backend_config_json`. There is no credential and this backend never touches the OS keychain. - `create_local_folder_account` validates the folder at save time by ACTUALLY writing, syncing and removing a probe file - a read-only mount, a restrictive ACL, an immutable flag and a full filesystem all pass a permissions-bit check and then fail the first real write. - `LocalFolderForm.vue` (new, with a vitest mount test) plus a store/wizard test file covering the non-OAuth branch end to end, on the wizard's step 2. With two credential-free destinations now in the picker, step 2 dispatches on the backend ID rather than on a second boolean - "which form" is a per-backend question. The folder is chosen through the backend-owned native dialog (SPEC s11.6.1 / C1), never typed. - Step 3 hides the Drive folder picker for this destination and shows the resolved `<root>/<sub-folder>` read-only instead. ## Windows The validation is platform-aware, not POSIX-shaped with the Windows case untested: - `D:\Backups` and a UNC share `\\server\share\Backups` (how a Windows user names a NAS - a destination this backend exists for) are ACCEPTED. - `Backups`, `..\Backups`, `\Backups` (relative to the current DRIVE) and `D:Backups` (relative to the current directory ON D:) are all REFUSED. The last two look rooted but move with the process's cwd, which is not a property a backup destination may have. - Trailing-separator trimming caught a genuine Windows bug (this is the one worth reading): `D:\` trimmed to `D:`, which is the drive-RELATIVE current directory rather than the drive root - so a user who picked the whole stick would have had their backups written wherever that process's cwd on `D:` happened to point. The trim now only applies while the result is still absolute, which covers POSIX `/` -> `""` by the same rule. Windows also accepts `/` as a separator, so both are trimmed there and only `/` on unix (a backslash is a legal unix filename character). The tests for this are platform-AWARE (`#[cfg(windows)]` constants selecting the path shapes) rather than `#[cfg(unix)]`-gated, so the Windows behaviour is actually asserted on windows-latest instead of skipped - including four `driven_localfs::config` tests that were `cfg(unix)` and now run on both. Two assertions are deliberately NOT made, each with the reason in a comment rather than a blanket gate: - A forward-slash UNC (`//server/share`) is not asserted as accepted. Rust's Windows path parser detects a UNC prefix by the literal `\\`, so that form has no prefix and `is_absolute()` is false. That is a limit on what the validator can PROVE, not a claim about Windows - and it is not a real input, because the folder always arrives from the backend-owned native dialog, which returns backslashes. - The 280-char-name e2e round trip is `#[cfg(not(windows))]`. A ~205-byte filename is well inside every target filesystem's 255-byte COMPONENT limit, but Windows also caps a full path at 260 characters unless long paths are enabled system-wide AND the process opts in via its manifest, which a `cargo test` binary does not - so the failure would be the harness's MAX_PATH, not the store's. The truncate-and-digest algorithm is pure string logic and is covered platform-neutrally in `names.rs`. Because cross-compiling to `x86_64-pc-windows-msvc` is blocked locally (`ring` needs Windows C headers), the Windows-only code paths were type-checked by temporarily inverting every `cfg(windows)`/`cfg(unix)` gate and compiling on macOS. That is what would have caught the stray `#[test]` attribute on a `#[cfg(windows)]` const that broke the previous Windows run - it is invisible to a macOS compile because the item it attaches to is stripped. ## Verification Round-trip tested against **three real filesystems** on `hdiutil`-created volumes plus a temp directory, via `tests/localfs_e2e.rs` (`DRIVEN_TEST_LOCALFS_ROOTS=/Volumes/A,/Volumes/B,...`; honest gate, not `#[ignore]`). 14 scenarios x 4 destinations, all green: | Destination | Result | |---|---| | tempdir (APFS, case-insensitive) | 14/14 | | `/Volumes/DrivenLfAPFS` (APFS) | 14/14 | | `/Volumes/DRIVENEXFAT` (exFAT) | 14/14 | | `/Volumes/DRIVENFAT` (FAT32 / msdos) | 14/14 | Covered: upload/list/download round trip with byte-for-byte and md5 comparison; nested folders; update-merges-properties; resumable upload across 4 MiB wire chunks; resume after a simulated process restart; wrong-offset refusal; trash + delete idempotence; `find_by_op_uuid`; the source audit; the case-collision case; the hostile-filename corpus (including MS-DOS device names and AppleDouble look-alikes); 280-char names sharing a prefix (the shape an ENCRYPTED source produces, since it encrypts filenames); `about()`. All volumes were detached and every `.dmg` deleted; `ls /Volumes` shows only `Macintosh HD`. ### Finding 1: macOS writes AppleDouble `._*` files on exFAT/FAT32 The first run against real exFAT and FAT32 FAILED, and the reason was worth the exercise. On a filesystem with no native xattr support, macOS transparently writes the xattrs and resource fork of `X` into a sibling `._X` - including `._.driven-meta` and `._.driven-destination.json`. Unfiltered they appeared in the destination picker, doubled every `list_folder`, and - much worse - would have been carried into the remote-existence audit as objects Driven owns with no `file_state` row, which the audit tries to heal forever. `._` is now a reserved control prefix (filtered from listings and audits, and escaped by the encoder so a user file genuinely named `._notes` cannot be mistaken for one). ### Finding 2: FAT32's 2-second mtime granularity is inert here Measured directly (the test prints it): | Destination | mtime granularity | |---|---| | tempdir / APFS | fine (ms-resolution, distinct within one second) | | exFAT | fine (~10 ms, distinct within one second) | | **FAT32 (msdos)** | **coarse - two writes 30 ms apart share one second-aligned timestamp** | It does not reach change detection. `RemoteEntry.modified_time` is read in exactly two places, both tie-breakers among duplicates (`find_by_op_uuid` choosing the most recent of several objects carrying one op uuid, and Drive's `ensure_folder` choosing the oldest of several same-named folders). Change detection reads the SOURCE file's mtime, which lives on the source volume and is unaffected by the destination's format; the sidecar carries its own millisecond timestamp; and correctness rests on the content digest, not the clock. The test pins that a same-second rewrite is still detected by content and still restores byte-for-byte. ### Finding 3: the FAT32 4 GiB ceiling, verified for real Verified against a 5 GB sparse FAT32 image by streaming a 4 GiB + 1 MiB object through `create()`. It fails with `EFBIG` (errno 27), surfaces as `drive.unreachable` carrying `"the destination filesystem refused the file as too large (FAT32 cannot store a single file of 4 GiB or more; reformat the volume as exFAT to lift the limit)"`, publishes no partial object, and leaves no temp file behind. Kept in the suite behind `DRIVEN_TEST_LOCALFS_FAT32_ROOT` (a 512 MB image cannot exercise it, so it prints why it did nothing when unset). There is no portable way to learn a destination's per-file ceiling before writing, so this is a post-hoc errno mapping by design - a ~6 GiB video on FAT32 fails after writing 4 GiB, not in milliseconds. ## Resume contract This backend deviates from the HTTP ones on `resume_chunk` after a restart. S3 returns `InProgress { received: 0 }` to force a full rewind; `LocalFsStore` instead hydrates from the temp file's on-disk length and re-hashes that prefix, so the reported `received` is derived from the bytes that ACTUALLY survived the crash rather than a remembered count, and the executor replays only what is genuinely missing. That relies on the executor treating `received` as authoritative in BOTH directions, not merely as a rewind signal. Checked, and it does: `executor.rs::push_chunks` re-slices the body with `offset = received` on every `InProgress` (it is also the crash-resume path, entered with `start_offset = acked_offset`). The streamed path never hydrates - it always opens a fresh session at offset 0 - so `received` there is always exactly `offset + chunk.len()`. Noted in the `resume_chunk` doc so a future edit to either side does not silently break it. ## Judgment calls - **No `verify_writes` knob.** Always re-read. A knob would invite someone to disable the only real corruption check on this backend. - **`supports_folder_picker = false`.** The destination root is chosen with the OS dialog; browsing below it would only offer a way to nest one backup inside another. Step 3 shows the resolved destination path read-only instead. - **Per-source destination sub-folder.** `add_source` requires a non-empty, whitespace-free `drive_folder_id`, so the wizard derives one from the source folder's own name (whitespace collapsed to `-`): `~/Documents` lands in `<destination>/Documents/`, which keeps the backup browsable and hand-restorable. Two sources whose names collapse to one id would share a sub-tree - exactly what happens on Drive when a user picks one destination folder twice, and visible on the source step. - **An unannotated file already sitting at a target path is overwritten**, with a warning. It is either a crashed create's orphan (where overwriting is required for the replay to land correctly) or a file the user put in Driven's destination folder; refusing would wedge the backup forever. - **Sidecars, not a `.tar`-style container.** A single file holding header + payload would make the commit perfectly atomic, but it would break `download()` streaming and destroy the "copy your files off the stick by hand" property that is most of the reason to back up to a folder you own. ## Not verified / known caveats - The cross-filesystem ROUND TRIPS ran on macOS 26 only. Windows and Linux path handling is unit-tested (and the path-shape tests really do run on windows-latest in CI), but no Windows NTFS/exFAT volume round trip was performed, and the MS-DOS device-name escaping was not validated against a real Windows volume. `volume_capacity` is deliberately unimplemented on Windows (`about()` reports an unknown limit rather than a guessed one). - **Windows MAX_PATH is a real product caveat, not just a test one.** A deeply nested destination plus long encoded names can exceed 260 characters, which Windows refuses unless long paths are enabled. No per-component cap can fix path DEPTH; a user hitting it sees the I/O error mapped through `crate::error`. Worth a follow-up (prefixing `\\?\` on Windows would lift it) but out of scope here. - The cross-process name-claim race is documented, not closed. - `guard_root` is a small file read plus a JSON parse on EVERY trait call. Hot in the page cache locally, but on the SMB/NFS mounts this backend explicitly targets, close-to-open semantics could make it a network round trip per object. A short-TTL cache would keep the safety property and remove the per-object cost; deliberately not done here, because correctness on an unplugged drive is the whole reason the check exists and a TTL is exactly where that gets subtle. - An interrupted resumable upload leaves its `SessionState` (and its name claim) in the in-process map until the store is dropped. Bounded by interruptions per app run, and the temp file itself is swept from disk; the map is not. ## Note for the reviewer This branch will conflict with #207 in `crates/driven-remote/src/backend.rs` (the `BackendKind` enum and its four `match`es), `crates/driven-backend/src/lib.rs`, `src-tauri/src/lib.rs`, `src-tauri/src/commands/accounts.rs`, `ui/src/stores/setup.ts`, `ui/src/views/SetupWizard.vue` and `ui/src/locales/en-US.json`. Whichever lands first, the other needs a re-cut; the conflicts are all additive-arm/additive-key. `cargo test --workspace` is green on the pushed commit (53 test binaries, 0 failures - both hangers pass here now that #200's keyring-mock fix is on main), as are `cargo clippy --workspace --all-targets -D warnings`, `cargo fmt --all --check`, `cargo deny check`, and the full UI suite (581 tests, including 6 new `LocalFolderForm` mount tests and 8 covering the local-folder store actions and wizard branch). 🤖 Generated with [Claude Code](https://claude.com/claude-code) https://claude.ai/code/session_01Qu8GxMwkuxF7JBzwRjtcw7
pmaxhogan
added a commit
that referenced
this pull request
Jul 30, 2026
Rebased onto `main` @ 9d67765 (after #201, #216, #213, #212, #203, #219). #207 already fixed the root cause for the two crates it touched, so this PR is now the **audit**, the **consolidation**, and the **docs** - the three halves it did not cover. ## Root cause (recap, for context) `keyring` 4.x's `Entry::new` installs the PLATFORM-NATIVE store as the process default on its **first** call, overwriting whatever default is already set (`keyring-4.1.5/src/v1.rs`, the `SET_CREDENTIAL_STORE` latch). So installing `keyring-core`'s mock *before* the first real `Entry` is silently undone, and every "mock" write lands in the real OS keychain. On macOS each rebuild is a new binary identity, so the OS re-prompts on every single rebuild and the run blocks on the modal dialog. Confirmed against the maintainer's login keychain before touching anything - it held the tests' own hard-coded account ids: ``` driven.google.refresh_token / acct-with-token (created 20260729180400Z) driven.google.client_creds / acct-byo driven.s3.credentials / acct-s3-round-trip (created 20260729181820Z) ``` ## 1. The audit: are there any other paths? **Verified answer: no. Every test that reaches a real `keyring::Entry` is now behind the shared helper - there are no unisolated paths left.** Done empirically rather than by inspection. A temporary probe panicked at all **four** `Entry` construction sites in the workspace - `driven-crypto/src/keystore.rs:63`, `driven-drive/src/google/token_store.rs:102` and `:157`, `driven-s3/src/config.rs:221` - and `cargo test --workspace --no-fail-fast` ran every test binary on this exact tree. Exactly **13** tests reach an entry, across 5 binaries: | Crate | Tests reaching an entry | Services touched | |---|---|---| | `driven-backend` | 3 (pre-existing) | `driven.google.refresh_token`, `driven.google.client_creds` | | `driven-s3` | 4 (pre-existing) | `driven.s3.credentials` | | `driven-crypto` | 3 (new here) | `dev.maxhogan.driven` | | `driven-drive` | 2 (new here) | `driven.google.refresh_token`, `driven.google.client_creds` | | `src-tauri` | 1 (new here) | `dev.maxhogan.driven` | All 13 go through `driven_test_fixtures::keychain::isolated()`. The probe hit list and the isolated-test list match exactly, with no remainder. Also checked and clear: `driven-cli` (its integration tests only exercise `--help` / missing-argument paths, so the spawned binary never opens an entry), every integration test under `crates/*/tests` and `src-tauri/tests`, and the two new crates from this week - `driven-localfs` and `driven-rclone` reference neither `keyring` nor any of the credential-store types. ## 2. Consolidation: one helper, not three `driven_test_fixtures::keychain` (`crates/driven-test-fixtures/src/keychain.rs`) is now the single implementation. Both of #207's local copies are repointed at it and `keyring-core` is dropped from both crates' dev-deps: - `crates/driven-backend/src/lib.rs` - 60-line local helper -> one-line delegate - `crates/driven-s3/src/config.rs` - same It keeps #207's load-bearing ordering (burn the latch with a throwaway `Entry`, *then* install the mock) and adds one thing #207's version lacks: **An I/O-free precondition ahead of the proof write.** The installed default store must report `CredentialPersistence::ProcessOnly`, which by definition means its credentials cannot outlive the process; a real OS keychain reports `UntilDelete`/`UntilReboot`. So a defeated mock is caught **before anything is written**. Under #207's version the sentinel write is itself the first thing that would leak into a real keychain when the mock fails. The sentinel round trip still runs, after the precondition passes, as the functional check. Constructing the throwaway `Entry` is safe: `build` in `apple-native-keyring-store-1.0.1/src/keychain.rs` is pure struct construction with no keychain I/O, so it raises no prompt and creates nothing. **Drift protection:** `the_test_suite_is_isolated_from_the_os_keychain` in every crate that owns a keychain call site - `driven-crypto`, `driven-drive`, `driven-s3`, `driven-backend`, `src-tauri`. These *assert* rather than skip, so a future `keyring` upgrade that breaks the mechanism fails loudly instead of the suite quietly resuming real writes. All five crates now have the dev-dep wired, so isolating a new test is a one-line change with no `Cargo.toml` work. Production is untouched: no shipped code path changed, and `keyring-core` is a direct dependency only of `driven-test-fixtures`, which is `publish = false` and only ever a `[dev-dependencies]` entry. ## 3. The docs half ### README - new "macOS re-prompts for keychain access after every update" Placed in the existing macOS caveats, between the APFS locked-file section and the auto-updater caveat. Verified, not restated: - Driven's macOS build carries **no Developer ID signature**. It is only ad-hoc (linker) signed, so it has no stable [designated requirement](https://developer.apple.com/library/archive/technotes/tn2206/_index.html) and its cdhash changes with every build. (`tauri.conf.json` sets no `signingIdentity`; `release.yml` runs no `codesign`.) - macOS pins keychain ACLs to the identity of the binary that was granted access. Apple states the rule directly: ["This dialog appears if you recently updated your system software or the app, or if the app has been modified"](https://support.apple.com/guide/keychain-access/if-a-trusted-app-asks-for-keychain-access-kyca1331/mac). So "Always Allow" does not carry across an update, for any of the four services Driven uses (`dev.maxhogan.driven`, `driven.google.refresh_token`, `driven.google.client_creds`, `driven.s3.credentials`). - Denying it is safe by construction - an encrypted source whose master key cannot be read fails closed (`crypto.key_missing`) rather than uploading plaintext - but it stalls encrypted sources until the user re-authorizes. - The only fix is a Developer ID signature. Stated without overstating: no entitlement or partial workaround makes an ad-hoc-signed build's grants survive an update. **Aligned with #216, not contradicting it.** #216's `fdaBanner.unsignedNote` already covers the TCC/Full Disk Access half ("macOS ties this permission to the app's signature, and Driven is not signed yet ... remove Driven from the Full Disk Access list and add it back"). The README's FDA bullet gives the same mechanism and the same remove-and-re-add remedy, and now explicitly says the in-app banner says the same thing. The genuinely new material is the **keychain** half, which nothing documented. ### DESIGN - 3.6 gains "macOS, second cost: permission grants do not survive an update" next to the existing "no Apple Developer ID" material. It defers the TCC half to 5.3.3 rather than restating it, and documents the keychain half. - 5.3.3 (#216's FDA onboarding section, which already explains the cdhash binding for TCC) gains a short pointer noting the same binding governs keychain ACLs, with a cross-reference to 3.6. ### CONTRIBUTING A local-gates subsection: the suite is keychain-isolated, how to isolate a new test, why macOS makes it load-bearing, and cleanup commands for anyone who ran the suite while the flawed helper was on `main`. ## Bonus: coverage that was previously impossible The old module docs in `driven-crypto/src/keystore.rs` and `driven-drive/src/google/token_store.rs` claimed the mock store was unusable and steered contributors away from testing these paths. Both are corrected, and the paths they excluded are now covered for real against the in-memory store: - `Keystore` store/load/delete master key, `NotFound` on a wiped keychain, idempotent delete, per-account scoping - `KeyringTokenStore` and `ClientCredsStore` round trips, incl. the empty-secret PKCE case and per-account isolation - `KeystoreCryptoProvider` with an encrypted source that HAS a wrapped key but no master key - the only path there that actually opens the keystore, which no existing test reached (every existing one short-circuits on a missing wrapped key). This is the GA-critical fail-closed rule. ## Verification: zero keychain prompts Prompts are interactive and the machine is locked, so this is proved, not asserted. Headless throughout - no GUI automation, no app launch. 1. **Complete audit.** The 4-site panic probe above enumerated every test in the workspace that constructs a keychain entry; all 13 are isolated. 2. **The five guard tests pass**, so the mock really is the effective default store in each of those binaries - the exact thing that was silently false. 3. **mdat unchanged.** After a full `cargo test --workspace`, the S3 item's `mdat` is still `20260729181820Z` - unchanged across multiple full runs and a targeted `cargo test -p driven-s3`, whose `credentials_round_trip_through_the_keychain` stores to that exact account. Nothing wrote. 4. **Nothing created.** The two Google items were cleaned up mid-session, so runs since then started from an empty state for those services. After the final full run, `dev.maxhogan.driven`, `driven.google.refresh_token`, `driven.google.client_creds`, the helper's own `driven.test.keyring-latch` / `driven.test.keyring-sentinel` and the test round-trip service are **all absent** - despite the suite performing real store/load/delete round trips against every production service. 5. **No dialog.** Every run executed non-interactively in the background and exited on its own, so nothing blocked on a modal prompt. ## Gates (all re-run after the rebase onto 9d67765) - `SQLX_OFFLINE=true cargo test --workspace` - pass, 0 failures - `SQLX_OFFLINE=true cargo clippy --workspace --all-targets -- -D warnings` - clean - `cargo fmt --all -- --check` - clean - `git diff --check` - clean - `cargo deny check` - `advisories ok, bans ok, licenses ok, sources ok` - No em/en dashes in the diff (checked) - UI suite not run: no UI file is touched by this PR. Note: `test:` is a hidden changelog type here, so the user-facing README/DESIGN macOS caveat will not appear in the release notes. Flagging deliberately - a follow-up `docs:` commit is your call.
pmaxhogan
added a commit
that referenced
this pull request
Jul 30, 2026
…tory Pass 2 of the 2.5.0 docs refresh (pass 1 was PR #209). Every DRAFT feature merged since then, so this flips them live in README.md and site-landing/index.html, verified against the merged code rather than restated from PR descriptions: - S3-compatible backend (#207), local/removable-folder backend (#212), scheduled integrity scrub (#203), and the rclone config importer (#213) all move from DRAFT comments to real bullets/feature cards. - Restore drill (#215) stays commented out: the core engine landed but is marked DRAFT/unmerged with no UI surface yet. - Guided macOS Full Disk Access onboarding (#216, already live in README prose from an earlier feature PR) gets its landing-page card. Footnote 6 no longer claims Google Drive is the only backend. The comparison table's "multiple storage backends" row flips to a checkmark; "point-in-time restore" drops to partial with a new footnote, because versioning does not actually retain older copies on S3 or local-folder destinations (issue #220) - a versioned change overwrites the same deterministic key on both backends, and nothing in the UI currently gates the toggle to prevent turning it on there. Docs now say exactly what the in-app copy already says instead of promising more. design/ROADMAP.md's "Beyond V1" section is checked off for everything that shipped, including the APFS snapshot broker actually being wired into driven-vss's map_for_volume seam (it was not, pre-#201) and FDA onboarding UI existing (it did not, pre-#216) - while restating, not watering down, that a snapshot never substitutes for Full Disk Access. design/DESIGN.md's crate table and non-goals section now reflect three destinations (driven-drive, driven-s3, driven-localfs) behind the driven-remote::RemoteStore trait plus the driven-backend factory, rather than Drive plus "maybe someday." The point-in-time-versioning section (s5.5.1) gets a post-V1 note explaining exactly why the Drive-trash-based mechanism it describes does not carry over to a backend with a deterministic remote key. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01Qu8GxMwkuxF7JBzwRjtcw7
pmaxhogan
added a commit
that referenced
this pull request
Jul 30, 2026
…tory (#225) Pass 2 of the 2.5.0 docs refresh (pass 1 was #209, which left DRAFT blocks commented out for features still in flight). Everything in those blocks has now merged, so this flips them live - verified against the merged code, not restated from PR descriptions. ## What flipped from DRAFT to live - **README.md** Features list + **site-landing/index.html** feature grid: S3-compatible backend (#207), local/removable-folder backend (#212), scheduled integrity scrub (#203), rclone config importer (#213). Guided macOS Full Disk Access onboarding (#216) gets its landing-page card (it was already live in README prose from an earlier pass). - **Restore drill (#215) stays commented out**: the core engine landed but the PR is DRAFT/unmerged with no UI surface a user can click yet. ## Corrections to reality, not just restated PR text - **Footnote 6** no longer says "Google Drive is Driven's only backend today" - flagged stale by two prior agents and left for this pass. - **Versioning does not work on S3 or local-folder destinations** (issue #220): a versioned change forces the create path, and both backends derive a deterministic remote key, so the re-upload overwrites the previous bytes. Nothing on `main` currently gates the per-source versioning toggle to prevent turning it on there (verified in `SourceTable.vue` and `executor.rs`); a fix is open in PR #224 but unmerged as of this writing. The comparison table drops "point-in-time restore" to partial for Driven with a footnote, and the docs now say exactly what the in-app copy already says instead of promising more. - **The integrity scrub's checksum coverage is backend-dependent**, not uniform: full on Drive and `driven-localfs` (which always re-hashes its own committed bytes), but size-only for an S3 object uploaded via multipart, because S3's ETag for one is a digest of part digests, not a plain content digest (`driven-s3`'s own `metadata()` doc comment says as much). The scrub's `classify()` correctly reports these as `Unverifiable` rather than a false pass - the docs now say so too, instead of implying uniform checksum verification. - **APFS snapshots are not a Full Disk Access substitute**, restated everywhere the feature is described (they were already careful about this pre-existing README prose; carried the same discipline into the ROADMAP/DESIGN updates). - Dropped a false "no code change needed to add the next backend" claim and an unconditional "Driven uses your own Google OAuth credentials" line that doesn't hold for the two non-OAuth backends. - Added a compact "Local / removable-folder caveats" note: no trash, FAT32's 4 GiB per-file ceiling (fatal, with a clear message), and Windows' classic 260-character path limit being a real, still-open caveat (`driven-localfs` has no `\\?\` long-path prefixing yet - per PR #212's own "not verified / known caveats" section). ## design/ROADMAP.md and design/DESIGN.md - ROADMAP's "Beyond V1" section is checked off for everything that shipped, including two things that were previously false even after the code landed: the APFS broker being wired into `driven-vss`'s `map_for_volume` seam (confirmed at the actual `assembly::build_vss` macOS call site, not just crate-doc claims - this exact class of gap is what pass 1 caught before), and the FDA onboarding UI existing. - DESIGN.md's crate table (s4.2) and non-goals (s2) now describe three destinations (`driven-drive`, `driven-s3`, `driven-localfs`) behind the `driven-remote::RemoteStore` trait plus the `driven-backend` factory, instead of Drive plus "maybe someday." The point-in-time-versioning section (s5.5.1) gets a note explaining why its Drive-trash-based design does not carry over to a backend with a deterministic key. ## Verification Every claim above was checked against the actual code on `main` (`crates/driven-s3`, `crates/driven-localfs`, `crates/driven-rclone`, `crates/driven-remote`, `crates/driven-apfs`, `crates/driven-vss`, `crates/driven-core/src/scrub.rs`, `ui/src/components/SourceTable.vue`, `ui/src/locales/en-US.json`, `src-tauri/src/assembly.rs`), not restated from PR descriptions. `git diff --check` is clean and the diff introduces no em/en-dashes or other non-ASCII beyond the file's existing superscript footnote / `§` conventions. CHANGELOG.md and version numbers are untouched (release-please's territory). 🤖 Generated with [Claude Code](https://claude.com/claude-code) https://claude.ai/code/session_01Qu8GxMwkuxF7JBzwRjtcw7 Co-authored-by: Claude Opus 5 <noreply@anthropic.com>
pmaxhogan
added a commit
that referenced
this pull request
Jul 30, 2026
🤖 I have created a release *beep* *boop* --- ## [2.5.0](v2.4.0...v2.5.0) (2026-07-30) ### Features * **cli:** import destinations from an existing rclone config ([#213](#213)) ([baaf7bd](baaf7bd)) * **core:** enable macOS locked-file backup via the APFS snapshot broker ([#201](#201)) ([ada822e](ada822e)) * **core:** local and removable-folder backup destination ([#212](#212)) ([c416a24](c416a24)) * **core:** macOS APFS snapshot broker for locked files ([#196](#196)) ([a5f105e](a5f105e)) * **core:** pluggable backup destination backends ([#200](#200)) ([871df59](871df59)) * **core:** S3-compatible backup destination ([#207](#207)) ([37acb03](37acb03)) * **core:** scheduled integrity scrub of remote objects ([#203](#203)) ([049c62a](049c62a)) * **ui:** guide macOS users to grant Full Disk Access when files are denied ([#216](#216)) ([aa5327e](aa5327e)) ### Bug Fixes * **ci:** wait for MinIO readiness before the S3 integration suite ([#226](#226)) ([fff471a](fff471a)) * **core:** classify macOS locked and permission-denied opens into the skip-and-report path ([#195](#195)) ([08d2864](08d2864)) * **core:** downgrade the APFS helper-dir check from fatal to advisory ([#211](#211)) ([65010ac](65010ac)) * **net:** redact proxy credentials from the diagnostic bundle ([#190](#190)) ([8e514f3](8e514f3)) * **net:** redact userinfo from PAC source in logs ([#208](#208)) ([8692bc0](8692bc0)) * **net:** refresh stale PAC scripts instead of pinning them for the process ([#191](#191)) ([18b0d43](18b0d43)) * **scanner:** route the deep-verify hash through the platform-open helper ([#193](#193)) ([3af5c65](3af5c65)) * **ui:** do not offer versioning on destinations that cannot honour it ([#224](#224)) ([857c8ba](857c8ba)) * **ui:** make the destination step backend-driven and stop copy claiming Drive behaviour ([#219](#219)) ([9d67765](9d67765)) * **ui:** tear down exclusion-preview listeners lost to an unmount race ([#206](#206)) ([2656c9f](2656c9f)) * **ui:** use a template tray icon on macOS ([#202](#202)) ([eaefa9a](eaefa9a)) --- This PR was generated with [Release Please](https://github.com/googleapis/release-please). See [documentation](https://github.com/googleapis/release-please#release-please).
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Reopened: the original #205 was auto-closed when its base branch
feat/macos-apfs-wiringwas deleted on #201's merge. Same content, rebased onto
mainso it now gets the full CImatrix (a stacked PR only ever ran the title check).
Completes the macOS locked-file story started by #195 (classification), #196 (the APFS
snapshot broker) and #201 (wiring). Those handle files a lock blocks; this handles the
other half - files macOS PRIVACY blocks.
What it does
When a cycle skips files with
local.permission_denied, the UI surfaces a dismissiblehint explaining that macOS privacy protection (TCC) is blocking them, with a button that
opens System Settings directly at the Full Disk Access pane.
The deep link is
x-apple.systempreferences:com.apple.preference.security?Privacy_AllFiles,verified four ways on macOS 26.6 rather than assumed:
Full Disk Access.Privacy & Security, so theanchor is load-bearing rather than a false positive.
openUrlgoes through the plugin's scope rather thanopen(1), the glob waschecked directly: it matches the target and rejects
file:///etc/passwd,https://evil.exampleandx-apple.systemprefsX:y.Two bugs caught that tests could not have
opener:defaultwould have silently blocked the link. Its scope permits onlymailto:/tel:/http/https; a custom scheme throws at runtime while every mockedtest passes. Fixed with a scoped
opener:allow-open-urlentry, confirmed present in thegenerated
capabilities.json.launch_status()is not apure read - its
NotAttempted -> InFlighttransition is what spawns consent. Statusaccessors are polled whenever the Settings tab opens, so merely opening Settings would
have asked for a password. Fixed with a regression test.
The accuracy point this PR exists to get right
A snapshot cannot substitute for Full Disk Access, and no doc here implies otherwise. The
-o noownersbypass was CVE-2020-9771, is patched, and is now an EDR-flagged signature -Driven never uses it. So the README carries a table splitting
local.file_locked(fix:the APFS toggle) from
local.permission_denied(fix: FDA, and nothing else will do).Also documented: because Driven ships unsigned, a granted FDA can silently lapse when the
binary is rebuilt, since macOS ties the grant to code identity.
Reconciled with #209
#209's docs refresh landed mid-flight saying "there is currently no in-app guided flow -
planned for a future release". Rather than adding a second FDA section, this edits #209's
in place, adds the grant steps and the unsigned-binary caveat it lacked, keeps its
"not a substitute for FDA" paragraph, and flips its
DRAFT - do not uncommentroadmapbullet live.
Banner dismissal is per-session rather than persisted:
localStorageappears nowhere inui/src, and the condition is genuinely unresolved until FDA is granted.