Skip to content

protect-immutable-adrs.sh: HARD-DENY bypassed when CLAUDE_PROJECT_DIR does not prefix the edited path #60

Description

@j4th

Where: .claude/hooks/protect-immutable-adrs.sh at 74edf84, lines 54–57:

rel="${file_path#"$PROJECT_DIR/"}"
if [[ "$rel" =~ ^docs/adr/[0-9]{4}-.*\.md$ ]]; then

Defect: bash's # prefix-strip is a silent no-op when $PROJECT_DIR (CLAUDE_PROJECT_DIR, else $PWD) does not prefix file_path — a session launched in another worktree or a nested checkout. rel stays absolute, the ^docs/adr/… anchor never matches, and the HARD-DENY falls through to exit 0 with no stderr at all — not the documented fail-open path, a silent match-miss.

Repro (throwaway tree): CLAUDE_PROJECT_DIR=<checkout A>, payload {"tool_name":"Edit","tool_input":{"file_path":"<checkout B>/docs/adr/0099-x.md"}} with that file existing → exit 0, empty stderr. Expected: exit 2.

Fix applied in a target project (https://github.com/crease-data/crease/pull/270, commit fix(hooks): protect-immutable-adrs denies by path suffix…): match the payload path itself, path-independently — [[ "$file_path" =~ (^|/)docs/adr/[0-9]{4}-[^/]*\.md$ ]] — and keep rel for the message only; plus a test whose env dir and file dir disagree. An alternative is deriving the root from the file's own checkout (git -C "$(dirname "$file_path")" rev-parse --show-toplevel), the pattern analyze-on-edit.sh already uses.

Also worth a fixture case: hook-contract-fixture.sh probes the drain and the readers but never a payload whose path lies outside CLAUDE_PROJECT_DIR.

Found by the review floor (pr-review-toolkit silent-failure-hunter) on the harvest-4 sync of a target project.

Activity

  1. j4th commented on Sep 28, 2026

    @j4th
    OwnerAuthor

    From echosphere, 2026-09-28: the same bug class, measured against this hook and against the suffix-match fix above

    echosphere's own protect-frozen-corpus.sh (a project hook, not a kit copy) matched paths the way this hook does. Rewriting it on j4th/echosphere#55 (draft) took two red-team rounds. The spellings those rounds found were then run against this hook as it stands on main, and against the suffix match proposed above:

    [[ "$file_path" =~ (^|/)docs/adr/[0-9]{4}-[^/]*\.md$ ]]

    The test setup:

    • a throwaway tree with an existing docs/adr/0001-x.md;
    • CLAUDE_PROJECT_DIR set to the tree's root unless a row says otherwise;
    • an Edit payload piped straight to the hook.

    Every row should exit 2.

    Spelling of the existing ADR kit main suffix match
    control: the absolute path 2 2
    a .. segment (docs/sub/../adr/0001-x.md) 0 0
    a . segment and a doubled slash 0 0
    CLAUDE_PROJECT_DIR with a trailing slash 0 2
    a relative path from a subdirectory (../adr/0001-x.md) 0 0
    a symlinked directory into docs/adr/ 0 0
    a symlink to the ADR file 0 0
    a hardlink to the ADR file 0 0
    a path through /proc/self/root 0 2
    a lone UTF-16 surrogate elsewhere in the payload (jq fails, so tool_name reads empty and the hook allows it) 0 0
    this issue's own case: CLAUDE_PROJECT_DIR set to another directory 0 2

    The suffix match closes 3 of the 10. Not tested here: which of these spellings Claude Code passes to a hook without normalizing it first. A guard should not depend on that.

    What closed them in echosphere. The fixture landed red first, in 03d2562. The fixes followed in 0eb993c, ffff50a and 07b52e6:

    • Read the payload exactly, and refuse what can't be read.
      • Read file_path and cwd exactly: jq -j …; printf x, then strip the sentinel, so a trailing newline survives.
      • Refuse a payload jq cannot parse, where the hook used to fail open.
    • Resolve the path two ways, and deny if either reading lands in the guarded tree.
      • Lexically, collapsing .. the way a writer that normalizes the path would.
      • Physically, one component at a time through symlinks, reading each link's text exactly. A symlink that cannot be read is refused.
    • Refuse any path through /proc. Its magic links resolve per process, so a readlink subprocess sees its own /proc/self.
    • Compare device and inode. The target is checked against every guarded directory and file with stat -L -c '%d:%i' (BSD stat -f as the fallback). That is what catches a hardlink or a bind mount.
    • Guard two roots. One is the checkout the hook lives in, derived lexically from BASH_SOURCE so that a symlinked .claude/hooks cannot move it. The other is CLAUDE_PROJECT_DIR.
    • The fixture is .claude/workflows/tests/frozen-corpus-hook-fixture.sh, 32 cases. Its allow side covers the neighbours a looser match catches: a sibling that shares the directory name as a prefix, another checkout's copy, and a symlink loop.

    One difference matters for a port: this hook allows creating a new ADR, so its existence test has to run on the resolved path, after canonicalization. echosphere's hook names four residuals and does not close them:

    • writes through the Bash tool;
    • case-insensitive filesystems;
    • hand edits;
    • a symlink swapped between the check and the write.

    The frozen-corpus recipe names a CI job that nothing verifies

    The kit's cbk-conventions.md template row for a frozen corpus says "scaffold registers the hook, CI job, .gitattributes and editor entries that keep it byte-stable". consultation/references/frozen_corpus_ingestion.md § The enforcement set asks for "a hook and a CI job on the corpus pattern", and the bootstrap checklist carries all four items as one row.

    echosphere filled that row in naming a frozen-corpus CI job, but scaffold had never built one. From the harvest-3 sync (j4th/echosphere#37, 2026-09-13) until 2026-09-27, two files named it as the hook's backstop:

    • .claude/settings.json's hook registry: "its CI twin is the frozen-corpus job";
    • cbk-conventions.md § Mutation discipline, in the table row and in the two-views paragraph.

    Nothing under .github/ read the corpus, and the only required checks were ADR immutability and Gate. A write through the Bash tool reached the corpus with nothing behind it. echosphere has since built the check as a leg of its required gate, mise run frozen-corpus-check, in defc860, 3972783 and 5db9c76, with a 20-case fixture.

    Two suggestions for the kit:

    • The verification block already checks the hook registry against the hook files. It could also check that every CI job the mutation table or the registry names exists under .github/workflows/, or as a task the gate runs.
    • Give the four enforcement items a row each in the bootstrap checklist, so one of them cannot be ticked off along with the others.

    The ADR-immutability job the recipe says to extend has two measured gaps

    These gaps are in the kit's .github/workflows/adr-immutability-check.yml on main; echosphere's copy has a byte-identical run body. The job reads git diff --name-status through awk '$2 ~ /^docs\/adr\/[0-9]{4}-.*\.md$/', so a path that git quotes, or that awk splits, never matches:

    A modified existing ADR exit why
    0001-plain.md (control) 1
    0002-café.md 0 git prints the path quoted: "docs/adr/0002-caf\303\251.md"
    0003-two words.md 0 awk's $2 is docs/adr/0003-two

    A body that parses no paths closes both. It uses a :(glob) pathspec, filters out additions with --diff-filter=a, and fails on any output:

    set -euo pipefail
    changes=$(git diff --no-renames --diff-filter=a --name-status "$BASE_SHA" "$HEAD_SHA" -- ':(glob)docs/adr/[0-9][0-9][0-9][0-9]-*.md')
    [ -z "$changes" ] && { echo "No existing numbered ADR changed."; exit 0; }
    printf '::error::ADR immutability violation:\n%s\n' "$changes"; exit 1

    Run against the same tree, it gives the expected result in all nine cases:

    • exit 1: a modification (with an ASCII, a non-ASCII or a spaced name), a deletion, a rename, and a mode change;
    • exit 0: a new ADR, a README.md edit, and a nested docs/adr/sub/0005-x.md.

    you-are-hear's Frozen corpus job, which sits beside the ADR job, fails on any git diff output and so does not have this gap. Extending the ADR job, as the recipe asks, would inherit it.

    echosphere's leg had the mirror image of this defect. Its byte-check loop read git ls-tree without -z, so a corpus file named café.md came out quoted and read as deleted, and the leg failed even though nothing changed. It failed closed rather than open, and no corpus file has such a name today. Fixed on j4th/echosphere#55 in b1d647c: the list is read with -z through xargs -0, since the leg runs under dash, whose read has no -d.

  2. j4th commented on Sep 29, 2026

    @j4th
    OwnerAuthor

    From crease, 2026-09-28: where each target stands, and what crease is porting

    Hook status across targets

    • crease: protect-immutable-adrs.sh carries this issue's suffix match (from crease #270).
    • echosphere: main (b1c4fa5) still matches ^docs/adr/… on the prefix-stripped path, which is the bug this issue names. Diffed on 2026-09-28. The hardening in the comment above went into echosphere's frozen-corpus hook, not its ADR hook.
    • The suffix match is a partial fix. By the table above it closes 3 of the 10 spellings.

    What crease is porting

    Results against the 10-row table and the 9-case tree will be posted here when the port lands.

  3. j4th commented on Sep 29, 2026

    @j4th
    OwnerAuthor

    Applied to crease, 2026-09-28: crease-data/crease#281 (open, not yet merged)

    The hook. The hardened resolution recipe is a shared helper, .claude/hooks/lib/resolve-path.sh (fca9a4c), sourced by the ADR hook and by a new guard on crease's frozen docs/upstream/ (a9e6444). Against this issue's table, the ADR hook now exits 2 on all ten spellings plus the control: .., . and //, a trailing-slash project dir, a relative path, a symlinked directory, a symlink to the file, a hardlink, /proc/self/root, a lone surrogate, and a foreign project dir. It also catches a bind mount of docs/adr/. The fixture is .claude/workflows/tests/protected-paths-hook-fixture.sh (red first in 6a55be5; 57 cases at the PR head, including each guard's no-jq fail-open branch, added at review in e0027f4). Three differences from echosphere's frozen-corpus hook are worth knowing for the kit's port:

    • The ADR deny stays path-independent. An existing docs/adr/NNNN-*.md is denied in any checkout: every kit target keeps immutable ADRs at that path, and a session in one target edits its siblings. The corpus guard is root-scoped instead.
    • Linked worktrees are roots too. A worktree session driven by the main checkout's hook would otherwise pass the two-root model. The helper recognizes a linked worktree of a root from its .git file, whose gitdir: resolves under <root>/.git/worktrees/, in pure bash.
    • Device and inode use bash's -ef over a glob walk, echosphere's final form, rather than stat -L, so no missing tool can switch the check off.

    A trap for any target with the toptal Python .gitignore. Its unanchored lib/ hides .claude/hooks/lib/, and git add silently skips the helper. crease needed an anchored negation, !/.claude/hooks/lib/.

    The ADR job. The :(glob) body is live (1cd6b8a), and #61's fail-closed behaviour is kept. .claude/workflows/tests/adr-ci-body-fixture.sh extracts the job's own run: body from the YAML and runs it on a throwaway repository. It gives the expected exit on all nine cases and fails closed on a bad SHA. It was red first on 0002-café.md (490762a).

    Suggestion 1, built. The verification block now fails when the mutation table or the hook registry names a .github/workflows/*.yml, scripts/* or mise run <task> that does not exist (8fb30af, b3b1035). The checker is asked about bogus names first, so it cannot pass by matching nothing.

    The frozen-corpus CI leg, for a corpus frozen whole. scripts/check-frozen-upstream.sh is adapted from echosphere's leg, but fails on additions too (82fb35c; fixture b4c4f63). It runs in the required Python job, whose checkout needed fetch-depth: 0.

    Two defects in crease's adaptation, caught at review (effa880; fixture now 29 cases).

    • A dead || fail. The symlink branch read link=$(readlink -- "$path"; printf x) || fail …. With ;, the substitution's status is printf's, so a failed readlink was hashed as an empty link text. It is && now.
    • A pass-open. crease branched on the base's mode (120000), not on the path's type on disk. A corpus file replaced by a symlink to an identical copy, and hidden by assume-unchanged, therefore passed with exit 0: hash-object follows the link.
    • The fix: the leg compares type before content, and reports either type change as disk: T.
    • echosphere's leg is not affected by the pass-open. It branches on [ -L "$f" ] and hashes the link text. Its readlink …; printf x carries no || fail, so nothing goes dead there either.
    • For the kit's port: branch on the disk type and compare it with the base's mode.
  4. j4th commented on Sep 30, 2026

    @j4th
    OwnerAuthor

    Applied to crease — the harvest-5 final check (2026-09-29)

    Three more items for this issue's surface: the guards, their CI backstops, and the frozen-corpus twin of the ADR guard. They are applied in crease-data/crease#281 (open) and complement the application recorded in crease's earlier comment above.

    1. The frozen-corpus CI job's red-team closures belong in consultation's enforcement set

    consultation/references/frozen_corpus_ingestion.md § The enforcement set asks scaffold for "a hook and a CI job on the corpus pattern" and says nothing about how such a job is fooled. The job is the backstop the hook cannot be, because a Bash write, a hand edit or a case-insensitive filesystem never reaches the hook.

    echosphere red-teamed its frozen-corpus leg twice (2026-09-27). crease's docs/upstream leg (scripts/check-frozen-upstream.sh, adapted from it) added the type-first comparison and the readlink failure. Each closure is a case in its leg's fixture: 29 cases in crease's frozen-upstream-leg-fixture.sh. The section now lists them:

    • the base resolves from fully qualified refs (refs/remotes/origin/<default>, then refs/heads/<default>) — a tag or local branch named origin/main would shadow the bare name, and a full-history fetch brings every tag; no base fails, never skips;
    • on the default branch, or a branch with no commits of its own, the merge base is HEAD, so the last commit is compared with its parent;
    • GIT_DIR, GIT_WORK_TREE, GIT_INDEX_FILE and their kin are cleared, and replace refs and grafts ignored;
    • nothing is sourced from the tree under check — a PR-supplied helper could define a git function that silences every diff;
    • the commits, the index and the working tree are each compared with the base, so an edit staged or committed with the tree put back is caught;
    • the bytes on disk are hashed with --no-filters against the base blob — git's own diffs trust the index (assume-unchanged, skip-worktree) and apply checkout conversions (core.autocrlf, a .gitattributes eol, encoding or filter a PR can commit);
    • a path's type is compared before its content, so a symlink standing in for a file is a change even when what it reaches is identical; a failed readlink stops the job;
    • every path list is read NUL-separated (-z) — a quoted non-ASCII name otherwise reads as a missing file;
    • every git call fails the job on error; an error is never read as "unchanged".

    Whether an added file passes (a newly frozen document added by hand in a reviewed commit) is the target's call: one allows additions, the other freezes the directory whole.

    Crease commit 7949947.

    2. The sourced-helper contract (already in crease's § Hook authoring; recording the kit home)

    lib/resolve-path.sh, which this issue's resolver suggestion became, is sourced by two guards, and that raised a contract the kit does not state:

    • A hook that sources a helper keeps it in .claude/hooks/lib/, sources it only after the stdin drain, and names it on its Depends: line.
    • It fails open (exit 0) naming the surviving backstop when the helper is missing.
    • The helper defines functions only and never reads stdin.
    • hook-contract-fixture.sh's early-reader tripwire reads lib/*.sh too. An early-exiting pipeline planted in a helper was otherwise invisible to it.

    The toptal Python template's unanchored lib/ in .gitignore hides .claude/hooks/lib/, which needs an anchored negation (!/.claude/hooks/lib/). All of it is in crease's § Hook authoring ("A project's own hooks are held to the same contract, and a sourced helper fails open") since b3b1035. Crease's earlier comment named the helper and the gitignore trap, but not the contract.

    3. The always-run ADR job can check out blobless

    The ADR immutability job has no paths: filter, so it runs on every PR, and it needs the full commit graph for base..head. It does not need blobs: under the :(glob) body's --no-renames, git diff --name-status compares tree entries. A blobless partial clone's missing-object count stays unchanged across the diff (measured 2026-09-28). crease's checkout:

            with:
              fetch-depth: 0
              filter: blob:none   # an input of actions/checkout (v7.0.1 here)

    This is low priority, an efficiency change on every PR. Crease commits 6b35137 (on crease-data/crease#270) and 0fdcc20 (the comment corrected, on crease-data/crease#281).

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Metadata

Metadata

Assignees

No one assigned

    Labels

    No labels
    No labels

    Projects

    No projects

      Milestone

      No milestone

      Relationships

      None yet

      Development

      No branches or pull requests

      Issue actions