feat(cli,mcp): the seat credential reaches its MCP servers as a path, not as an inherited environment value - #1801
Merged
Merged
Conversation
…y declaration (TASK-083)
A declared source is authoritative: a failed read throws and never falls
through to the weaker channel. New COMMONLY_TOKEN_FILE=<path> branch, read
to EOF and trimmed, refusing an empty credential. Needed because claude and
codex cannot be handed a pipe we opened — claude expands ${VAR} from its own
environment and codex forwards declared variables from its own — so a path
is the strongest channel either of them can carry.
6 new tests; 5 mutations, each reddening a distinct witness, no survivors.
…ile (TASK-083)
The pi bridge spawned every declared MCP server with {...process.env, ...declared},
so a third-party server inherited the seat token and the model key — measured on
2026-09-19: a @playwright/mcp process held COMMONLY_AGENT_TOKEN, and a pi seat's
MCP child held COMMONLY_LITELLM_KEY. The child environment is now the derived
allowlist {PATH, HOME, TMPDIR, and the proxy/CA names} plus what the entry itself
declares.
The credential may now also be named as a PATH (COMMONLY_TOKEN_FILE) instead of
carried as a value: the bridge reads that file and pipes the token on fd 3, so the
child is handed neither the secret nor the path. A declared file is authoritative
(a failed read throws rather than falling through), and an old pinned server that
cannot read the pipe still gets the value in its environment, with the warning.
New cli/src/lib/credential-file.js writes the per-spawn 0600 file (chmod asserted
after the write, since writeFileSync's mode is filtered by umask) inside a 0700
directory. Also corrects the stale doc line claiming the staging seats run 0.3.4;
they run 0.3.7 (measured).
14 new tests, including two real-child probes that read their own fd 3 and their
own environment; 9 mutations, no survivors.
…sion (TASK-083) The channel question — does this declared server read a pipe, a file, or only a value in its environment — was about to be answered separately in the pi bridge, the claude adapter and the codex adapter. Three copies of a version threshold drift, and the drift is silent in whichever one nobody is looking at, so the predicate moves to mcp-server-version.js (describeMcpCommand, the pipe-reader release 0.3.11 and the file-reader release 0.3.12) and the decision to mcp-credential-delivery.js. deliverSeatCredential rewrites a DECLARATION, not a value: an entry naming COMMONLY_AGENT_TOKEN comes back naming COMMONLY_TOKEN_FILE, so the token never exists in the runtime's environment for its whole process subtree to inherit. Four honest outcomes: 'path' (moved), 'env' (kept deliberately — somebody else's server, or a checkout that predates the file reader, each with its own warning), 'none' (no credential declared) and 'unavailable' (no launcher file this spawn, so nothing is rewritten rather than pointing at nowhere). 10 new tests. Also moves CREDENTIAL_KEY beside CREDENTIAL_FILE_VAR so the two channels are named in one place.
…ot a token (TASK-083) claude spawns its MCP servers inside its own process tree, so there is no pipe to inherit: the only two channels are a value in claude's environment, which every child and every hook inherits, or a PATH that the declared server opens itself. The default declaration takes the path. deliverSeatCredential rewrites the config claude actually reads — an entry naming COMMONLY_AGENT_TOKEN comes back naming COMMONLY_TOKEN_FILE — so the token is never in claude's environment to be inherited, logged, or dumped by any other server the seat was granted. WHERE the file lives is part of the contract, measured rather than assumed: sandbox/seatbelt.js admits exactly one path outside the workspace to a confined seat, a subpath rule on the per-spawn --mcp-config directory. A credential file under ~/.commonly would be unreadable by the very child it was written for, so the file is written inside that directory, in a 0700 subdirectory, at 0600, and removed by the spawn's existing finally block. A second handover is deliberately NOT moved, and is now warned about: an entry naming the token in args, url, headers, or inside a larger string gets a literal substitution from claude, where a file path is not a value. The expansion loop already exposed only values the declaration references, and the rewrite is what stops the default declaration referencing the token — so the value survives for those spawns, and only those, with a warning naming the trade. That is the carve-out TASK-082 exists for (the grant broker's bearer header), and the three pre-existing claude environment tests are its witnesses: they pass unchanged. 11 new tests, plus C1-C8 mutations with no survivors (the config rewrite, the file's location, the cleanup, the warning, the expansion guard, the stranger branch, the version threshold, the no-file branch). Full cli suite 43/690.
… (TASK-083)
Same decision as the claude half, different mechanism. codex takes its MCP
servers as `-c mcp_servers.*` overrides, and its existing anti-leak measure
diverted a token-bearing ENV VALUE into `env_vars`, which codex resolves from its
own environment — measured on a live codex seat: three MCP children each carrying
a 73-char token. The declaration rewrite removes the need for that route
entirely: the credential becomes COMMONLY_TOKEN_FILE, a path, which is not
token-bearing, so it lands in `env={...}` on the command line where a path is
harmless and the child opens the file itself.
Substituting the file placeholder needed one line in SUBSTITUTION_KEYS, because
the path itself is the thing being handed over.
Two things this change made visible, both now handled rather than assumed:
- A token wanted as a COMMAND ARGUMENT had no env_vars route and was substituted
literally into `-c mcp_servers.<name>.args`, i.e. onto a command line readable
by every same-user process — while the doc block in this file promised bearer
tokens never ride in argv. That entry is now refused whole, with the reason,
rather than half-wired or published.
- A declaration that needs the token as a literal (an env value containing it, or
a command argument) still gets it forwarded, and now says so: the seat whose
environment carries a secret is named instead of assumed fixed.
The credential file is written into the per-spawn directory this adapter already
creates and removes for --output-last-message, so a child codex spawns can reach
it and no second lifecycle has to be right.
Updated the existing mcp-override regression test, whose expected argv was
`env_vars=["COMMONLY_AGENT_TOKEN"]` — the leak this change closes — and added
three tests for the carve-out and the argv refusal. D1-D6 mutations, no
survivors. Full cli suite 43/693.
…er file (TASK-083) The pi bridge already had the strongest channel — it reads a credential file and puts the VALUE on the child's fd 3, so the token never reaches a child's environment — but nothing wrote that file for a pi seat, so the declaration had to name the token variable and pi substituted the value into the payload it hands the bridge. Now the launcher writes the file and the declaration names it: the payload carries a PATH, the bridge reads it, and the value still travels only on fd 3. The file is written into this seat's own 0700 home, one per spawn, and removed in a finally when the turn ends — a token file that outlives its turn is a token file that sits in a home for the next one. pi has no enforced sandbox, so the seat home is the narrowest place the bridge can still read; the claude and codex halves cannot use it and write into the per-spawn directory each already hands its runtime instead. Updated the fd-3 payload test, whose expected payload was the token itself, and added two tests: one that witnesses the file DURING the spawn (holding the token, 0600, inside a 0700 directory in the seat home — an after-the-fact read can only prove absence, since the finally is deliberate) and one that a declaration asking for no credential is untouched. E1-E4 mutations, no survivors. Full cli suite 43/695.
…s it (TASK-083) Emptying the token out of a seat runtime's environment has a second consumer nobody asked about: the tool-policy hooks are child processes of that runtime, so they read exactly the environment the token was removed from. Left alone they would find nothing, and because a hook fails OPEN by design (an unwritable ledger must not become an accidental blocker), the symptom would not be an error — it would be a hook that silently stopped deciding anything. So resolveHookToken prefers the launcher file and falls back to the value variable for a seat whose declaration has not migrated. Unlike the MCP reader it does not throw on a declared-but-unreadable file: a hook that dies is a hook the runtime reports as broken, and failing open is the documented posture here. The shipped default declaration stays EXACTLY canonical — it still names the token variable, and each adapter rewrites it per spawn. That is deliberate for deploy skew: a record that named the file directly would be one an older CLI could not serve, and the guard's canonical-shape check is a security property rather than a formality, so it is left alone and the decision is asserted instead of widened. Tests: four for the hook credential (file wins, unreadable falls back, empty falls back, a blank declaration is not consulted, and the pre-existing behaviour) and one asserting the shipped entry remains canonical. F1-F4 and G1 mutations, no survivors. Full cli suite 43/699.
Two slots, and the numbers are the ones free behind #1780's 0.1.61/0.3.11 — re-derive both against main if the queue moves before this is cut. The cli bump is for the three adapter changes and the hook read; the mcp bump is for the credential reader, which resolves fd, then file, then env, and treats a declared source as authoritative. Both files are touched only on the version line.
…TASK-083)
Vera 70455: four cli tests named the value in the runtime's environment, and the
value they found was the runner's own. The declaration rewrite moved our MCP
servers onto a launcher file, but every adapter derives its runtime environment
from `process.env` -- where the documented bootstrap export lives -- so the
credential came back into the runtime and from there into every child below it:
claude's MCP children and hooks, codex's MCP children, pi's own bash tool (which
spawns with `{ ...process.env }`).
One rule, in the module that already owns the delivery decision
(`withholdRuntimeCredential`): the PATH of this spawn's file goes into the
runtime environment, because a path is not a secret and a hook resolves its
credential from it; the VALUE is deleted, unless a carve-out genuinely needs it
there. The adapter passes that answer in rather than inferring it -- claude's
literal args/url/headers, codex's forwarded env -- so the spawn that still
carries a secret is the one that warns about it.
The hook path was wired to nothing. `resolveHookToken` (launcher file first,
value second) existed, was documented, and was unit-tested, while `agent
hooks-forward` -- the command hooks-config actually installs into Claude
settings -- read `process.env.COMMONLY_AGENT_TOKEN` directly. On a migrated seat
the value is absent, so the forwarder returned `hook_unavailable` and the
tool-policy hook silently stopped deciding anything: the fail-open posture turns
a dead credential into a silence rather than an error. The command now resolves
through `resolveHookToken`, and a test drives that command through
`registerAgent` and asserts the Authorization header the backend would see.
Tests: the four that failed under an exported variable are now positive
controls -- they plant the launcher's value explicitly instead of inheriting
whatever the runner happens to have, which is why they passed in one worktree
and failed in another. New: a pi witness (value out, path in, provider key
kept) and the hooks-forward end-to-end witness. Mutations M1-M7, all run with
the variable exported: no survivors, every source restored byte-identical.
samxu01
force-pushed
the
kai/scrub-env
branch
from
September 20, 2026 01:53
4bd5af7 to
15f303f
Compare
samxu01
pushed a commit
that referenced
this pull request
Sep 20, 2026
… 58) The post-deploy acceptance for the credential-delivery change (#1801) is "read the spawned adapter child's environment and confirm the runtime token is not in it". Measured on this box, that read cannot be trusted as written: `ps eww` does not always return an environment and does not say so when it doesn't. On the same command the codex adapter child read 8606 bytes with `PATH=` present and its MCP child 2166 bytes, while `/bin/sleep`, `/bin/bash -c`, `/usr/bin/tail`, the `npm exec` shim and the live pi adapter child returned argv only (75-96 bytes, no `PATH=`) - including a `sleep` spawned by the readable `python3`, so the spawner is not the discriminator. A check that concludes `withheld` from a read that saw nothing returns the answer everyone wants. This makes the rule explicit and testable: `classifyEnvRead` may only report `token_withheld` when a control variable is present in the same output, reports `unreadable` otherwise, and lets a *present* token stand on its own because finding it proves the read reached the block. `scripts/verify-seat-credential- delivery.mjs` walks the live process tree and prints that verdict per child, with `--self-test` failing if the rule regresses; the classifier's unit test covers the empty read, the two live reads, the file variable, configurable variable names, and the well-formedness of parsed assignments. Corrects an earlier revision of this branch, which attributed the blind read to the seatbelt sandbox the adapters use; a codex adapter child reading in full on the same host - with nothing under `sandbox-exec` in its tree - contradicts that, so the notes now state the measured per-process behaviour and say the cause is not established. Comments and the docs entry only: every code line outside the comment headers is byte-identical to the cleared head `a531965e`. No version slot: `scripts/` only.
samxu01
pushed a commit
that referenced
this pull request
Sep 20, 2026
… 58) The post-deploy acceptance for the credential-delivery change (#1801) is "read the spawned adapter child's environment and confirm the runtime token is not in it". Measured on this box, that read cannot be trusted as written: `ps eww` does not always return an environment and does not say so when it doesn't. On the same command the codex adapter child read 8606 bytes with `PATH=` present and its MCP child 2166 bytes, while `/bin/sleep`, `/bin/bash -c`, `/usr/bin/tail`, the `npm exec` shim and the live pi adapter child returned argv only (75-96 bytes, no `PATH=`) - including a `sleep` spawned by the readable `python3`, so the spawner is not the discriminator. A check that concludes `withheld` from a read that saw nothing returns the answer everyone wants. This makes the rule explicit and testable: `classifyEnvRead` may only report `token_withheld` when a control variable is present in the same output, reports `unreadable` otherwise, and lets a *present* token stand on its own because finding it proves the read reached the block. `scripts/verify-seat-credential- delivery.mjs` walks the live process tree and prints that verdict per child, with `--self-test` failing if the rule regresses; the classifier's unit test covers the empty read, the two live reads, the file variable, configurable variable names, and the well-formedness of parsed assignments. Corrects an earlier revision of this branch, which attributed the blind read to the seatbelt sandbox the adapters use; a codex adapter child reading in full on the same host - with nothing under `sandbox-exec` in its tree - contradicts that, so the notes now state the measured per-process behaviour and say the cause is not established. Comments and the docs entry only: every code line outside the comment headers is byte-identical to the cleared head `a531965e`. No version slot: `scripts/` only.
samxu01
pushed a commit
that referenced
this pull request
Sep 20, 2026
… 58) The post-deploy acceptance for the credential-delivery change (#1801) is "read the spawned adapter child's environment and confirm the runtime token is not in it". Measured on this box, that read cannot be trusted as written: `ps eww` does not always return an environment and does not say so when it doesn't. On the same command the codex adapter child read 8606 bytes with `PATH=` present and its MCP child 2166 bytes, while `/bin/sleep`, `/bin/bash -c`, `/usr/bin/tail`, the `npm exec` shim and the live pi adapter child returned argv only (75-96 bytes, no `PATH=`) - including a `sleep` spawned by the readable `python3`, so the spawner is not the discriminator. A check that concludes `withheld` from a read that saw nothing returns the answer everyone wants. This makes the rule explicit and testable: `classifyEnvRead` may only report `token_withheld` when a control variable is present in the same output, reports `unreadable` otherwise, and lets a *present* token stand on its own because finding it proves the read reached the block. `scripts/verify-seat-credential- delivery.mjs` walks the live process tree and prints that verdict per child, with `--self-test` failing if the rule regresses; the classifier's unit test covers the empty read, the two live reads, the file variable, configurable variable names, and the well-formedness of parsed assignments. Corrects an earlier revision of this branch, which attributed the blind read to the seatbelt sandbox the adapters use; a codex adapter child reading in full on the same host - with nothing under `sandbox-exec` in its tree - contradicts that, so the notes now state the measured per-process behaviour and say the cause is not established. Comments and the docs entry only: every code line outside the comment headers is byte-identical to the cleared head `a531965e`. No version slot: `scripts/` only.
lilyshen0722
added a commit
that referenced
this pull request
Sep 20, 2026
…g a verdict (AX 58) (#1806) * test(scripts): the seat credential check proves its read was live (AX 58) The post-deploy acceptance for the credential-delivery change (#1801) is "read the spawned adapter child's environment and confirm the runtime token is not in it". Measured on this box, that read cannot be trusted as written: `ps eww` does not always return an environment and does not say so when it doesn't. On the same command the codex adapter child read 8606 bytes with `PATH=` present and its MCP child 2166 bytes, while `/bin/sleep`, `/bin/bash -c`, `/usr/bin/tail`, the `npm exec` shim and the live pi adapter child returned argv only (75-96 bytes, no `PATH=`) - including a `sleep` spawned by the readable `python3`, so the spawner is not the discriminator. A check that concludes `withheld` from a read that saw nothing returns the answer everyone wants. This makes the rule explicit and testable: `classifyEnvRead` may only report `token_withheld` when a control variable is present in the same output, reports `unreadable` otherwise, and lets a *present* token stand on its own because finding it proves the read reached the block. `scripts/verify-seat-credential- delivery.mjs` walks the live process tree and prints that verdict per child, with `--self-test` failing if the rule regresses; the classifier's unit test covers the empty read, the two live reads, the file variable, configurable variable names, and the well-formedness of parsed assignments. Corrects an earlier revision of this branch, which attributed the blind read to the seatbelt sandbox the adapters use; a codex adapter child reading in full on the same host - with nothing under `sandbox-exec` in its tree - contradicts that, so the notes now state the measured per-process behaviour and say the cause is not established. Comments and the docs entry only: every code line outside the comment headers is byte-identical to the cleared head `a531965e`. No version slot: `scripts/` only. * feat(scripts): route 2 for seats whose child route 1 cannot read (AX 58) wren 70575: the script is route one of 70567 only - a positive-controlled read at a process `ps` can see - while route two, the child reporting its own environment, has no instrument in it, and the live run exits 1 on the pi child by design. That is the route that answered for the pi seat, and leaving it as ad-hoc bash means the next acceptance of that shape is hand-rolled again. `--self-report` (run inside the seat, `--seat <name>` to label it) classifies this process's own environment with the same three verdict words. It shares the parser with route 1 and differs only where it must: a process reading itself cannot read nothing, so `token_withheld` is not gated on the control variable, which is reported as context instead. Both routes now check the criterion as written - no `cm_agent_` value in any adapter child's environment - rather than the absence of the declared token variable, so a credential that moved to another name is a finding either way, and `--self-report` names the variable it found it under. Ledger re-run at this head with two mutations the first pass did not have: M6 route 2 gated on the control (RED), M7 credential values not detected under unexpected names (RED), M8 route 1 keyed on the declared name only (RED), with M1-M4 RED and M5 no-op GREEN as before. No survivors, sources restored. The first M6 attempt SURVIVED, which is how the missing test was found: a withheld fixture answers the same whether or not route 2 is gated, so the test now asserts the direction that matters - a present credential with no control variable. No version slot: `scripts/`, `backend/__tests__/` and a docs entry only. * fix(scripts): route 2 says where it ran, and checks its label (wren 70593) wren's hold: route 2's evidence is only as good as WHERE it ran, and `--seat` is a label the reporter never verifies. A self-report is evidence about *some* process, so the line now carries pid, ppid and the parent argv chain (read with `ps -o ppid=,args=`, which prints argv without the environment and is therefore unaffected by the per-process env-read behaviour this instrument is about), and the `--seat` label is checked against that chain rather than asserted. The check has to exclude the invocation's own argv, or a labelled run matches `--seat kai` in its own command line and confirms the label with the label. With that exclusion, a live labelled run reports: kai (pi): route 2 (self-report) — token withheld, no cm_agent_* value in any variable pid=68579 ppid=68578 started under: 68579 node ... --seat kai --self-report <- 68578 /bin/bash -c ... <- 68378 pi <- 28808 /opt/homebrew/bin/node /opt/homebrew/bin/commonly agent run kai seat label "kai" appears in ancestor pid 28808 (.../commonly agent run ...) A wrong label prints `does NOT appear in any ancestor argv — the label is unverified`, and a verdict with no chain still reports the verdict without a claim about where it was made. Unit suite 13/13. Ledger re-run with two more mutations: M9 route 2 drops where it ran (RED), M10 the label check matches the invocation's own argv (RED), alongside M1-M4 and M6-M8 RED, M5 no-op GREEN. No survivors, sources restored. * fix(scripts): the seat label is confirmed by the launcher's argument shape, not a substring (vera 70596) vera's finding, reproduced live on the head wren asked for the chain on: the label check confirmed labels that are wrong. --seat commonly -> "appears in ancestor pid 28808 (/opt/homebrew/bin/node /opt/homebrew/bin/commonly agent run kai)" --seat run -> same --seat node -> same --seat pi -> "appears in ancestor pid 73825 (pi)" `args.includes(seat)` matches the supervisor's own command line by substring, and a standalone token matches the adapter binary - which is named after the adapter, not after the seat. Both were reported as confirmation. Now the only confirmation is the shape the launcher itself uses, `agent run <seat>`, matched on whitespace-delimited tokens so a name inside a path never matches at all; a standalone token is reported as "not the seat argument, so the label is NOT confirmed"; anything else is unverified. Live at this head: `--seat kai` -> "is the seat argument of ancestor pid 28808 (…/commonly agent run kai)"; commonly/run/node no longer confirm; `pi` reports token-only and unconfirmed. Unit 13/13. Two more ledger mutations: M11 a substring anywhere counts as the label (RED, vera's finding), M12 a standalone token counts as the seat argument (RED). M10 (the invocation's own argv) SURVIVED the first re-run because the new strong tier short-circuits it - which is how the redundant-mechanism question surfaced - so its test now pins the case where the seat appears in the invocation argv and nowhere else. M1-M4, M6-M12 all RED, M5 no-op GREEN, no survivors, sources restored. * fix(scripts): route 2 exits 1 when it cannot place itself, and a prompt is not the launcher (wren 70599/70600) Two more lines of wren's hold, at the head that already anchored the label check to `agent run <seat>`: 1. A claude seat's ancestor is `claude -p <whole prompt>`, pod text included, so a prompt that quotes the launcher can reproduce the token sequence. The anchor is now the full launcher shape - `commonly agent run <seat>`, the CLI token included - and a prompt-bearing argv (`-p`, `--print`, `exec`) is never the launcher however convincing its tokens look. It reports `PROMPT-BEARING argv … NOT confirmed`. The trade-off is a false negative for a launcher that itself uses `exec` in its argv, which is the safe direction: it delays a pass rather than fabricating one. 2. An unverified label still exited 0 when the verdict was withheld, so a script got "withheld" about a process nobody placed. Now: 0 only when the token is withheld AND the report is placed (the label is the launcher's own seat argument), 1 when this run cannot answer (unverified, matched only as text, or no `--seat` at all - it prints `UNPLACED: …` and says to re-run with a label), 3 when the token is present, 2 on a bad invocation. A present credential stays distinguishable from a non-answer. Live: `--seat kai` from this seat -> exit 0, `is the seat argument of ancestor pid 28808 (/opt/homebrew/bin/node /opt/homebrew/bin/commonly agent run kai)`; `--seat otto` -> exit 1, `UNPLACED`; `COMMONLY_AGENT_TOKEN=cm_agent_… --seat otto` -> exit 3, TOKEN PRESENT. Unit 14/14. Three more ledger mutations: M13 an unplaced report exits 0 (RED, wren's second line), M14 a prompt-bearing argv counts as the launcher (RED, wren's first), M15 a token or prompt match counts as placed (RED). M1-M4 and M6-M15 all RED, M5 no-op GREEN, no survivors, sources restored. The verdict rule is untouched.
lilyshen0722
added a commit
that referenced
this pull request
Sep 20, 2026
…X 58) (#1808) The deferred second half of AX entry 58, held back because it could not ride #1806 without moving a head both reviewers had stamped. Two rules, both earned on the #1801 acceptance instrument: - A seat-level verdict is evidence about the code that process loaded, and a seat supervisor loads its cli modules at start. A peer's --self-report returned 3 (TOKEN PRESENT) at the fixed instrument, and the reason was her supervisor: started Sep 3 23:03 against a cli installed Sep 19 19:09. Compare `ps -o lstart= -p <supervisor pid>` with the install mtime before believing any seat-level verdict — a pre-fix seat's finding is evidence about the old code, and a pre-fix seat's `withheld` would be evidence about nothing. - Then run it against the candidate, not the population already under discussion. The check was applied to the seats under discussion and not to the two that could supply the leg — started 19 and 21 seconds after that install, so they needed a turn, not a restart, and the sentence written about them added a step nobody needed. Corollary: a post-fix seat that shares no pod with the measurer cannot be asked at all (the agent-DM route refuses with `sharePod`), so reachability is as load-bearing as freshness. Docs only — no version slot.
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.
The seat credential reaches its MCP servers as a path, not as an inherited environment value
TASK-083 (carries TASK-082's token-channel ruling, folded in per wren 70275).
Cut from main at
f064d578, after #1780 merged — that PR supplies the readerthis depends on (a declared fd/file source is authoritative,
@commonlyai/mcp0.3.11), and this one supplies the declaration rewrite that lets a runtime stop
carrying the value at all. The eight commits replay onto
f064d578cleanly; thepre-merge copy of the reader commit the branch was built on is dropped, because
main now carries it. Versions:
cli0.1.63 → 0.1.64,@commonlyai/mcp0.3.11 → 0.3.12. (
cli0.1.63 landed on main in the meantime — TASK-063's daemon-half guard — so this takes the next slot rather than a lower one.)What was measured before anything changed
ps ewwon live seats, 2026-09-19:COMMONLY_AGENT_TOKEN(73 chars) andCOMMONLY_LITELLM_KEY(25 chars);COMMONLY_AGENT_TOKEN(73);-pseat and its children:COMMONLY_AGENT_TOKEN(73), as did anunrelated
@playwright/mcpchild of the same seat;commonly agent run <seat>process: clean — so the valuewas being added by the adapters, not inherited from the launcher.
The ruling (Sam, 70227 + 70274): the runtime is spawned without the token,
and the child that needs it is told where to read it — a launcher credential
file, only its PATH in the environment.
What this PR does
One decision, in one place (
mcp-credential-delivery.js), applied at eachadapter's own seam, because the three adapters cannot use the same channel:
env={...}on the-coverrideenv_varsroute — the one that put the value in codex's environment — is no longer needed for our server.The second half of the same leak: the runtime's own environment
Rewriting the declaration decides what a CHILD is told. It does not decide what
the runtime process itself carries, and the credential is exported for bootstrap
(
agent run, the daemon), so every adapter that derives its runtime environmentfrom
process.envhanded the value straight back — to claude's MCP children andhooks, to codex's MCP children, to pi's own
bashtool.One rule, in the module that owns the delivery decision
(
withholdRuntimeCredential): the PATH of this spawn's file goes into the runtimeenvironment, the VALUE is deleted, unless a carve-out genuinely needs it there —
which the adapter passes in rather than infers. This is what Vera's four failures
measured (70455): they passed in a runner without the variable and failed in one
with it, and the value they found was the runner's own.
The hook path, which was wired to nothing
resolveHookToken(launcher file first, value second) was written, documented andunit-tested — while
agent hooks-forward, the commandhooks-configactuallyinstalls into Claude settings, read
process.env.COMMONLY_AGENT_TOKENdirectly.So the migration was real in the library and absent in production: on a migrated
seat the value is gone, the forwarder returned
hook_unavailable, and atool-policy hook silently stopped deciding anything — the fail-open posture
turns a dead credential into a silence, not an error. The command now resolves
through
resolveHookToken, and a test drives that command throughregisterAgentand asserts the Authorization header the backend would see.A declared file is authoritative for our server: it wins over a literal token
in the same declaration, because preferring the literal would keep the secret
exactly where the launcher channel exists to remove it. If no file exists for a
spawn, nothing is rewritten — a path to nowhere is worse than the status quo.
Where the file lives is part of the contract, and it was measured
sandbox/seatbelt.jsadmits exactly one path outside the workspace to aconfined seat: a subpath rule on the per-spawn
--mcp-configdirectory. Nothingunder
~/.commonlyis admitted except the seat's ownstatePath. A credentialfile under
~/.commonly/credentialswould therefore be unreadable by the verychild it was written for,
readTokenwould throw on its declared-but-unreadablefile, and that seat would lose its tools.
So the file is written inside the per-spawn directory each adapter already
hands its runtime — claude's
--mcp-configdir, codex's--output-last-messagedir, pi's seat home — at0600in a0700directory(
chmodasserted after the write, sincewriteFileSync's mode is umask-filtered),and removed by the spawn's existing
finally.The carve-out, which is TASK-082's measured case
Three handovers cannot move, and they are not silent:
args,url,headers, or inside a largerstring — claude/codex substitute those literally and a file path is not a
value. The value survives for that spawn, and only that spawn, with a warning
naming the trade.
adapters.claude.environment.test.mjs's three pre-existingtests are this case's witnesses, and they pass unchanged — which is how I
know the carve-out was measured rather than invented;
hand-patched staging checkout whose
package.jsonsays0.3.7. That readerunderstands only the environment, so it keeps the value with a warning
naming the pin. Handing it a path it cannot open would take away the seat's
tools, which is the worse failure;
@commonlyai/mcp: its declaration is its own, so itis left alone (with a warning). Measured: of the 32 fleet entries that name the
credential variable, all 32 are the
commonlyentry — 27 floating@commonlyai/mcp@latest(which move to the path) and the 5 staging seatsabove. No third-party server references the seat credential today.
A finding this PR made, and repaired rather than documented
codex's doc block promised bearer tokens never ride in argv. That held for env
values (diverted to
env_vars) and not for command arguments: a token incommand/argswas substituted literally into-c mcp_servers.<name>.args, on acommand line every same-user process can read. That entry is now refused whole
(skipped, with the reason) rather than half-wired. Measured before the guard was
written.
The consumer nobody had named
Tool-policy hooks are child processes of the seat runtime, so they read exactly
the environment the token was removed from — and because a hook fails open by
design, the symptom would not be an error: it would be a hook that silently
stopped deciding anything.
resolveHookTokenprefers the launcher file and fallsback to the value variable for a seat whose declaration has not migrated. Unlike
the MCP reader it does not throw on a declared-but-unreadable file: a hook that
dies is a hook the runtime reports as broken.
What is deliberately not changed
The shipped default declaration stays exactly canonical — it still names the
token variable, and each adapter rewrites it per spawn. A record that named the
file directly would be one an older CLI could not serve, and the guard's
canonical-shape check is a security property. The decision is asserted in a test
rather than implemented by widening that check.
Release order (a real sequencing risk, documented not hidden)
@commonlyai/mcp@0.3.12must be on npm before a host runscli@0.1.64;npm-publish.ymlpublishes both packages in one parallel matrix with no order.A seat that gets the new cli before the new server is served by the old-pin
branch: it keeps the value and says so, which is a degradation, not a break.
Evidence
cli: 45 suites / 739 tests — 729 passed, 10 skipped — green atHEADon1b7e8a04(main after feat(cli): the daemon withholds the grant broker from a seat it cannot confine (TASK-063) #1798),eslint srcclean, and green both with and withoutCOMMONLY_AGENT_TOKENexported into the runner's environment. That pairingis the instrument: the four tests that only failed under an exported variable
were the ones that found this defect, so they are now positive controls that
plant the launcher's value explicitly instead of inheriting it.
commonly-mcp: 3 suites / 72 tests green.kept) and the end-to-end hooks-forward command.
source authoritative, 6), the delivery decision (10), claude's channel (11),
codex's carve-out + argv refusal (3 + the rewritten mcp-override test), pi's
launcher/pipe split (4), the hook credential (4).
D1–D6 (codex), E1–E4 (pi), F1–F4 + G1 (hooks + the canonical-shape assertion).
Every mutated source was restored byte-identical.
M1 claude call-site removed, M2 codex call-site removed, M3 pi call-site
removed, M4 the helper deletes unconditionally (ignoring a carve-out), M5 the
helper never writes the path, M6
hooks-forwardreads the bare variable again,M7 the helper never deletes — all RED, no survivors, sources byte-identical.
A mutation in this set can only be seen under the exported configuration, which
is precisely why the guard was invisible before.
reads
ctx.runtimeToken || '', so a mutation that edits only that line ismasked by the loop's own guard; C5 witnesses the guard instead.
Honest limits
published, on the supported path. It cannot be taken from the pre-merge branch
without either faking it or running real seat turns against the live fleet.
The claim it makes is exact (wren, 70462): no
cm_agent_value in anychild's environment of any adapter (
ps ewwis the instrument), plus theLiteLLM key only in the model-calling process. It does not claim the seat's
bashcannot reach the token: the file channel removes the token from theprocess table, not from a same-user reader —
COMMONLY_TOKEN_FILErides in theruntime environment, and
0600stops other users, not other processes of thesame user. Unlinking the file after the server reads it, or an fd channel for
claude and codex, needs a launcher that owns the server lifecycle (claude and
codex may respawn it), so it is a separate row rather than a promise here.
Same-user confinement as a general question stays TASK-063's.
HTTPS_PROXY,HTTP_PROXY,NO_PROXY,NODE_EXTRA_CA_CERTS) are absent on a live child on this box, so they areincluded in the base set by declaration and cannot be exercised here.
/proc/<pid>/fd/3inspection is Linux-only and cannot be measured on macOS;the fd-3 channel is witnessed by real-child probes in the test suite instead.