fix(server): Antigravity keeps its workspace containment - #13613
Conversation
| allowedRoots: [cwd, options.serverConfig.attachmentsDir], | ||
| request, | ||
| }), | ||
| writeTextFile: (request, cwd) => |
There was a problem hiding this comment.
🔴 Critical Adapters/AntigravityAdapterV2.ts:190
readTextFile and writeTextFile still follow a final symlink after the containment check, so an in-workspace symlink targeting outside cwd or attachmentsDir can disclose or overwrite arbitrary server files. Update readAntigravityClientTextFile and writeAntigravityClientTextFile to validate the canonicalized target path (or reject final-component symlinks), not only dirname(resolved).
🚀 Reply "fix it for me" or copy this AI Prompt for your agent:
In file @apps/server/src/orchestration-v2/Adapters/AntigravityAdapterV2.ts around line 190:
`readTextFile` and `writeTextFile` still follow a final symlink after the containment check, so an in-workspace symlink targeting outside `cwd` or `attachmentsDir` can disclose or overwrite arbitrary server files. Update `readAntigravityClientTextFile` and `writeAntigravityClientTextFile` to validate the canonicalized target path (or reject final-component symlinks), not only `dirname(resolved)`.
Thread transfer impact✅ Thread transfer remains within every enforced ceiling.
Baseline: unavailable · PR result: Scenario and decoded snapshot size10 historical turns, 5 command tools per turn, 878.9 KiB retained MCP result per historical turn, and a 1.05 MiB retained result in the measured turn.
Updated in place by a trusted workflow. PR artifacts are strictly validated and never executed. |
ApprovabilityVerdict: Not approved Macroscope's review found this PR not approvable — This PR changes production filesystem authorization and workspace containment for Antigravity, including symlink resolution and no-workspace behavior. An unresolved critical symlink-escape finding makes human verification necessary despite the apparent canonical-path fix in the current diff. Not approved because:
Adjust the Minimum Blocking Severity for this repo — including turning it Off — in Settings. You can add or adjust custom eligibility rules. Learn more. |
e5a3e33 to
dcb5ce1
Compare
The generic ACP fs handlers registered after Antigravity's own and replaced them (effect-acp keeps the last handler per method), so Antigravity could read and write any path the T3 server can. The flavor now supplies its handlers through `clientFileSystem`, and the adapter serves them behind the runtime policy guard instead of the unconfined generic ones. Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com>
…f the server cwd A session policy without a cwd made the server's own cwd Antigravity's containment root. Pass the nullable policy cwd through and allow only the attachments dir in that case. Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com>
…nt workspace Two escapes from Antigravity's workspace containment: - The check resolved only the parent directory, so a symlink inside the workspace pointing outside it could be read or overwritten through. The final target is now canonicalized before the check, and a link that exists but cannot be resolved (dangling) is refused rather than written through. - The allowed roots came from the policy the session opened with. They now come from the policy active when the request arrives, the one the policy guard already checks, so a session carried into a turn for another workspace cannot reach the previous one. Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com>
0abd34b to
bae8e22
Compare
Antigravity is the one ACP agent T3 gives client file access on purpose, and its handlers confine reads and writes to the workspace and the attachments dir (
AntigravityClientFiles.ts). In V2 those handlers stopped doing anything:AntigravityAdapterV2registered them insidemakeRuntime, thenwireAcpRuntimeHandlersregistered the genericacpReadTextFile/acpWriteTextFileon the same runtime, and effect-acp keeps only the last handler per method (packages/effect-acp/src/client.tshandleReadTextFile/handleWriteTextFileassigncoreHandlers.*). Under auto or full access the policy guard allows every read and write, so Antigravity could read or write any path the T3 server can.This is layer 1 of the ACP stack the maintainer approved: "rely on the agent's own sandboxes and permission models rather than implementing our own, like Claude and Codex".
Stacked on #13616 (Grok launches in the thread's permission mode), which this stack builds on; it will retarget to
t3code/codex-turn-mappingwhen that lands.What changed
AcpAdapterV2Flavorgets an optionalclientFileSystem: { readTextFile, writeTextFile }that receives the session cwd. The adapter still runs the runtime-policy guard first, then calls the flavor's handler instead of the generic one.AntigravityAdapterV2moves its containment handlers there (roots: session cwd + attachments dir, unchanged) and no longer registers them on the runtime itself.The handler gets the policy cwd as-is (
string | null). A session without a workspace gets only the attachments dir as a root instead of falling back to the server's ownprocess.cwd(). In production the runtime policy always carries the thread's worktree or project root (RuntimePolicy.tslayerFromProjectRepositoryfails without a project), so this only matters for callers that build a policy withcwd: null.Containment resolves the final path, not only its directory. A symlink inside the workspace that points outside it is refused for reads and writes. A dangling link is refused rather than followed, so it can't create a file outside the workspace.
The allowed roots come from the policy active when the request arrives, the same one the runtime policy guard reads (
clientPolicyContext), instead of the policy the session opened with. In today's orchestrator an ACP session serves one thread (supportsMultipleProviderThreadsPerSession: false), and a workspace change detaches the session (Orchestrator.ts"Workspace changed."). So this closes a footgun rather than a live escape:startTurnalready switcheslatestRuntimePolicyto the incoming turn's policy.Decisions (override if you disagree)
mainopts it in for the same reason,AntigravityAcpSupport.ts). Later layers turn client fs off for everyone else.Verification
vp test run src/orchestration-v2/Adapters/AntigravityAdapterV2.test.ts src/orchestration-v2/Adapters/AcpAdapterV2.test.ts src/provider/Drivers/AntigravityDriver.test.ts src/provider/acp/AntigravityAcpSupport.test.ts(apps/server): 165 passed.AntigravityAdapterV2 workspace changes > confines file requests to the workspace of the turn in progress. A session opened for workspace A runs a turn for workspace B: B's file is served and A's is denied. It fails if the handler uses the cwd the session opened with.AntigravityAdapterV2 client file system > confines agent file requests to the workspace under full accessopens a real Antigravity adapter session against the ACP mock agent (Antigravity profile) underfull-access, then calls the fs handlers the runtime actually kept: a write inside the workspace and a read from the attachments dir succeed; a read and a write outside both fail and nothing is written. With the fix reverted the test fails (expected false to be trueon the outside read).vp exec tsc --noEmit -p .(apps/server): no errors or warnings.vp linton the touched files: only the pre-existing unusedNodePathimport warning inAcpAdapterV2.ts.Model: Claude Opus 5.5 (Claude Code)
🤖 Generated with Claude Code