feat(security): deny-by-default child process environments (CF-02) - #142
Merged
Conversation
Four launchers spread `...process.env` into a child: the bundled brain, the local model child, the preview supervisor, and the device runtime. A parent holding AETHER_TOKEN, provider API keys, a device command key, or an MCP connector token handed every one of them to whatever it started. None of those children needs a credential. The dev server needs PATH and a HOME; the model child needs PATH and PYTHONUTF8. They inherited secrets because `...process.env` is the shortest thing to type. Adds src/core/child_env.ts and switches all four sites to it. The direction of the default is the whole design. This denies by default and passes by exception. Copy-everything-then-delete-the-sensitive-names has the failure mode every denylist has: the newest secret is the one it does not know about. A launcher that starts denying and forgets to allow something fails loudly in development; a launcher that starts copying and forgets to deny something ships a credential into a subprocess nobody audits. A test seeds a brand-new provider variable no pattern knows about and asserts it is still withheld. The name detector is reused, not reinvented: SENSITIVE_KEY comes from src/core/redaction.ts. Findings and diagnostics carry variable NAMES and a count, never values — an error that prints the secret it is complaining about has published it to every log that catches the throw. One bug found by this suite on its first run. SENSITIVE_KEY contains `pat` (for PAT tokens) and is case-insensitive, so it matches PATH, and an unqualified name check flagged the one variable every child needs most. That pattern documents itself as deliberately over-matching, and it can afford to be — its other consumers only fire on 32+ char hex values. Here it classifies bare names, where over-matching is not free. The name heuristic now applies only to names nobody has reviewed; the value-shape check still runs on everything, so an allowlisted HTTPS_PROXY smuggling `user:password@` is still caught. Both halves have regression tests. Tests: 23 in test/child_env.test.ts, including a seeded-credential corpus of 13 variables asserted absent from a child by name AND by value, so a rename cannot slip one through. Hermetic — every case passes an explicit source and nothing reads the real process.env. SCOPE: this is the child-inheritance half of CF-02. OS-native credential custody (Keychain / Credential Manager / libsecret) behind the existing TokenStore, and the mcp.json authToken migration, are NOT in this PR — src/core/auth.ts still carries its "prefer the OS keychain" TODO. Doing those properly needs platform verification on macOS and Linux that this change does not, and pretending otherwise in one PR would make the migration receipts untrustworthy.
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.
Lane
CF-02— the Agent half of the Credential Firewall. Specifically: the child-process inheritance gap the spec names.Base:
main@6c61ea17203a181ef5c04f682e57b634f24f1bf9(three-way verified). Cloud counterpart: AETHER-CLOUD#1512.The gap
The spec says "Several Agent child-process launch paths inherit the full ambient environment." That is exactly right, and it's four sites:
src/core/brain_bundled_child.ts:32env: { ...process.env }src/core/brain_local.ts:76env: { ...process.env, PYTHONUTF8: "1", ... }src/core/preview_supervisor.ts:173env: { ...process.env, HOST: "127.0.0.1", ... }src/commands/device.ts:274env: { ...process.env, AETHER_DEVICE_RUNTIME: "1" }A parent holding
AETHER_TOKEN, provider API keys, a device command key, or an MCP connector token handed all of it to a bundled model, a Python process, a dev server, and a device helper. None of them needs a credential — the dev server needsPATHand aHOME.The design decision worth reviewing
Deny by default, pass by exception. The alternative — copy everything, then delete what looks sensitive — has the failure mode every denylist has: the newest secret is the one it does not know about. A launcher that starts denying and forgets to allow something fails loudly in development. One that starts copying and forgets to deny something ships a credential into a subprocess nobody audits.
test("a brand-new credential name nobody anticipated is still withheld")seedsFUTURE_PROVIDER_XYZand asserts it never reaches a child — no pattern update required.The name detector is reused, not reinvented:
SENSITIVE_KEYcomes fromsrc/core/redaction.ts. Findings and diagnostics carry variable names and a count, never values — an error that prints the secret it's complaining about has published it to every log that catches the throw.A bug this suite caught on its first run
SENSITIVE_KEYcontainspat(for PAT tokens) and is case-insensitive — so it matchesPATH. An unqualified name check flagged the one variable every child needs most.That pattern documents itself as deliberately over-matching, and it can afford to be: its other consumers only fire on 32+ char hex values. Here it classifies bare names, where over-matching is not free. The name heuristic now applies only to unreviewed names; the value-shape check still runs on everything, so an allowlisted
HTTPS_PROXYsmugglinguser:password@is still caught. Both halves have regression tests, including one that assertsSENSITIVE_KEY.test("PATH")is genuinely true so the regression can't rot into a tautology.Test plan
The seeded corpus is 13 credential variables, asserted absent from a child by name and by value — a rename cannot slip one through. Hermetic: every case passes an explicit
source; nothing reads or mutates the realprocess.env. Four tests read the launcher sources and assert...process.envis gone andchildEnv()is called, so the fix cannot silently regress.⚠ Scope: this is half of CF-02
Not in this PR: OS-native credential custody (macOS Keychain / Windows Credential Manager / libsecret) behind the existing
TokenStore, and themcp.jsonauthTokenmigration.src/core/auth.ts:8still carries itsTODO: prefer the OS keychain over the file store, andsrc/core/mcp_store.ts:99still reportspermission bits not checked on Windows (ACLs not verified).Both are real and both are named in the spec. They need platform verification on macOS and Linux that this change does not, and the spec's own migration protocol demands a read-test inside the secure adapter before the pointer switches. Shipping that half unverified would make the migration receipts untrustworthy, which is worse than shipping it later — so it is called out rather than half-done.
READY_FOR_MERGE=true·MERGED=false·ENABLED=false— not self-merging, per the fanout handoff contract.