Keep check text and API keys away from the worker

[solvyxtech]

Fixes the two confidentiality findings: hidden check text readable from argv (#27 bug-hunt) and keys readable from /proc/$PPID/environ (#23 bug-hunt). Also two items from the #29 round-2 review. Stacked on #36.

1. Check text never in argv (src/run.ts)

runCommand(..., { hideCommand: true }) spawns <sh> -c HIDDEN_WRAPPER, a fixed script:

__maat_c=$(cat <&3) || exit 125; exec 3<&-; eval "unset __maat_c; $__maat_c"

The command comes in on fd 3, a socket that can't be reopened through /proc/<pid>/fd. The wrapper reads it to the end and closes fd 3 before the check runs, so nothing the check starts inherits it. The variable is never exported and is unset before the first command of the check. This is used at every check run site on this branch: runCheck, mutation baseline/mutated/negated runs, preflightCriteria trials (drafting and the seal-time screen), the reference try, and mission assertions. It covers all check runs, hidden or not. A test fails if a runCommand in bar/criteria/mission, or the reference check.run call, is missing hideCommand: true. #34's late pre-work tries and #28's objection runs aren't on this chain. They need the flag when they rebase, and that guard test will flag them if they're missing it.

Overlaps #38, which uses the same fd-3 approach. Differences are in the review comment there. The main one is that #38's eval "$(cat <&3)" leaves fd 3 open, so the deliverable inherits it.

2. Keys out of every environment (src/secrets.ts)

• captureSecrets() runs first in the CLI and in Electron main. It moves every credential-shaped variable out of process.env into memory: *_API_KEY, *_TOKEN, *_SECRET, *_ACCESS_KEY, *_SECRET_KEY, *_PASSWORD, *_CREDENTIAL(S), XAI_*, GROK_*, TOGETHER_*, MAAT/MOLT_API_KEY, MAAT/MOLT_JUDGE_KEY. env(), judgeTarget and redaction read the in-memory copy. ACP/OpenCode backends get their keys back with withSecrets. The worker's bash and background jobs get scrubEnv (same shapes). MAAT_KEEP_ENV=NAME,... lets a task keep its own token.
• /proc/<pid>/environ keeps the exec-time copy, so the real fix is to keep keys out of the exec env entirely: MAAT_KEYS_FD (read and closed at startup) or MAAT_KEYS_FILE (must be a 0600 regular file owned by Maat's user, otherwise Maat refuses to start). Either one takes NAME=value lines or JSON.
• bench/local/run.py passes keys on a pipe (pass_fds, verified through runuser in the container). run-in-container.sh mounts a root-only key file instead of --env-file, which had put the key in the container env and in docker inspect.
• Degraded path (Run the worker's tools as another user: --worker-user (builds on #27) #29, --worker-user without --worker-strict): that code is only on claude/priv-sep. Here scrubbedEnv() now calls scrubEnv, so when Run the worker's tools as another user: --worker-user (builds on #27) #29 rebases onto this the fallback path gets the wide list. Until then Run the worker's tools as another user: --worker-user (builds on #27) #29's fallback still uses the six-name SECRET_ENV.

3. Check copy re-checks at copy time (src/scratch.ts)

copyRegular opens with O_NOFOLLOW|O_NONBLOCK, fstats the descriptor and copies only regular files. A symlink or FIFO swapped in after readdir is skipped.

Same-uid is best effort

docs/secrets.md covers what this does and doesn't protect against. Recommendation: make --worker-user the default for unattended and bench runs. It's the real boundary: a same-uid worker can still read Maat's auth file and, where ptrace_scope=0 (it was 0 in the node:22-bookworm container), Maat's memory. I haven't flipped that default because #29 owns it.

Tests

• test/secret-hygiene.test.ts, 14 tests:
  • A marker check runs while every process's cmdline and environ is scanned (/proc on Linux, ps -E on macOS). An unhidden control run must be found by the same scan.
  • A child of the check walks every ancestor's argv/environ and checks fd 3. This runs through runCommand, runCheck (hidden task check) and preflightCriteria.
  • copyRegular with a symlink and a FIFO.
  • Capture, fd and file loading, and the refused 0644 file.
  • The real CLI with a stub provider whose bash call dumps its own env and every ancestor's argv/environ, with keys given by fd, by file and by env. With env delivery, the Linux control confirms the key is still in /proc/$PPID/environ, which is why the fd/file path exists.
• bench/local/tests/test_keys_handover.py.
• npm run check with ~/.local/bin off PATH: 2038/2038 pass, e2e and self-check PASS.
• node:22-bookworm container, as the non-root node user: secret-hygiene 14/14 pass. Fd handover through runuser: the key arrives on the fd and is absent from /proc/self/environ.

Gaps left open

• A check whose own argv contains its logic (python3 -c "...") still exposes that argv to the deliverable it imports.
• Same-uid ptrace/memory access, Maat's auth.json, and backend processes (Grok/OpenCode) that hold keys in their env.
• bench/harbor still passes MOLT_API_KEY via exec env.
• maskText is still exact-substring.

[solvyxtech]

Verification for this PR:

• npm run check with ~/.local/bin removed from PATH: typecheck, 2038/2038 tests, e2e and self-check all PASS.
• node:22-bookworm container, non-root node user: test/secret-hygiene.test.ts passes 14/14. That includes the /proc scan of all processes, ancestor walks from a deliverable run by a hidden bar check and by a drafting trial, and the real CLI with keys delivered by fd and by file (no key found anywhere). The env-delivery control confirms the key is still in /proc/$PPID/environ when keys come in through the environment.
• Bench: pytest bench/local/tests 23 pass. In the container, run.keys_by_fd through runuser -u node: the key arrives on the fd and is absent from /proc/self/environ.

This overlaps #38 on the check-text half. My review there covers the inherited fd 3, the missing mission site and the Linux-only tests. Whichever lands first, the other rebases onto it.

[solvyxtech]

Review of #40 (Grok, automated)

Verdict: no blocking issues. I'd approve once CI has actually run. It's posted as a comment review because GitHub won't let this account approve its own PR.

Pairing rules

• The branch is claude/secret-hygiene into claude/completion-reserve, so nothing targets main. ✅
• No subscription backends are added or brought back. ✅
• No competitor names appear in product copy. docs/secrets.md only names OPENROUTER_API_KEY as an example variable, and the code already uses that name. ✅
• Nothing touches fine-tune data or the Grok exclusion. ✅

Verification (Linux box, Node 20.19)

• tsc passes for both tsconfig.json and tsconfig.app.json.
• npm test gives 2038/2038 pass, and the a check's command text / secret-hygiene suites ran green. Node's coverage reporter crashed afterwards (Could not report code coverage ... reading 'line'), so the runner exited 1. I think that's this box's Node version, not this PR.
• I ran the wrapper by hand under dash and bash. A heredoc whose terminator is the last line works, and so do set -e, exit N, return inside a function, and keeping trailing newlines in output. A syntax error gives exit 2, $0 is the shell, and __maat_c is unset when the check runs.
• CI did not run. typecheck-and-test failed in 5s with "The job was not started because recent account payments have failed or your spending limit needs to be increased." Please don't merge on that red X or treat it as green. Re-run it once billing is sorted.

Blocking

None.

Non-blocking

1. A directory swapped for a symlink can still get followed into the check copy. src/scratch.ts:242 (and :212, :337)
Confidence: medium-high that the hole is real, medium that it matters before #29 lands.

• copyRegular uses O_NOFOLLOW, but that only protects the last part of the path. copyDir decides something is a directory from the dirent, then calls copyDir(s, …) → readdir(s), and both follow symlinks.
• Failure scenario: a worker on another uid (the --worker-user setup from #29) swaps pkg/ for a symlink to a directory only Maat can read, such as Maat's config dir. It does this after the parent readdir and before the recursive call. readdir and every open(pkg/<file>, O_NOFOLLOW) then go through the symlink, and Maat-only files end up in the copy where the deliverable runs.
• Why it matters: the PR body says "a symlink or FIFO swapped in after readdir is skipped". That's true for files but not for directories, and #29 is the case this defence exists for.
• Suggested fix: open each directory with O_RDONLY|O_DIRECTORY|O_NOFOLLOW. Compare fstat against the lstat dev/ino from the listing, then walk through /proc/self/fd/<n>/ on Linux so the directory can't change under you. On macOS, the closest you can get is checking dev/ino before and after each file. At minimum, narrow the claim in the PR body and docs/secrets.md.

2. ACP backends get every captured credential, not just their own. src/acp.ts:536, src/secrets.ts:217
Confidence: high that it happens; low-medium on how much it matters.

• withSecrets puts back everything in the store, including GITHUB_TOKEN, cloud keys and NPM_TOKEN, because they all match the new wider patterns.
• This isn't a regression, since before this PR the backend inherited all of process.env. But the backend can run some tools of its own (the unaccountedTools path), so those inherit the lot.
• Suggestion: give each backend only the keys its spec names, the way opencodeChildEnv already drops other providers' keys.

3. The worker's environment changes for real tasks. src/secrets.ts:31
Confidence: high.

• The *_TOKEN / *_CREDENTIALS patterns now take GITHUB_TOKEN, GH_TOKEN, NPM_TOKEN/NODE_AUTH_TOKEN and GOOGLE_APPLICATION_CREDENTIALS (a file path) away from the worker and from checks. That means npm ci against a private registry, gh calls, or cloud SDK tests will fail without explaining why.
• MAAT_KEEP_ENV covers it, but only if someone knows it exists. Suggestion: when a turn's command fails and a removed name looks relevant, add a one-line info event such as "removed from the worker env: NPM_TOKEN (keep with MAAT_KEEP_ENV)", and list it in the CHANGELOG.

4. Check copies lose copy-on-write cloning. src/scratch.ts:347
Confidence: medium.

• copyFile(…, COPYFILE_FICLONE) used to clone on APFS and btrfs. copyRegular now copies every byte, so trees near maxBytes will copy noticeably slower on the Mac bench.
• open(d, "wx") also throws EEXIST where copyFile used to overwrite. One case: a case-sensitive source tree with Foo and foo, copied into a case-insensitive /var/folders. The whole copy then falls back to running in place.
• Possible fix: once the open descriptor has passed the regular-file check, copy from it with copyFile on /dev/fd/<n>, or accept the cost and note it.

5. The wrapper needs cat on PATH. src/run.ts:53
Confidence: low.

• If a check's env has no usable PATH, it now exits 125 before running, and that looks like the check failing.
• A loop using only the shell's built-in read (while IFS= read -r l <&3 || [ -n "$l" ]; do …) avoids the outside binary. Alternatively, map 125 to "infrastructure error" rather than "check failed".

Overlap with #38 (grok/hide-check-argv)

I checked the claim and it's right: #38 runs eval "$(cat <&3)" without closing fd 3, so the deliverable inherits it. #40's version is the more complete fix and covers more call sites. I suggest landing the argv part from #40 and closing #38 once this merges. I'll leave that decision to Tyler.

[solvyxtech]

Verdict: APPROVE (pending a real CI run)

Prior items: there were no blockers before, and there are none now. Non-blocking items 1–5 are all still open, because the PR hasn't changed. The head is still 09794ce, the commit I reviewed in 5450220906, and the base claude/completion-reserve hasn't moved either.

Fresh pass from Grok, comment only. I re-ran it at 09794ce: npm run typecheck is clean and the suite passed 2037 of 2038. The one failure was acp-deadline "ends the job on time…", a timing test that ran over while three suites were going at once on this box. On its own it passes 13 of 13, and the same flake showed up on #45 and #46, so it isn't this PR. CI hasn't run because of the account-wide GitHub Actions billing block, so please re-run it before merging. I found nothing new.

Still recommended, in order: close #38 once this merges (this is the better fd-3 fix), make the directory walk in src/scratch.ts symlink-safe before #29's separate worker user lands, and add a CHANGELOG line for the *_TOKEN env removal (MAAT_KEEP_ENV).

[solvyxtech]

Round-3 review: no blocking items. Integration (claude/integration-all@3d1ca16, #49) takes this PR's fd-3 wrapper, and #38 is not merged. Under --worker-user the wrapper and fd 3 also go through checkSpec/commandSpec. The post-work audit's check runs (#42) were added to the hideCommand source guard. The withhold.ts masking note from #38 was carried over. The non-blocking items (directory symlink swap, per-backend keys, env-removal notice, FICLONE, cat on PATH) are still open. Integration supersedes this branch.