From f7fae6b73874fe07e0b60d1ea8722e91eade8cdf Mon Sep 17 00:00:00 2001 From: Devin Foley Date: Tue, 3 Nov 2026 13:15:01 -0800 Subject: [PATCH] fix(runner): preserve native Claude GitHub broker transport (#18850) ## Thinking Path > - Paperclip is the open source app people use to manage AI agents for work. > - Shared agents use managed GitHub launchers to acquire credentials for the active run. > - Native Claude runs through the ACPX provider environment boundary. > - That boundary keeps the launcher paths but removes their broker transport settings. > - Shell commands then report `capability_missing`, even when connector tools work. > - This pull request retains the three transport settings for explicit Claude managed launchers. > - Shell commands can use the existing session broker and its current access checks. ## Linked Issues or Issue Description **What happened?** Native Claude through ACPX cannot authenticate shell `git` and `gh` commands through an eligible managed GitHub connection. The launcher reports `GitHub capability_missing`. Connector tools can still authenticate through their own path. **Expected behavior** Managed shell commands acquire credentials for the active run. Provider processes do not receive raw GitHub tokens or ambient host credentials. **Steps to reproduce** 1. Configure an eligible managed GitHub connection and a native Claude ACPX execution. 2. Start a task and run `gh api user` through its terminal. 3. Observe `capability_missing`, although the corresponding connector identity request succeeds. **Paperclip version or commit** Reproduced from source at `6ef37398c8d799c5142a3a6590ecc70ad352b663` and current master `0b87308d5de93b4b8f18eeea89afc0bb4d9480a7` with controlled local dependencies. The affected runtime is native Claude through ACPX in a managed sandbox. Related: #18212 preserves Claude launcher selectors. This change supplies the missing broker transport. #18081 and #17599 address a separate Legacy Codex shell-policy boundary. #15354 addresses Legacy ACPX session hashes. The duplicate search found no open PR for this native Claude boundary. ROADMAP.md already lists managed personal GitHub identities; this restores that existing behavior. ## What Changed - Retain the broker URL, session capability and bridge capability within the existing explicit Claude managed-launcher allowlist. - Continue to reject raw GitHub tokens, ambient broker settings, task-supplied overrides and transport for other ACPX providers. - Add real Bash and launcher subprocess controls with a loopback broker and fixture Git/gh executables. Cover isolated, prepared and preconfigured homes; changed operation identities; revocation; and recovery. - Confirm that broker capabilities stay out of persisted session environment data. Update the execution guide. The ACPX environment owner retains this behavior. Broker authorization and credential resolution do not change. No shared API or schema changes are needed. ## Verification - Passing: 84 focused tests across `environment.test.ts`, `github-launcher.test.ts`, `runtime-sandbox.test.ts` and `configured-environment.test.ts` in the Runner package. - Passing: `pnpm --filter @paperclipai/paperclip-runner typecheck:typescript` and repository-wide `pnpm -r typecheck`. - Passing: repository-wide `pnpm build`. - Passing: 60 existing adapter-utils launcher tests; one unrelated test is skipped. - Passing: `node scripts/check-maps.mjs`, `node scripts/check-module-boundaries.mjs`, and `git diff --check`. - The new regression failed before the fix: both shell commands ran anonymously and the unit check showed the three missing broker settings. After the fix, the local broker receives each operation, changes identity for later commands, and denies revoked access without reusing a prior token. - The prepared-home fixture first rejected a macOS temporary-directory alias. It now resolves that fixture directory to its canonical path. A launcher-control command also named a nonexistent package config; the corrected run uses the root Vitest config. - Both `pnpm test:run` attempts stopped in chat-suite setup when embedded Postgres initialization exited with no numeric status. No chat tests ran. An independent database-support probe between attempts succeeded. The startup cause is not established; the full local suite is not verified. No further identical retry was made. - [CI run 41713841404, attempt 2](https://github.com/paperclipai/paperclip/actions/runs/41713841404) passed on `92fd931722486c111030a7efe19dfc73f530b4c8`: 48 successful jobs and one conditional skip. This includes browser shards, general and serialized tests, Runner static/Rust/package/native-server checks, build, typecheck and the canary dry run. No CI jobs remain pending or failed. - The first CI attempt was interrupted on multiple runners. Browser and server/chat jobs logged runner shutdown signals followed by cancellation. The Runner static check reported lost communication with its runner. No failing browser assertion was recorded before termination; the summary gates failed on interrupted jobs. The successful retry used the same source revision. It shows recovery but does not establish why the hosts stopped. - Greptile reviewed `92fd931722486c111030a7efe19dfc73f530b4c8` at 5/5 with no outstanding findings. All review threads are resolved, and GitHub reports the branch as mergeable with no conflicts. The source tests use fixture credentials and a loopback broker on macOS arm64 with Node 25.6.1. They do not qualify a live Claude process, deployed sandbox, Windows shell or GitHub write. No existing run or deployed configuration was changed. ## Risks - Claude shell commands can read the explicitly supplied broker capabilities. Managed launchers need those capabilities. The existing broker still enforces active session/run binding, responsible identity, trust and connection access. - The exception applies only when the controller supplies a Claude managed-launcher directory. Raw GitHub tokens remain excluded, and broker settings remain absent from persisted session environment data. - No migration is required. Existing provider processes need their normal retirement and restart to receive a new launch environment. Deployment and live sandbox verification remain separate steps. ## Model Used OpenAI `gpt-6.1-sol` through the Codex harness, with xhigh reasoning. Used repository inspection, code editing and shell execution. The context window size is not exposed in this session. ## Checklist - [x] I have included a thinking path that traces from project context to this change - [x] I have specified the model used (with version and capability details) - [x] I have checked ROADMAP.md and confirmed this PR does not duplicate planned core work - [x] I have searched GitHub for duplicate or related PRs and linked them above - [x] I have either (a) linked existing issues with `Fixes: #` / `Closes #` / `Refs #` OR (b) described the issue in-PR following the relevant issue template - [x] I have not referenced internal/instance-local Paperclip issues or links (only public GitHub `#NNN` / `github.com/paperclipai/paperclip` URLs) - [x] My branch name describes the change (e.g. `docs/...`, `fix/...`) and contains no internal Paperclip ticket id or instance-derived details - [x] I have run tests locally and they pass - [x] I have added or updated tests where applicable - [x] I have updated relevant documentation to reflect my changes - [x] I have considered and documented any risks above - [x] All Paperclip CI gates are green - [x] Greptile is 5/5 with no open P2s, recommendations, or follow-ups - [x] I will address all Greptile and reviewer comments before requesting merge --------- Co-authored-by: Paperclip --- doc/execution-github-identity.md | 8 ++ .../src/drivers/acpx/environment.test.ts | 14 ++- .../src/drivers/acpx/environment.ts | 3 +- .../src/drivers/acpx/github-launcher.test.ts | 100 ++++++++++++++++++ 4 files changed, 121 insertions(+), 4 deletions(-) create mode 100644 packages/paperclip-runner/src/drivers/acpx/github-launcher.test.ts diff --git a/doc/execution-github-identity.md b/doc/execution-github-identity.md index 53a4cc5d99a..02e2ef50fdd 100644 --- a/doc/execution-github-identity.md +++ b/doc/execution-github-identity.md @@ -54,6 +54,14 @@ runner startup and provider startup. Shell operations acquire credentials for the active run through that broker. They do not require a persistent GitHub CLI login or inherit an ambient host broker capability. +Claude through native ACPX retains the controller-projected broker URL and +session/bridge capabilities alongside its managed launcher paths. Both isolated +and preconfigured homes use this transport for shell `git` and `gh` operations. +These capabilities stay out of persisted session environment data. Raw GitHub +tokens remain confined to each managed command; ambient host capabilities and +task-supplied broker overrides are excluded. A working connector tool alone does +not prove that the terminal received its broker transport. + Without a managed identity, local commits default to `Paperclip `. The launcher writes this token-free default to a private per-operation global Git configuration file, removed on exit. diff --git a/packages/paperclip-runner/src/drivers/acpx/environment.test.ts b/packages/paperclip-runner/src/drivers/acpx/environment.test.ts index fa6db41055c..641cd1eb5cb 100644 --- a/packages/paperclip-runner/src/drivers/acpx/environment.test.ts +++ b/packages/paperclip-runner/src/drivers/acpx/environment.test.ts @@ -14,11 +14,13 @@ describe("ACPX launch environment", () => { expect(() => createSanitizedAcpxSpawnInput({[key]:value, [CONFIGURED_ENVIRONMENT_KEYS]:JSON.stringify([key])}, "claude")).toThrow("configured environment"); } }); - it("keeps explicit Claude managed-command selectors without forwarding GitHub credentials", () => { + it("keeps explicit Claude managed-command transport without forwarding GitHub credentials", () => { const helpers = { PATH: "/managed:/usr/bin", BASH_ENV: "/managed/.bashrc", ZDOTDIR: "/managed", - GH_CONFIG_DIR: "/managed", PAPERCLIP_GITHUB_LAUNCHER_DIR: "/managed" }; + GH_CONFIG_DIR: "/managed", PAPERCLIP_GITHUB_LAUNCHER_DIR: "/managed", + PAPERCLIP_GITHUB_BROKER_URL: "http://127.0.0.1:3210", PAPERCLIP_GITHUB_BROKER_TOKEN: "session-capability", + PAPERCLIP_GITHUB_BRIDGE_TOKEN: "bridge-capability" }; const source = { ...helpers, GH_TOKEN: "host-secret", GITHUB_TOKEN: "host-secret", - PAPERCLIP_GITHUB_BROKER_TOKEN: "host-capability", NODE_OPTIONS: "--require=untrusted" }; + PAPERCLIP_GIT_TOKEN: "host-secret", NODE_OPTIONS: "--require=untrusted" }; expect(createSanitizedAcpxSpawnInput(source, "claude").env).toEqual(helpers); for (const [key, value] of Object.entries(helpers)) vi.stubEnv(key, value); const ambient = createSanitizedAcpxSpawnInput(undefined, "claude").env; @@ -27,6 +29,12 @@ describe("ACPX launch environment", () => { expect(ambient).not.toHaveProperty(key); expect(sibling).not.toHaveProperty(key); } + const { PAPERCLIP_GITHUB_LAUNCHER_DIR: _launcher, ...withoutLauncher } = source; + expect(createSanitizedAcpxSpawnInput(withoutLauncher, "claude").env).toEqual({ PATH: helpers.PATH }); + for (const [key, value] of Object.entries(helpers).filter(([key]) => key.startsWith("PAPERCLIP_GITHUB_"))) { + expect(configuredEnvironmentProjection({ [key]: value })).not.toHaveProperty(key); + expect(() => createSanitizedAcpxSpawnInput({ [key]: value, [CONFIGURED_ENVIRONMENT_KEYS]: JSON.stringify([key]) }, "claude")).toThrow("configured environment"); + } }); it("does not inherit ambient prepared storage", () => { vi.stubEnv("PAPERCLIP_RUNNER_PREPARED_HARNESS_HOME", "/ambient/claude"); diff --git a/packages/paperclip-runner/src/drivers/acpx/environment.ts b/packages/paperclip-runner/src/drivers/acpx/environment.ts index ca2bf78e80b..8b5a9a0d68a 100644 --- a/packages/paperclip-runner/src/drivers/acpx/environment.ts +++ b/packages/paperclip-runner/src/drivers/acpx/environment.ts @@ -153,7 +153,8 @@ export function createSanitizedAcpxSpawnInput( // The environment owner supplies helper paths, not a replacement provider executable. ...(agent === "claude" && environment !== undefined ? ["MCP_TOOL_TIMEOUT"] : []), ...(agent === "claude" && environment?.PAPERCLIP_GITHUB_LAUNCHER_DIR?.trim() - ? ["BASH_ENV", "ZDOTDIR", "GH_CONFIG_DIR", "PAPERCLIP_GITHUB_LAUNCHER_DIR"] : []), + ? ["BASH_ENV", "ZDOTDIR", "GH_CONFIG_DIR", "PAPERCLIP_GITHUB_LAUNCHER_DIR", + "PAPERCLIP_GITHUB_BROKER_URL", "PAPERCLIP_GITHUB_BROKER_TOKEN", "PAPERCLIP_GITHUB_BRIDGE_TOKEN"] : []), ...(usesPreconfiguredHome(source) ? [...PRECONFIGURED_HOME_KEYS, "PAPERCLIP_RUNNER_HOME_MODE", "ACPX_CLAUDE_INCLUDE_USER_SETTINGS"] : []), ...(agent === "claude" && environment?.PAPERCLIP_RUNNER_PREPARED_HARNESS_HOME ? [...PRECONFIGURED_HOME_KEYS, "PAPERCLIP_RUNNER_PREPARED_HARNESS_HOME"] : []), diff --git a/packages/paperclip-runner/src/drivers/acpx/github-launcher.test.ts b/packages/paperclip-runner/src/drivers/acpx/github-launcher.test.ts new file mode 100644 index 00000000000..64ee858df53 --- /dev/null +++ b/packages/paperclip-runner/src/drivers/acpx/github-launcher.test.ts @@ -0,0 +1,100 @@ +import { execFile } from "node:child_process"; +import { mkdir, mkdtemp, readFile, realpath, rm, writeFile } from "node:fs/promises"; +import { createServer } from "node:http"; +import { tmpdir } from "node:os"; +import { dirname, join } from "node:path"; +import { promisify } from "node:util"; +import { afterEach, expect, it } from "vitest"; +import { githubLauncherSource } from "../../../../adapter-utils/src/github-launcher.js"; +import { resolveQualifiedAcpxProfile } from "./qualified-profiles.js"; +import { createAcpxRecoveryBinding } from "./recovery-identity.js"; +import { prepareAcpxRuntimeSandbox } from "./runtime-sandbox.js"; + +const execute = promisify(execFile); +const cleanups: Array<() => Promise> = []; + +afterEach(async () => { + for (const cleanup of cleanups.splice(0).reverse()) await cleanup(); +}); + +it.skipIf(process.platform === "win32").each(["isolated", "prepared", "preconfigured"])("authenticates Claude shell commands through its session broker (%s home)", async homeMode => { + const root = await realpath(await mkdtemp(join(tmpdir(), "paperclip-acpx-github-"))); + cleanups.push(() => rm(root, { recursive: true, force: true })); + const workspace = join(root, "workspace"); + const runtimeDirectory = join(root, "runtime"); + const managed = join(root, "managed"); + const real = join(root, "real"); + const home = join(root, "home"); + await Promise.all([workspace, runtimeDirectory, managed, real, home].map(path => mkdir(path))); + await mkdir(join(home, ".claude")); + const binding = await createAcpxRecoveryBinding({ + runtimeDirectory, + normalizedSessionId: "github-session", + workingDirectory: workspace, + profile: resolveQualifiedAcpxProfile("claude", "claude-sonnet-5"), + requestedModel: "claude-sonnet-5", + permissionMode: "approve-reads", + }); + let identity: "first" | "second" | null = "first"; + let requests = 0; + const server = createServer((req, res) => { + req.resume(); + requests++; + if (req.method !== "POST" || req.url !== "/runtime-tools/github/credentials" + || req.headers.authorization !== "Bearer fixture-bridge" + || req.headers["x-paperclip-github-capability"] !== "fixture-session") { + res.writeHead(403).end(); + return; + } + res.setHeader("Content-Type", "application/json"); + res.end(JSON.stringify(identity + ? { status: "available", env: { GH_TOKEN: `fixture-operation-${identity}` } } + : { status: "unavailable", reason: "Fixture connection revoked" })); + }); + await new Promise(resolve => server.listen(0, "127.0.0.1", resolve)); + cleanups.push(() => new Promise(resolve => { server.closeAllConnections(); server.close(() => resolve()); })); + const url = `http://127.0.0.1:${(server.address() as { port: number }).port}`; + const path = [managed, real, dirname(process.execPath), "/usr/bin", "/bin"].join(":"); + await writeFile(join(managed, ".bashrc"), `export PATH='${path}'\n`); + const executable = `#!/usr/bin/env node +const token = process.env.GH_TOKEN; +const identity = token === 'fixture-operation-first' ? 'first' : token === 'fixture-operation-second' ? 'second' : 'anonymous'; +console.log(identity); +`; + for (const program of ["git", "gh"]) { + await writeFile(join(managed, program), githubLauncherSource(), { mode: 0o700 }); + await writeFile(join(real, program), executable, { mode: 0o700 }); + } + const environment = { + PATH: path, + BASH_ENV: join(managed, ".bashrc"), + ZDOTDIR: managed, + GH_CONFIG_DIR: managed, + PAPERCLIP_GITHUB_LAUNCHER_DIR: managed, + PAPERCLIP_GITHUB_BROKER_URL: url, + PAPERCLIP_GITHUB_BROKER_TOKEN: "fixture-session", + PAPERCLIP_GITHUB_BRIDGE_TOKEN: "fixture-bridge", + GH_TOKEN: "ambient-must-not-leak", + GITHUB_TOKEN: "ambient-must-not-leak", + ...(homeMode === "preconfigured" ? { HOME: home, PAPERCLIP_RUNNER_HOME_MODE: "preconfigured" } : {}), + ...(homeMode === "prepared" ? { PAPERCLIP_RUNNER_PREPARED_HARNESS_HOME: join(home, ".claude") } : {}), + }; + const sandbox = await prepareAcpxRuntimeSandbox({ binding, agent: "claude", environment }); + const commands = "git ls-remote https://github.com/example/fixture; gh api user"; + for (const currentIdentity of ["first", "second", null] as const) { + identity = currentIdentity; + const result = await execute("/bin/bash", ["-c", commands], { cwd: workspace, env: sandbox.launchEnvironment, timeout: 10_000 }); + expect(result.stdout.trim().split("\n")).toEqual([identity ?? "anonymous", identity ?? "anonymous"]); + if (identity) expect(result.stderr).toBe(""); + else expect(result.stderr).toContain("Fixture connection revoked"); + } + expect(requests).toBe(6); + expect(sandbox.launchEnvironment.GH_TOKEN).toBeUndefined(); + expect(sandbox.launchEnvironment.GITHUB_TOKEN).toBeUndefined(); + expect(JSON.stringify(sandbox.persistedEnvironment)).not.toMatch(/fixture-(?:session|bridge|operation)|PAPERCLIP_GITHUB_BROKER|PAPERCLIP_GITHUB_BRIDGE/); + const recovered = await prepareAcpxRuntimeSandbox({ binding, agent: "claude", environment }); + identity = "first"; + expect((await execute("/bin/bash", ["-c", commands], { cwd: workspace, env: recovered.launchEnvironment, timeout: 10_000 })).stdout.trim().split("\n")).toEqual(["first", "first"]); + expect(requests).toBe(8); + expect(await readFile(join(managed, "gh"), "utf8")).not.toContain("fixture-session"); +});