Skip to content

fix(providers): keep the MCP bearer out of ACP and Pi child environments - #17957

Open
only21mil wants to merge 3 commits into
pingdotgg:mainfrom
only21mil:fix/acp-pi-mcp-token-off-env
Open

only21mil wants to merge 3 commits into
pingdotgg:mainfrom
only21mil:fix/acp-pi-mcp-token-off-env

Conversation

@only21mil

@only21mil only21mil commented Oct 11, 2026 •

Copy link
Copy Markdown
Contributor

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-code MCP server. The /mcp endpoint 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.Authorization over 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:

  • Registry ACP agents get T3_ACP_MCP_AUTHORIZATION in their own environment, and each of their client terminals gets it in the terminal's environment, through the adapter's processEnvironment overlay (packages/provider-acp/src/server/adapter.ts:714 on main).
  • Every ACP agent, Grok and Antigravity included, is told in session/new to spawn the t3 acp-mcp-bridge stdio server with T3_ACP_MCP_AUTHORIZATION in that server's environment (mcpServers, adapter.ts:705). Grok and Antigravity ignore processEnvironment, so this is the one place their sessions put the token.
  • Pi gets T3_MCP_BEARER_TOKEN in its environment (packages/provider-pi/src/server/mcpInjection.ts:313).

An environment variable gets copied far beyond the process it was meant for:

  • Every process those children start inherits it: shell and terminal commands, test suites, package scripts. Anything among them that prints or reports its environment (a log line, test output, an error reporter) copies the token with it.
  • On Linux hosts where core_pattern pipes crashes to systemd-coredump, a crash of any of these processes writes its whole environment to the journal as COREDUMP_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 ignores RLIMIT_CORE except 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 0600 file 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 Authorization header in a file. Only the file's path goes into the environment: T3_ACP_MCP_AUTHORIZATION_FILE for the ACP agent, its bridge and its client terminals, and T3_MCP_AUTHORIZATION_FILE for Pi. Both files hold the full header value (Bearer ...), hence the names. The old variables are gone as channels.

