Skip to content

Gate 2's approval token can be self-minted by any agent it blocks, defeating the control it exists to enforce - #866

Open
JoshuaVSherman wants to merge 1 commit into
devfrom
agy/808-gate-2-s-approval-token-can-be-self-mint
Open

Gate 2's approval token can be self-minted by any agent it blocks, defeating the control it exists to enforce#866
JoshuaVSherman wants to merge 1 commit into
devfrom
agy/808-gate-2-s-approval-token-can-be-self-mint

Conversation

@JoshuaVSherman

Copy link
Copy Markdown
Contributor

Summary

  • Adds a Gate 2 authorization check to scripts/write_issue_approval_token.ts (web-jam-tools#808, decision 21 of design-issue-enhancements-design-2026-08-23.md): the writer now refuses to mint a token unless the most recent own-session, non-sidechain user turn invoked /design-issue or /file-issue, and refuses unconditionally when the current invocation is itself a dispatched subagent's own turn — closing the two reproduced bypasses where an agent denied by Gate 2 simply ran the writer itself.
  • New hooks/lib/check_token_write_authorization.ts: the pure, testable decision logic. Scans backward through own-session user turns (reusing hooks/lib/select_transcript_entry.ts's isOwnSessionUserTurnBoundary, extractEntryText) for the most recent authorizing skill invocation, mirroring decision 17's opus-delegation-gate.sh scan-for-most-recent-occurrence approach rather than checking only the literal last turn — a literal-last-turn check would break /design-issue's legitimate multi-turn Gate 2 flow, where approval routinely arrives many turns after the /design-issue invocation itself.
  • Claude Code self-discovery (resolveClaudeCodeWriteContext): since this script has no hook-delivered transcript_path and its own working directory can move (e.g. into a /work-issue worktree), it locates the invoking session's own transcript by searching every ~/.claude/projects/*/ directory for a file named <session-id>.jsonl — session ids are UUIDs, so this is unambiguous without needing to reconstruct Claude Code's project-slug algorithm. Subagent detection (tailIsCurrentlySidechain) reads the transcript's own tail entry's isSidechain flag, the same real-time signal opus-delegation-gate.sh already relies on.
  • Antigravity best-effort discovery (resolveAntigravityWriteContext): reuses the existing /tmp/agy-hook-invocations.jsonl record (hooks/lib/agy_hook_shim.ts's recordInvocation, built for an unrelated purpose in web-jam-tools#816) to recover the invoking conversation's identity and transcript path. Documented as a known limitation, not silently assumed solved: it is a shared, cross-session log, and Antigravity's transcript shape carries no in-band subagent marker at all (an acknowledged non-goal of the underlying reader, web-jam-tools#841), so isSubagentInvocation is only computed mechanically on Claude Code — on Antigravity the authorizing-turn text scan does the work instead, which in practice still denies a dispatched subagent's own composed prompt (virtually never a literal /file-issue//design-issue invocation) short of an adversarial one.
  • New --transcript-path/--conversation-id CLI flags let a caller (or a test) supply the authorization context explicitly instead of relying on auto-discovery.
  • buildApprovalToken/writeApprovalToken/writeApprovalTokenSync are deliberately left unauthorized — nothing else in the repo imports them directly, so gating the CLI's import.meta.main block is the one real enforcement point, and the existing unit tests of those three functions are unaffected.
  • Version bump: deno.json 1.32.321.32.33.

Closes #808

How to test locally

Working directory: /home/joshua/WebJamApps/web-jam-tools (Deno, no package.json).

  1. Full gate suite:
deno task fmt:check
deno task lint
deno task check
deno task test

Expect: all four green.

  1. The check that exercises the change itself — reproduce the two recorded bypasses and confirm each is now refused, then confirm the legitimate paths still work:
# From ordinary chat, with no filing skill invoked on the most recent user turn:
deno task write_issue_approval_token --session-id "$CLAUDE_CODE_SESSION_ID" \
  --repo WebJamApps/web-jam-tools --title "anything" --token-path /tmp/test-token.json

Expect: refused, non-zero exit, message naming that no /design-issue or /file-issue invocation was found — no token file written.

# A dispatched subagent's own turn is refused even with an authorizing invocation present
# (regression test: "CLI: refuses a token write when the current invocation is a dispatched
# subagent's own turn, even though an authorizing invocation is present").
deno test --allow-all --filter "dispatched subagent's own turn" test/write_issue_approval_token.test.ts

Expect: passes.

# The legitimate paths still succeed, once per authorizing skill.
deno test --allow-all --filter "succeeds when the most recent authorizing turn" test/write_issue_approval_token.test.ts

Expect: both pass (one for /file-issue, one for /design-issue several turns before the write, proving the Gate 2 multi-turn flow still works).

  1. fmt/lint scope note: deno task fmt:check/lint/check are scoped to src/ test/ per deno.json and CI's own .circleci/config.yml, so they don't directly cover the changed hooks/ and scripts/ files. Ran deno fmt --check, deno lint, and deno check src/ test/ (which transitively type-checks the changed files via test/write_issue_approval_token.test.ts's imports) directly against the changed files as well — all clean.

  2. Both-surfaces note: this fix adds no new hook or skill registration — check_token_write_authorization.ts is a library import (like the existing check_issue_approval_token.ts), and write_issue_approval_token.ts is already a registered deno task, unchanged in registration. Both surfaces run the identical deno task write_issue_approval_token invocation, so no installer run is required for this change to take effect on either surface.

Test evidence

1. Full gate suite

$ deno task fmt:check
Task fmt:check deno fmt --check src/ test/
Checked 201 files

$ deno task lint
Task lint deno lint src/ test/
Checked 165 files

$ deno task check
Task check deno check src/ test/
Check src/book-gig/browser.ts
...
Check test/write_issue_approval_token.test.ts

$ deno task test
...
CLI: refuses when no authorizing skill invocation is found anywhere in the transcript ... ok (42ms)
CLI: refuses a token write when the current invocation is a dispatched subagent's own turn, even though an authorizing invocation is present ... ok (48ms)
CLI: succeeds when the most recent authorizing turn invoked /design-issue, several turns before the write (Gate 2 flow) ... ok (52ms)
CLI: succeeds when the most recent authorizing turn invoked /file-issue ... ok (88ms)
filingSkillInvoked: recognizes /file-issue and /design-issue at the start of the text ... ok (222µs)
filingSkillInvoked: does not match a mid-sentence mention, only an invocation ... ok (95µs)
checkTokenWriteAuthorization: refuses when isSubagentInvocation is true regardless of transcript content ... ok (189µs)
checkTokenWriteAuthorization: refuses when ownConversationId is undetermined ... ok (74µs)
checkTokenWriteAuthorization: finds the most recent authorizing turn, skipping a subagent's interleaved sidechain turns ... ok (289µs)
checkTokenWriteAuthorization: Antigravity entries only count when conversationId matches ... ok (129µs)
tailIsCurrentlySidechain: true only when the LAST entry is flagged isSidechain ... ok (123µs)
resolveClaudeCodeWriteContext: returns null when no session id is given ... ok (180µs)
resolveClaudeCodeWriteContext: returns null when no matching transcript file exists ... ok (1ms)
authorizeWrite: end-to-end via an explicit --transcript-path-equivalent options object ... ok (1ms)
Round-trip: written token allows matching issue_write create without a prompt ... ok (32ms)
Round-trip: written token DENIES unapproved title ... ok (58ms)
Round-trip: written token DENIES when token is expired ... ok (45ms)
Round-trip: written token DENIES when session ID does not match ... ok (30ms)

ok | 1952 passed | 0 failed (55s)

deno task fmt:check/lint/check are scoped to src/ test/ per deno.json and CI's own .circleci/config.yml, so hooks/lib/check_token_write_authorization.ts and scripts/write_issue_approval_token.ts aren't in their direct path args. Ran deno fmt --check/deno lint directly against the three changed files too — both clean (Checked 3 files, no diffs). deno check src/ test/ also transitively type-checks the changed files via test/write_issue_approval_token.test.ts's imports.

2. New-test-fails-against-unfixed-code proof

Reverted scripts/write_issue_approval_token.ts to its pre-#808 committed state (git show 7eb8bb4:...) while keeping the new test file, and reran:

$ deno test --allow-all test/write_issue_approval_token.test.ts
Check test/write_issue_approval_token.test.ts
TS2305 [ERROR]: Module '".../scripts/write_issue_approval_token.ts"' has no exported member 'authorizeWrite'.
TS2305 [ERROR]: Module '".../scripts/write_issue_approval_token.ts"' has no exported member 'resolveClaudeCodeWriteContext'.
Found 2 errors.
error: Type checking failed.

Restored the fix afterward; full suite passes again (see section 1).

3. Live reproduction against a real running session (not just fixtures)

$ deno task write_issue_approval_token --session-id "$CLAUDE_CODE_SESSION_ID" --repo WebJamApps/web-jam-tools --title "anything" --token-path /tmp/test-token.json
Task write_issue_approval_token deno run --allow-env --allow-read --allow-write scripts/write_issue_approval_token.ts '--session-id' '2c2fd037-0096-4590-99e8-b469ec925d41' '--repo' 'WebJamApps/web-jam-tools' '--title' 'anything' '--token-path' '/tmp/test-token.json'
Refused to write approval token: Refused: no /design-issue or /file-issue invocation found in this session's own transcript. Get Josh's explicit approval for this plan first, or ask him directly.
(exit 1)

Run against this PR's own actual, live Claude Code session (real $CLAUDE_CODE_SESSION_ID, real transcript on disk), whose most recent user turn was /work-issue, not /file-issue or /design-issue — the writer's Claude Code discovery path (glob every ~/.claude/projects/*/ for <session-id>.jsonl) found the real transcript and correctly refused. No token file was written.

4. Scope notes

  • No hook or skill registration changed — check_token_write_authorization.ts is a library import (like the existing check_issue_approval_token.ts), and write_issue_approval_token.ts was already a registered deno task. Both surfaces run the same deno task write_issue_approval_token invocation, so no scripts/install-hooks.sh/scripts/install-skills.ts run is required for this to take effect.
  • /learn was not run: it is an agy/Flash-native command (confirmed against this session's own memory of that convention) with no Claude Code equivalent, and /work-issue's own Steps section does not call for it — only the generic dispatch-template boilerplate does.
  • The Antigravity discovery path (resolveAntigravityWriteContext) is unit-tested via injected fixtures (the checkTokenWriteAuthorization/Antigravity-shaped-entries tests) but not live-verified against a real agy session, since this PR is implemented and reviewed from Claude Code. Its known limitation is documented in the function's own doc comment.

🤖 Work by Claude Code — Sonnet 5

…kill authorized it

scripts/write_issue_approval_token.ts now refuses to write unless the most recent
own-session, non-sidechain user turn invoked /design-issue or /file-issue, and a
dispatched subagent's own turn is refused regardless of what an authorizing turn
elsewhere in the transcript says (web-jam-tools#808, decision 21 of
design-issue-enhancements-design-2026-08-23.md).

- hooks/lib/check_token_write_authorization.ts: the pure, testable decision — scans
  backward through own-session user turns (hooks/lib/select_transcript_entry.ts's
  isOwnSessionUserTurnBoundary) for the most recent /design-issue or /file-issue
  invocation, mirroring decision 17's opus-delegation-gate.sh scan-for-most-recent
  approach rather than checking only the literal last turn (which would break
  /design-issue's multi-turn Gate 2 flow).
- scripts/write_issue_approval_token.ts: resolveClaudeCodeWriteContext() locates the
  invoking session's own transcript by globbing every ~/.claude/projects/*/ directory
  for a file named <session-id>.jsonl, since a bare CLI script has no hook-delivered
  transcript_path and the project-slug cannot be reconstructed from Deno.cwd() once a
  prior `cd` (e.g. into a /work-issue worktree) has moved it. resolveAntigravityWriteContext()
  is a documented best-effort fallback via hooks/lib/agy_hook_shim.ts's existing
  /tmp/agy-hook-invocations.jsonl record, with an acknowledged limitation: Antigravity's
  transcript shape carries no in-band subagent marker (web-jam-tools#841 non-goals), so
  isSubagentInvocation is only computed mechanically on Claude Code (tailIsCurrentlySidechain).
- New --transcript-path/--conversation-id CLI flags let a caller (or a test) supply the
  authorization context explicitly instead of relying on auto-discovery.
- buildApprovalToken/writeApprovalToken/writeApprovalTokenSync stay unauthorized on
  purpose — the CLI's import.meta.main block is the only caller in this repo and is
  now the sole gated entry point.

Bumps deno.json to 1.32.33.
@JoshuaVSherman
JoshuaVSherman marked this pull request as ready for review August 28, 2026 16:54

@JoshuaVSherman JoshuaVSherman left a comment

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

PR Review Summary

🛑 Changes Requested

🛑 Must Fix Items

  • 🛑 The new --transcript-path / --conversation-id CLI flags reopen the exact bypass this PR exists to close: they let the caller supply the authorization evidence it is being judged against (scripts/write_issue_approval_token.ts:373-385, resolveWriteContext at :238-254).

    resolveWriteContext() returns early at :241 when options.transcriptPath is set — it loads that file as the transcript, takes ownConversationId from options.conversationId || options.sessionId, and derives isSubagentInvocation from that same file's tail. Every input the decision rests on then comes from the caller. An agent denied by Gate 2 writes four lines of JSONL, passes --transcript-path, and the check returns ok.

    This is not a hypothetical, and it does not require forging anything: the PR's own passing tests are a working proof-of-concept. test/write_issue_approval_token.test.ts:430-465 ("CLI: succeeds when the most recent authorizing turn invoked /design-issue…") writes {"type":"user","message":{"role":"user","content":"/design-issue token savings"}} to a temp file, runs the real CLI with --transcript-path, and asserts exit 0 with a token on disk. writeAuthorizingTranscriptFixture at :89-107 packages the maneuver into a reusable helper. Anything a test can do here, the agent the gate just denied can do — with one flag that the --help output at :313-316 advertises.

    This is materially worse than the pre-fix state rather than equal to it. The design document's threat model does accept that an agent sharing the operating-system user could tamper with files (which is why decision 21 rejected the cryptographic and filesystem-permission candidates) — but that reasoning covers an adversary going out of its way, not a documented, supported flag whose stated purpose is to supply the authorization context directly. Reproduced bypass #2 in web-jam-tools#808 "Gate 2's approval token can be self-minted by any agent it blocks, defeating the control it exists to enforce" was an agent that had read the issue describing the bypass and used it anyway; a self-documenting flag will be found and used the same way.

    Same root cause, second vector: resolveAntigravityWriteContext() (:199-231, doc comment from :179) takes both conversationId and transcriptPath from the last parseable line of /tmp/agy-hook-invocations.jsonl — a world-writable append-only log, with the transcript path it names then read as authoritative. An agent appends one line pointing at a file it wrote. The doc comment discloses the concurrency risk on that log ("the last line could belong to a different session's call") but not that the log is an agent-writable authorization input, so the limitation as written understates it.

    The fix keeps the testability without the hole: the pure decision logic is already injectable through the exported checkTokenWriteAuthorization(ctx) and authorizeWrite({...})test/write_issue_approval_token.test.ts:606-628 exercises exactly that path with no CLI flag at all. Drop the two flags from the import.meta.main block so a real invocation has only discovery available, and move the four CLI success/refusal tests onto authorizeWrite. If an end-to-end CLI test is genuinely wanted, gate the override behind an env-var test seam that is inert in normal use, matching the seam convention already documented in docs/scripts.md.

  • 🛑 The authorization scan is unbounded, so one /file-issue early in a session authorizes every token write for the rest of that session's life — including from ordinary chat, which is reproduced bypass #2 (hooks/lib/check_token_write_authorization.ts:113-118).

    Both the design document (decision 21, and the "A skill invocation is what authorizes the approval token" section) and web-jam-tools#808's first acceptance criterion specify the writer "refuses unless the most recent non-sidechain user turn invoked one of the two filing skills". The implementation instead walks the whole transcript backward and returns ok on the first authorizing turn it finds anywhere in history, with no bound on how far back or how many unrelated turns have intervened.

    I want to be fair about why: a literal most-recent-turn test genuinely would break the flow decision 21 protects, since /design-issue's Gate 2 approval routinely lands many turns after the invocation — the PR's reasoning on that point is correct, and the test at :430 (whose most recent user turn is "looks good, approved") shows the case. So the answer is not "use the last turn". The answer is a bound, which is the half of decision 17 that did not come across. That grant pairs its scan with an independent expiry — it holds only while the working tree stays on the issue's branch, and "expires when the branch changes, which is when the work is done". This mechanism has no counterpart: the doc comment nominates the token's 4h TTL, but that bounds the life of a token after minting, not the reach of the scan that authorizes minting, so a session that ran /file-issue this morning can mint fresh 4h tokens all day.

    Concretely, after this lands: any session that has invoked either filing skill at any earlier point can mint a token for any title from ordinary chat, which is bypass #2 with one precondition added — a precondition that was true of the orchestrating session in bypass #2 itself. Worth noting that the live reproduction in the PR body's evidence section 3 does not cover this: it refused because that session had no filing-skill invocation anywhere in its transcript, so it demonstrates the no-invocation case only.

    Since the implementation and the stated acceptance criterion disagree here, this needs Josh's call on the bound rather than a silent choice either way — a scope-ending event (the next non-sidechain invocation of a different skill), a turn/time window on the scan, or a per-run marker consumed on use.

Checklist Verification

  • Mergeability: ✅ MERGEABLE, no conflicts with dev.
  • Snyk: ✅ No Snyk check reported on this PR.
  • Scope: ✅ Tight — the three files the issue's "Files changed" section named, plus deno.json. No stray refactors, and buildApprovalToken/writeApprovalToken/writeApprovalTokenSync are correctly left alone.
  • Semver Bump: ✅ deno.json 1.32.33 strictly exceeds origin/dev at 1.32.32, bumped on the PR's single commit.
  • Package-lock engine alignment: ✅ N/A — Deno repo, no engines change.
  • Test plan: ✅ Concrete — reproduces both recorded bypasses and names the specific --filter runs, not a bare suite invocation.
  • Issue acceptance criteria: 🛑 Criterion 1 ("the most recent non-sidechain user turn") is not met as written — see the second Must Fix.
  • Architecture — authorization input integrity: 🛑 The decision rests on inputs the caller controls — see the first Must Fix.
  • Secret literal safety: ✅ No credentials, tokens, or private URLs in the diff.
  • Guardrails (no raw any): ✅ Clean — the record parsing at scripts/write_issue_approval_token.ts:262-270 uses unknown narrowed to Record<string, unknown> at :216-218.
  • Fail-closed behavior: ✅ Correct where it is reached — an undetermined conversation identity, an unreadable transcript, and a sidechain tail all refuse rather than guess, and the ordering in checkTokenWriteAuthorization puts the subagent test ahead of the scan.

🟡 Suggestions

  • 🟡 filingSkillInvoked (hooks/lib/check_token_write_authorization.ts:41-52) matches only a leading /design-issue or /file-issue. Josh routinely invokes skills by name rather than by slash command ("file an issue", "run design-issue"), and the file-issue skill's own description lists those phrasings as triggers. Those turns will not authorize a write, so a legitimate run started that way hits the refusal. Worth deciding deliberately whether the slash form is the only authorizing form — if it is, the refusal message should say so, since "no /design-issue or /file-issue invocation found" reads as an error to an agent whose user did invoke the skill in words.

  • 🟡 The resolveAntigravityWriteContext doc comment (scripts/write_issue_approval_token.ts:179-198) says a dispatched subagent's composed prompt is "virtually never a literal /file-issue//design-issue invocation, so the scan denies it in practice". Given the unbounded scan above, that reasoning does not hold as stated: the subagent does not need its own prompt to be an invocation — it needs only an authorizing turn somewhere earlier in the conversation it is scanning. If the scan gains a bound, this comment should be re-derived against it rather than carried over.

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.

Gate 2's approval token can be self-minted by any agent it blocks, defeating the control it exists to enforce

1 participant