diff --git a/.abcd/work/DECISIONS.md b/.abcd/work/DECISIONS.md index 1ce81b8..f707174 100644 --- a/.abcd/work/DECISIONS.md +++ b/.abcd/work/DECISIONS.md @@ -30,3 +30,4 @@ a `.abcd/development/decisions/` ADR when it shapes architecture or is expensive - 2026-08-13: Bug-hunt round 7 (fresh hunt, 4 dimensions, 4 hunters; 23 unique candidates, one independent adversarial refuter each — 19 confirmed, 4 refuted). Fixed nine substantive defects: every work verb refuses a cargo store that is or resolves into `~/.ssh` before any read/write on the path (the store was the one configurable, ferry-written path with no ssh guard — `work pack` created a world-writable directory inside `~/.ssh` and wrote bundles there, while the guarded repo/`--out` paths prove hand-edited config is in the threat model); the pre-commit secret gate walks a collapsed untracked directory (`?? dir/` at default -unormal EISDIR'd the fail-closed read, wedging every sync after a capture created a new repo subdirectory, with advice a directory can never satisfy — the sibling backup pass already walked exactly this shape, and the round-3 test dodged it with a staged `add -A`); `ferry init ` wiring the already-configured repo carries `managed` (the round-6 carry keyed on the no-arg route, so `ferry init .` from inside the configured repo dropped the flag with no supported way back — `init --github` refuses while a repo is configured; the carry is gated on the route having WIRED a pre-existing worktree, because a clone or fresh seed can land on the recorded path after the old repo was deleted and must never inherit the flag — the final dual review caught exactly that hole in this round's first cut, both reviewers NO-SHIP, fixed in the single remediation round with an eval pinning the recreated-at-same-path case); `bundle import` pins HEAD to the managed branch before its initial commit (the same unpinned `git init` round 6 fixed for the fresh path, left on the import route); `init --github`'s success banner and existing-repo hint name `ferry sync` as the publish verb (the flow's final instruction claimed capture pushes and apply pulls — capture never commits or pushes, apply never touches the network, and the banner is the last thing the flow prints); drift/conflict guidance is keyed on the registry's Captures() flag, so status stops steering terminals/keybindings/emacs/iterm2-profiles at `ferry capture` and iterm2-profiles stops falling through apply/diff's hand-maintained domain list to the dotfiles wording (an existing eval forbade exactly this wording for diff but never ran status); `apply --deps` states when a failed installed-set snapshot suppressed the restore-record (the fail-closed suppression printed the same "installed 0 package(s)" as a benign re-run, and restore --packages silently lost the run); release.yml asserts the built binary reports the tag before checksumming (Go's linker silently drops -X for a moved symbol, so a rename would publish -dev-reporting binaries with valid checksums and provenance, nothing red) and verifies the published assets against the published checksums.txt after the release (the pairing install.sh depends on; runs on private repos where the attestation check is gated off); consistency-lint invariant 5 fails if `.abcd/.work.local/` is ever tracked, and `make preflight` runs the lint so the pre-push hook catches it before publication (the exclude rule is per-clone and absent in a fresh checkout, so `git add -A` staged the private tier with every gate green — ADR 0002's exclude-not-gitignore choice honoured, enforcement added where none existed). Docs: the compatibility contract covers the four work-domain versioned files (including the machine-crossing cargo manifest) and states their unversioned-is-refused rule plus the no-retention policy for journal runs and snapshots; README/tutorial scope the "never requires sudo" claim to name the apt `apply --deps` exception; doctor's synopsis says git (required) vs zsh/package manager (recommended); install.sh documents FERRY_VERSION and its pipe-placement footgun; evals README lists init_preserve_test.go; the drift page's remedy bullet covers all four repo-authoritative domains; the stale lint step labels name all the invariants; prune-releases refuses to prune when the just-published release is missing from a listing whose fetch failure the empty-grep guard would otherwise swallow. Refuted as non-defects: Brewfile inline-comment rejection (CHANGELOG documents the fail-closed trade-off verbatim, and comment-stripping would regress a legal `#` inside an option value); the release verify 20-minute timeout (measured runs finish in ~2 minutes — 9x headroom — and the burnt-version consequence is documented and one-click recoverable); the refusal-constant concatenation (vet folds constant concatenation and the const-ness is compile-enforced); FERRY_NO_NETWORK's absent header doc (a contract-optional test hook that installs nothing). Carried forward: capture's own completion hint never names `ferry sync` for a managed repo; installApt/installBrew swallow the package manager's stderr (exit-status-only errors); the round-6 `carryMachineScoped` sites now have eval coverage but the unit seam stays untested; work-domain state growth has no pruner (documented this round as deliberate for snapshots; journal runs are inert dead bytes a future keep-last-N sweep could clear); guided_apply buckets the repo-authoritative domains under a "dotfiles" label. - 2026-08-14: Bug-hunt round 8 (fresh hunt, 4 dimensions, 4 hunters; 22 unique candidates, one independent adversarial refuter each — 12 confirmed, 12 refuted, 2 hunter-self-refuted). Fixed six code defects: guided apply's risky walkthrough groups every FileDomain under its own name in registry order (a two-name literal switch predating fn-5 dropped keybindings/emacs/iterm2-profiles into the "dotfiles" bucket, so the group header and the bulk consent named a domain the user was not reviewing and one "yes" spanned the mixed bucket); capture's brew/npm re-dumps count a change only when the manifest's bytes actually changed (`brew bundle dump --force` rewrites unconditionally, so every clean re-run claimed "wrote 1 change(s)" over an empty `git status` and "nothing has drifted" was unreachable on any machine managing either domain), with the npm list written via backup.AtomicWrite and left untouched when identical; capture's terminal preference-domain compare resolves the local overlay first via terminalRepoStatusSource, exactly as apply/status/diff do (a domain captured to [l]ocal was re-offered as drifted forever, and re-accepting it shared would promote machine-divergent settings); the deps install/dump rails carry the package manager's combined output in their errors as the uninstall rail already did (a failed apply --deps surfaced as a bare "exit status 1"); backup.Engine snapshots run the resolved-parent containment guard inside snapshotCurrent so the exported Snapshot used by work receive is guarded identically to restore (a parent symlink out of $HOME let the pre-receive snapshot read and persist out-of-home content the write boundary then refused); the release version-stamp gate compares exactly against "ferry " in release.yml and release.sh (the dev line is next-release-plus--dev, so the substring grep passed an unstamped binary on precisely the modal release). Docs: the README/tutorial "one privileged step" claim covers restore --packages (the apt uninstall rail runs apt-get as root, per the code's own comments, and no user-facing page said so); the compatibility contract's claim-file row names pack, receive, and take-back as writers (receive.AppendClaim writes claim..json); the commands reference gains the agents parent-noun row; make preflight enters AGENTS.md/CONTRIBUTING.md; the consistency lint's private-tier advice mandates amending committed history (git rm --cached alone greens the gate while the push still publishes). Refuted as non-defects: the terminal-capture placeholder plaintext leak (placeholders exist only for content GateValue already rates High, and the re-export re-trips the gate); the hand-over sudo mkdir (round-1 adjudication stands — a host-OS setup example, not a ferry operation); the release how-to's five-step summary (explicitly non-exhaustive by its own structure); +build-metadata tags becoming latest (SemVer-correct and deliberate per the workflow's own comment); the attestation docs' missing visibility caveat (true of everything a reader can download); the retention-policy scoping, [work]-in-ferry.toml, topic-plan lifecycle, behavioral-in-evals-README, "works today" framing, attribution-rule carve-out (rule already scoped to credit; amending owner policy is an owner call), the pre-push hook's absent in-repo installation (dispatcher is deliberately external; hazard population equals hook population), and a range-based invariant-5 check (breaks CI's detached checkout — the advice-text fix shipped instead). Merge-gate remediation (one round, regressions reviewer's blocker): commands.md's apply row still enumerated the walkthrough groups as "dotfiles / agents / terminals" — the exact list the code fix deleted — now naming all six domains; folded in the reviewers' safe notes: the status drift dump was a fifth output-discarding site (internal/deps/status.go), a dump that creates an empty manifest now counts as a change (nil-vs-empty bytes compare equal, so existence is checked), groupRisky gained the unit tests it shipped without, the dump.go comment and capture's "exactly as apply" claim were corrected, and the CHANGELOG narrates the npm 0644 normalisation and receive's abort-first failure mode. Carried forward: capture's completion hint still never names ferry sync for a managed repo (adjudicated a nitpick, unfixed this round); the managed carry stays path-keyed (an origin-URL comparison would close it); terminalRepoStatusSource ignores an extensionless local overlay that apply's terminalExportBlob honours, and falls back to shared behind a poisoned overlay where apply fails closed (pre-existing, inherited by capture's compare); guided-apply's per-domain groups have no direct interactive eval; the darwin-only defaults export has no injectable seam, so the terminal-capture fix is pinned at the terminalRepoStatusSource seam rather than end-to-end. - 2026-08-15: Bug-hunt round 9 (fresh hunt, 4 dimensions, 4 hunters; 36 candidates, one independent adversarial refuter per finding with same-site findings grouped — 28 confirmed, 8 refuted/rejected). Fixed thirteen substantive defects: the pre-commit secret scan Lstat-gates every changed path (a symlink-following read pulled `~/.ssh/id_ed25519` into the process on the default sync path — a direct boundary breach — and a dirty submodule or symlink-to-directory EISDIR-wedged every sync with unsatisfiable advice); the collapsed-untracked-directory scan enumerates via `git ls-files --others --exclude-standard` so gitignored files and nested `.git` trees no longer false-block a sync `git add -A` would never publish; a shared terminal capture removes the per-machine overlay that would otherwise shadow it forever (status/apply/capture disagreed indefinitely, re-offering the domain on every run); status and capture render `{{ferry.secret …}}` placeholders through the store before comparing terminal domains — with the placeholder write made a byte-exact round-trip — so a secret-routed capture converges instead of reporting permanent drift while apply says in-sync; `npm ls -g` is parsed from stdout alone with stderr spliced into real errors (CombinedOutput fusion made the documented non-zero-exit tolerance dead code, disabling the domain on any peer-dep warning); `ferry diff` predicts the empty-over-substantial refusal as "would refuse" and an aborted apply states its rollback (the guard's abort policy itself adjudicated deliberate and untouched); repos ferry creates gitignore `deps/Brewfile.*.local` on every init route with the `--github` gate model rendered from the writer's own pattern set (the documented per-machine overlay was committed, synced, installed fleet-wide, and bundled); release publishing is idempotent and auto-release's heal predicate requires a published, asset-complete release (a red post-publish check was permanently un-re-runnable while a full re-run went green with the bypassed gates unrun); the by-hand checksum recipe is the single `make checksums VERSION=` invocation (the two-line form silently re-built dev-stamped binaries under a self-consistent manifest); the compatibility contract names restore snapshots as the second version-independent store (the refuse-newer rule never gated `restore --undo`); configuration.md stops directing users to "extend" the compiled-in iTerm2 allowlist and documents the committed-plist route that actually works; the single-branch `main` constraint on sync enters the reference, tutorial, and help text. Nitpicks fixed: conflict items in the guided walkthrough say "yes" will not overwrite them; clean skip-always targets are silent and counted in-sync; the receive unlock accumulates releases (latent double-lock leak); consistency-lint invariant 3 exempts only test files; zizmor's SARIF upload skips fork PRs (audit still gates); install.sh names a failed binary download; ci.yml's cross-compile comment stops claiming a pre-tag gate it is not (auto-release tags the same push in ~20s); the release how-to documents step 7's local NEXT.md reset; allowlist categories, route-jargon, ssh.md's enumeration, scaffold help, AGENTS.md's CI list, and the tutorial's drift-vs-conflict wording corrected. Refuted as non-defects: restore --packages count (unreachable via ferry-written state); auto-merge retitle disarm (same end state as the round-4/6 refutation, one-admin actor set, disarm step racy and clobbers hand-armed PRs); the green terminal-state annotation (three red runs already notified; a transition-fail fires unreliably on a non-monotonic two-counter sum); +metadata tags as latest (round-8 adjudication stands — the workflow comment weighs the latest/install.sh consequence explicitly, retention-escape applies to every suffixed tag by documented design); stale-binary checksums (local rehearsal file only, gitignored); the doubled push/pull_request CI runs (different trees, and a shared group would cancel required checks); the wizard interactive fallback (disclosed twice on the same doc page, conservative and secret-safe by design — Codex M3 chose fallback over error for redirected runs; wording tightened anyway). Carried forward: terminalRepoStatusSource's extensionless/poisoned-overlay divergences from terminalExportBlob (hand-authored state only; one shared resolver would close it); Apple Terminal's unfiltered export routinely trips the entropy gate on NSKeyedArchiver base64, possibly making that domain uncapturable on customised machines (new lead from the refutation pass, unadjudicated); the deps overlay glob is duplicated in cmd/init.go rather than shared from internal/deps; release.sh's render_next_md "also sourced by tests" claim matches nothing, and a NEXT.md missing its carry markers makes step 7 exit the driver non-zero after a successful tag push; the heal predicate hardcodes the 5-asset count; sync's branch-rename hint still omits the remote-side default-branch step; machine-config `managed` remains path-keyed. +- 2026-08-16: Bug-hunt round 10 (fresh hunt, 4 dimensions, 4 hunters; ~30 unique candidates, 6 adversarial refuters over grouped clusters — 8 substantive and ~19 nitpicks confirmed, ~10 refuted). Fixed eight substantive defects: the terminal capture secret gate scans a ``-masked view of the exported plist (NSKeyedArchiver base64 archives — any customised Apple Terminal profile — scored as high-entropy credentials, blocking both repo routes and leaving the domain uncapturable; `` values stay fully scanned and an unterminated `` ends the masking); the secret-routed terminal capture removes a superseded local overlay exactly as a shared accept does (the placeholder was written behind the overlay, so apply kept importing — and silently re-applying — the stale bytes forever); the status-side terminal resolver probes both overlay spellings apply probes; aptInstalledSet treats an output-less probe error as snapshot-unreliable rather than "absent" (a before-snapshot probe failure recorded a pre-existing package as ferry-installed, handing `sudo restore --packages` a package the user already had; a symmetric lookup failure silently lost the record with no warning — the brew prober had exactly this guard and test, apt did not); `work pack --acknowledge`'s unmatched list masks a PathSecret component instead of echoing the path the same message withholds; `bundle import` refuses secret-shaped path components via the same per-component predicate export uses (moved to internal/secret so the two sides cannot drift; refusal withholds the path); gitconfig's inline-header split and subsection derivation share one close-bracket scanner honouring git's in-quote `\"`/`\\` escapes (a legal escaped quote swallowed a same-line assignment into the header's raw bytes, which for non-includeIf sections passed the identity firewall into SharedContent — the non-conservative direction; round-trip fuzz re-proved byte-exact); release.sh refuses a version that is not the newest dated CHANGELOG heading (the driver tags HEAD, recreating the mis-pointed-immutable-tag hazard auto-release refuses) and pre-checks NEXT.md's carry markers before the tag push. The CHANGELOG's [Unreleased] section is consolidated (the round-7 directory-walk entry contradicted the round-9 ls-files entry that replaced it — both would have shipped verbatim in the Release body — and two stray blank lines split the Fixed list). Docs: the drift explanation subjects first-touch adoption to the risky gate (it claimed unconditional deployment; scripted apply fails closed non-zero); scaffold help names NEXT.md (the --private list named it, so the omission read as a false negative); the release how-to documents step 7's post-tag NEXT.md reset (the round-9 log recorded this fix as shipped when no such sentence existed — the addition makes that ledger line true) and the widened re-release condition (missing, draft, or short of assets); the terminal secret route and placeholder are documented; the bundle local-layer gate is scoped to `local/**` + `ferry.local.toml` (a force-tracked deps overlay travels like any tracked file — round 9's eval adjudicated gitignore, not a wider gate, as the remedy); deps/README names the `brew = true` gate its own npm section already mirrors; the evals map lists deps_local_overlay_ignore_test.go (40th file); roadmap clauses leave commands.md and the tutorial; `init --fresh` enters the commands reference; sync's branch-rename hint names the remote default-branch step. Hygiene: registry_test's four-domain enumeration, writeDomain's no-op sort (the TOML encoder documents key sorting), groupRisky's misattributed fallback comment and test name (the defensive branch stays), the unreachable SkipDir arm in agents adoption (a latent trap: had it ever fired for a non-dir it would skip the remaining siblings), sync_round3_test's stale walk-parity claim, printPlan's consumer comments, allowlist.go's ambiguous "extend" advice, the Overlay comment's missing gitconfig, and release.sh's untrue "sourced by tests" claim. Refuted as non-defects: gitleaks/zizmor gating no release path ("blocking" is the required-check sense fixed in the v0.4.0 plan of record; the release comment shows the three-gate mirror was a considered boundary; a tag names an already-scanned merged main commit); the how-to's step list omitting verify (names verify in the sentence introducing the list; round-8 bar); prune's undocumented listing-integrity exits (error handling, not retention policy); install.sh "verifies each binary" (the set it downloads is a singleton, stated one line later); the binary-blob push-range scan gap (ScanText is a strict superset of HasBinarySecret; the residual divergence over-blocks); PathSecret findings being unacknowledgeable (a correctly-typed ack works; the real gap was the echo, fixed); reDump's existedBefore (any non-NotExist Lstat failure also fails the dump; documented tolerance); planSummary's parameter order (deliberate and documented in place, pinned by test); the .gitignore non-atomic write (the premise "every other repo write is atomic" is false — init-route writers are uniformly plain; class-wide call left to the owner); groupRisky's fallback as a defect (defensive default, kept). Carried forward: a present-but-refused local terminal overlay still falls back to shared in status where apply fails closed (surfacing "refused" needs a tri-state through reportTerminalStatus — an existing test pins the fallback); work pack's path gate scans the whole rel where export gates per component, missing nested high-entropy filenames despite its parity comment (new lead from the refutation pass, unadjudicated); the auto-release heal predicate hardcodes the 5-asset count; machine-config `managed` remains path-keyed; the secret scan's Lstat-to-read TOCTOU (O_NOFOLLOW would close it); journal runs remain unpruned; the deps-overlay glob stays duplicated in cmd/init.go; the placeholder-render fix stays forward-only for pre-round-9 placeholders. diff --git a/CHANGELOG.md b/CHANGELOG.md index 81d2f0a..dad553b 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -46,6 +46,74 @@ called out in a **Breaking** section. See ### Fixed +- **A customised Apple Terminal profile no longer trips the capture secret + gate.** The whole-plist export carries profile colours and fonts as base64 + `` archives, whose entropy the scanner read as credential material, + so any customised profile blocked both repo routes and offered only reject + or the out-of-repo store — leaving the domain effectively uncapturable, and + non-portable via the store. The gate scans a view of the plist with + `` payloads masked: a pasteable token lives in a `` value, + which stays fully scanned, while base64-encoded binary the text scanner + could never read into stops feeding the entropy detector. An unterminated + `` element ends the masking, so malformed input is scanned in full. +- **A secret-routed terminal capture supersedes the local overlay.** A + shared accept removes a superseded per-machine overlay, but the secret + route wrote its placeholder behind one, so `apply` kept importing the + stale overlay — silently reverting the settings the capture had just + routed out of band — while `status` and `capture` re-offered the domain + forever. The placeholder write removes the overlay exactly as a shared + accept does, reporting the removal; a symlinked overlay is left untouched. +- **`status` and `capture` honour an extensionless terminal overlay.** + `apply` probes both overlay spellings (`.plist` and bare ``); the + status-side resolver probed only the `.plist` spelling, so a hand-authored + extensionless overlay was what `apply` imported while `status` and + `capture` compared against the shared copy. Both spellings are probed + everywhere. +- **A failed apt probe cannot record a pre-existing package as + ferry-installed.** `dpkg-query` reports a not-installed package with a + diagnostic in its output; an exec-level probe failure returns no output at + all, which the installed-set prober read as "absent" — so a probe failure + in the before-snapshot could hand `restore --packages` a package the user + already had, and a lookup failure hitting both snapshots silently lost the + run's record with no warning. A probe error with no output marks the + snapshot unreliable, suppressing the restore record and saying so. +- **`work pack --acknowledge` never echoes a withheld path.** A finding on a + secret-shaped file name renders as "(name withheld)", but the acknowledge + retry's "covered some findings, but not: …" list printed the raw path in + the same message. The unmatched list masks the secret-shaped component, + keeping the surrounding path visible so the finding stays identifiable. +- **`bundle import` refuses a secret-shaped path component.** Export + withholds such entries, but import validated only the canonical path + shape, so a handcrafted bundle could plant a token-named file that wedged + every later `ferry sync` far from the cause. Import applies the same + per-component gate export uses — shared as one predicate so the two sides + cannot drift — refusing with the path withheld. +- **gitconfig section headers honour backslash escapes.** The inline-header + split and the subsection derivation both mis-read a `\"` (and an unquoted + `]` inside quotes) in a quoted subsection name, swallowing a same-line + assignment into the header's raw bytes, where the identity firewall could + pass it through to the shared repo. One shared scanner honours git's + in-quote escapes for both; parsing stays total and reassembly byte-exact. +- **`release.sh` refuses a version that is not the newest dated CHANGELOG + section.** The driver tags HEAD, so accepting an older still-untagged + version would point an immutable tag at newer main code under an older + name — the mis-pointing the automatic path already refuses. The driver + also checks NEXT.md's carry markers before the tag push, so a malformed + local file fails the run before the irreversible act instead of after it. +- **`ferry sync`'s branch-rename hint covers the remote side.** The hint + named only the local `git branch -M main`; it adds setting `main` as the + remote's default branch so other clones follow. +- **Docs corrected against the code.** The drift explanation subjects + first-touch adoption to the risky-change gate (confirmed interactively, + refused with a non-zero exit unattended); the scaffold help names NEXT.md + in the runtime layout both modes create; the release how-to documents the + driver's post-tag NEXT.md reset and the real re-release condition + (missing, still a draft, or short of its assets); the configuration + reference documents the terminal secret route and its placeholder, and + scopes the bundle's local-layer gate to `local/**` and `ferry.local.toml`; + deps/README names the `brew = true` gate; the evals file map lists the + deps-overlay eval; roadmap clauses leave the reference and tutorial; the + commands reference notes `init --fresh`. - **The pre-commit secret scan never reads through a repo symlink.** `ferry sync`'s changed-file scan opened paths with a symlink-following read, so an untracked or modified symlink in the config repo — for @@ -60,13 +128,6 @@ called out in a **Breaking** section. See read aborted every sync with advice ("re-run once the file is readable") a directory can never satisfy. Such entries are skipped — a gitlink's content never enters the push range. -- **The untracked-directory secret scan matches what `git add -A` - stages.** The walk over a collapsed untracked directory read every file - beneath it, including gitignored files and nested repositories' `.git` - internals, so a token in a file git would never commit blocked the sync - with no way forward. The directory is now enumerated with - `git ls-files --others --exclude-standard`, scanning exactly the - stageable set; a failed enumeration still aborts. - **A shared terminal capture behind a local overlay converges.** Accepting a terminal preference domain to shared wrote the shared plist, but the per-machine overlay from an earlier local capture kept @@ -150,7 +211,6 @@ called out in a **Breaking** section. See the cargo-store guard; the scaffold help names exactly what it creates; AGENTS.md's CI list names the gitleaks and zizmor gates; the tutorial distinguishes local drift from a conflict. - - **A cargo store under `~/.ssh` is refused.** The `[work] store` path is hand-configured, and every other configurable path ferry writes through — the repo path, `bundle import --out` — is guarded against resolving into `~/.ssh` @@ -163,9 +223,11 @@ called out in a **Breaking** section. See collapsed entry, which the pre-commit secret gate tried to read as a file; the resulting fail-closed abort ("could not read a changed file … is a directory") recurred on every run, with advice a directory can never - satisfy, until the user staged the files by hand. The gate now walks a - collapsed directory and scans every file inside, exactly as the sync backup - pass already did. + satisfy, until the user staged the files by hand. The gate enumerates a + collapsed directory with `git ls-files --others --exclude-standard`, + scanning exactly the set `git add -A` would stage — gitignored files and + nested repositories' `.git` internals stay outside the gate — and a + failed enumeration still aborts. - **`ferry init ` naming the configured repo keeps `managed`.** The carry-forward shipped earlier keyed on the no-argument re-run route, so passing the already-configured repo's path positionally (`ferry init .` from @@ -348,7 +410,6 @@ called out in a **Breaking** section. See set, `make release` documentation passes `VERSION=vX.Y.Z`, and CONTRIBUTING's commit-type list includes the `ci` type the history and auto-merge workflow already use. - - **The configuration reference documents the Apple Terminal domain.** The `terminal` key has been implemented end-to-end — apply, capture, restore, and the eval suite — but appeared nowhere in the reference: the `[manage]` diff --git a/cmd/agents.go b/cmd/agents.go index e1730ba..5af999b 100644 --- a/cmd/agents.go +++ b/cmd/agents.go @@ -36,7 +36,7 @@ var agentsScaffoldCmd = &cobra.Command{ Short: "Set a project repo up for the multi-tool agent pipeline", Long: `Set a project repo up for the multi-tool agent pipeline. -Both modes create the local-only runtime layout: .abcd/.work.local/{scratch,logs}, +Both modes create the local-only runtime layout: .abcd/.work.local/{NEXT.md,scratch,logs}, hidden via the git info/exclude mechanism (never committed; .gitignore is never touched). diff --git a/cmd/apply.go b/cmd/apply.go index cb0c682..8f2d1ce 100644 --- a/cmd/apply.go +++ b/cmd/apply.go @@ -1082,7 +1082,8 @@ func applyNpmGlobals(ctx *cmdContext, out io.Writer) error { return nil } -// printPlan renders the planned actions (diff / status). For dotfile/overlay +// printPlan renders the planned actions (diff, and init's closing plan +// preview; status renders its own report). For dotfile/overlay // targets it prints the REAL three-way classification computed during planning // (it.state) — the same resolution apply acts on — rather than a blanket "would // deploy": a clean target is shown clean, a conflict as conflict, a missing diff --git a/cmd/capture.go b/cmd/capture.go index 6d60fac..d2c5544 100644 --- a/cmd/capture.go +++ b/cmd/capture.go @@ -1097,7 +1097,7 @@ func captureTerminalDomain(cc captureCtx, domain string) (wrote bool, offered bo // MANDATORY secret gate BEFORE any write: scan the WHOLE exported plist value. A // high-confidence secret blocks every repo route (shared AND local); only reject // / the out-of-repo secret store are offered. - gate := secret.GateValue(string(liveBlob)) + gate := secret.GateValue(terminalGateInput(liveBlob)) if gate.BlockedFromRepo { w, gerr := captureBlockedTerminal(cc, domain, prefID, repoDest, liveBlob) return w, true, gerr @@ -1135,6 +1135,62 @@ func captureTerminalDomain(cc captureCtx, domain string) (wrote bool, offered bo } } +// terminalGateInput reduces what the MANDATORY pre-write secret gate READS for a +// whole-plist terminal domain (both Apple Terminal's raw export and iTerm2's +// allowlist-filtered one): the base64 payload inside every XML plist +// … element is masked out. Only the gate INPUT changes — the routes, +// the gate's authority, and the bytes written on accept are all untouched; the +// blob that reaches the repo is still the raw export. +// +// A element is base64 of binary NSKeyedArchiver output — how Apple +// Terminal stores every customised profile attribute (colours, font, cursor). The +// TEXT scanner cannot read into base64 anyway: a token encoded there is +// unmatchable by every named detector, so masking loses no real coverage. What it +// does remove is a permanent false positive — wrapped base64 lines clear the +// entropy detector's length and shape floors, so any customised Apple Terminal +// profile blocked the whole domain from the repo forever, leaving only reject and +// the (non-portable) secret store. values, where a pasteable token would +// actually live, stay fully scanned. +func terminalGateInput(blob []byte) string { + const openTag, closeTag = "", "" + rest := string(blob) + var b strings.Builder + b.Grow(len(rest)) + for { + open := strings.Index(rest, openTag) + if open < 0 { + break + } + payload := open + len(openTag) + end := strings.Index(rest[payload:], closeTag) + if end < 0 { + // UNTERMINATED : the payload's extent is unknown, so nothing more + // is masked. Malformed input fails toward scanning MORE, never less. + break + } + b.WriteString(rest[:payload]) + b.WriteString(maskBase64Payload(rest[payload : payload+end])) + rest = rest[payload+end:] + } + b.WriteString(rest) + return b.String() +} + +// maskBase64Payload replaces every base64-alphabet character of a plist +// body with 'x', leaving whitespace (and anything else) in place so the masked +// text keeps the original's line structure. A run of 'x' carries zero entropy and +// no digit, so it trips neither the entropy heuristic nor any named detector. +func maskBase64Payload(payload string) string { + return strings.Map(func(r rune) rune { + switch { + case r >= 'a' && r <= 'z', r >= 'A' && r <= 'Z', r >= '0' && r <= '9', + r == '+', r == '/', r == '=': + return 'x' + } + return r + }, payload) +} + // captureBlockedTerminal handles a terminal domain whose exported plist holds a // high-confidence secret. It is NEVER written to shared or local (both live in the // repo worktree); only reject or the out-of-repo secret store are offered, mirroring @@ -1162,33 +1218,48 @@ func captureBlockedTerminal(cc captureCtx, domain, prefID, repoDest string, live return false, err } fmt.Fprintf(cc.out, " %s: secret stored out-of-band in ~/.config/ferry/secrets-local; a placeholder was written to the repo\n", domain) + // The placeholder lands at the SHARED repo path, so a surviving per-machine + // overlay from an earlier [l]ocal capture would keep winning every comparison + // and apply would keep importing the stale overlay instead. Drop it on the same + // terms the shared accept does (the stored secret is neither re-read nor + // re-written here). + if err := removeSupersededLocalOverlays(cc.out, cc.repoPath, domain, prefID); err != nil { + return true, err + } return true, nil } // acceptTerminalShared writes an accepted whole-domain export to the SHARED repo // path and SUPERSEDES any per-machine local overlay for the same domain, so the // shared accept actually converges. +func acceptTerminalShared(out io.Writer, repo, domain, prefID string, blob []byte) error { + dest := terminalRepoDest(repo, domain, prefID) + if err := writeRepoFile(repo, dest, blob); err != nil { + return err + } + fmt.Fprintf(out, " %s: captured -> shared (%s)\n", domain, relTo(repo, dest)) + return removeSupersededLocalOverlays(out, repo, domain, prefID) +} + +// removeSupersededLocalOverlays drops any per-machine local overlay for a terminal +// domain after something has been written to that domain's SHARED repo path (an +// accepted export, or the placeholder of a secret-routed capture). // // Every comparison of this domain (status's terminalLiveDiffers, capture's own // compare, apply's terminalExportBlob) resolves LOCAL-WINS: while an overlay -// exists it shadows the shared copy. A shared capture written behind a surviving -// overlay therefore changed nothing observable — status reported drift forever, -// capture re-offered the domain forever, and apply kept importing the stale -// overlay. The accept is an explicit instruction to make THESE bytes the ones -// this machine carries, so the superseded overlay is removed and the removal is -// reported by path and reason. +// exists it shadows the shared copy. A shared write left behind a surviving +// overlay therefore changes nothing observable — status reports drift forever, +// capture re-offers the domain forever, and apply keeps importing the stale +// overlay. The capture is an explicit instruction to make the shared bytes the +// ones this machine carries, so the superseded overlay is removed and the removal +// is reported by path and reason. // // A removal failure is an ERROR, never a silent continue: leaving the overlay in // place would keep the machine on the stale bytes while the capture reported // success. An overlay that ferry REFUSES to read (symlinked/escaping — see // regularRepoFile, which guards before it stats) never wins a comparison in the // first place, so it is left exactly as found. -func acceptTerminalShared(out io.Writer, repo, domain, prefID string, blob []byte) error { - dest := terminalRepoDest(repo, domain, prefID) - if err := writeRepoFile(repo, dest, blob); err != nil { - return err - } - fmt.Fprintf(out, " %s: captured -> shared (%s)\n", domain, relTo(repo, dest)) +func removeSupersededLocalOverlays(out io.Writer, repo, domain, prefID string) error { // Probe BOTH overlay names apply's terminalExportBlob accepts (.plist and // the extensionless ), so no spelling of the overlay is left to shadow the // shared copy on the next apply. @@ -1206,7 +1277,7 @@ func acceptTerminalShared(out io.Writer, repo, domain, prefID string, blob []byt if err := os.Remove(safe); err != nil { return fmt.Errorf("remove superseded local overlay %s: %w", relTo(repo, cand), err) } - fmt.Fprintf(out, " %s: removed the local overlay %s (superseded by this shared capture; it would otherwise keep winning over the shared copy)\n", domain, relTo(repo, cand)) + fmt.Fprintf(out, " %s: removed the local overlay %s (superseded by this capture of the shared copy; it would otherwise keep winning over the shared copy)\n", domain, relTo(repo, cand)) } return nil } diff --git a/cmd/export.go b/cmd/export.go index 4dee007..d26b323 100644 --- a/cmd/export.go +++ b/cmd/export.go @@ -275,13 +275,12 @@ func isLocalLayerRel(slash string) bool { // secretInPath reports whether any component of a forward-slash relative path is a // high-confidence secret-shaped token (M10). Each component is scanned as an opaque // value so a token used as a filename is caught. +// +// It delegates to secret.IsPathBlockedFromRepo, the ONE owner of the predicate: +// bundle import/validate applies the same function, so the export and import path +// gates cannot drift apart. func secretInPath(slash string) bool { - for _, seg := range strings.Split(slash, "/") { - if secret.GateValue(seg).BlockedFromRepo { - return true - } - } - return false + return secret.IsPathBlockedFromRepo(slash) } // isProbablyText reports whether data is safe to run the text secret gate over. A diff --git a/cmd/guided_apply.go b/cmd/guided_apply.go index 92e2bf4..b1afe6a 100644 --- a/cmd/guided_apply.go +++ b/cmd/guided_apply.go @@ -290,8 +290,11 @@ func groupRisky(risky []planItem) []riskyGroup { for _, it := range risky { name := it.fileDomain if name == "" { - // Non-FileDomain items (terminal preference domains) carry no - // fileDomain; they keep their pre-existing dotfiles-bucket home. + // Defensive default for an item that reached the walkthrough + // without a fileDomain stamp. Unreachable today — the plan driver + // stamps every FileDomain item, and preference items are filtered + // out before grouping — but a future planner that forgets the + // stamp degrades to a visible bucket instead of an unnamed group. name = "dotfiles" } if !slices.Contains(order, name) { diff --git a/cmd/guided_apply_group_test.go b/cmd/guided_apply_group_test.go index 5ad7db6..d19b36a 100644 --- a/cmd/guided_apply_group_test.go +++ b/cmd/guided_apply_group_test.go @@ -5,8 +5,10 @@ import "testing" // groupRisky once bucketed every FileDomain it did not name into "dotfiles", // so the walkthrough's group header and bulk consent named a domain the user // was not reviewing. These tests pin the fix: every domain groups under its -// own name, in registry order, and only non-FileDomain items (which carry no -// fileDomain) fall back to the dotfiles bucket. +// own name, in registry order, and an item missing its fileDomain stamp +// (unreachable in production — the plan driver stamps every FileDomain item, +// and preference items are filtered out before grouping) degrades to the +// visible dotfiles bucket instead of an unnamed group. func TestGroupRiskyGroupsEveryDomainUnderItsOwnName(t *testing.T) { risky := []planItem{ {domain: "iterm2-profiles:p.json", fileDomain: "iterm2-profiles"}, @@ -44,12 +46,15 @@ func TestGroupRiskyNeverLumpsOtherDomainsIntoDotfiles(t *testing.T) { } } -func TestGroupRiskyFileDomainlessItemsKeepDotfilesBucket(t *testing.T) { +func TestGroupRiskyUnstampedItemsDegradeToDotfilesBucket(t *testing.T) { + // Hand-built state: no production path emits a risky item without a + // fileDomain stamp. The defensive default must keep such an item visible + // under a named bucket rather than an unnamed group. risky := []planItem{ - {domain: "iterm2 preference domain", fileDomain: ""}, + {domain: "unstamped item", fileDomain: ""}, } groups := groupRisky(risky) if len(groups) != 1 || groups[0].name != "dotfiles" { - t.Fatalf("fileDomain-less item grouped as %+v, want the pre-existing dotfiles bucket", groups) + t.Fatalf("unstamped item grouped as %+v, want the dotfiles fallback bucket", groups) } } diff --git a/cmd/registry.go b/cmd/registry.go index ec5fb1f..cf130c9 100644 --- a/cmd/registry.go +++ b/cmd/registry.go @@ -143,9 +143,10 @@ type dotfilesFileDomain struct{} func (dotfilesFileDomain) Name() string { return "dotfiles" } // Overlay reports the per-machine overlay strategy for a bare dotfile name. The -// include-style names — the zsh family (zshrc/zshenv/zprofile) and tmux -// (tmux.conf) — compose their overlay as a sourced sidecar (their format has a -// real include point: shell `source`, tmux `source-file`); every other dotfile +// include-style names — the zsh family (zshrc/zshenv/zprofile), tmux +// (tmux.conf) and git (gitconfig) — compose their overlay as a sourced sidecar +// (their format has a real include point: shell `source`, tmux `source-file`, +// git's native `[include]`); every other dotfile // is a whole-file replace. This is the authoritative replacement for the isZsh() // oracle, and the single trigger the two-strip contract keys off (via // usesIncludeSidecar). diff --git a/cmd/registry_test.go b/cmd/registry_test.go index aef0df8..05ab9ce 100644 --- a/cmd/registry_test.go +++ b/cmd/registry_test.go @@ -83,10 +83,10 @@ func TestFileDomainCaptures(t *testing.T) { } // TestBuildRegistryOrder pins the LOAD-BEARING registry order: FileDomains are -// [dotfiles, agents, terminals, keybindings] and ResourceDomains are -// [iterm2, terminal], matching the pre-fn-5 dispatch sequence so -// plan/status/diff/capture output ordering is unchanged (keybindings appends -// last, after terminals, so existing ordering is unchanged). Each FileDomain must +// [dotfiles, agents, terminals, keybindings, emacs, iterm2-profiles] and +// ResourceDomains are [iterm2, terminal], matching the pre-fn-5 dispatch +// sequence so plan/status/diff/capture output ordering is unchanged (new +// domains append last, so existing ordering is unchanged). Each FileDomain must // also satisfy the cmd filePlanner upcast the plan driver relies on. func TestBuildRegistryOrder(t *testing.T) { reg := buildRegistry(&cmdContext{RepoPath: t.TempDir()}) diff --git a/cmd/status.go b/cmd/status.go index 5ab5648..b3a5e7b 100644 --- a/cmd/status.go +++ b/cmd/status.go @@ -263,13 +263,22 @@ func terminalSecretStore() *secret.Store { // (local//.plist) when present, else the shared committed copy // (iterm2/.plist or terminal/.plist). func terminalRepoStatusSource(repo, domain, prefID string) string { - local := filepath.Join(repo, "local", domain, prefID+".plist") - // Guard-first probe: regularRepoFile routes `local` through safeRepoPath BEFORE - // any stat, so a symlinked repo plist (or a symlinked PARENT into ~/.ssh) is - // never stat'd here — it refuses before the read. Falling through to the shared - // copy keeps status read-only and safe. - if regularRepoFile(repo, local) { - return local + // Probe BOTH overlay spellings apply's terminalExportBlob accepts (.plist + // and the extensionless ): an overlay apply imports but this resolution + // skipped past left status reporting drift forever and capture re-offering the + // domain forever. + // + // Guard-first probe: regularRepoFile routes each candidate through safeRepoPath + // BEFORE any stat, so a symlinked repo plist (or a symlinked PARENT into + // ~/.ssh) is never stat'd here — it refuses before the read. Falling through to + // the shared copy keeps status read-only and safe. + for _, local := range []string{ + filepath.Join(repo, "local", domain, prefID+".plist"), + filepath.Join(repo, "local", domain, prefID), + } { + if regularRepoFile(repo, local) { + return local + } } if domain == "iterm2" { return filepath.Join(repo, "iterm2", prefID+".plist") diff --git a/cmd/sync.go b/cmd/sync.go index 6184f6b..64504f7 100644 --- a/cmd/sync.go +++ b/cmd/sync.go @@ -290,7 +290,7 @@ func refuseInProgressGitOp(repo string) error { return fmt.Errorf("sync: refusing to run on a detached HEAD — check out the `%s` branch first (`git checkout %s`), then re-run `ferry sync`", syncBranchName, syncBranchName) } if branch != syncBranchName { - return fmt.Errorf("sync: refusing to run — HEAD is on branch %q, but `ferry sync` integrates and pushes `%s`. Check out `%s` first (or rename this branch with `git -C branch -M %s`), then re-run", ghcli.Redact(branch), syncBranchName, syncBranchName, syncBranchName) + return fmt.Errorf("sync: refusing to run — HEAD is on branch %q, but `ferry sync` integrates and pushes `%s`. Check out `%s` first (or rename this branch with `git -C branch -M %s`, then set `%s` as the remote's default branch so other clones follow), then re-run", ghcli.Redact(branch), syncBranchName, syncBranchName, syncBranchName, syncBranchName) } return nil } diff --git a/cmd/sync_round3_test.go b/cmd/sync_round3_test.go index 64c4a8d..8629a60 100644 --- a/cmd/sync_round3_test.go +++ b/cmd/sync_round3_test.go @@ -170,7 +170,8 @@ func TestScanWorktreeBlocksRealSecretInTerminals(t *testing.T) { } // MAJOR: an UNTRACKED directory (git status collapses it to one `?? dir/` entry at -// the default -unormal) must be walked file-by-file, exactly as backupOutOfBand does: +// the default -unormal) must be enumerated file-by-file (the gate uses +// `git ls-files --others --exclude-standard`, scanning exactly the stageable set): // reading the entry as a file EISDIRs, which is neither a deletion nor a not-exist, // and the fail-closed abort wedges every `ferry sync` after a capture that creates a // new repo subdirectory (agents/, terminals/, ...) — with advice ("re-run once the diff --git a/cmd/terminal_gate_test.go b/cmd/terminal_gate_test.go new file mode 100644 index 0000000..c8db652 --- /dev/null +++ b/cmd/terminal_gate_test.go @@ -0,0 +1,144 @@ +package cmd + +// Gate-input coverage for the whole-plist terminal preference domains. +// +// Apple Terminal stores every customised profile attribute (colours, font, +// cursor) as an NSKeyedArchiver blob, which `defaults export` renders as an XML +// plist element: wrapped base64 lines that clear the entropy detector's +// length and shape floors. Scanning that base64 as text blocked EVERY customised +// Apple Terminal domain from the repo permanently — the only routes left were +// reject and the out-of-repo secret store, and the secret store makes the domain +// non-portable (another machine reports a missing ref forever). +// +// The gate itself is mandatory and stays pre-write; what changes is the INPUT: +// the base64 payload inside elements is masked before the gate sees it, +// while every value — where a pasteable token would actually live — +// stays fully scanned. + +import ( + "encoding/base64" + "strings" + "testing" + + "github.com/REPPL/ferry/internal/secret" +) + +// archivedDataPayload renders n bytes of DETERMINISTIC pseudo-binary as the +// wrapped base64 body of a plist element, the way `defaults export` +// writes an NSKeyedArchiver blob. The bytes come from a fixed LCG, so the +// fixture's entropy is identical on every run and the test cannot flake. +func archivedDataPayload(n int) string { + raw := make([]byte, n) + x := uint32(0x9E3779B9) + for i := range raw { + x = x*1664525 + 1013904223 + raw[i] = byte(x >> 24) + } + enc := base64.StdEncoding.EncodeToString(raw) + var b strings.Builder + for i := 0; i < len(enc); i += 68 { + end := i + 68 + if end > len(enc) { + end = len(enc) + } + b.WriteString("\t\t\t\t" + enc[i:end] + "\n") + } + return b.String() +} + +// appleTerminalPlist builds an Apple Terminal-shaped `defaults export` blob: a +// Window Settings profile carrying NSKeyedArchiver attributes, plus +// whatever extra key/value markup a case wants inside the profile dict. +func appleTerminalPlist(extra string) string { + return ` + + + + Default Window Settings + Ferry + Window Settings + + Ferry + + BackgroundColor + +` + archivedDataPayload(180) + ` + Font + +` + archivedDataPayload(240) + ` + name + Ferry +` + extra + ` + + + +` +} + +// (a) A customised Apple Terminal profile is CAPTURABLE: its archived +// blobs must not block the domain from the repo. +func TestTerminalGateInput_ArchivedDataDoesNotBlock(t *testing.T) { + blob := appleTerminalPlist("") + + // Guard the fixture: the RAW export must be blocked, otherwise this test + // proves nothing about the gate-input reduction. + if !secret.GateValue(blob).BlockedFromRepo { + t.Fatalf("fixture does not trip the raw gate; it cannot demonstrate the reduction") + } + + if secret.GateValue(terminalGateInput([]byte(blob))).BlockedFromRepo { + t.Errorf("a customised Apple Terminal profile is blocked from the repo by its archived blobs; the domain would be permanently uncapturable") + } +} + +// (b) Masking loses no real coverage: a pasteable token in a +// value still blocks the whole domain. +func TestTerminalGateInput_TokenInStringStillBlocks(t *testing.T) { + cases := map[string]string{ + "named token": " CommandString\n" + + " export GH=ghp_ABCdefGHIjklMNOpqrSTUvwx0123\n", + "PEM header": " CommandString\n" + + " -----BEGIN OPENSSH PRIVATE KEY-----\n", + } + for name, extra := range cases { + t.Run(name, func(t *testing.T) { + blob := appleTerminalPlist(extra) + if !secret.GateValue(terminalGateInput([]byte(blob))).BlockedFromRepo { + t.Errorf("a secret in a value no longer blocks the domain; the gate-input reduction masks more than the base64 payload") + } + }) + } +} + +// (c) A plist with no element at all is untouched by the reduction. +func TestTerminalGateInput_NoDataElementStillBlocks(t *testing.T) { + blob := ` + + + CommandString + ghp_ABCdefGHIjklMNOpqrSTUvwx0123 + + +` + if !secret.GateValue(terminalGateInput([]byte(blob))).BlockedFromRepo { + t.Errorf("a token in a -free plist is not blocked") + } +} + +// (d) Malformed input fails toward scanning MORE: an unterminated has no +// known extent, so everything after it stays unmasked and fully scanned. +func TestTerminalGateInput_UnterminatedDataStaysScanned(t *testing.T) { + blob := ` + + + Broken + +` + archivedDataPayload(120) + ` CommandString + ghp_ABCdefGHIjklMNOpqrSTUvwx0123 + + +` + if !secret.GateValue(terminalGateInput([]byte(blob))).BlockedFromRepo { + t.Errorf("content after an unterminated was masked; malformed input must fail toward scanning more, never less") + } +} diff --git a/cmd/terminal_round9_test.go b/cmd/terminal_round9_test.go index ae0bdd2..fb9430c 100644 --- a/cmd/terminal_round9_test.go +++ b/cmd/terminal_round9_test.go @@ -17,6 +17,7 @@ package cmd // coverage here is at the extracted helper seams both platforms compile. import ( + "bufio" "bytes" "os" "path/filepath" @@ -170,3 +171,91 @@ func TestTerminalPlaceholderBlob_RoundTripsExport(t *testing.T) { } } } + +// A secret-routed capture converges the same way a shared one does: the +// placeholder lands at the SHARED repo path apply reads, so a per-machine local +// overlay left over from an earlier [l]ocal capture must not survive to shadow +// it — while it exists, apply keeps importing the stale overlay and status and +// capture disagree about the domain forever. +func TestCaptureBlockedTerminal_SupersedesLocalOverlay(t *testing.T) { + repo := t.TempDir() + const prefID = "com.apple.Terminal" + overlay := terminalLocalDest(repo, "terminal", prefID) + if err := os.MkdirAll(filepath.Dir(overlay), 0o755); err != nil { + t.Fatal(err) + } + if err := os.WriteFile(overlay, []byte("stale overlay\n"), 0o644); err != nil { + t.Fatal(err) + } + + var out bytes.Buffer + cc := captureCtx{ + out: &out, + in: bufio.NewReader(strings.NewReader("x\n")), + repoPath: repo, + secretStore: secret.OpenAt(t.TempDir()), + } + shared := terminalRepoDest(repo, "terminal", prefID) + wrote, err := captureBlockedTerminal(cc, "terminal", prefID, shared, []byte(syntheticExport)) + if err != nil { + t.Fatalf("captureBlockedTerminal: %v", err) + } + if !wrote { + t.Fatal("the secret route reported nothing written") + } + + // The placeholder is written BYTE-EXACT at the shared path. + got, rerr := os.ReadFile(shared) + if rerr != nil { + t.Fatalf("placeholder not written at %s: %v", shared, rerr) + } + if want := string(terminalPlaceholderBlob(prefID + ".captured")); string(got) != want { + t.Errorf("shared copy = %q, want the placeholder %q", got, want) + } + // And the overlay that would otherwise keep winning every comparison is gone. + if _, serr := os.Lstat(overlay); !os.IsNotExist(serr) { + t.Errorf("the local overlay %s survived the secret-routed capture (apply would keep importing the stale bytes): stat err = %v", overlay, serr) + } + if src := terminalRepoStatusSource(repo, "terminal", prefID); src != shared { + t.Errorf("after the secret-routed capture the compare source = %q, want the shared copy %q", src, shared) + } + msg := out.String() + if !strings.Contains(msg, relTo(repo, overlay)) || !strings.Contains(msg, "superseded") { + t.Errorf("the removed overlay is not reported by path and reason:\n%s", msg) + } +} + +// A refused (symlinked) overlay never wins a comparison, so the secret-routed +// capture leaves it exactly as found — and does not turn the refusal into an error. +func TestCaptureBlockedTerminal_LeavesSymlinkedOverlay(t *testing.T) { + repo := t.TempDir() + const prefID = "com.apple.Terminal" + outside := filepath.Join(t.TempDir(), "elsewhere.plist") + if err := os.WriteFile(outside, []byte("outside\n"), 0o644); err != nil { + t.Fatal(err) + } + overlay := terminalLocalDest(repo, "terminal", prefID) + if err := os.MkdirAll(filepath.Dir(overlay), 0o755); err != nil { + t.Fatal(err) + } + if err := os.Symlink(outside, overlay); err != nil { + t.Fatal(err) + } + + var out bytes.Buffer + cc := captureCtx{ + out: &out, + in: bufio.NewReader(strings.NewReader("x\n")), + repoPath: repo, + secretStore: secret.OpenAt(t.TempDir()), + } + if _, err := captureBlockedTerminal(cc, "terminal", prefID, terminalRepoDest(repo, "terminal", prefID), []byte(syntheticExport)); err != nil { + t.Fatalf("captureBlockedTerminal: %v", err) + } + if _, serr := os.Lstat(overlay); serr != nil { + t.Errorf("a refused (symlinked) overlay was removed; it never wins the comparison: %v", serr) + } + if _, serr := os.Lstat(outside); serr != nil { + t.Errorf("the symlink target outside the repo was touched: %v", serr) + } +} diff --git a/cmd/terminal_status_source_test.go b/cmd/terminal_status_source_test.go index eaf5d32..5b55207 100644 --- a/cmd/terminal_status_source_test.go +++ b/cmd/terminal_status_source_test.go @@ -60,3 +60,21 @@ func TestTerminalRepoStatusSourceIgnoresSymlinkedOverlay(t *testing.T) { t.Errorf("symlinked overlay must be refused, source = %q, want shared %q", got, shared) } } + +// Apply's terminalExportBlob accepts BOTH overlay spellings (.plist and the +// extensionless ), so the resolution status and capture share must probe both +// too: an extensionless overlay that apply imports but status resolved past made +// status report drift forever while capture re-offered the domain forever. +func TestTerminalRepoStatusSourceAcceptsExtensionlessOverlay(t *testing.T) { + repo := t.TempDir() + local := filepath.Join(repo, "local", "terminal", "com.apple.Terminal") + if err := os.MkdirAll(filepath.Dir(local), 0o755); err != nil { + t.Fatal(err) + } + if err := os.WriteFile(local, []byte("overlay"), 0o644); err != nil { + t.Fatal(err) + } + if got := terminalRepoStatusSource(repo, "terminal", "com.apple.Terminal"); got != local { + t.Errorf("with an extensionless local overlay present, source = %q, want the overlay %q (apply imports it)", got, local) + } +} diff --git a/cmd/work.go b/cmd/work.go index 34b04ee..1fa1d97 100644 --- a/cmd/work.go +++ b/cmd/work.go @@ -236,7 +236,15 @@ func runWorkPack(c *cobra.Command, args []string) error { var unmatched []string for _, f := range sge.Findings { if !named[f.Item+"/"+f.Path] { - unmatched = append(unmatched, f.Item+"/"+f.Path) + // A PathSecret finding's name IS the secret, which the gate error + // itself withholds; the unmatched list must not re-leak it in the + // same message. Mask the secret-shaped COMPONENT only, so the rest + // of the path still tells the user which file to act on. + label := f.Item + "/" + f.Path + if f.PathSecret { + label = redactSecretPath(label) + } + unmatched = append(unmatched, label) continue } ctx.state.Acks = append(ctx.state.Acks, work.Ack{ diff --git a/cmd/work_ack_redact_test.go b/cmd/work_ack_redact_test.go new file mode 100644 index 0000000..8adc99d --- /dev/null +++ b/cmd/work_ack_redact_test.go @@ -0,0 +1,81 @@ +package cmd + +// The `work pack --acknowledge` retry must not undo the secret gate's own +// withholding: a PathSecret finding's name IS the secret, so the "not covered by +// --acknowledge" list has to name it REDACTED, exactly as the gate error does. + +import ( + "bytes" + "os" + "path/filepath" + "strings" + "testing" + + "github.com/spf13/cobra" + + "github.com/REPPL/ferry/internal/work" +) + +// newWorkPackTestCmd builds a cobra command carrying `work pack`'s flag set. +func newWorkPackTestCmd() (*cobra.Command, *bytes.Buffer) { + c := &cobra.Command{Use: "pack", RunE: runWorkPack} + c.Flags().StringArray("exclude", nil, "") + c.Flags().Bool("allow-empty", false, "") + c.Flags().StringArray("acknowledge", nil, "") + c.Flags().Bool("allow-sync-root", false, "") + out := &bytes.Buffer{} + c.SetOut(out) + c.SetErr(out) + c.SetIn(strings.NewReader("")) + return c, out +} + +// TestWorkPackAckUnmatchedRedactsSecretPath: with a non-matching --acknowledge, +// the retry aborts and lists the findings it could not pin. A PathSecret +// finding must appear REDACTED there — printing it raw would re-leak, in the +// very same message, the token the gate deliberately withheld. +func TestWorkPackAckUnmatchedRedactsSecretPath(t *testing.T) { + home, project := workLockHome(t) + + // The required handover note, so the pack reaches the secret gate. + workLocal := filepath.Join(project, ".abcd", ".work.local") + if err := os.MkdirAll(workLocal, 0o755); err != nil { + t.Fatal(err) + } + if err := os.WriteFile(filepath.Join(workLocal, "NEXT.md"), []byte("handover\n"), 0o644); err != nil { + t.Fatal(err) + } + + // A cargo file whose NAME is secret-shaped: the finding the gate withholds. + const secretComp = "AKIAIOSFODNN7EXAMPLEXYZ.txt" + memory := filepath.Join(home, ".claude", "projects", work.ClaudeProjectsKey(project), "memory") + if err := os.MkdirAll(memory, 0o755); err != nil { + t.Fatal(err) + } + if err := os.WriteFile(filepath.Join(memory, secretComp), []byte("notes\n"), 0o644); err != nil { + t.Fatal(err) + } + + c, _ := newWorkPackTestCmd() + if err := c.Flags().Set("acknowledge", "agent-memory/some-other-file.md"); err != nil { + t.Fatal(err) + } + err := runWorkPack(c, []string{project}) + if err == nil { + t.Fatal("pack with a secret-shaped cargo path and a non-matching --acknowledge: want an error") + } + msg := err.Error() + if !strings.Contains(msg, "not:") { + t.Fatalf("want the unmatched-findings list in the error, got: %s", msg) + } + if strings.Contains(msg, secretComp) || strings.Contains(msg, "AKIAIOSFODNN7EXAMPLE") { + t.Errorf("the unmatched list echoed the withheld secret-shaped path: %s", msg) + } + if !strings.Contains(msg, "") { + t.Errorf("want the unmatched entry redacted, got: %s", msg) + } + // Fail-closed: nothing acknowledged is persisted by a partially-matched run. + if _, err := os.Stat(filepath.Join(workLocal, work.HandoverMarkerName)); err == nil { + t.Errorf("a refused pack must not write the handover marker") + } +} diff --git a/deps/README.md b/deps/README.md index c7506d2..fa34173 100644 --- a/deps/README.md +++ b/deps/README.md @@ -2,7 +2,10 @@ One manifest per platform/manager. `ferry` selects the right one on this machine by `runtime.GOOS` plus the detected package manager (`internal/deps.SelectManifest`), -and the gated `ferry apply --deps` step installs from it. Default file-only +and the gated `ferry apply --deps` step installs from it — but only when +`brew = true` is declared under `[manage]` (see +[Dependencies](../docs/reference/configuration.md#homebrew)); without that +declaration `apply --deps` skips the OS package manager entirely. Default file-only `apply` never touches these — package installs may prompt, need `sudo`, or run remote scripts, so they are an explicit step. diff --git a/docs/explanation/reconciling-drift-and-conflicts.md b/docs/explanation/reconciling-drift-and-conflicts.md index 27a1ed2..3d65154 100644 --- a/docs/explanation/reconciling-drift-and-conflicts.md +++ b/docs/explanation/reconciling-drift-and-conflicts.md @@ -48,9 +48,13 @@ Two details worth calling out: - **First-touch adoption is not a conflict.** The first time `apply` meets an in-scope file that ferry has never managed — one with no last-applied record — it takes ownership rather than refusing: it backs the live file up to an - immutable baseline and then deploys the repo content. Because there is no - last-applied record, there is no uncaptured edit to protect, so this is - treated as repo-ahead, not conflict. The backup keeps it reversible. + immutable baseline and then deploys the repo content. Adoption is still a + *risky* change, so it goes through [`apply`](../reference/commands.md)'s + risky-change gate: the guided walkthrough asks you to confirm it, while a + non-interactive run — or one with `--skip-wizard` — refuses it, listing it + unapplied and exiting non-zero. Because there is no last-applied record, there + is no uncaptured edit to protect, so this is treated as repo-ahead, not + conflict. The backup keeps it reversible. - **A conflict is reserved for the genuine case:** ferry *has* a last-applied record for the file, your live copy differs from it (you edited a file ferry manages, without capturing), *and* the repo has also moved on. Only diff --git a/docs/how-to/cutting-a-release.md b/docs/how-to/cutting-a-release.md index 41bb9e6..6cfcf43 100644 --- a/docs/how-to/cutting-a-release.md +++ b/docs/how-to/cutting-a-release.md @@ -30,8 +30,9 @@ tag yet, it creates the annotated `vX.Y.Z` tag at that commit and invokes the [`release` workflow](../../.github/workflows/release.yml) directly as a reusable workflow. An ordinary push — no newly promoted CHANGELOG heading — tags nothing and does nothing. Tags are immutable: only the newest dated version is ever tagged, and an -already-tagged version is re-released only if its GitHub Release is missing (a -transient publish failure), built from the tagged commit, never a moved-on `main`. +already-tagged version is re-released only if its GitHub Release is missing, still +a draft, or short of its five assets (a transient publish failure), built from the +tagged commit, never a moved-on `main`. Because the tag exists moments after the promotion commit lands, run the local gates **before** pushing that commit. The driver itself cannot run at this point — its @@ -160,6 +161,13 @@ promotion commit as soon as it is pushed, so a driver run started afterwards fai its tag step on the existing tag — that is the expected outcome, not an error in the release: the gates have still run, and the tag already points at the right commit. +After the tag push the driver does one last piece of housekeeping, all of it +local: it archives `.abcd/.work.local/NEXT.md` into `.abcd/.work.local/history/` +(as `NEXT-vX.Y.Z-.md`) and regenerates the file with its carry region +preserved. Under `--dry-run` it prints the current-to-regenerated diff instead of +writing anything, and a tree with no `NEXT.md` skips the step. Nothing here is +staged, committed, or pushed — `.abcd/.work.local/` is local-only. + To prepare a release-ready tree locally (e.g. to inspect the manifest before tagging, or if you publish by hand): diff --git a/docs/reference/cli/ferry_agents_scaffold.md b/docs/reference/cli/ferry_agents_scaffold.md index a353358..1669ca3 100644 --- a/docs/reference/cli/ferry_agents_scaffold.md +++ b/docs/reference/cli/ferry_agents_scaffold.md @@ -6,7 +6,7 @@ Set a project repo up for the multi-tool agent pipeline Set a project repo up for the multi-tool agent pipeline. -Both modes create the local-only runtime layout: .abcd/.work.local/{scratch,logs}, +Both modes create the local-only runtime layout: .abcd/.work.local/{NEXT.md,scratch,logs}, hidden via the git info/exclude mechanism (never committed; .gitignore is never touched). diff --git a/docs/reference/commands.md b/docs/reference/commands.md index 2e596a8..1774471 100644 --- a/docs/reference/commands.md +++ b/docs/reference/commands.md @@ -4,7 +4,7 @@ Every command is run as `ferry ` (e.g. `ferry init`). | Command | What it does | |---|---| -| `init` | First-run setup: locate/clone the config repo into ferry's own space (`~/.config/ferry/repo` by default), write ferry's config. On a fresh interactive run (stdin and stdout both ttys) an adoption **wizard** scans your existing `~/.zshrc` and lets you keep it as-is, route it per block (shared / local / drop), or start fresh from a portable starter: nothing is written before the preview confirm, the original is kept in a timestamped `~/.zshrc.ferry-.bak`, and secret-shaped lines are always routed to the out-of-repo secret store or dropped, never seeded. Non-interactively (a non-tty run, or `--wizard=off`) the same adopt happens without prompts: secrets are extracted to the store automatically (refs listed on stderr) and everything else is kept shared verbatim. The plugin set is currently zsh (`~/.zshrc`); more domains come with later releases. init handles setup only; the single guided reconcile walkthrough lives in `apply`, which init points you to (or runs directly with `--apply`). | +| `init` | First-run setup: locate/clone the config repo into ferry's own space (`~/.config/ferry/repo` by default; `--fresh ` puts a new repo somewhere else instead), write ferry's config. On a fresh interactive run (stdin and stdout both ttys) an adoption **wizard** scans your existing `~/.zshrc` and lets you keep it as-is, route it per block (shared / local / drop), or start fresh from a portable starter: nothing is written before the preview confirm, the original is kept in a timestamped `~/.zshrc.ferry-.bak`, and secret-shaped lines are always routed to the out-of-repo secret store or dropped, never seeded. Non-interactively (a non-tty run, or `--wizard=off`) the same adopt happens without prompts: secrets are extracted to the store automatically (refs listed on stderr) and everything else is kept shared verbatim. The plugin set is zsh (`~/.zshrc`). init handles setup only; the single guided reconcile walkthrough lives in `apply`, which init points you to (or runs directly with `--apply`). | | `init --yes` | Assume yes for init's confirmations — the `--github` create-confirm and the closing apply confirm with `--apply`. It does **not** skip the wizard (use `--wizard=off` for that). | | `init --wizard=off\|interactive\|answers:` | Choose the first-run wizard mode. `off` skips it (the non-interactive adopt-and-extract fallback); `interactive` forces the TUI (needs a tty pair); `answers:` drives every wizard decision from a TOML answers file (same gates, preview, backup, and confirm, no tty needed; schema in [Configuration](configuration.md#the-wizard-answers-file)). Default (unset): interactive on a real tty pair, else `off`. | | `init --repair` | Opt into the wizard's repair review: hardcoded `/Users/` paths to `$HOME`, duplicate `PATH` exports, dead `source` lines: each fix is accepted or declined individually. Needs a running wizard, so it conflicts with `--wizard=off` and (in interactive mode) a non-tty run — unless `--wizard=answers:` drives it, which satisfies the consent requirement. | diff --git a/docs/reference/configuration.md b/docs/reference/configuration.md index 21dbab5..14acbfc 100644 --- a/docs/reference/configuration.md +++ b/docs/reference/configuration.md @@ -214,6 +214,10 @@ The `.local` layer applies whole-domain: a committed `local/iterm2/com.googlecode.iterm2.plist` is imported instead of the shared copy on machines that need a wholesale-divergent global set. +A filtered export that trips the secret scan takes the same store route as the +Apple Terminal plist below, with the same placeholder-and-skip consequences: see +[Apple Terminal](#apple-terminal). + ## Apple Terminal Apple Terminal's preferences are carried as a **whole-domain** `defaults` @@ -229,7 +233,15 @@ Enable it with `terminal = true` (one character away from `terminals`, the config-file emulators above — they are different domains). On `capture`, ferry exports the live domain and offers the whole plist through the same accept and route flow as other captures — shared, local, or reject; a shared-routed -capture commits it at `terminal/com.apple.Terminal.plist`. On `apply`, ferry +capture commits it at `terminal/com.apple.Terminal.plist`. An export that trips +the secret scan is barred from **both** repo routes and offered the out-of-repo +secret store instead: the value is stored on this machine and the committed +plist path carries a `{{ferry.secret …}}` placeholder in its place. `apply` +renders that placeholder back before the import and `status` compares through +it, so a stored secret does not read as drift. The value itself lives in the +per-machine store, so the domain applies only on a machine that holds it; +elsewhere `apply` skips the whole domain and names the missing reference rather +than importing a placeholder. On `apply`, ferry imports the committed plist, which replaces the whole domain; relaunch Terminal for the change to take effect (`cfprefsd` may serve a cached copy until then). The domain is macOS-only and skips cleanly elsewhere. The `.local` layer applies @@ -621,7 +633,11 @@ untracked, ignored, or editor-backup files). It never carries secrets, anything `~/.ssh/`, or the per-machine `.local` layer. The per-machine local layer (`local/**`, `ferry.local.toml`) is **gitignored by -design**, so it is not tracked and does not enter the bundle. `--include-local` adds +design**, so it is not tracked and does not enter the bundle. Those two paths are +exactly what the bundle's local-layer gate covers: `deps/Brewfile..local` is +gitignored rather than gated, so a force-tracked overlay travels with the repo like +any other tracked file, in the ordinary export set and without `--include-local`. +`--include-local` adds back the local-layer files that are actually **tracked** in the repo (the same `git ls-files` set `bundle export` collects from) — so it bundles a local-layer file only if you have force-tracked it. The local layer lands on the other side only when you pass diff --git a/docs/tutorials/getting-started.md b/docs/tutorials/getting-started.md index b121ff0..109f0eb 100644 --- a/docs/tutorials/getting-started.md +++ b/docs/tutorials/getting-started.md @@ -135,8 +135,7 @@ If you have no `~/.zshrc` (and skip the starter), ferry seeds no shell source at file once you have one. A symlinked or unreadable `~/.zshrc` is left entirely alone: ferry declares it but does not manage, back up, or replace it. -The wizard's plugin set is currently **zsh** (`~/.zshrc`); more config domains come -with later releases. +The wizard's plugin set is **zsh** (`~/.zshrc`). `ferry capture` is interactive and selective: it shows you each change and lets you route it **shared** (synced to every machine) or **local** (this machine only). Things diff --git a/evals/README.md b/evals/README.md index 4e6b6d3..9df246f 100644 --- a/evals/README.md +++ b/evals/README.md @@ -138,7 +138,7 @@ The remaining eval files, by feature area: | Agents capture | `capture_agents_test.go` | | Guided apply | `guided_apply_test.go` | | Terminal-adjacent domains | `emacs_test.go`, `tmux_test.go`, `keybindings_test.go`, `npmrc_test.go`, `iterm2_profiles_test.go`, `iterm2_staging_test.go`, `terminal_config_test.go` | -| Dependency tracking | `deps_track_test.go` | +| Dependency tracking | `deps_track_test.go`, `deps_local_overlay_ignore_test.go` (a fresh-init repo leaves `deps/Brewfile..local` untracked across a real `sync`) | | Safety overlays and edge cases | `safety_apply_test.go`, `safety_overlay_bypass_test.go`, `edgecases_test.go`, `fn5_migration_test.go` | ## Related documentation diff --git a/internal/agents/capture.go b/internal/agents/capture.go index dfa0ba1..68e0bdb 100644 --- a/internal/agents/capture.go +++ b/internal/agents/capture.go @@ -240,9 +240,9 @@ func AdoptCandidates(in PlanInput) ([]AdoptCandidate, []string, error) { return werr } if d.Type()&fs.ModeSymlink != 0 { - if d.IsDir() { - return fs.SkipDir // never descend through a symlinked dir into e.g. ~/.ssh - } + // WalkDir never follows symlinks (a symlinked entry reports + // IsDir() == false, so it is never descended into — e.g. a + // link into ~/.ssh); the entry is simply not adopted. return nil } if d.IsDir() || !d.Type().IsRegular() { diff --git a/internal/bundle/bundle_test.go b/internal/bundle/bundle_test.go index 0ac29be..795d1ce 100644 --- a/internal/bundle/bundle_test.go +++ b/internal/bundle/bundle_test.go @@ -283,6 +283,32 @@ func TestValidateSecretInEntry(t *testing.T) { } } +// TestValidateRejectsSecretShapedPath: import mirrors export's path gate. Export +// withholds any tracked file with a secret-shaped path COMPONENT, so a bundle +// carrying one is handcrafted; importing it would plant a token-named file that +// later wedges sync's fail-closed push gate, diagnosed far from the cause. The +// refusal must not echo the offending path (that would re-leak the token). +func TestValidateRejectsSecretShapedPath(t *testing.T) { + dir := t.TempDir() + const secretComp = "AKIAIOSFODNN7EXAMPLEXYZ.txt" + body := []byte("harmless\n") + m := bundleManifest{FormatVersion: 1, FerryVersion: "v", Entries: []bundleEntry{ + {Path: "conf/" + secretComp, Size: int64(len(body)), SHA256: hexSum(body)}, + }} + path := filepath.Join(dir, "b.zip") + writeZip(t, path, manifestJSON(t, m), []member{{name: "conf/" + secretComp, data: body}}) + _, err := Validate(path, "", false) + if err == nil { + t.Fatal("expected a secret-shaped path refusal") + } + if !strings.Contains(err.Error(), "secret-shaped path") { + t.Errorf("expected a secret-shaped path refusal, got %v", err) + } + if strings.Contains(err.Error(), secretComp) || strings.Contains(err.Error(), "AKIAIOSFODNN7EXAMPLE") { + t.Errorf("refusal echoed the secret-shaped path: %v", err) + } +} + func TestValidateEntryCountCap(t *testing.T) { dir := t.TempDir() m := bundleManifest{FormatVersion: 1, FerryVersion: "v"} diff --git a/internal/bundle/validate.go b/internal/bundle/validate.go index f47fbd9..16ecefc 100644 --- a/internal/bundle/validate.go +++ b/internal/bundle/validate.go @@ -166,6 +166,17 @@ func Validate(path, expectedSHA256 string, includeLocalWanted bool) (*Validated, var actualTotal uint64 // cumulative ACTUAL decompressed bytes, capped at maxTotalSize for _, e := range manifest.Entries { + // Secret-shaped PATH gate FIRST, symmetric with export (which WITHHOLDS such + // a file, so no ferry-written bundle carries one — reaching here means the + // bundle was handcrafted). Refusing at import keeps a token-named file from + // being planted in the config repo, where it would later wedge sync's + // fail-closed push gate far from its cause. It runs ahead of every other + // per-entry check because those name the path in their refusals, and echoing + // a secret-shaped path re-leaks the token — so this one WITHHOLDS it, the + // same withholding export's report applies. + if secret.IsPathBlockedFromRepo(e.Path) { + return nil, fmt.Errorf("bundle: a manifest entry has a secret-shaped path component (path withheld) — refusing to import") + } clean, err := canonicalRel(e.Path) if err != nil { return nil, fmt.Errorf("bundle: manifest entry %q: %w", e.Path, err) diff --git a/internal/deps/deps_test.go b/internal/deps/deps_test.go index 3e376de..b719196 100644 --- a/internal/deps/deps_test.go +++ b/internal/deps/deps_test.go @@ -357,11 +357,17 @@ func TestInstallApt_AlreadyInstalledNotRecorded(t *testing.T) { // aptStateRunner answers dpkg-query from a before/after status map and flips // after apt-get install runs. A package absent from the map is "not installed": -// dpkg-query exits non-zero with empty output, which the prober reads as absent. +// dpkg-query exits non-zero AND writes "no packages found matching " to the +// stream the runner returns — CommandRunner.Run returns COMBINED stdout+stderr, +// so that diagnostic IS the returned output, and the prober reads it as absent. +// probeFailBefore names a package whose BEFORE probe fails at the exec level +// instead (empty output, error) — the shape a runner returns when the binary +// cannot be resolved or forked. type aptStateRunner struct { - before, after map[string]string - installed bool - installArgs []string // argv of the apt-get install invocation, once seen + before, after map[string]string + probeFailBefore string + installed bool + installArgs []string // argv of the apt-get install invocation, once seen } func (a *aptStateRunner) Run(args ...string) (string, error) { @@ -377,15 +383,52 @@ func (a *aptStateRunner) Run(args ...string) (string, error) { if a.installed { m = a.after } + if !a.installed && pkg == a.probeFailBefore { + // Exec-level failure: no output at all, only an error. + return "", errors.New("fake: dpkg-query could not be run") + } if status, ok := m[pkg]; ok { return status, nil } - // Not installed: dpkg-query exits non-zero with empty status. - return "", errors.New("dpkg-query: no packages found matching " + pkg) + // Not installed: dpkg-query exits non-zero and its diagnostic is part of + // the combined output. + msg := "dpkg-query: no packages found matching " + pkg + return msg, errors.New(msg) } return "", nil } +// TestInstallApt_FailedBeforeSnapshot_RecordsNothing is the apt counterpart of +// TestInstallBrew_FailedBeforeSnapshot_RecordsNothing: a dpkg-query probe that +// fails with NO output is an exec-level failure, not the "package absent" +// signal. Reading it as absent would make a PRE-EXISTING package look newly +// installed, so a later restore --packages would uninstall something the user +// already had. The snapshot must be unreliable and nothing recorded. +func TestInstallApt_FailedBeforeSnapshot_RecordsNothing(t *testing.T) { + dir := t.TempDir() + apt := filepath.Join(dir, "apt.txt") + writeFile(t, apt, "zsh\nzoxide\n") + m := Manifest{Manager: platform.ManagerApt, GOOS: "linux", Shared: apt} + + // zsh is already installed, but its BEFORE probe fails at the exec level. + r := &aptStateRunner{ + before: map[string]string{"zsh": "install ok installed"}, + after: map[string]string{"zsh": "install ok installed", "zoxide": "install ok installed"}, + probeFailBefore: "zsh", + } + + res, err := install(m, r) + if err != nil { + t.Fatalf("install apt: %v", err) + } + if got := res.RecordedInstalledSet(); len(got) != 0 { + t.Errorf("failed before-probe must record nothing, got %v", got) + } + if !res.SnapshotUnreliable { + t.Errorf("failed before-probe must be flagged SnapshotUnreliable so the caller can tell the user the record was suppressed") + } +} + // TestInstallBrew_AbsentSharedBrewfileNotBundled checks fix #3: a shared // Brewfile that does not exist must NOT be handed to `brew bundle` (which would // fail on a missing file); the present .local overlay is still processed. diff --git a/internal/deps/install.go b/internal/deps/install.go index 51afa9f..2b7edda 100644 --- a/internal/deps/install.go +++ b/internal/deps/install.go @@ -241,10 +241,15 @@ func installApt(m Manifest, runner CommandRunner) (InstallResult, error) { // and whether the query was RELIABLE. We query each package with // `dpkg-query -W -f=${Status} `: a "install ok installed" status means // present. dpkg-query exits non-zero for a not-installed package, which is the -// normal absent signal — NOT a query failure — so absence is read from the -// status text, and only an unexpected empty status with no error is treated as -// present-unknown. ok is false only when we cannot tell (kept conservative so a -// genuine tool failure suppresses recording rather than over-recording). +// normal absent signal — NOT a query failure — but it says so: it writes +// "dpkg-query: no packages found matching " to the COMBINED stdout+stderr +// the runner returns. So absence is only read from a FAILED probe that produced +// output which does not read as installed; a probe that fails with NO output is +// an exec-level failure (the binary could not be resolved or run) and must never +// be mistaken for absence — an absent BEFORE snapshot would make a pre-existing +// package look ferry-installed and let restore --packages uninstall it. ok is +// false whenever we cannot tell (fail closed: suppress the record rather than +// over-record). func aptInstalledSet(runner CommandRunner, pkgs []string) (set map[string]struct{}, ok bool) { set = map[string]struct{}{} for _, pkg := range pkgs { @@ -254,10 +259,16 @@ func aptInstalledSet(runner CommandRunner, pkgs []string) (set map[string]struct // (see runner.ExecRunner.Run) since this probe runs under `sudo ferry`. out, err := runner.Run(dpkgQueryBin, "-W", "-f=${Status}", "--", pkg) if err != nil { + // No output at all: the probe never ran (unresolvable binary, fork/exec + // failure). That is not the absent signal — dpkg-query always reports a + // missing package in its output — so the snapshot is unreliable. + if strings.TrimSpace(out) == "" { + return nil, false + } // Not-installed packages make dpkg-query exit non-zero; treat that as - // "absent" only when the output confirms it (empty / no "installed"). - // Any other error text we cannot interpret means an unreliable probe. - if strings.TrimSpace(out) == "" || !strings.Contains(out, "installed") { + // "absent" only when the output confirms it (no "installed"). Any other + // error text we cannot interpret means an unreliable probe. + if !strings.Contains(out, "installed") { continue } return nil, false diff --git a/internal/dotfile/apply.go b/internal/dotfile/apply.go index e267024..5fc6a17 100644 --- a/internal/dotfile/apply.go +++ b/internal/dotfile/apply.go @@ -422,10 +422,11 @@ func StripFerryOverlayDirective(content []byte) []byte { // count when dangerous is true (0 otherwise). // // It is the ONE predicate behind both the write-time refusal -// (guardEmptyOverSubstantial) and the read-only preview (`ferry diff` / -// `ferry status`, which render a "would refuse" line instead of promising an -// update that aborts). Exported so the preview shares this definition rather -// than re-deriving the thresholds and drifting from the guard. +// (guardEmptyOverSubstantial) and the read-only preview (`ferry diff` and +// `ferry init`'s plan preview, which render a "would refuse" line instead of +// promising an update that aborts). Exported so the preview shares this +// definition rather than re-deriving the thresholds and drifting from the +// guard. func WouldRefuseEmptyOverSubstantial(t Target, desired []byte) (liveSize int, dangerous bool) { if !isNearEmpty(stripFerryOverlayDirective(desired)) { return 0, false // user's managed source has real content: not the dangerous transition. diff --git a/internal/gitconfig/parse.go b/internal/gitconfig/parse.go index a770f99..0a8ac6d 100644 --- a/internal/gitconfig/parse.go +++ b/internal/gitconfig/parse.go @@ -195,9 +195,30 @@ func inlineCloseIdx(raw string) int { if open < 0 { return -1 } + return closeBracketIdx(raw, open+1) +} + +// closeBracketIdx returns the byte index of the `]` that closes a section header +// in s, scanning from index from. A `]` inside a quoted subsection does not close +// the header, and git's in-quote escapes are honoured: inside `"…"` a backslash +// escapes the following byte (`\"` and `\\`), so that byte can neither close the +// quote nor the header. The scan never crosses a newline and returns -1 when no +// closing `]` is found. +// +// inlineCloseIdx (the physical-line split) and parseSectionHeader (the section / +// subsection derivation) BOTH go through here so the two can never disagree about +// where a header ends. It only ever computes indices — no byte is rewritten, so +// the callers keep their Raw bytes and Reassemble(Parse(x)) == x holds. +func closeBracketIdx(s string, from int) int { inQuote := false - for i := open + 1; i < len(raw); i++ { - switch raw[i] { + for i := from; i < len(s); i++ { + switch s[i] { + case '\\': + // A trailing backslash before the newline is git's value-continuation + // signal, so it is never consumed as an escape. + if inQuote && i+1 < len(s) && s[i+1] != '\n' { + i++ + } case '"': inQuote = !inQuote case ']': @@ -279,7 +300,9 @@ func parseSectionHeader(trimmed string) (section, subsection string, ok bool) { return "", "", false } inner := trimmed[1:] - if close := strings.IndexByte(inner, ']'); close >= 0 { + // Same quoting/escaping rule the inline split uses: a `]` inside the quoted + // subsection is part of the subsection, not the header's terminator. + if close := closeBracketIdx(inner, 0); close >= 0 { inner = inner[:close] } inner = strings.TrimSpace(inner) diff --git a/internal/gitconfig/parse_test.go b/internal/gitconfig/parse_test.go index 2625fb6..1c76324 100644 --- a/internal/gitconfig/parse_test.go +++ b/internal/gitconfig/parse_test.go @@ -35,6 +35,8 @@ var realisticConfigs = []string{ "[core] editor = vim\n\tpager = less\n", "[core] ; trailing comment\n", "[url \"https://x]y/\"] insteadOf = git://x/\n", // ']' inside a quoted subsection + "[user \"a\\\"b\"] email = x@y.z\n", // escaped '"' inside a quoted subsection + "[user \"a\\\\\"] email = x@y.z\n", // escaped '\' closing a quoted subsection // Backslash value-continuation (odd run continues, even run does not). "[user]\n\temail = foo\\\nbar@example.com\n", "[user]\n\temail = \\\nalice@example.com\n", @@ -81,6 +83,50 @@ func TestParseIncludeIfContext(t *testing.T) { } } +// TestParseInlineHeaderEscapedQuote: git escapes a '"' inside a quoted +// subsection as \". The inline split must skip that escaped quote instead of +// reading it as the closing one — otherwise the header never closes, the whole +// physical line becomes ONE Section line with the assignment swallowed into its +// Raw bytes, and for a non-includeIf section that line survives verbatim into +// the shared repo, carrying the identity value with it. +func TestParseInlineHeaderEscapedQuote(t *testing.T) { + const in = "[user \"a\\\"b\"] email = x@y.z\n" + lines := Parse([]byte(in)) + if len(lines) != 2 { + t.Fatalf("want a Section line plus a KeyValue line, got %d: %+v", len(lines), lines) + } + if lines[0].Kind != Section || lines[0].Section != "user" { + t.Errorf("line 0 = %+v, want a `user` Section header", lines[0]) + } + // Returned VERBATIM: escape handling moves indices only, it never rewrites bytes. + if want := `a\"b`; lines[0].Subsection != want { + t.Errorf("subsection = %q, want %q", lines[0].Subsection, want) + } + if lines[1].Kind != KeyValue || lines[1].FullKey() != "user.email" { + t.Errorf("line 1 = %+v, want the user.email assignment", lines[1]) + } + if got := string(Reassemble(lines)); got != in { + t.Errorf("round-trip mismatch\n input: %q\noutput: %q", in, got) + } +} + +// TestParseSubsectionWithBracket: a ']' inside the quoted subsection does not +// close the header, so the subsection must be derived whole — the same quoting +// rule the inline split already applies. +func TestParseSubsectionWithBracket(t *testing.T) { + const in = "[user \"a]b\"] email = x\n" + lines := Parse([]byte(in)) + if len(lines) != 2 || lines[0].Kind != Section { + t.Fatalf("want a Section line plus a KeyValue line, got %+v", lines) + } + if lines[0].Subsection != "a]b" { + t.Errorf("subsection = %q, want %q", lines[0].Subsection, "a]b") + } + if got := string(Reassemble(lines)); got != in { + t.Errorf("round-trip mismatch\n input: %q\noutput: %q", in, got) + } +} + // FuzzReassembleParseRoundTrip is the STOP-condition proof: for ANY input, // Reassemble(Parse(x)) == x byte-for-byte. A failure here is the plan's // "any parser fails Reassemble(Parse(x))==x -> stop" condition. diff --git a/internal/secret/path.go b/internal/secret/path.go new file mode 100644 index 0000000..40c880b --- /dev/null +++ b/internal/secret/path.go @@ -0,0 +1,27 @@ +package secret + +import "strings" + +// IsPathBlockedFromRepo reports whether any COMPONENT of a forward-slash relative +// path is itself a high-confidence secret — a token used as a filename or a +// directory name. It is the path counterpart of IsBlockedFromRepo (which gates +// file CONTENT): a path is carried in manifests, refusal messages, and git output +// long after the file it names, so a secret-shaped component must be kept out of +// the repo just as a secret-shaped line is. +// +// Each component is scanned as an OPAQUE VALUE (GateValue), never the whole +// relative path at once: the separators and the ordinary components around a +// token dilute the entropy heuristic, so a whole-path scan would miss what a +// per-component scan catches. +// +// Bundle EXPORT and bundle IMPORT/validate both gate through this ONE function so +// their path predicates cannot drift — a path export withholds is a path import +// refuses. +func IsPathBlockedFromRepo(slash string) bool { + for _, seg := range strings.Split(slash, "/") { + if GateValue(seg).BlockedFromRepo { + return true + } + } + return false +} diff --git a/internal/secret/store.go b/internal/secret/store.go index 5815ace..e9ef0a6 100644 --- a/internal/secret/store.go +++ b/internal/secret/store.go @@ -5,7 +5,6 @@ import ( "os" "path/filepath" "regexp" - "sort" "strings" "unicode/utf8" @@ -194,23 +193,14 @@ func (s *Store) readDomain(domain string) (map[string]string, error) { } // writeDomain writes a domain map back to its TOML file with mode 0600, -// deterministically (keys sorted) so the file is stable across writes. The store -// is treated as secret-bearing: perms are never loosened. +// deterministically so the file is stable across writes (the TOML encoder +// documents that map keys are sorted alphabetically). The store is treated as +// secret-bearing: perms are never loosened. func (s *Store) writeDomain(domain string, m map[string]string) error { path := s.domainFile(domain) - keys := make([]string, 0, len(m)) - for k := range m { - keys = append(keys, k) - } - sort.Strings(keys) - var b strings.Builder enc := toml.NewEncoder(&b) - ordered := make(map[string]string, len(m)) - for _, k := range keys { - ordered[k] = m[k] - } - if err := enc.Encode(ordered); err != nil { + if err := enc.Encode(m); err != nil { return fmt.Errorf("encode secret domain %s: %w", path, err) } // Write atomically: a temp file in the SAME directory, fsync'd, then renamed diff --git a/internal/terminal/allowlist.go b/internal/terminal/allowlist.go index 97a9d15..8b91c3b 100644 --- a/internal/terminal/allowlist.go +++ b/internal/terminal/allowlist.go @@ -23,8 +23,11 @@ import ( // outside any profile. // // The set below is a curated STARTING POINT of stable, machine-agnostic global -// behaviour keys; extend it to taste in the repo's own review. A key absent from -// the list is simply not carried (a no-op), never an error. +// behaviour keys. It is compiled in, not manifest-configurable, so extending it +// means changing THIS file in ferry's own source repo and reviewing it here; a +// user's config repo carries an extra key instead by committing it directly to +// the domain's plist (apply imports the committed bytes verbatim). A key absent +// from the list is simply not carried (a no-op), never an error. var ITerm2GlobalAllowlist = map[string]bool{ // Quit / close confirmations. "PromptOnQuit": true, diff --git a/scripts/release.sh b/scripts/release.sh index f9472c3..a3e4d72 100755 --- a/scripts/release.sh +++ b/scripts/release.sh @@ -32,7 +32,8 @@ EOF } # --------------------------------------------------------------------------- -# Functions (also sourced by tests; see the executed-directly guard at the end). +# Functions. The executed-directly guard at the end lets a shell source this +# file to exercise them in isolation without running main. # --------------------------------------------------------------------------- die() { echo "release: $*" >&2; exit 1; } @@ -115,6 +116,17 @@ main() { [ "$local_main" = "$remote_main" ] || \ die "local main ($local_main) != origin/main ($remote_main); pull/push to sync first" + # Step 7 regenerates .abcd/.work.local/NEXT.md after the tag push; check its + # carry markers here so a malformed local file fails the driver BEFORE the + # irreversible push, not after it. render_next_md re-checks at use. + local next_precheck="$ROOT/.abcd/.work.local/NEXT.md" + if [ -f "$next_precheck" ]; then + grep -qF 'next:carry:start' "$next_precheck" || \ + die ".abcd/.work.local/NEXT.md is missing the '' marker; fix it before releasing (step 7 would fail after the tag push)" + grep -qF 'next:carry:end' "$next_precheck" || \ + die ".abcd/.work.local/NEXT.md is missing the '' marker; fix it before releasing (step 7 would fail after the tag push)" + fi + # ------------------------------------------------------------------------- # Gate 2 — CHANGELOG: the version must be promoted out of [Unreleased]. # ------------------------------------------------------------------------- @@ -122,7 +134,16 @@ main() { if ! grep -Eq "^## \[${dot_re}\] - " "$ROOT/CHANGELOG.md"; then die "CHANGELOG.md has no '## [${ver_no_v}] - ' section; promote it out of [Unreleased] first" fi - echo "release: gate CHANGELOG — '## [${ver_no_v}]' present." + # The version must also be the NEWEST dated section. The driver tags HEAD, + # so accepting an older still-untagged version would point an immutable tag + # at newer main code under an older name — the exact mis-pointing the + # automatic path refuses by tagging only the first dated heading. + local newest + newest="$(grep -E -m1 '^## \[[0-9]+\.[0-9]+\.[0-9]+\] - ' "$ROOT/CHANGELOG.md" | sed -E 's/^## \[([0-9.]+)\].*/\1/')" + if [ "$newest" != "$ver_no_v" ]; then + die "'${ver_no_v}' is not the newest dated CHANGELOG section (${newest:-none} is); tagging it at main HEAD would mis-point the tag" + fi + echo "release: gate CHANGELOG — '## [${ver_no_v}]' present and newest." # ------------------------------------------------------------------------- # Gate 3 — docs currency (deterministic lint). The tool is maintainer-local @@ -225,7 +246,7 @@ main() { echo "release: reset .abcd/.work.local/NEXT.md (carry region preserved)." } -# Run main only when executed directly; when sourced (tests), just define the +# Run main only when executed directly; when sourced, just define the # functions above so render_next_md can be exercised in isolation. if [ "${BASH_SOURCE[0]}" = "$0" ]; then main "$@"