The files. makeMcpCredentialFiles in packages/provider-core/src/server/mcpSession.ts owns the files for one adapter session.

  • Each thread gets one path for the life of the provider session: a file in its own fresh mkdtemp directory (0700) under the OS temp dir, created with Effect's makeTempFileScoped. On Linux and macOS the file has mode 0600, so only its owner can read it. Windows ignores the mode, so there the file gets the ACL of the user's temp directory.
  • To write a new header, T3 deletes any existing <path>.tmp, creates a new one exclusively with mode 0600 and 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.
  • Removing a thread's credential deletes the file and any .tmp, also after a failed first write. A later write puts it back at the same path.
  • Closing the session scope removes every file and its directory.
  • Writes and removals take one semaphore permit, so they never interleave. remove forgets 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-bridge and 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.
  • Without a readable file the bridge exits with acp-mcp-bridge requires T3_ACP_MCP_ENDPOINT and a readable T3_ACP_MCP_AUTHORIZATION_FILE. It no longer reads the old variable.
  • The in-process MCP-over-ACP bridge keeps the header in T3's memory, as before. It never reaches a child.

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_ENV in provider-core lists the three names older builds used (T3_ACP_MCP_AUTHORIZATION, T3_MCP_BEARER_TOKEN and Claude's T3_CODE_MCP_AUTHORIZATION), and withoutRawMcpCredentials drops 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.
  • Pi's launch environment, which also drops its own stale T3_MCP_URL and T3_MCP_AUTHORIZATION_FILE before 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.

  • ACP logs 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.
  • Pi fails openSession through its existing provideCacheFs mapping, the same way it fails when it cannot write its extension file.

Where the file lives. The file is in a fresh private mkdtemp directory under the OS temp dir rather than secretsDir. 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, not McpSessionRegistry: 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.md shows T3_MCP_AUTHORIZATION_FILE in Pi's environment block and says commands Pi runs do not inherit the token, so it is not in their COREDUMP_ENVIRON, and that on POSIX the file has mode 0600 while on Windows it gets the user temp directory's ACL. Its Codex section still described bearer_token_env_var and T3_MCP_BEARER_TOKEN, which main no longer uses. It now says Codex gets http_headers.Authorization in the config of thread/start, thread/resume and thread/fork over JSON-RPC.

Why a file and not a protocol channel

Channel Token in the env of the child and its subprocesses Token in COREDUMP_ENVIRON Same user can read the token Available for ACP stdio servers and Pi Secret file to manage
Env var (main) yes yes yes, from /proc/<pid>/environ yes no
Protocol or control channel (Codex JSON-RPC, Claude mcp_set_servers) no no from process memory only no, see below no
0600 file, path in env (this PR) no, path only no, path only yes, from the file, by design yes yes: one per thread, removed after a clear and on session close
  • ACP: a stdio entry in mcpServers is 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.
  • Pi: --mode rpc has no command that hands an extension a value. The extension could request one with an extension_ui_request that 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

  • Processes running as the same user, the agent's own commands included, can still read the file through the path in their environment. acp-mcp-call depends 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's 0600 file has the same property.
  • A t3 acp-mcp-bridge started 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-call follows the rotation.
  • Stale T3_ACP_MCP_ENDPOINT and T3_ACP_MCP_AUTHORIZATION_FILE inherited 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.
  • The token is still in the memory of the bridge, acp-mcp-call and the Pi extension, so a full core image can contain it if the core file itself is stored. COREDUMP_ENVIRON no longer contains it.
  • If the T3 server itself dies, scope finalizers do not run and the files stay in the OS temp dir until the OS cleans it. The tokens in them are dead, because McpSessionRegistry keeps credentials in memory only. The directories are 0700.
  • Revoking the credential when an agent subprocess crashes or the provider exits without a stream failure (MCP bearer token passed via environment is captured in systemd-coredump journal entries on provider crashes #12031 triage item 3). Main already releases the session on a provider event-stream failure (ProviderSessionManager.ts:1978).

Scope and approval

  • #12031 is triaged and labeled accepted. The maintainer triage confirmed the coredump exposure and asked for a non-env channel.
  • One problem: ACP and Pi pass the bearer through child environment variables. Both move to the same helper.
  • Three commits on 477282263a: the fix, a knip follow-up that un-exports LEGACY_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:

vp test run --maxWorkers=2 apps/server/src/orchestration-v2/Adapters/AcpAdapterV2.test.ts apps/server/src/provider/acp/AcpJsonRpcConnection.test.ts apps/server/src/mcp/AcpMcpStdioBridge.test.ts packages/provider-acp/src/server/clientTerminals.test.ts packages/provider-pi/src/server/adapter.test.ts packages/provider-pi/src/server/mcpInjection.test.ts packages/provider-pi/src/server/mcpExtensionSource.test.ts packages/provider-core/src/server/mcpSession.test.ts
 Test Files  8 passed (8)
      Tests  312 passed (312)

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 parent process.env, do not reach the spawned agent. The mock agent's probe checks all three.
  • The rotation test also seeds a stale raw value in the agent's base environment for a Grok-like flavor. The native fork test seeds one in 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 names for the bridge and acp-mcp-call.
  • hands Pi the MCP credential as a session-owned file, not in its environment: the file is 0600 and gone after close. The Pi launch test in mcpInjection.test.ts seeds 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. For makeMcpCredentialFiles on the real filesystem: an unchanged header is a no-op (same inode); rotation keeps the path and mode 0600 and replaces the inode; remove then write puts the file back at the same path; an injected rename failure leaves no .tmp, remove clears a key with no recorded header, and a failed rotation keeps the old header; closing the scope removes every key's directory. For withoutRawMcpCredentials: all three names masked and the rest kept; an unset environment starts from process.env.
  • writes the file again after a removal that stopped part way: remove deletes 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: remove deletes 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 .tmp left with mode 0644 does not survive the next write, and the file ends up 0600 with the new header.
  • does not write into a temporary file created after the leftover is deleted: a readable .tmp that appears between the delete and the exclusive create makes that create fail with AlreadyExists. The file keeps the old header, the planted file never gets the new one, and no .tmp is left.

Related suites, every test that builds AcpSessionRuntime since its spawn environment changed. Both runs are on the fix commit 336a55f9e7, before the two follow-ups:

vp test run apps/server/src/orchestration-v2/Adapters/AcpRegistryAdapterV2.test.ts apps/server/src/orchestration-v2/Adapters/AntigravityAdapterV2.test.ts apps/server/src/orchestration-v2/Adapters/GrokAdapterV2.test.ts packages/provider-acp/src/server packages/provider-acp-registry/src/server apps/server/src/provider/acp apps/server/src/provider/AcpRegistryProbe.test.ts apps/server/src/provider/AntigravityAuth.test.ts apps/server/src/provider/AntigravityProvider.test.ts apps/server/src/textGeneration/AntigravityTextGeneration.test.ts
 Test Files  1 failed | 23 passed (24)
      Tests  1 failed | 429 passed (430)

vp test run apps/server/src/orchestration-v2/testkit/OrchestratorReplayFixtures.integration.test.ts packages/provider-pi/src/server packages/provider-grok/src/server packages/provider-core/src/server
 Test Files  35 passed | 1 skipped (36)
      Tests  579 passed | 4 skipped (583)

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/sh is bash, also on the earlier main a8bdfdb5 before the rebase, and passes where /bin/sh is 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 against 477282263a or a8bdfdb5, where the *_AUTHORIZATION_FILE assertions would fail too.

Red/green for the hardening commit: with the partial-removal and leftover-.tmp tests 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 mode 0644). The interrupted-removal and planted-.tmp tests each fail when one part of the change is undone. With remove forgetting the header after the deletes again, the interrupted-removal test fails because the file is missing, and so does the partial-removal one. With flag: "wx" dropped and the pre-delete kept, the planted-.tmp test fails because the rotation succeeds. With the change, mcpSession.test.ts passes 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.

Mutation Result
Agent spawn without the strip helper 3 fail: the parent-seed, rotation and write-failure tests (the agent sees the raw variable)
Client terminals without the strip helper fork test fails: expected 'Bearer stale-dummy-credential|' to equal '|'
Rotation deletes the old file and writes a new path rotation test fails: NotFound: FileSystem.readFile on the path the agent and terminals were given
remove leaves the file on clear rotation test fails: expected true to be false on exists(file) after clear
ACP write failure raised instead of logged write-failure test fails: ProviderAdapterOpenSessionError ... PermissionDenied: FileSystem.makeTempFileScoped
A failed write keeps its .tmp "leaves no temporary file behind when a write fails" fails: expected true to be false on exists(<path>.tmp)
remove returns early when no header is recorded same test fails: the empty file still exists after remove
Pi deletes only T3_MCP_BEARER_TOKEN Pi launch test fails: expected 'Bearer stale-token' to equal undefined
The shared list loses T3_CODE_MCP_AUTHORIZATION 2 fail: the strip helper's strict-shape test and the Pi launch test
write drops the unchanged-header check "keeps a key's path through a rotation and skips an unchanged header" fails on the inode

The rename step and the semaphore have no mutation check, because no test races a reader or a second writer against a write.

  • tsc --noEmit through each package's typecheck script in provider-core, provider-acp, provider-pi and apps/server: exit 0, 0 errors.
  • vp lint --report-unused-disable-directives on the changed .ts files: 0 errors. 4 warnings, all on lines this PR does not change.
  • vp fmt --check on the changed files: All matched files use the correct format.
  • On the hardening commit: the provider-core, provider-acp and apps/server typechecks exit 0 with 0 errors (it does not touch provider-pi), vp lint and vp fmt --check on its six files are clean, and vp run knip:check reports 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 a8bdfdb5 and on this change on that base (36da67d1). The rebase onto 477282263a was 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 one export. The hardening commit changes how mcpSession.ts writes the .tmp and when remove forgets 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 -i with a private HOME and TMPDIR. A temporary test file that is not part of this PR drove the real code: makeAcpRegistryAdapterV2 spawning the repo's acp-mock-agent.ts, makeAcpAdapterV2 with mcpServers pointing at the clone's real t3 acp-mcp-bridge, and buildPiRpcLaunch. Each child's /proc/<pid>/environ was 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's Authorization header. Both clones were clean afterwards.

Child environments. ACP agent process (the T3_ACP_MCP_NODE and T3_ACP_MCP_ENTRYPOINT lines omitted):

# before (main)
T3_ACP_MCP_AUTHORIZATION=Bearer dummy-e1-00112233445566778899aabbccddeeff
T3_ACP_MCP_ENDPOINT=http://127.0.0.1:43123/mcp

# after (this branch)
T3_ACP_MCP_AUTHORIZATION_FILE=$TMPDIR/t3-mcp-XXXXXX/<random>
T3_ACP_MCP_ENDPOINT=http://127.0.0.1:43123/mcp

The bridge, the client terminal and the Pi child show the same change (T3_MCP_BEARER_TOKEN becomes T3_MCP_AUTHORIZATION_FILE for 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:

# before, server env has T3_ACP_MCP_AUTHORIZATION: ACP agent and its client terminal
T3_ACP_MCP_AUTHORIZATION=Bearer dummy-stale-inherited

# before, server env has T3_MCP_BEARER_TOKEN and T3_CODE_MCP_AUTHORIZATION: ACP agent and its client terminal
T3_CODE_MCP_AUTHORIZATION=Bearer dummy-stale-inherited
T3_MCP_BEARER_TOKEN=dummy-stale-inherited

# after, both runs: ACP agent and its client terminal
<no T3 MCP variables>

T3 MCP variables in each child's environment, before and after

Crash dumps. For each captured environment, a dummy /usr/bin/sleep ran as a transient user unit with exactly those T3 MCP variables, was crashed with SIGSEGV, and COREDUMP_ENVIRON was filtered to T3 MCP lines. The dummy core files were deleted afterwards.

$ systemd-run --user --collect --unit=$UNIT -p LimitCORE=infinity -E T3_...=... /usr/bin/sleep 300
$ systemctl --user kill -s SEGV $UNIT
$ journalctl --user COREDUMP_USER_UNIT=$UNIT.service -o json --all --output-fields=COREDUMP_ENVIRON

# before, t3 acp-mcp-bridge environment
T3_ACP_MCP_AUTHORIZATION=Bearer dummy-e1-00112233445566778899aabbccddeeff
T3_ACP_MCP_ENDPOINT=http://127.0.0.1:33005/mcp

# after
T3_ACP_MCP_AUTHORIZATION_FILE=$TMPDIR/t3-mcp-XXXXXX/<random>
T3_ACP_MCP_ENDPOINT=http://127.0.0.1:46271/mcp

All four channels (agent, bridge, terminal, Pi) held the dummy bearer before and only the path after.

COREDUMP_ENVIRON after a SIGSEGV, before and after

The credential still works. On this branch, the bridge started from the exact mcpServers spec, acp-mcp-call from a client terminal, and the Pi extension (loaded with Pi's environment, real fs and real fetch) each sent the issued header. After a rotation the file at the same path held the new header and acp-mcp-call sent it:

native session: mock-session-1; agent processes started: 1
file before rotation: Bearer dummy-... (matches dummy: true)
file after rotation:  Bearer dummy-... (matches rotated dummy: true)
new terminal's T3_ACP_MCP_AUTHORIZATION_FILE is the same path: true
acp-mcp-call stdout: {"content":[{"type":"text","text":"e1 probe ok"}]}
server received tools/call: Authorization: Bearer dummy-... (matches rotated dummy: true)

The file was 0600 in a 0700 directory, and both were gone once the owning scope closed. No t3-mcp-* directory was left in TMPDIR. The bridge given only the old raw variable exits with code 2.

The bridge, acp-mcp-call and the Pi extension authenticating from the file, rotation, file mode and cleanup

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 call orchestrator_capabilities. The call went through t3 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. The codex-acp process had T3_ACP_MCP_AUTHORIZATION_FILE, T3_ACP_MCP_ENDPOINT, T3_ACP_MCP_ENTRYPOINT and T3_ACP_MCP_NODE. The bridge had T3_ACP_MCP_AUTHORIZATION_FILE and T3_ACP_MCP_ENDPOINT. Neither had T3_ACP_MCP_AUTHORIZATION.

ACP thread on this branch: orchestrator_capabilities completed

The orchestrator_capabilities result in the ACP thread

Not checked:

  • macOS and Windows. os.tmpdir() is per user on both and rename replaces 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.
  • A real pi binary. The Pi environment came from buildPiRpcLaunch without starting Pi, and the extension ran in a vm context, as the repo's testkit does.
  • Real Grok, Antigravity or other registry agent binaries. The manual checks used the repo's mock agent, and the live thread used codex-acp through the registry driver.
  • A real crash of a real provider. The crash dumps came from a dummy sleep carrying each captured environment.
  • A T3 server crash, and a reader or second writer racing a write.

Model and harness: Claude Opus 5.5 in Claude Code, run from T3 Code.

🤖 Generated with Claude Code

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>
@github-actions github-actions Bot added the vouch:unvouched PR author is not yet trusted in the VOUCHED list. label Oct 11, 2026
@github-actions github-actions Bot added the size:L 100-499 changed lines (additions + deletions). label Oct 11, 2026
@macroscopeapp

macroscopeapp Bot commented Oct 11, 2026

Copy link
Copy Markdown
Contributor

Approvability

Verdict: 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>
@coderabbitai

coderabbitai Bot commented Oct 11, 2026 •

Copy link
Copy Markdown

Review in Change Stack →

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info
⚙️ Run configuration
  • Configuration used: Path: .coderabbit.config.ts
  • Review profile: CHILL
  • Plan: Advanced
  • Run ID: 3ec165d9-fb6f-4169-88f2-f19945cc2654

📥 Commits

Reviewing files that changed from the base of the PR and between 0b0028b and b979146.


📒 Files selected for processing (6)
  • apps/server/scripts/acp-mock-agent.ts
  • apps/server/src/provider/acp/AcpJsonRpcConnection.test.ts
  • docs/orchestration-v2/orchestrator-mcp-server.md
  • packages/provider-acp/src/server/adapter.ts
  • packages/provider-core/src/server/mcpSession.test.ts
  • packages/provider-core/src/server/mcpSession.ts

🚧 Files skipped from review as they are similar to previous changes (2)
  • docs/orchestration-v2/orchestrator-mcp-server.md
  • packages/provider-acp/src/server/adapter.ts

Included review availability: This review used your included allowance. Your plan provides up to 10 included reviews per hour; 2 remain after this review.



📝 Walkthrough
📝 Walkthrough

Walkthrough

ACP and Pi now pass MCP authorization through scoped credential files instead of raw environment values. Provider environments mask legacy MCP credentials. ACP bridge handling, launch configuration, tests, and documentation reflect the file-based flow.

Changes

MCP credential-file flow

Layer / File(s) Summary
Credential files and environment sanitization
packages/provider-core/src/server/mcpSession.ts, packages/provider-core/src/server/mcpSession.test.ts
Provider core adds scoped credential-file management and masks three legacy raw MCP credential variables. Tests cover credential updates, removal, cleanup, permissions, and environment fallback.
ACP credential-file integration
packages/provider-acp/src/server/adapter.ts, packages/provider-acp/src/server/AcpSessionRuntime.ts, packages/provider-acp/src/server/clientTerminals.ts, apps/server/src/orchestration-v2/Adapters/AcpAdapterV2.test.ts, apps/server/src/provider/acp/AcpJsonRpcConnection.test.ts, apps/server/scripts/acp-mock-agent.ts, docs/orchestration-v2/orchestrator-mcp-server.md
ACP session setup passes credential-file paths to MCP consumers and filters child and terminal environments. Tests cover session updates, forks, rollback replacement, and credential-file failures. The documentation describes Codex MCP configuration in JSON-RPC requests.
ACP bridge credential-file reader
apps/server/src/mcp/AcpMcpStdioBridge.ts, apps/server/src/mcp/AcpMcpStdioBridge.test.ts, apps/server/src/cli/acpMcpBridge.ts
The bridge reads and trims authorization from the configured file. Tests check missing and unreadable credentials. The command documentation describes the file-based configuration.
Pi credential-file integration
packages/provider-pi/src/server/adapter.ts, packages/provider-pi/src/server/mcpInjection.ts, packages/provider-pi/src/server/mcpExtensionSource.ts, packages/provider-pi/src/server/status.ts, packages/provider-pi/src/server/textGeneration.ts, packages/provider-pi/src/server/*test*, docs/orchestration-v2/orchestrator-mcp-server.md
Pi launch configuration now carries an authorization-file path. The extension reads authorization from that file. Tests check that the token is absent from arguments and environment and that the file is removed after the scoped runtime ends. The documentation describes the new environment setting.

Priority: ➖ Normal

Estimated code review effort: 3 (Moderate) | ~25 minutes

Change: Bug fix

Sequence Diagram(s)

sequenceDiagram
  participant AcpAdapter
  participant makeMcpCredentialFiles
  participant AcpMcpStdioBridge
  participant CredentialFile
  AcpAdapter->>makeMcpCredentialFiles: Write authorization for thread
  makeMcpCredentialFiles-->>AcpAdapter: Return credential-file path
  AcpAdapter->>AcpMcpStdioBridge: Pass endpoint and credential-file path
  AcpMcpStdioBridge->>CredentialFile: Read and trim authorization
Loading
sequenceDiagram
  participant PiAdapter
  participant makeMcpCredentialFiles
  participant buildPiRpcLaunch
  participant PiExtension
  participant CredentialFile
  PiAdapter->>makeMcpCredentialFiles: Write session authorization
  makeMcpCredentialFiles-->>PiAdapter: Return credential-file path
  PiAdapter->>buildPiRpcLaunch: Pass endpoint and file path
  buildPiRpcLaunch->>PiExtension: Set endpoint and file-path environment
  PiExtension->>CredentialFile: Read and trim authorization
Loading

Suggested reviewers: juliusmarminge


Merge Risk: ⚪ Minimal · up to b9791

The credential-file handoff has no identified merge-blocking issue; the Pi MCP request retains a valid authorization header.

Security Architecture Review

Security architecture risk: 🔵 Low · up to b9791

The change reduces accidental bearer-token exposure in child environments. The inspected paths use scoped credential files, atomic replacement and environment sanitization without establishing a new authorization bypass. Remaining uncertainty concerns Windows file permissions and credential-file recovery after abnormal shutdown.

Retained concerns
No architecture-level concerns identified.

Security review details

Security Blast Radius

  • inferred — The affected bearer grants access to existing orchestration operations, not merely a child-process connection. Documented operations include project-thread reads, mutations and delegation across supported provider instances, so credential compromise can affect more than the originating conversation. Exact handler-level authorization limits were not reverified.

Security Findings and Attack Paths

  • inferred — A provider or terminal with the same filesystem identity can still read the credential through its supplied path. The ACP reader does not confine an environment-supplied path, but this does not establish a new privilege boundary crossing: those processes previously received the bearer directly. The change reduces accidental environment disclosure rather than isolating credentials from malicious same-user code.

Trust Boundaries and Controls

  • observed — Legacy raw credential names are masked after ACP terminal environment overlays and before ACP process spawning. Pi sanitizes its launch environment, supplies only its current MCP path and endpoint, and spawns without extending the inherited environment.

Resilience and Maintainability Implications

  • observed — Credential cleanup is attached to the provider-session scope. ACP file-update errors produce an MCP-disabled context; Pi file-creation errors fail session opening. Pi's short-lived fork worker receives no MCP configuration and disables extensions and tools. Inspected tests assert credential removal when scopes close.

Hardening Proposals

  • proposed — Validate Windows ACL isolation and abnormal-shutdown residue handling explicitly. Recovery should account for credential lifetime and revocation before removing abandoned directories. These are assurance improvements, not verified vulnerabilities.

Pre-merge checks | Passed 4
✅ Passed checks (4 passed)
Check name Status Explanation
Linked Issues check Passed Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check Passed Check skipped because no linked issues were found for this pull request.
Title check Passed The title clearly and concisely describes the main change: preventing MCP bearer credentials from entering ACP and Pi child environments.
Description check Passed The description includes complete Problem, Change, Scope and approval, and Verification sections. It explains the security issue, implementation, approval context, focused test results, manual checks,…

✨ Finishing Touches
🧪 Generate unit tests (beta)
  • Create a new PR

  • Autofix · Keep fixing CodeRabbit findings and required CI, and resolving merge conflicts

Comment @coderabbitai help to get the list of available commands.

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

No deployments
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

size:L 100-499 changed lines (additions + deletions). vouch:unvouched PR author is not yet trusted in the VOUCHED list.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant