Repository navigation
fix: harden Claude credential guards for launches and relaunches - #7
Merged
Merged
Conversation
…ocs updates - Extract Claude API key guard into bin/fm-api-key-guard-lib.sh for reuse - Replace inline guard in fm-spawn.sh with library call - Add guard pre-check in fm-control before do_exit (R2-5 option a) - Check tmux GLOBAL environment in addition to session env (R2-1) - Update docs for pin exemption, tmux check, pane-rc/direnv note (R2-4, R2-6) - Fix shellcheck warnings
…very non-opt-in claude launch - Strip credential vars from worker env for all claude launches without --allow-api-key, closing the relaunch-into-old-pane gap - Document known gap in docs/configuration.md - Add tests for the env -u prefix in launch command
…e other 2 checks do not come from this PR. 1. "Behavior portable serial 8". This PR caused it. The failing test was tests/fm-spawn-dispatch-profile.test.sh. Its message was "no-profile claude launch did not use the canonical launch kind". This PR adds `env -u ANTHROPIC_API_KEY -u ANTHROPIC_AUTH_TOKEN` to every Claude launch that does not use --allow-api-key and has no worker-account pin (bin/fm-spawn.sh:5161). That is the intended behavior. But the test helper `claude_expected_launch` (tests/fm-spawn-dispatch-profile.test.sh:1679) still built the old launch line. All 4 callers of that helper are non-opt-in launches with no pin. So I added the same credential shed to the expected line in the helper. I did not change product code. - Proof: I ran `bash tests/fm-spawn-dispatch-profile.test.sh` locally. It printed "# all fm-spawn-dispatch-profile tests passed" and exited 0. shellcheck -x on the file also passed. The test took over 20 minutes here because other jobs were loading the machine. It did not hang. 2. "Behavior portable serial 2". The failure is in tests/fm-calm-pi-extension.test.sh: "rendered export DOM violated the Calm conversation boundary". This PR did not cause it. The same test fails the same way in CI run 36658046545 on an unrelated PR (fm/fm-subagent-guard-fix, PR 6). This PR does not touch any Pi or Calm code. The main branch CI passed on 2026-09-29, so the cause is probably outside the repo (for example, a newer Pi or Chrome version in CI). I made no change. 3. "Behavior portable serial 4". The failure is in tests/fm-pi-branch-extension.test.sh: "Calm-off ToolExecutionComponent rendering differs from Pi stock". This PR did not cause it either. The same failure shows up in the PR 6 run above. I made no change. What changed: 1 line in tests/fm-spawn-dispatch-profile.test.sh. What you do now: nothing for shard 8. Shards 2 and 4 will stay red until someone fixes the Pi test drift in a separate change
…ork-pr6 # Conflicts: # bin/fm-control.sh # bin/fm-spawn.sh # docs/configuration.md # tests/fm-spawn-claude-api-key-guard.test.sh # tests/fm-spawn-dispatch-profile.test.sh
…d shell compatibility
MrGTV-love
changed the title
… 2, 4, 7, and 8. Their assertions depended on shell-command quoting that changed when credential shedding wrapped the complete Claude launch expression. Affected tests now execute captured launches with model-free probes and assert received argv/environment. The shared TeamClaude assertion retains executable proxy/CA checks instead of inspecting launcher spelling. Removed obsolete snapshot helpers and updated the contributor helper inventory. Production credential shedding and explicit opt-in behavior are unchanged. Verification: all six failing test scripts passed locally, plus the API-key guard and worker-account regression suites. Compound-launch credential cases passed. Stock Bash syntax checks passed for all six modified shell files, documentation validation passed, and a shell smoke verified NUL-delimited arguments preserve spaces, empty values, and newlines. An initial run used an invalid in-repository temporary secondmate layout; affected scripts passed after correcting that setup. One optional Cursor case skipped because its executable was unavailable. Some existing cleanup/BSD-sed warnings were emitted, but the scripts returned 0. Full CI shards were not rerun
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.
Intent
ship
(Captain's board-88 answer, 2026-09-28, on the card for the two API-key guard commits that never landed.) Later, 2026-09-29: "I don't like dangling work. Why was it not pushed, PR'd and merged?"
Context: fork PR 2 (#2, "refuse Claude worker launch when an Anthropic API key would reach it") landed the first two commits of branch fm/fm-claude-worker-api-key-guard. Two later fix commits never shipped: 958c95ed "Fix round 2: extract guard to shared lib, add fm-control pre-check, ..." and 341b4d0c "Fix R3-1: apply env -u ANTHROPIC_API_KEY -u ANTHROPIC_AUTH_TOKEN to ...". Source copy: ~/.treehouse/firstmate-7bab20/10/firstmate (task fm-claude-worker-api-key-guard; keep it until this PR merges).
What Changed
Risk Assessment
🚨 High: The claimed durable credential containment remains bypassable through the supported compound raw-command interface, allowing unintended API billing without explicit opt-in.
Testing
Reproduced the baseline credential escape and verified the target through real Claude/Codex CLI launches, isolated tmux environment checks, persisted metadata, and unchanged foreground-Claude state after refused relaunches. Focused guard/control regressions passed after resolving the initial driver timeout; successful authenticated account-pin launches could not be exercised live. Product transcripts and observations were saved, and disposable validation state was removed.
Evidence: Live spawn and control CLI transcript
Source: Live spawn and control CLI transcript
Evidence: Baseline leakage versus target credential containment
Source: Baseline leakage versus target credential containment
Evidence: Compound-launch credential observations
Source: Compound-launch credential observations
Evidence: Real Claude foreground process preserved by refused relaunch
Source: Real Claude foreground process preserved by refused relaunch
Evidence: Replacement metadata retaining explicit API billing opt-in
Source: Replacement metadata retaining explicit API billing opt-in
Evidence: Actual terminal transcript with Claude and Codex version output
Source: Actual terminal transcript with Claude and Codex version output
Evidence: Disposable Claude account authentication status
Source: Disposable Claude account authentication status
Evidence: Private lab teardown and socket-path accommodation
Source: Private lab teardown and socket-path accommodation
Pipeline
Updates from git push no-mistakes
✅ **intent** - passed
✅ No issues found.
✅ **Rebase** - passed
✅ No issues found.
bin/fm-spawn.sh:5177- The stale-pane fix does not cover an entire supported compound raw launch. With clean caller/tmux environments, no account pin or allowlist, and an existing pane exporting ANTHROPIC_API_KEY, a raw command such asclaude --version && claude --dangerously-skip-permissions ...passes preflight. The generatedenv -u ...cleans only the first command; the second Claude process inherits the unchanged pane credential and can bill the API without --allow-api-key. Compound raw commands are supported by tests/fm-spawn-dispatch-profile.test.sh:474, and docs/configuration.md:1019 explicitly includes raw Claude launches. Apply credential shedding at the shared execution boundary enclosing the entire launch expression, preserving opt-in behavior. Related sites: bin/fm-spawn.sh:5161 and :5163—the pinned-account prefixes likewise cover only the first raw command and must retain their broader shedding invariant; bin/fm-spawn.sh:5373—an allowlist explicitly retaining either credential leaves the same compound-command escape reachable; docs/configuration.md:1033—the unconditional captured-pane containment claim is not currently true. Both ANTHROPIC_API_KEY and ANTHROPIC_AUTH_TOKEN are affected.tests/fm-spawn-dispatch-profile.test.sh:1614- The change restores an unused expected-command snapshot path: claude_worker_add_dirs at :1614 is used only by claude_expected_launch at :1622, which is used only by test_claude_permission_mode_bypass_matches_absent_launch at :1631. That test is never invoked, and its expected-command helper references ai_trailer_hooks_prefix, which has no definition in the repository. No API-key guard intent requirement needs this dormant permission-mode test or parallel launch-command reconstruction. Remove the three definitions rather than activating and repairing this unrelated path.🔧 Fix applied.
1 warning still open:
tests/fm-spawn-dispatch-profile.test.sh:1614- The change restores an unused expected-command snapshot path: claude_worker_add_dirs at :1614 is used only by claude_expected_launch at :1622, which is used only by test_claude_permission_mode_bypass_matches_absent_launch at :1631. That test is never invoked, and its expected-command helper references ai_trailer_hooks_prefix, which has no definition in the repository. No API-key guard intent requirement needs this dormant permission-mode test or parallel launch-command reconstruction. Remove the three definitions rather than activating and repairing this unrelated path.✅ **Test** - passed
✅ No issues found.
claude --versionconfirmed the installed real CLI: Claude Code 2.1.291.bin/fm-lab-home.sh create <worktree>/lcreated a disposable marked home; real tmux ran on its privatefm-labsocket with a 120×40 pane.bin/fm-spawn.sh guard-fresh-02 <disposable-project> --scout --harness claude --backend tmuxrefused each ambient Anthropic credential before creating a task record or worker window.bin/fm-spawn.sh guard-live-01 --relaunch --harness claudeand its compound raw-command variant exercised ambient, tmux-session, tmux-global, and allowlist credential refusals.Real tmuxset-environmentoperations exercised session removal markers and empty session values overriding a global credential.bin/fm-spawn.sh guard-live-01 --relaunch --harness 'claude --version && ... && claude --version && ...'exercised stale-pane credentials, retaining/filtering allowlists, and--allow-api-key; shell observations recorded the environment surrounding both real Claude invocations.Executed the base revision'sfm-spawn.shwith only its self-location bootstrap adapted forbash -c; reproduced credential leakage, then reran the same scenario against the target and observed both credentials unset.bin/fm-control.sh guard-live-01 relaunch --note 'Live pre-stop guard validation'refused ambient and tmux credentials; against a foreground real Claude process, its process identity and terminal contents remained unchanged.bin/fm-spawn.sh guard-live-01 --relaunch --harness 'codex --version'succeeded with an ambient Anthropic key and producedcodex-cli 0.160.0.bin/fm-control.sh guard-live-01 relaunch --harness claude --note 'Keep deliberately opted-in API billing'carried recordedapi_key=allowinto replacement metadata.claude auth status --jsonand actual spawn attempts with ordinary and named isolated account pins confirmed those disposable accounts were not signed in.bash tests/fm-spawn-claude-api-key-guard.test.shcompleted successfully with a longer allowance after the initial 180-second driver timeout.Executed only five guard-related functions fromtests/fm-control-relaunch.test.sh: pre-stop refusal, replacement-profile selection, tmux refusal, opt-in persistence, and direct-relaunch opt-in removal. All completed successfully; these were supplemental mock-backed regressions.Killed only the privatefm-labtmux server, confirmed it was absent, checked for remaining processes referencing the disposable test root, and removed both worktree-local validation directories.✅ **Document** - passed
✅ No issues found.
🔧 **Lint** - 1 issue found → auto-fixed ✅
🔧 Fix applied.
✅ Re-checked - no issues remain.
✅ **Push** - passed
✅ No issues found.