Skip to content

fix(farm): scrub dispatcher secrets from the pluggable mutation hook env#143

Merged
SUaDtL merged 2 commits into
mainfrom
fix/farm-mutation-env-scrub
Jun 26, 2026
Merged

fix(farm): scrub dispatcher secrets from the pluggable mutation hook env#143
SUaDtL merged 2 commits into
mainfrom
fix/farm-mutation-env-scrub

Conversation

@SUaDtL

@SUaDtL SUaDtL commented Jun 26, 2026

Copy link
Copy Markdown
Collaborator

What

Closes a least-privilege gap in the farm dispatcher: the pluggable FARM_MUTATION_CMD mutation hook spawned with { ...process.env }, so an operator-wired (possibly third-party) mutation framework inherited FARM_API_KEY and CLAUDE_CODE_OAUTH_TOKEN. It was the one child-spawn path that bypassed run()'s existing secret scrub, against the intent stated in run()'s own comment.

The fix adds a single shared helper in exec.ts:

export function scrubbedEnv(extra?) {
  const env = { ...process.env, ...(extra ?? {}) };
  delete env.FARM_API_KEY;
  delete env.CLAUDE_CODE_OAUTH_TOKEN;   // deleted LAST, after the merge
  return env;
}

Both run() and the mutation-hook spawn now route through it, so there is one scrub, not two that can drift. The hook still receives the FARM_MUTATION_* contract vars; only the credentials are stripped.

Why

