Skip to content

fix(guardrail): close the remaining loose ends - #20

Merged
fas89 merged 1 commit into
mainfrom
fix/guardrail-loose-ends
Sep 6, 2026
Merged

fas89 merged 1 commit into
mainfrom
fix/guardrail-loose-ends

Conversation

@fas89

@fas89 fas89 commented Sep 6, 2026

Copy link
Copy Markdown
Contributor

Four things, all previously reported and consciously deferred, now closed.

WAIVER LIFECYCLE. An approval the owner actually gave was silently discarded if
the confirmation dialog had been open longer than the 600s TTL: the note was
consumed and the waiver never written, so CI then failed on a finding they had
explicitly allowed. The TTL exists to stop a DENIED note being reused by a
later edit — but the content hash already proves the note belongs to this exact
edit, and that guard does not expire. Aged notes are now honoured when they
carry a content hash, and dropped when they do not.

CONCURRENCY. _append_grace was an unlocked read-modify-write of a file the
checker also reads, using two SHARED temp filenames, so two edits in flight
could interleave or collide. It now takes an exclusive lock and stages through
per-process temp files. Locking is best-effort and never blocks an edit.

install-hooks.sh HARDCODED .git/hooks, which is wrong in a git worktree where
.git is a FILE — the redirect failed with "Not a directory" and no hook was
installed, silently, in exactly the checkouts this team works in. It now uses
git rev-parse --git-path hooks, which also honours core.hooksPath.

SYNC CLASSIFIED BY FOLDER NAME. The public/private decision keyed on the
destination DIRECTORY name, so a worktree of command_center called anything
else classified as public and had its payload replaced with the redacted one.
Fail-closed meant nothing leaked, but it is still the wrong content in the
wrong repo — and this run reproduced it. Classification now reads the
destination git remote, so what a checkout is called no longer matters; a
destination whose remote cannot be read is refused rather than guessed.

Verified: an approval survives a 30-minute-old dialog; a denial still never
becomes a waiver; six concurrent recorders leave the profile parseable with no
stray lock or temp files; the installer works in a worktree and is idempotent;
a command_center checkout named "totally-unrelated-name" classifies as private;
and every repo holds the payload variant it should.


Closes the last items left open after the Product Guardrail merged. Each was reported by review and consciously deferred as low severity; none was a security hole, and each is now closed with a control proving it.

🤖 Generated with Claude Code

Four things, all previously reported and consciously deferred, now closed.

WAIVER LIFECYCLE. An approval the owner actually gave was silently discarded if
the confirmation dialog had been open longer than the 600s TTL: the note was
consumed and the waiver never written, so CI then failed on a finding they had
explicitly allowed. The TTL exists to stop a DENIED note being reused by a
later edit — but the content hash already proves the note belongs to this exact
edit, and that guard does not expire. Aged notes are now honoured when they
carry a content hash, and dropped when they do not.

CONCURRENCY. _append_grace was an unlocked read-modify-write of a file the
checker also reads, using two SHARED temp filenames, so two edits in flight
could interleave or collide. It now takes an exclusive lock and stages through
per-process temp files. Locking is best-effort and never blocks an edit.

install-hooks.sh HARDCODED .git/hooks, which is wrong in a git worktree where
.git is a FILE — the redirect failed with "Not a directory" and no hook was
installed, silently, in exactly the checkouts this team works in. It now uses
`git rev-parse --git-path hooks`, which also honours core.hooksPath.

SYNC CLASSIFIED BY FOLDER NAME. The public/private decision keyed on the
destination DIRECTORY name, so a worktree of command_center called anything
else classified as public and had its payload replaced with the redacted one.
Fail-closed meant nothing leaked, but it is still the wrong content in the
wrong repo — and this run reproduced it. Classification now reads the
destination git remote, so what a checkout is called no longer matters; a
destination whose remote cannot be read is refused rather than guessed.

Verified: an approval survives a 30-minute-old dialog; a denial still never
becomes a waiver; six concurrent recorders leave the profile parseable with no
stray lock or temp files; the installer works in a worktree and is idempotent;
a command_center checkout named "totally-unrelated-name" classifies as private;
and every repo holds the payload variant it should.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
@fas89
fas89 merged commit 8fef4bc into main Sep 6, 2026
9 checks passed
@fas89
fas89 deleted the fix/guardrail-loose-ends branch September 6, 2026 15:05
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant