Repository navigation
Conversation
ACP agents, the `t3 acp-mcp-bridge` stdio server they spawn, ACP client terminals and Pi received the thread's MCP bearer in an environment variable. Everything those processes spawn inherits it, and on Linux systemd-coredump writes the environment of a crashing process to the journal as COREDUMP_ENVIRON. ACP and Pi now write the Authorization header to an owner-only file in a private temporary directory and pass only its path (T3_ACP_MCP_AUTHORIZATION_FILE, T3_MCP_AUTHORIZATION_FILE). The bridge, `t3 acp-mcp-call` and the Pi extension read the file. ACP keeps one path per thread for the life of the provider session. A rotated header is written beside the file and renamed over it. Once the credential is cleared, the next read deletes the file, and a reissued credential comes back at the same path. A failed write leaves no temporary file. Pi writes its file once when the session opens. Closing the session removes the files. If the file cannot be written, ACP runs the session without T3's MCP server and Pi fails to open the session, as it does when it cannot write its extension. The raw-credential variables older builds exported (T3_ACP_MCP_AUTHORIZATION, T3_MCP_BEARER_TOKEN, T3_CODE_MCP_AUTHORIZATION) are dropped where every ACP flavor spawns its agent, where ACP client terminals start, and from Pi's launch environment, whether or not the session has a credential. The MCP server doc now describes how Codex receives its header (JSON-RPC thread config, not an environment variable). Refs pingdotgg#12031 Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Contributor
ApprovabilityVerdict: Not approved Macroscope's review found this PR not approvable — This PR changes the default authentication-credential transport for ACP and Pi processes, adding session-owned files, rotation and cleanup logic, and broad child-environment sanitization. Because it affects security-sensitive production behavior across multiple provider paths, human review is required. You can add or adjust custom eligibility rules. Learn more. |
knip flagged the export; it is only used inside mcpSession.ts. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
remove() now forgets the recorded header before it deletes anything, so a removal that fails or is interrupted part way cannot leave a header that a later write of the same value trusts while the file is gone. write() deletes any leftover .tmp and creates the new one exclusively with mode 0600, so it never writes into a .tmp that already exists, whether it was left over or appeared after the delete. Also: the ACP log says "update" since it covers clears too; the mock agent's probe checks all three legacy names, and the test that seeds them in the parent environment now seeds all three; the docs and the helper comments say what protects the file on POSIX and on Windows; one test is renamed to say what it checks. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
This branch has not been deployed
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.
ACP and Pi still hand each provider session's MCP bearer token to their children in environment variables, so it lands in every copy of those environments, crash-dump journal entries included. This PR moves both to a file per thread, owner-only on Linux and macOS, and puts only the file's path in the environment.
Problem
Each provider session gets a bearer token for the
t3-codeMCP server. The/mcpendpoint sits outside the environment auth stack, so this token is the only thing guarding the session's orchestrator tools (#12031 triage).Codex and Claude no longer put the token in a child environment on main. Codex receives
http_headers.Authorizationover its app-server JSON-RPC connection. Claude moved from argv to an env var in #17408, then to the Agent SDK control channel in #17898. ACP joined the env channel with the V2 orchestrator (#2829), after the triage was written. ACP and Pi still pass the token itself through environment variables:T3_ACP_MCP_AUTHORIZATIONin their own environment, and each of their client terminals gets it in the terminal's environment, through the adapter'sprocessEnvironmentoverlay (packages/provider-acp/src/server/adapter.ts:714on main).session/newto spawn thet3 acp-mcp-bridgestdio server withT3_ACP_MCP_AUTHORIZATIONin that server's environment (mcpServers,adapter.ts:705). Grok and Antigravity ignoreprocessEnvironment, so this is the one place their sessions put the token.T3_MCP_BEARER_TOKENin its environment (packages/provider-pi/src/server/mcpInjection.ts:313).An environment variable gets copied far beyond the process it was meant for:
core_patternpipes crashes to systemd-coredump, a crash of any of these processes writes its whole environment to the journal asCOREDUMP_ENVIRON(MCP bearer token passed via environment is captured in systemd-coredump journal entries on provider crashes #12031). That entry outlives the process, and journal-reading admin groups and anyone holding a copy of the journal can read it. The core size limit does not reliably prevent it. For a piped dump the kernel ignoresRLIMIT_COREexcept for the value 1, which aborts the dump. At 0, systemd-coredump declines to store the core file but still sends the journal entry, environment included. A crashing subprocess does not end the provider session, so the token in that entry stays valid until T3 releases, rotates or revokes the session.The #12031 triage already asked for a non-env channel: item 1 proposes a
0600file for Codex, and item 5 says that Claude, leaving argv, should not stop at a child env var on Linux coredump hosts. The same reasoning applies to ACP and Pi.Change
ACP and Pi now get the
Authorizationheader in a file. Only the file's path goes into the environment:T3_ACP_MCP_AUTHORIZATION_FILEfor the ACP agent, its bridge and its client terminals, andT3_MCP_AUTHORIZATION_FILEfor Pi. Both files hold the full header value (Bearer ...), hence the names. The old variables are gone as channels.The files.
makeMcpCredentialFilesinpackages/provider-core/src/server/mcpSession.tsowns the files for one adapter session.mkdtempdirectory (0700) under the OS temp dir, created with Effect'smakeTempFileScoped. On Linux and macOS the file has mode0600, so only its owner can read it. Windows ignores the mode, so there the file gets the ACL of the user's temp directory.<path>.tmp, creates a new one exclusively with mode0600and renames it over the file, so the path never goes missing and a reader never sees half a header. If the write or the rename fails, T3 tries to delete the.tmp..tmp, also after a failed first write. A later write puts it back at the same path.removeforgets the recorded header before it deletes anything, so after a removal that stops part way, a later write of the same header still rewrites the file.ACP syncs the file each time the adapter reads the thread's credential: on a turn, a resume, a snapshot read, a fork or a rollback. A rotated header is written over the file on that read, and the next read after a clear deletes it. An idle session keeps the file until that read or until the session closes; the token in it was already revoked by the clear. The adapter's credential reads for one session never interleave either. Every read after open holds the session's runtime transition lock, so an older header cannot overwrite a newer one.
The path has to stay put. The agent's environment, the bridge it registered and the terminal environments the adapter remembers are all fixed when the native session starts. When the credential rotates and the next turn runs on the same native session, the adapter rewrites the file those processes already name. Pi reads the registry once per
openSession, so it uses the same helper with one key.The readers.
t3 acp-mcp-call, the fallback an agent runs from a client terminal, reads the file on every call, so it follows rotation.t3 acp-mcp-bridgeand the Pi extension read it at start, as they read the env var before. "What this does not fix" covers what that means for the bridge after a rotation.acp-mcp-bridge requires T3_ACP_MCP_ENDPOINT and a readable T3_ACP_MCP_AUTHORIZATION_FILE.It no longer reads the old variable.Old variables. A T3 server started from the terminal of an older build's provider child can carry a raw credential in its own environment. On main that copy reached ACP agents and their terminals even in sessions with no credential.
LEGACY_RAW_MCP_CREDENTIAL_ENVin provider-core lists the three names older builds used (T3_ACP_MCP_AUTHORIZATION,T3_MCP_BEARER_TOKENand Claude'sT3_CODE_MCP_AUTHORIZATION), andwithoutRawMcpCredentialsdrops all three in three places:AcpSessionRuntime.make, where every ACP flavor spawns its agent (registry, Grok, Antigravity, and the registry probe and auth runtimes).makeAcpClientTerminals.T3_MCP_URLandT3_MCP_AUTHORIZATION_FILEbefore it adds the current session's values.This applies whether or not the session has a credential and whether or not the file write worked. When no environment is configured the helper starts from
process.env, which Node would pass on anyway, so a terminal keeps the server's environment minus those names.When the file cannot be written.
Could not update the T3 Code MCP credential file.and opens the session without the t3-code MCP server and without the file variable. T3's tools are an addition to the session, and two ACP call sites have error channels that cannot take a new error.openSessionthrough its existingprovideCacheFsmapping, the same way it fails when it cannot write its extension file.Where the file lives. The file is in a fresh private
mkdtempdirectory under the OS temp dir rather thansecretsDir. One directory per file needs no shared directory and no cleanup across sessions. Where the temp dir is tmpfs, the file is never written to a disk file system. The adapters own the files, notMcpSessionRegistry: writing them from the registry would put every provider session's bearer on disk, including Codex, Claude, Cursor, OpenCode and Muse, which never read it.Docs.
docs/orchestration-v2/orchestrator-mcp-server.mdshowsT3_MCP_AUTHORIZATION_FILEin Pi's environment block and says commands Pi runs do not inherit the token, so it is not in theirCOREDUMP_ENVIRON, and that on POSIX the file has mode0600while on Windows it gets the user temp directory's ACL. Its Codex section still describedbearer_token_env_varandT3_MCP_BEARER_TOKEN, which main no longer uses. It now says Codex getshttp_headers.Authorizationin theconfigofthread/start,thread/resumeandthread/forkover JSON-RPC.Why a file and not a protocol channel
COREDUMP_ENVIRON/proc/<pid>/environmcp_set_servers)0600file, path in env (this PR)mcpServersis a process spec (command, args, env). It has no header field, so the bridge can only receive the header through its environment, its argv or a file.--mode rpchas no command that hands an extension a value. The extension could request one with anextension_ui_requestthat the adapter answers over stdin, but that is new protocol on both sides, and the token would then travel in the RPC stream that native protocol logging records.What this does not fix
acp-mcp-calldepends on that. The change takes the token out of every copy of the environment and off disk once the session closes, not out of the user's reach. The triage's0600file has the same property.t3 acp-mcp-bridgestarted before a rotation keeps the old header in memory, as on main (ProviderSessionManager.ts:445: rotating afterwards cannot repair an already-configured process). Its MCP HTTP session was opened under that header.acp-mcp-callfollows the rotation.T3_ACP_MCP_ENDPOINTandT3_ACP_MCP_AUTHORIZATION_FILEinherited by the server pass through when a session has no credential. At the spawn point the instance environment and the session overlay are already merged, so an inherited path looks like the current one. They hold an endpoint and a path, not a token. The server's own environment is not touched either, so children of other providers still pass on any legacy variable the server inherited, as on main.acp-mcp-calland the Pi extension, so a full core image can contain it if the core file itself is stored.COREDUMP_ENVIRONno longer contains it.McpSessionRegistrykeeps credentials in memory only. The directories are0700.ProviderSessionManager.ts:1978).Scope and approval
accepted. The maintainer triage confirmed the coredump exposure and asked for a non-env channel.477282263a: the fix, a knip follow-up that un-exportsLEGACY_RAW_MCP_CREDENTIAL_ENV, and a hardening commit for the credential-file helper. 20 files, +930 / -107, most of it tests.Part of #12031.
Verification
Tests, red/green, mutation checks, typecheck, lint and format
Focused tests, run from the repo root:
New and changed tests:
keeps an ACP thread's MCP credential file at one path through rotation and clear: after a rotation and a turn on the same native session there is still one runtime, the same path holds the new header, and a terminal started afterwards reads it through its remembered environment. The next read after a clear removes the file, a reissue restores it at the same path, and closing the session removes it.opens an ACP session without T3's MCP server when its credential file cannot be written: no t3-code server and no file variable, and the agent does not see a stale raw variable.drops a raw MCP credential the server inherited from an older build: raw values for all three legacy names, seeded in the parentprocess.env, do not reach the spawned agent. The mock agent's probe checks all three.clientTerminals.environment; terminals of a session without a credential and of a session with one both print it empty.reads the credential from the file the environment namesfor the bridge andacp-mcp-call.hands Pi the MCP credential as a session-owned file, not in its environment: the file is0600and gone after close. The Pi launch test inmcpInjection.test.tsseeds all three legacy names plus a stale URL and file variable and asserts each is gone.mcpSession.test.ts, 10 new tests, four of them in the next bullets. FormakeMcpCredentialFileson the real filesystem: an unchanged header is a no-op (same inode); rotation keeps the path and mode0600and replaces the inode; remove then write puts the file back at the same path; an injected rename failure leaves no.tmp,removeclears a key with no recorded header, and a failed rotation keeps the old header; closing the scope removes every key's directory. ForwithoutRawMcpCredentials: all three names masked and the rest kept; an unset environment starts fromprocess.env.writes the file again after a removal that stopped part way:removedeletes the file, then an injected failure stops it at the.tmp; a write of the same header puts the file back at the same path.writes the file again after a removal that was interrupted part way:removedeletes the file and is interrupted while it deletes the.tmp; a write of the same header puts the file back at the same path.replaces a leftover temporary file instead of writing into it: a.tmpleft with mode0644does not survive the next write, and the file ends up0600with the new header.does not write into a temporary file created after the leftover is deleted: a readable.tmpthat appears between the delete and the exclusive create makes that create fail withAlreadyExists. The file keeps the old header, the planted file never gets the new one, and no.tmpis left.Related suites, every test that builds
AcpSessionRuntimesince its spawn environment changed. Both runs are on the fix commit336a55f9e7, before the two follow-ups:The one failure is
AcpSessionRuntime.processTree.test.ts"contains a packaged-runtime command without requiring cgroup delegation" (expected 127 to be 125), and this PR does not touch that test or the wrapper it exercises. It fails the same way where/bin/shis bash, also on the earlier maina8bdfdb5before the rebase, and passes where/bin/shis dash, as in upstream CI on this PR. #17949 fixes it separately, carrying forward #15344.Red/green: with the new strip tests on the source from before the strip existed (file channel only), all four fail: the parent-seed test (the agent sees the raw variable), the rotation test (the agent sees the stale raw value, the Grok case), the write-failure test (same), and the fork test (the terminal of a session without a credential prints
Bearer stale-dummy-credential|). I did not run the new tests against477282263aora8bdfdb5, where the*_AUTHORIZATION_FILEassertions would fail too.Red/green for the hardening commit: with the partial-removal and leftover-
.tmptests on the helper from before it, both fail (the file is missing after the partial removal; the file ends up with the leftover.tmp's mode0644). The interrupted-removal and planted-.tmptests each fail when one part of the change is undone. Withremoveforgetting the header after the deletes again, the interrupted-removal test fails because the file is missing, and so does the partial-removal one. Withflag: "wx"dropped and the pre-delete kept, the planted-.tmptest fails because the rotation succeeds. With the change,mcpSession.test.tspasses 12 of 12.Mutation checks, one per new behavior, each source file restored and checksum-verified afterwards. The first five ran on an earlier revision with the same call sites, the last five on the fix commit's code, and the two Pi ones again after the rebase. All ten predate the hardening commit.
expected 'Bearer stale-dummy-credential|' to equal '|'NotFound: FileSystem.readFileon the path the agent and terminals were givenremoveleaves the file on clearexpected true to be falseonexists(file)after clearProviderAdapterOpenSessionError ... PermissionDenied: FileSystem.makeTempFileScoped.tmpexpected true to be falseonexists(<path>.tmp)removereturns early when no header is recordedremoveT3_MCP_BEARER_TOKENexpected 'Bearer stale-token' to equal undefinedT3_CODE_MCP_AUTHORIZATIONwritedrops the unchanged-header checkThe rename step and the semaphore have no mutation check, because no test races a reader or a second writer against a write.
tsc --noEmitthrough each package'stypecheckscript inprovider-core,provider-acp,provider-piandapps/server: exit 0, 0 errors.vp lint --report-unused-disable-directiveson the changed.tsfiles: 0 errors. 4 warnings, all on lines this PR does not change.vp fmt --checkon the changed files:All matched files use the correct format.provider-core,provider-acpandapps/servertypechecks exit 0 with 0 errors (it does not touchprovider-pi),vp lintandvp fmt --checkon its six files are clean, andvp run knip:checkreports nothing.Manual checks
Child environments, crash dumps, the credential at work, and a live T3 thread
These checks ran before the rebase, on the earlier main
a8bdfdb5and on this change on that base (36da67d1). The rebase onto477282263awas conflict-only: it left the ACP and core code byte-identical and changed Pi's launch code only by upstream's new extension-path lines. The checks also predate the two follow-up commits, and I did not repeat them. The knip commit drops oneexport. The hardening commit changes howmcpSession.tswrites the.tmpand whenremoveforgets the header, plus the ACP warning text, the mock agent's probe, a docs sentence, comments and tests. None of that decides what goes into a child's environment, so the environment and crash-dump results stand. The rotation, file mode and cleanup in the third image went through the earlier write code; the unit tests above cover the new one.Main and this change each ran in a throwaway clone, under
env -iwith a privateHOMEandTMPDIR. A temporary test file that is not part of this PR drove the real code:makeAcpRegistryAdapterV2spawning the repo'sacp-mock-agent.ts,makeAcpAdapterV2withmcpServerspointing at the clone's realt3 acp-mcp-bridge, andbuildPiRpcLaunch. Each child's/proc/<pid>/environwas read and filtered to T3 MCP variables. The credential was a dummy,Bearer dummy-e1-00112233445566778899aabbccddeeff, and the MCP endpoint was a local dummy HTTP server that logs each request'sAuthorizationheader. Both clones were clean afterwards.Child environments. ACP agent process (the
T3_ACP_MCP_NODEandT3_ACP_MCP_ENTRYPOINTlines omitted):The bridge, the client terminal and the Pi child show the same change (
T3_MCP_BEARER_TOKENbecomesT3_MCP_AUTHORIZATION_FILEfor Pi). Two more runs put stale raw copies in the environment of the process standing in for the T3 server and opened a session with no credential:Crash dumps. For each captured environment, a dummy
/usr/bin/sleepran as a transient user unit with exactly those T3 MCP variables, was crashed with SIGSEGV, andCOREDUMP_ENVIRONwas filtered to T3 MCP lines. The dummy core files were deleted afterwards.All four channels (agent, bridge, terminal, Pi) held the dummy bearer before and only the path after.
The credential still works. On this branch, the bridge started from the exact
mcpServersspec,acp-mcp-callfrom a client terminal, and the Pi extension (loaded with Pi's environment, realfsand realfetch) each sent the issued header. After a rotation the file at the same path held the new header andacp-mcp-callsent it:The file was
0600in a0700directory, and both were gone once the owning scope closed. Not3-mcp-*directory was left inTMPDIR. The bridge given only the old raw variable exits with code 2.A live T3 thread. A dev server from this branch, loopback only, with an isolated T3 home and an ACP registry instance running
codex-acp. One prompt asked the agent to callorchestrator_capabilities. The call went throught3 acp-mcp-bridge, completed, and the agent reported 10 providers. While the agent was alive I listed only the variable names in its environment and the bridge's. Thecodex-acpprocess hadT3_ACP_MCP_AUTHORIZATION_FILE,T3_ACP_MCP_ENDPOINT,T3_ACP_MCP_ENTRYPOINTandT3_ACP_MCP_NODE. The bridge hadT3_ACP_MCP_AUTHORIZATION_FILEandT3_ACP_MCP_ENDPOINT. Neither hadT3_ACP_MCP_AUTHORIZATION.Not checked:
os.tmpdir()is per user on both andrenamereplaces an existing file on both, but I ran neither. On Windows the mode is ignored and the file gets the ACL of the user's temp directory; I did not check that ACL.pibinary. The Pi environment came frombuildPiRpcLaunchwithout starting Pi, and the extension ran in avmcontext, as the repo's testkit does.codex-acpthrough the registry driver.sleepcarrying each captured environment.Model and harness: Claude Opus 5.5 in Claude Code, run from T3 Code.
🤖 Generated with Claude Code