fix: round five — a CLI harness quoting its own key - #218
Merged
Conversation
The previous sweep flagged this as a gap it could not reproduce without a real harness. It reproduces fine with a stub one, and it is a real leak. A CLI route is handed its credential in an environment variable, and a harness reporting an auth failure can print it back. Nothing removed it. A stub harness writing `auth error: rejected key <key>` to stderr produced that string verbatim in the terminal and in logs/dispatches.jsonl, measured against the built binary before and after. Narrower than the endpoint leaks beside it: `api_key:` on a CLI route reclassifies it as metered, which billing policy blocks until the user explicitly opts in. Confirmed by hitting that block first. Once opted in it is the same disclosure. The scrub wraps the dispatcher's whole event stream rather than each result site, and that is the entire point. This class has now been found five times and four were "the fix landed in one branch, the sibling beside it kept leaking" — twice inside a single file. #runStream has five result sites plus the chunk events; a sixth added later would silently miss a per-site scrub. It cannot miss the wrapper. Chunks are scrubbed too: they become partialOutput and stdout.log on disk, which is the same disclosure a beat earlier. A route with no key — every subscription CLI — takes the untouched path and pays nothing. Sabotage: bypassing the wrapper fails exactly the two scrub tests, while the no-key test keeps passing, which is what says the common path is unaltered. npm run check: 1231 passed, 8 skipped, exit 0.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
The previous sweep flagged this as a gap it could not reproduce without a real harness. It reproduces fine with a stub one, and it is a real leak.
The finding
A CLI route is handed its credential in an environment variable, and a harness reporting an auth failure can print it back. Nothing removed it.
Measured against the built binary, before:
After:
How narrow it is
Reaching it needs
api_key:on a CLI route, which reclassifies the route as metered — billing policy then blocks it until the user explicitly opts in. I hit that block first and had to opt in to reproduce, so it is genuinely narrower than the endpoint leaks beside it. Once opted in it is the same disclosure. Three shipped harnesses (claude_code,codex,cursor) declare anapi_key_env_var, and setting a key on them is documented.Why a wrapper and not five scrub calls
This class has now been found five times, and four of those were the fix landed in one branch and the sibling beside it kept leaking — twice inside a single file.
#runStreamhas five result sites plus the chunk events, and a sixth added later would silently miss a per-site scrub. It cannot miss the wrapper.Chunks are scrubbed too — they become
partialOutputandstdout.logon disk, the same disclosure a beat earlier. A route with no key (every subscription CLI) takes an earlyyield*and pays nothing.Sabotage
Bypassing the wrapper fails exactly the two scrub tests. The third test — a no-key route's output arriving byte-identical — keeps passing, which is what says the common path is unaltered rather than merely untested.
npm run check: 1231 passed, 8 skipped, exit 0.