Development record
GHSA-4q78-ccfv-f374 (CWE-345, advisory severity low): the owned PATH-copy verification is self-referential under an env-supplied CLAUDE_PLUGIN_DATA. `internal/core/ahoy/data_dir.go:pluginDataDir` is `os.Getenv("CLAUDE_PLUGIN_DATA")` and nothing else; `owned_copy.go:cacheRecordedSHA` reads `binary_sha256` from a `binary-meta` beside the artefact under that same directory; `apply.go:installOwnedEntry` hashes the artefact, compares it to that record, writes the bytes 0755 as the owned PATH entry and records provenance in `~/.abcd/path-entry`. The bootstrap's manifest authentication (`hooks/bootstrap.sh`, cache_trust=manifest, iss-2608210934566228) covers only the cache-to-plugin-root promotion; the cache-to-PATH promotion re-verifies against the co-located record only, which an attacker who writes both satisfies trivially — adr-46 decision 3 states exactly this principle for the bootstrap. Reproduced at v0.7.0: with CLAUDE_PLUGIN_DATA and ABCD_PLUGIN_ROOT pointed at attacker-chosen directories, `ahoy install --yes --adopt` installed a one-byte file as `~/.local/bin/abcd`, recorded its hash and the fake plugin root in path-entry, and `classifyBinTarget` then vouched for it as an owned copy.
GHSA-4q78-ccfv-f374 (CWE-345, advisory severity low): the owned PATH-copy verification is self-referential under an env-supplied CLAUDE_PLUGIN_DATA. internal/core/ahoy/data_dir.go:pluginDataDir is os.Getenv("CLAUDE_PLUGIN_DATA") and nothing else; owned_copy.go:cacheRecordedSHA reads binary_sha256 from a binary-meta beside the artefact under that same directory; apply.go:installOwnedEntry hashes the artefact, compares it to that record, writes the bytes 0755 as the owned PATH entry and records provenance in ~/.abcd/path-entry. The bootstrap's manifest authentication (hooks/bootstrap.sh, cache_trust=manifest, iss-2608210934566228) covers only the cache-to-plugin-root promotion; the cache-to-PATH promotion re-verifies against the co-located record only, which an attacker who writes both satisfies trivially — adr-46 decision 3 states exactly this principle for the bootstrap. Reproduced at v0.7.0: with CLAUDE_PLUGIN_DATA and ABCD_PLUGIN_ROOT pointed at attacker-chosen directories, ahoy install --yes --adopt installed a one-byte file as ~/.local/bin/abcd, recorded its hash and the fake plugin root in path-entry, and classifyBinTarget then vouched for it as an owned copy.
DESIGN DECISION NEEDED — no mechanical fix closes the class without changing a documented contract (spc-35: "the data dir is taken from the harness or not at all"; adr-38: implicit checks are disk-only; the ahoy brief documents install as a no-network verb). Options:
A. Authenticate at install time against the published checksums.txt (reuse the update package's manifest fetch and origin seam). Puts a network GET on ahoy install; offline must degrade to the existing loud pinned-symlink fallback or to corruption-evidence with an honest notice. Needs an adr-38 reading and a docs change (install how-to, brief 01-ahoy).
B. Bind the cache to a record the env does not choose alone (RECOMMENDED). The bootstrap — which runs with the harness's real CLAUDE_PLUGIN_DATA and, online, has just authenticated the cache against the manifest — writes a home-scoped attestation (extend ~/.abcd/path-entry or a sibling ~/.abcd/cache-dir) naming the data dir, the manifest-authenticated binary_sha256 and cache_trust. installOwnedEntry accepts an env data dir only when it equals the attested one and the co-located record equals the attested hash. The trust floor moves from "env" to "home write", which adr-46 decision 4 already treats as the ownership root, and the terminal (where CLAUDE_PLUGIN_DATA is unset) gains a route to the cache as a side effect. Touches hooks/bootstrap.sh and its test, owned_copy.go, apply.go, spc-35 wording, an adr-46 amendment.
C. Accept as a documented residual (env control is at least a PATH foothold), record it in adr-46 beside the offline residual, close won't-fix — consistent with the decisions dcn-2608290000000002 and dcn-2608010000000008 that keep PATH as the operator's environment.
Rejected mechanical partial, so nobody re-derives it: cross-checking the recorded hash against the plugin-root binary only defeats the literal recipe and breaks dogfood installs (a source checkout as ABCD_PLUGIN_ROOT with a locally built binary never matches the cache). One caveat on the premise: if the harness honours a committed project settings file's environment block, a hostile checkout can set this variable for its own sessions, and "env control is already a strong foothold" is weaker than the advisory assumes; not verified here. The strict hardening that does not depend on the choice (a data dir that is relative, inside the repository, or world-writable is never used as a cache) is captured and fixed separately and keeps this record open for the decision.
Grounds
- pursued: the class closes because the trust floor for the cache-to-PATH promotion moves from a value the environment supplies alone to a write into the caller's own home, which adr-46 decision 4 already treats as the ownership root — an attacker who can write ~/.abcd already owns path-entry and everything it vouches for, so the attestation grants nothing new. What would show it wrong: a route by which environment or repository control reaches the owned copy without a home write (a harness honouring a committed project settings file's environment block only reaches the data dir, not the home, so it is covered; a bypass of readCacheAttestation's directory-plus-hash equality, or a bootstrap path that attests on offline trust, would not be).
Frontmatter
| id | iss-2609012039102770 |
|---|---|
| type | issue |
| lifecycle | resolved |
| severity | minor |
.abcd/work/issues/resolved/iss-2609012039102770-ghsa-4q78-owned-copy-verification-self.md
open on GitHub ↗ · commit history ↗
mentions11
- adr-38Implicit checks are disk-only — the network answers only an explicit ask
- adr-46Persisting the hook binary never weakens the verification posture — every promotion re-verifies, and SessionEnd performs no network work
- adr-2609151706587280The cache reaches PATH only through a home-scoped attestation the environment cannot write
- dcn-26080100000000082026-08-01 — itd-105 / spc-21, THIRD-round fix pass: the environment-driven URL seam in `hooks/bootstrap.sh` is REMOVED rather than patched a third time. Two independent adversarial reviews had already returned BLOCK on two successive allowlist attempts at the same defect — first a bare `${VAR:-default}` read, then a `case http://127.0.0.1:*|http://localhost:*` glob — and both were defeated empirically by fresh reviewers: URL userinfo (`http://127.0.0.1:1@attacker.example`, where everything before the `@` is a username and the real host is the attacker's) walked straight through the glob, and the override path also cleared the `--proto '=https' --proto-redir '=https'` pin, so a redirect could downgrade the transport on top. The pattern was the finding, not the instances: an allowlist over URL syntax is a parser-differential surface, and the thing it guards is a code-execution primitive — the binary AND the `checksums.txt` that verifies it come from the same origin, so whoever names the origin supplies both the payload and the manifest that "verifies" it, and what is installed then runs unattended as the Bash shell guard on every tool call. So `ABCD_BOOTSTRAP_BASE_URL` and `ABCD_BOOTSTRAP_API_URL` are gone; `grep ABCD_BOOTSTRAP hooks/bootstrap.sh` now returns nothing, the script's only environment read at all is `${CLAUDE_PLUGIN_ROOT:-}` (a filesystem destination, never a fetch origin), and the two transport flags are literal on all four curl calls with no variable to clear. This supersedes claim (1) of the fix-pass entry above and the `ABCD_BOOTSTRAP_BASE_URL` mention in spc-21's Testing section, which named a seam that no longer exists. THE TEST SEAM MOVED TO THE TEST SIDE. `internal/surface/cli/bootstrap_test.go` reads the committed script's bytes, replaces the two quoted origin literals with an `httptest` base, writes the result to a temp file, `chmod +x`es it and execs THAT — the shipped bytes are what runs on a user's machine, with no test-only branch anywhere in them. Three properties make the copy trustworthy rather than a fixture that hides the bug (a failure this item already shipped once, when a friendlier fixture concealed that the release tag was always `unknown` in production): the match is the QUOTED, complete assignment value, so a longer origin cannot be prefix-rewritten into a mangled URL; each literal must appear exactly once before and zero times after, or the helper `t.Fatal`s naming the drift; and the happy path additionally asserts the fixture served at least one request, so a substitution that silently missed cannot pass by testing the real GitHub. The fixture speaks TLS (`httptest.NewTLSServer`) because the pin is unconditional, and trust is supplied through `CURL_CA_BUNDLE`/`SSL_CERT_FILE` — curl's own variables, which only ever ADD an issuer curl will accept and can never name a different server, so they are not the class of seam that was removed. Reviewer-facing residue, stated rather than implied: an actor who already controls the hook process's environment can still influence curl through curl's OWN variables (a proxy setting, a CA bundle), which is true of every curl invocation on the machine and is not something a shell script can close — such an actor can also replace `curl` on `PATH`; what is closed is the seam this script itself honoured. Detector-first throughout, each fix reverted in place and watched to fail: the reverted script reproduces the round-2 bypass verbatim (recorded fetch to `http://127.0.0.1:1@abcd-bootstrap.invalid/releases/latest`, no HTTPS pin), and against a live attacker-controlled release server on loopback the pre-fix script took four requests from it and installed its payload while the fixed script took zero and installed the genuine ELF release. Four smaller items in the same pass. (1) `plugin_sha`'s forty-hex gate rests on itd-105's unverified warrant that the harness names each plugin cache directory for its source commit; if that stops holding, every install records `unknown`, the skew notice goes silent permanently, and nothing anywhere says why — so the RAW basename is now recorded as `plugin_root_basename` beside the gated field, never compared and never rendered, purely so the failure is diagnosable from the file rather than only from a comment. Control characters are stripped from it and it is capped at 120 characters, since a directory name may contain a newline (forging a `key=value` line) or enough bytes to push `.binary-meta` past the guarded read and silence the notice by a second route. (2) `mkdir "$lock"` failing was indistinguishable from losing the race and took the same silent `exit 0`; it now refuses loudly when no lock DIRECTORY exists afterwards, because that case is permanent and repeats every session while the race case is transient and correctly quiet. (3) Both message sinks strip control characters from the values they echo, and the skew notice sanitises `release_tag` through `termsafe` like every other untrusted rendered string in this repo — the tag is read out of an HTTP redirect and is the one value in that line with no shape check. (4) `binary_sha256` is recorded and re-checked by nothing; the comment now says so plainly instead of implying an automatic check, because the fast path's whole contract is one file test and adding a SHA-256 recomputation to every session start would trade the AC away for a check nothing yet consumes. Two known asymmetries re-confirmed as disclosed rather than accidental: only the two binary-backed `SessionStart` entries carry the inline existence wrapper — `UserPromptSubmit`, `PreCompact` and `SessionEnd` still invoke the binary bare, which is itd-105's "every hook reports, for now" testing posture and not an oversight; and the unsupported-platform notice's move to exit 2 means it now repeats EVERY session for as long as the platform is unsupported, which is the accepted cost of that same posture (the one-notice-per-session aspiration is iss-168's). Not fixed, noted: the orphan sweep could still delete a live racer's temp directory in the already-accepted >10-minute stale-lock race, and the transcript scanner's `local_username` hard-fails on this script are the same pre-existing false-positive class already firing on `main` for `README.md`.
- dcn-26082900000000022026-08-29 — The gitleaks adapter admits a binary under an allow-shape, not a deny-list (GHSA-fg9r-3f8g-89m6, iss-2608291807456485): absolute path, resolved OUTSIDE the repository both lexically and after symlink resolution, regular, executable — applied identically to a committed `path` and to the PATH lookup result. Refusal is `ErrConfiguredPathRefused`, as loud as not-found, and a refused configured path never falls back to PATH. PATH itself stays trusted as the operator's environment (abcd already resolves git, gh and grep from it; a hostile checkout cannot set it); what the admission rule closes is a PATH entry or config that reaches INTO the checkout. Rejected: a fixed safe search path for the fallback (would break Homebrew/asdf/custom installs for no gain against an operator-level PATH compromise) and executing with an isolated env (the env is the operator's, like PATH — gitleaks does honour GITLEAKS_CONFIG, but a checkout cannot set it; cwd is pinned to the private temp dir instead). Containment routes through `fsutil.PathWithin` with the case-folding predicate AND an `os.SameFile` ancestor walk, because a byte-exact compare admitted a case-variant or NFD respelling of the root on APFS.
- dcn-26091500000000102026-09-15 — GHSA-4q78-ccfv-f374 (iss-2609012039102770) is closed by OPTION B, ruled by the maintainer at an interactive question: bind the cache to a record the environment does not choose alone. The owned PATH-copy promotion re-verified the cache only against the `binary-meta` beside it, and `CLAUDE_PLUGIN_DATA` is taken from the environment as given, so whoever chose the directory wrote both the bytes and the record that "verified" them — reproduced at v0.7.0 as a one-byte file installed 0755 as `~/.local/bin/abcd` with provenance recorded. Now the bootstrap, the one process holding the harness's real data dir that has just established manifest trust for the cache (an authenticated cache hit, or a fresh download verified against the same-origin manifest), writes `~/.abcd/cache-attestation` — `data_dir`, the manifest-authenticated `binary_sha256`, `cache_trust=manifest`, `attested_at`; 0600, temp-and-rename, beside `path-entry` — and `ahoy install` promotes a cache only when that record names the directory, the co-located record carries the attested hash, and the artefact hashes to it, whichever route (environment or the root's `.data-dir` stamp) named the directory; detection offers the heal on the same predicate. An offline run neither writes nor rewrites the attestation, so a cache provisioned offline waits for a networked session before it reaches PATH, said out loud. Why B: the trust floor moves from a value the environment supplies to a write into the caller's own home, which adr-46 decision 4 already treats as the ownership root, so the attestation grants nothing that authority did not hold and costs `ahoy install` no network (adr-38 stands). Rejected: A (a manifest GET on a disk-only verb, re-fetching what the session already proved); C (a documented residual — weaker than it reads, since a harness honouring a committed settings file's environment block lets a hostile checkout set the variable, and the owned-copy claim is what the hook shims trust); the mechanical partial of cross-checking against the plugin-root binary (breaks dogfood installs whose root binary is a local build); and a terminal rung through the attestation alone (it would heal a dogfood checkout's stable symlink into a release copy). adr-46 is superseded by adr-2609151706587280 (never amended, always superseded; retained because its numbered decisions are cited), spc-35 Design 2 step 1 and Design 3 are revised in place and dated, and brief invariant 12 gains the clause.
- iss-2608210934566228spc-35 BLOCK (security review): the cache promotion 're-verify against recorded binary_sha256' is a corruption check, not a tamper check. The cached artefact and its binary-meta (holding the expected hash) both live in ${CLAUDE_PLUGIN_DATA}/cache, equally same-UID-writable. An attacker writing BOTH — payload plus a meta recording its hash and the current release_tag — passes the promotion gate and gets an unverified binary installed at the guard path and onto PATH, with the success notice asserting 'checksum-verified'. Demonstrated end-to-end vs live v0.6.1. This is exactly adr-46's central claim: on main a poisoned plugin-root binary healed at the next update (harness re-clones a fresh root -> bootstrap re-downloads+verifies from GitHub); here the poisoned cache is preferred over the network indefinitely and survives every update. adr-46, invariant 12, and the notice assert a protection the code does not provide. Fix: in cache mode (which already resolves the latest tag over the network) GET that release's checksums.txt and verify cached_sha against the PUBLISHED digest before use_cache=yes — restores tamper-evidence online, offline stays corruption-only (document honestly). Plus the missing test: TestBootstrapCorruptCacheRefusesLoudly rewrites only the artefact, never the meta, so it never exercises the case that succeeds.
- iss-2609012039116200Sub-finding of GHSA-4q78-ccfv-f374 that does not need the design decision: `ahoy install` uses any CLAUDE_PLUGIN_DATA value as the verified cache, including one that is relative, lies inside the repository being installed, or is world-writable. `internal/core/ahoy/data_dir.go:pluginDataDir` returns the env value unexamined and `owned_copy.go:ownedCopySourceReady` plus `apply.go:installOwnedEntry` copy from it. The harness's persistent per-plugin data directory (spc-35) is always absolute, never inside a project checkout, and never world-writable, so refusing those shapes is a strict hardening of the current contract: a relative value resolves against the checkout the verb runs in, an in-checkout value blesses committed bytes as the owned PATH binary, and a world-writable cache lets any local user swap both artefact and record. The fix must establish that such a data dir is ignored with a note naming why, that install then degrades to the existing loud pinned-symlink path exactly as when no cache exists, and that a harness-shaped data dir still installs the owned copy; the display-only readers of the same variable (`vintage.go:readPinnedTag`, the skew notice) make no trust decision and are unchanged.
- itd-105A fresh plugin install just works: the hook binary survives install and every update
- spc-21a-fresh-plugin-install-just-works-the-hook-binary-survives
- spc-35hook-binary-to-persistent-data-dir