fix(bin): stop spawned workers from fetching operator credentials - #2428
Open
timidri wants to merge 1 commit into
Open
fix(bin): stop spawned workers from fetching operator credentials#2428timidri wants to merge 1 commit into
timidri wants to merge 1 commit into
Conversation
A worker doing an unrelated task previously fell back to the operator's unlocked password vault when its environment had no credential set, silently inheriting the operator's entire credential reach. Every generated brief now carries a standing hard safety contract: a worker never reads a credential store or runs its CLI, it stops and asks.
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
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
Stop spawned workers from reaching into the operator's password vault or any credential store. Credentials must be supplied to a worker deliberately at dispatch time, never discovered by it going looking. Concretely: (1) bin/fm-brief.sh's generated scaffold (ship, scout, and secondmate charter variants alike) now carries a standing 'Credentials - HARD SAFETY CONTRACT' clause, worded in the same hard-safety register as the existing worktree-isolation assertion (the closest precedent in that file): the worker must not read from the operator's password vault or any other credential store, must not run its CLI, and must not source any helper whose purpose is to populate credentials from one; if the task genuinely needs a credential, the worker stops and appends a needs-decision status line asking for it rather than fetching it itself. (2) AGENTS.md section 11 (Crewmate briefs) states this rule once, concisely, as a single sentence alongside the existing worktree-isolation sentence it is modeled on - no restatement of the reasoning, which stays in the scaffold. (3) Investigated whether bin/fm-spawn.sh's per-harness launch-command env scoping (its env -u / VAR= prefixes applied only to the launched worker process) could make this fail closed instead of relying on the worker following instructions. Conclusion: no reliable, clean, generic mechanism exists, and none was implemented - explicitly a documentation/instruction-only fix, not a mechanical one. Reasoning: this repo is deliberately vault-agnostic across operators (Bitwarden/rbw, 1Password, macOS Keychain, ssh-agent/gpg-agent, pass all differ), macOS Keychain access is session-based and not gated by any per-process environment variable at all, and blanket-unsetting the env vars that do gate some vaults (an SSH or GPG agent socket) would also break legitimate credential paths every worker is expected to use (gh-axi/git operations relying on agent-forwarded SSH or GPG signing) that have nothing to do with the vault-fallback failure mode being fixed. A narrower, tool-specific block would hardcode one operator's vault choice into a generic repo and risk false confidence for operators using a different vault. This finding must be stated plainly in the PR body: the constraint is enforced by instruction only, not mechanically, and that is a deliberate, investigated conclusion, not an oversight. Constraints honored: did not change how the operator's own vault, agent, or shell is configured anywhere outside this repo; did not read, print, or write any credential value; did not weaken or reword the existing worktree-isolation assertion while editing that file; comments explain why only, never restate the line below them, and never carry PR numbers; the words captain, first mate, crewmate, crew, and scout are avoided in commit messages and PR text. Verified by generating one brief of each variant (--mode no-mistakes, --scout, --secondmate --no-projects) and reading the rendered output to confirm the clause appears cleanly with no heredoc corruption in all three; added a new fm-brief.sh test (test_credentials_contract_renders_in_every_variant) covering all three variants, alongside the file's existing pattern for other safety contracts (e.g. the Herdr lab hard safety contract); ran bin/fm-lint.sh and bin/fm-doc-audience-check.sh clean; ran the full changed-tests suite and confirmed the only 4 failures are pre-existing on the unmodified base commit (unrelated environment/tooling issues: an unrelated PreToolUse hook test, a missing optional node package, a Kimi hook-install test, and a missing ruby asdf version needed by one CI-yml-parsing test), none related to fm-brief.sh, AGENTS.md, or the new test file.
What Changed
bin/fm-brief.sh's generated scaffold, worded in the same hard-safety register as the existing worktree-isolation assertion, across all three brief variants (ship/no-mistakes, scout, secondmate). The clause instructs a worker not to read from the operator's password vault or any credential store, not to run its CLI, and not to source any credential-populating helper — instead stopping and appending a needs-decision status line when a credential is genuinely required.AGENTS.mdsection 11 (Crewmate briefs) stating this rule alongside the existing worktree-isolation sentence.test_credentials_contract_renders_in_every_varianttotests/fm-brief.test.sh, verifying the clause renders cleanly in all three brief variants, following the file's existing pattern for other hard safety contracts.Note: this is a documentation/instruction-only fix.
bin/fm-spawn.sh's per-harness env scoping was investigated as a possible mechanical enforcement point but was found unsuitable — this repo is deliberately vault-agnostic (Bitwarden/rbw, 1Password, macOS Keychain, ssh-agent/gpg-agent, pass all differ), macOS Keychain access isn't gated by any per-process env var, and blanket-unsetting agent-socket env vars would break legitimate SSH/GPG-agent-backed operations every worker needs. No mechanical fail-closed guard was implemented as a result.Risk Assessment
✅ Low: Small, well-scoped documentation/instruction-only change: a verbatim credentials clause is correctly interpolated into all three brief heredocs via an existing pattern (mirroring HERDR_SECTION), AGENTS.md gets one concise sentence without disturbing the existing worktree-isolation sentence, a new test covers all three variants using existing assertion helpers, and no mechanical enforcement (e.g. in fm-spawn.sh) was added, consistent with the explicitly stated investigated conclusion.
Testing
Targeted testing confirms the credentials hard-safety-contract clause is correctly wired into bin/fm-brief.sh for all three scaffold variants: the existing fm-brief.sh test suite (21 assertions, including the new test_credentials_contract_renders_in_every_variant) passes cleanly, and manual generation of ship/scout/secondmate briefs shows the clause rendering verbatim and heredoc-corruption-free in each, matching the PR's stated verification steps. No findings.
Evidence: Rendered ship-variant brief showing the credentials clause
Evidence: Rendered scout-variant brief showing the credentials clause
Evidence: Rendered secondmate-charter brief showing the credentials clause
Evidence: tests/fm-brief.test.sh full run output
Pipeline
Updates from git push no-mistakes
✅ **intent** - passed
✅ No issues found.
✅ **Rebase** - passed
✅ No issues found.
✅ **Review** - passed
✅ No issues found.
✅ **Test** - passed
✅ No issues found.
bash tests/fm-brief.test.sh (full file, 21 checks, all pass)Manual generation: bin/fm-brief.sh <id> <repo> --mode no-mistakes (ship variant)Manual generation: bin/fm-brief.sh <id> <repo> --scout (scout variant)Manual generation: FM_SECONDMATE_CHARTER=... bin/fm-brief.sh <id> --secondmate --no-projects (secondmate variant)Visual grep confirmation of '# Credentials - HARD SAFETY CONTRACT' clause text in all three rendered briefsgit status --porcelain confirming no stray artifacts left in the working tree✅ **Document** - passed
✅ No issues found.
✅ **Lint** - passed
✅ No issues found.
✅ **Push** - passed
✅ No issues found.