This is the same least-privilege control run() already applied to git, gate, setup, and test commands (CodeQL #5). The mutation hook runs operator-supplied code in the worktree and has no need for the dispatcher's API key. Deleting the secrets after merging extra means a caller var can never re-introduce one.

Surfaced by auth-crypto-reviewer during the #140 (v2.rev.0020) review as a pre-existing gap, not introduced by that refactor. Conflict-hierarchy: a level-1 (security) tightening with no behavior change to any non-secret path.

Test plan

  • New subprocess integration test (farm.test.ts): the hook runs a committed probe that records its own env; the test asserts FARM_API_KEY / CLAUDE_CODE_OAUTH_TOKEN read ABSENT while FARM_MUTATION_FILES/TEST_PATH/TEST_CMD are present. Fails on the pre-fix code (the secret leaks), passes after.
  • New unit test (farm.unit.test.ts): pins the delete-last ordering by passing a secret in extra and asserting it is stripped, so a future reorder cannot silently break the defense.
  • Existing run() scrub test still green (now exercises the shared helper).
  • npx vitest run: 171/171. npx tsc --noEmit: clean. farm.js rebuilt and deterministic.

Reviews

All three path-matrix reviewers PASS, no CRITICAL or HIGH:

  • auth-crypto-reviewer: scrubbedEnv() removes both secrets after the merge; run() behavior identical; no real secret, TLS downgrade, or banned primitive.
  • security-reviewer: tightens the secret-store control; SHELL_OPTS cannot override the scrubbed env; the two sk-/tok- shaped values are fake test sentinels matching the sanctioned dummy-value convention.
  • coverage-auditor: about 85 percent, both scrubbedEnv paths exercised, both obligations verified. Its one LOW (untested delete-last ordering) is closed by the unit test in this PR.

Note on the commit gate

The compound-name secret detection from v2.rev.0016 (shipping in 2.6.0) flags the test's FARM_API_KEY:/CLAUDE_CODE_OAUTH_TOKEN: fixture lines. The secret-handling gate reviewed them (fake sentinels) and recorded a diff-bound pass before commit. The currently installed plugin (2.5.2) predates that detection, so its local hook does not flag them, which is expected release lag, not a gap.

Ref: v2.rev.0020 follow-up (auth-crypto-reviewer finding)

SUaDtL added 2 commits June 26, 2026 13:22
The pluggable FARM_MUTATION_CMD hook spawned with { ...process.env }, so an
operator-wired (third-party) mutation framework inherited FARM_API_KEY and
CLAUDE_CODE_OAUTH_TOKEN. It was the one child-spawn path that bypassed run()'s
least-privilege scrub, against the intent stated in run()'s own comment ("git,
operator gate/setup/test, mutation never need the dispatcher's secrets").

Add a shared scrubbedEnv(extra?) helper in exec.ts as the single source of
truth (merge caller vars onto process.env, then delete both secrets LAST so an
extra var cannot reintroduce one, CodeQL #5) and route both run() and the
mutation-hook spawn through it. The hook still receives the FARM_MUTATION_*
contract vars; only the credentials are stripped.

Proven by a subprocess test: the hook records its own env to a probe file; the
test asserts FARM_API_KEY/CLAUDE_CODE_OAUTH_TOKEN read ABSENT while the
FARM_MUTATION_* vars are present. Pre-existing gap surfaced by
auth-crypto-reviewer during the #140 (v2.rev.0020) review, not introduced there.

CHANGELOG: the pluggable mutation hook (FARM_MUTATION_CMD) no longer inherits
the dispatcher's API key or OAuth token, giving least-privilege parity with
every other child command.

Ref: v2.rev.0020 follow-up (auth-crypto-reviewer finding)
Claude-Session: https://claude.ai/code/session_0123LEtLuVCEzLMhWdktP7aC
Lock the security property that scrubbedEnv() deletes FARM_API_KEY /
CLAUDE_CODE_OAUTH_TOKEN AFTER merging caller-supplied vars, so an `extra`
entry can never reintroduce a scrubbed secret. The integration test proves the
mutation hook is scrubbed but does not pass a secret in `extra`, so a future
reorder of the delete before the merge would pass every test and silently break
the defense. This direct unit test pins the ordering.

Closes a coverage-auditor LOW finding on the env-scrub fix.

Claude-Session: https://claude.ai/code/session_0123LEtLuVCEzLMhWdktP7aC
Comment thread plugins/ca/tools/farm.js Dismissed
@SUaDtL
SUaDtL merged commit 836154a into main Jun 26, 2026
20 checks passed
@SUaDtL
SUaDtL deleted the fix/farm-mutation-env-scrub branch June 26, 2026 17:45
SUaDtL added a commit that referenced this pull request Jun 27, 2026
* docs(changelog): roll the 2.6.0 release notes

Reconcile the [2.6.0] CHANGELOG section for the release: broaden the intro
beyond the farm work, add context-drift provenance (#145) and file-scoped
context injection (#146) under Added, note the MIT to AGPLv3 relicense (#147,
ADR-0009) under Changed, and add a Fixed section for #143, #139, and #138.
Dated 2026-06-27.

Also flips v2.feature.0001 to done now that file-scoped context injection has
shipped and merged; the flip rides this release commit per ADR-0008.

No version bump: plugin.json is already 2.6.0 and the ca-scoped commit window is
unchanged. The relicense and docs commits touch no plugins/ca/ payload.

Claude-Session: https://claude.ai/code/session_01QaLw7nTrRbHwygFar2R6zu

* docs(license): reserve the dual-licensing right without offering it yet

Soften the README "Commercial licensing" wording. The prior text implied
proprietary licenses were actively on offer; the intent is narrower: the owner
retains sole ownership and reserves the right to dual-license, but is not
offering commercial licenses at this time. Inquiries may still be sent through
GitHub and will be considered if and when a commercial path is established.

The legal substance is unchanged. AGPLv3 plus sole copyright already secures the
right; this only corrects the public statement so it does not overstate an
active commercial offering. ADR-0009, the CLA, and the CHANGELOG already use the
reserves-the-right framing, so no other file changes.

Claude-Session: https://claude.ai/code/session_01QaLw7nTrRbHwygFar2R6zu

---------

Co-authored-by: SUaDtL <SUaDtL@users.noreply.github.com>
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.

2 participants