diff --git a/doc/execution-github-identity.md b/doc/execution-github-identity.md index d86c82d95b7..8c6ef1e2a05 100644 --- a/doc/execution-github-identity.md +++ b/doc/execution-github-identity.md @@ -12,9 +12,13 @@ Delegated work and interactions persist their originating context. Retries retai ## Managed GitHub operations -Executions with managed GitHub configured receive token-free `git` and `gh` launchers and a run-scoped capability. Each launcher invocation requests the active context through the authenticated runtime transport and resolves one eligible credential at operation start. A `gh` command's child Git processes inherit that command's captured identity. Later steering does not change already-started operations. When a subsequent run resumes a settled native conversation, the controller starts a fresh provider process with that run’s capability and rebinds its token-free launcher paths. The durable conversation and protected provider settings remain unchanged. Local and remote durable runners complete their bounded suspension before the controller releases the session for the next run, so a queued continuation cannot race unfinished cleanup. +Executions with managed GitHub configured receive token-free `git` and `gh` launchers. Each launcher invocation resolves one eligible credential from the current run's accepted identity at operation start. A `gh` command's child Git processes inherit that command's captured identity; later steering does not change already-started operations. -The broker endpoint rejects browser origins and session cookies, validates a distinct signed runtime scope, and rechecks the company, agent, and live run. Sandboxes relay the capability through the existing authenticated callback bridge. Tokens are returned only to the managed command process. They are not persisted in identity history or injected into the long-lived provider process. +Native runners retain a session-owned broker and launcher path across warm turns. The provider keeps an opaque transport token, not a GitHub credential or the previous run's signed capability. After acquiring exclusive session ownership, the controller binds the broker to the current company, agent, task, and run. Requests cannot choose another run or responsible person. The broker rejects requests while idle and discards credential responses if their run binding changed during acquisition. Each operation still rechecks the live run, accepted identity, grants, and trust policy through the shared credential resolver. Changing run IDs alone no longer replaces the provider process; changes to authentication mode, provider credentials, permissions, or other session configuration retain their existing retirement rules. + +The session broker listens only on controller loopback and accepts only its authenticated GitHub credential operation. Remote executions reach it through the existing authenticated callback bridge. Its transport and launchers are retired with the provider session. If remote bridge startup fails, anonymous launchers keep ordinary work available; the next run retries setup with a fresh session. Controller restart/cold recovery still uses the existing checkpoint and process-recovery rules; the broker's in-memory authority is not persisted for adoption. + +Other adapters continue using the run-scoped signed capability and public broker endpoint. That endpoint rejects browser origins and session cookies, validates a distinct signed runtime scope, and rechecks the company, agent, and live run. GitHub credentials are returned only to the managed command process, never persisted in identity history or injected into the long-lived provider process. Low-trust executions cannot receive raw GitHub credentials, including dedicated agent tokens. The broker rechecks current agent, project, task, and retained run @@ -123,6 +127,7 @@ Remote acceptance uses the existing paid runner workflow with a narrow selection Identity history survives deletion of the originating agent or run, so surviving subtasks and approvals retain their responsible person. The company foreign key and company-deletion service remove -these company-scoped records when their company is deleted. Completed runs remove their managed launcher files -before releasing a remote environment; same-run recovery retains them until the -terminal boundary. Cleanup failures are logged and do not change the run result. +these company-scoped records when their company is deleted. Run-scoped adapters remove their managed launcher files at the terminal boundary. +Native warm sessions retain their token-free launchers and inactive broker until +session retirement; environment deletion and orderly controller shutdown close +idle owners first. Cleanup failures are logged and do not change the run result. diff --git a/server/src/__tests__/github-operation-credentials.test.ts b/server/src/__tests__/github-operation-credentials.test.ts index cd9c4cc10c2..02d1c864bcd 100644 --- a/server/src/__tests__/github-operation-credentials.test.ts +++ b/server/src/__tests__/github-operation-credentials.test.ts @@ -1,3 +1,4 @@ +import { createNativeGitHubAccess } from "../services/native-runtime/native-github-access.js"; import express from "express"; import request from "supertest"; import { runtimeConnectionIntentRoutes } from "../routes/connection-intents.js"; @@ -800,6 +801,39 @@ const support = await getEmbeddedPostgresTestSupport(); } }); + it("reuses a session broker across runs while rechecking live-run identity and revocation", async () => { + const input = await seed(); + await grant(input, "A"); + await grant(input, "B"); + const broker = await createNativeGitHubAccess({ + scope: input, target: null, cwd: process.cwd(), env: { PATH: process.env.PATH }, + resolveCredentials: (binding) => resolveGitHubOperationCredentials(db, binding), + }); + const post = () => fetch(`${broker.env.PAPERCLIP_GITHUB_BROKER_URL}/runtime-tools/github/credentials`, { + method: "POST", headers: { authorization: `Bearer ${broker.env.PAPERCLIP_GITHUB_BRIDGE_TOKEN}` }, + }); + try { + const releaseA = broker.activate(input); + const a = await post(); + expect(a.status).toBe(200); + expect((await a.json()).login).toBe("A"); + await db.update(heartbeatRuns).set({ status: "succeeded" }).where(eq(heartbeatRuns.id, input.runId)); + // Even a delayed controller release cannot authorize a finished DB run. + expect((await post()).status).toBe(403); + releaseA(); + const next = { ...input, runId: randomUUID() }; + await db.insert(heartbeatRuns).values({ id: next.runId, companyId: next.companyId, agentId: next.agentId, status: "running", contextSnapshot: { issueId: input.issueId } }); + await initializeRunIdentity(db, { companyId: input.companyId, runId: next.runId, responsibleUserId: "B", cause: "instruction" }); + broker.activate(next); + const b = await post(); + expect(b.status).toBe(200); + expect((await b.json()).login).toBe("B"); + await db.update(connectionGrants).set({ status: "revoked" }).where(eq(connectionGrants.companyId, input.companyId)); + const revoked = await post(); + expect((await revoked.json()).env).toEqual({}); + } finally { await broker.stop(); } + }); + it("requires a run-scoped runtime capability and never accepts browser authentication or supplied identities", async () => { const input = await seed(); await grant(input, "A"); diff --git a/server/src/services/heartbeat-github-launchers.test.ts b/server/src/services/heartbeat-github-launchers.test.ts index 4d8771bc896..5bb4038bacb 100644 --- a/server/src/services/heartbeat-github-launchers.test.ts +++ b/server/src/services/heartbeat-github-launchers.test.ts @@ -11,6 +11,18 @@ import { prepareHeartbeatGitHubLaunchers } from "./heartbeat-github-launchers.js const target = { kind: "remote" as const, transport: "sandbox" as const, providerKey: "daytona", remoteCwd: "/workspace" }; describe("heartbeat GitHub launcher lifetime", () => { + it.each([target, null])("defers native managed authorization/staging to the session owner (%j)", async (target) => { + const prepare = vi.fn(); + const mint = vi.fn(); + const result = await prepareHeartbeatGitHubLaunchers({ + native: true, githubConfigured: true, agentId: "agent-a", runId: "run-a", target, + cwd: "/workspace", env: { GH_TOKEN: "ambient" }, brokerUrl: "https://paperclip.test", createBrokerToken: mint, + }, prepare); + expect(prepare).not.toHaveBeenCalled(); + expect(mint).not.toHaveBeenCalled(); + expect(result.cleanupLocation).toBeNull(); + expect(result.env).toMatchObject({ GH_TOKEN: "", PAPERCLIP_GITHUB_BROKER_TOKEN: "" }); + }); it("keeps anonymous native sandbox launchers stable without issuing a run capability", async () => { const createBrokerToken = vi.fn(() => "run-secret"); const prepareLaunchers = vi.fn(async (input) => input.env); @@ -31,7 +43,7 @@ describe("heartbeat GitHub launcher lifetime", () => { if (cleanupFails) throw new Error("cleanup unavailable"); }); await expect(prepareHeartbeatGitHubLaunchers({ - native: true, githubConfigured: true, agentId: "agent-a", target, + native: false, githubConfigured: true, agentId: "agent-a", target, runId: "failed-run", cwd: "/workspace", env: {}, brokerUrl: "https://paperclip.test", createBrokerToken: () => "current-run-secret", }, prepareLaunchers, cleanupLaunchers)).rejects.toBe(stagingError); @@ -50,7 +62,7 @@ describe("heartbeat GitHub launcher lifetime", () => { }); it.each([ - { native: true, githubConfigured: true, target }, + { native: false, githubConfigured: true, target }, { native: false, githubConfigured: false, target }, { native: true, githubConfigured: false, target: null }, ])("preserves run-scoped managed capabilities outside anonymous native sandboxes: %j", async (mode) => { diff --git a/server/src/services/heartbeat-github-launchers.ts b/server/src/services/heartbeat-github-launchers.ts index 5295c1f4564..83786a48eb2 100644 --- a/server/src/services/heartbeat-github-launchers.ts +++ b/server/src/services/heartbeat-github-launchers.ts @@ -15,10 +15,16 @@ export async function prepareHeartbeatGitHubLaunchers( prepareLaunchers = prepareGitHubOperationLaunchers, cleanupLaunchers = cleanupGitHubOperationLaunchers, ) { + // Native configured access is owned by the provider session supervisor. + // Defer staging until it has acquired that session; never mutate a live + // process's authorization from heartbeat preparation. + if (input.native && input.githubConfigured) { + return { env: githubBrokerEnvironment(input.env, { url: "", token: "" }), cleanupLocation: null }; + } // An unconfigured sandbox has no managed GitHub identity to broker. Its // token-free wrappers still isolate image credentials, but may live as long // as the workspace so a warm provider never inherits a deleted run path. - // Configured identities keep their run-scoped capability/retirement rules. + // Other adapter paths retain their run-scoped capability/retirement rules. const anonymous = input.native && !input.githubConfigured && input.target?.kind === "remote" && input.target.transport === "sandbox"; const location = { diff --git a/server/src/services/heartbeat.ts b/server/src/services/heartbeat.ts index 86578694376..b2fbcb5a584 100644 --- a/server/src/services/heartbeat.ts +++ b/server/src/services/heartbeat.ts @@ -24031,173 +24031,129 @@ export function heartbeatService( nativeDispatchAtMs, }), ); - // Native Git/gh uses the same authenticated remote callback - // transport as managed adapters. A bridge failure must not make - // GitHub a prerequisite for otherwise unrelated native work. - let nativeGitHubBridge: Awaited< - ReturnType - > = null; - if ( - executionTarget?.kind === "remote" && - adapterEnv.PAPERCLIP_GITHUB_BROKER_TOKEN - ) { - try { - nativeGitHubBridge = - await startAdapterExecutionTargetPaperclipBridge({ - runId: run.id, - target: executionTarget, - runtimeRootDir: path.posix.join( - executionTarget.remoteCwd, - ".paperclip-runtime", - "github", - run.id, - ), - adapterKey: "native-github", - hostApiToken: adapterEnv.PAPERCLIP_GITHUB_BROKER_TOKEN, - hostApiUrl: adapterEnv.PAPERCLIP_GITHUB_BROKER_URL, + const guardedDispatch = + await dispatchResolvedInteractionContinuationWithAtomicGate( + (markDispatchStarted) => + executePaperclipNativeSession({ + db, + execution: nativeExecution, + conversationMode: isConversation(issueContext), + turnTimeoutMs: Math.max(0, asNumber(runtimeConfig.timeoutSec, 0)) * 1_000, + runnerInstanceId: nativeRunnerInstanceId, + leaseOwner: runOptions.nativeLeaseOwner, + restartRecovery: runOptions.nativeRestartRecovery, + backend: + options.nativeSessionBackendFactory?.(nativeExecution), + useRunnerd: agent.adapterType === "paperclip_runner", + adapterType: agent.adapterType, + sessionGoalControl, + resumeSessionGoalHeartbeat: + context.resumeSessionGoalHeartbeat === true || + completedGoalControl, + onGoalCheckpoint: async (snapshot) => { + if (!taskKey) return; + const params = + attachPaperclipSessionMetadataToSessionParams( + { + ...runtimeSessionParamsForAdapter, + sessionId: snapshot.identity.sessionId, + cwd: executionWorkspace.cwd, + }, + configuredModel, + sessionConfigMetadata, + )!; + const displayId = + snapshot.providerSessionId ?? snapshot.sessionId; + await upsertTaskSession({ + companyId: agent.companyId, + agentId: agent.id, + adapterType: agent.adapterType, + taskKey, + sessionParamsJson: params, + sessionDisplayId: displayId, + lastRunId: run.id, + lastError: null, + }); + goalCheckpointSession.current = { params, displayId }; + }, onLog, - }); - } catch { - await onLog( - "stderr", - "[paperclip] GitHub runtime transport unavailable; continuing without managed GitHub access.\n", - ); - } - } - try { - const guardedDispatch = - await dispatchResolvedInteractionContinuationWithAtomicGate( - (markDispatchStarted) => - executePaperclipNativeSession({ - db, - execution: nativeExecution, - conversationMode: isConversation(issueContext), - turnTimeoutMs: Math.max(0, asNumber(runtimeConfig.timeoutSec, 0)) * 1_000, - runnerInstanceId: nativeRunnerInstanceId, - leaseOwner: runOptions.nativeLeaseOwner, - restartRecovery: runOptions.nativeRestartRecovery, - backend: - options.nativeSessionBackendFactory?.(nativeExecution), - useRunnerd: agent.adapterType === "paperclip_runner", - adapterType: agent.adapterType, - sessionGoalControl, - resumeSessionGoalHeartbeat: - context.resumeSessionGoalHeartbeat === true || - completedGoalControl, - onGoalCheckpoint: async (snapshot) => { - if (!taskKey) return; - const params = - attachPaperclipSessionMetadataToSessionParams( - { - ...runtimeSessionParamsForAdapter, - sessionId: snapshot.identity.sessionId, - cwd: executionWorkspace.cwd, - }, - configuredModel, - sessionConfigMetadata, - )!; - const displayId = - snapshot.providerSessionId ?? snapshot.sessionId; - await upsertTaskSession({ - companyId: agent.companyId, - agentId: agent.id, - adapterType: agent.adapterType, - taskKey, - sessionParamsJson: params, - sessionDisplayId: displayId, - lastRunId: run.id, - lastError: null, - }); - goalCheckpointSession.current = { params, displayId }; - }, - onLog, - onEvent: onAdapterEvent, - preparationSpans: nativeRunnerPreparationSpans, - // Bootstrap with executable/home discovery while keeping - // configured provider values and the server-selected - // workspace boundary authoritative. - managedAiCredentialIdentity: managedAiRuntime?.identity, - managedAiCredentialHome: managedAiRuntime ? String((managedAiRuntime.config.env as Record).CODEX_HOME) : undefined, - runnerEnvironment: { - ...buildNativeProviderEnvironment( - adapterEnv, - process.env, - executionWorkspace.cwd, - ), - ...(nativeGitHubBridge - ? { - PAPERCLIP_GITHUB_BROKER_URL: - nativeGitHubBridge.env.PAPERCLIP_API_URL, - PAPERCLIP_GITHUB_BRIDGE_TOKEN: - nativeGitHubBridge.env.PAPERCLIP_API_KEY, - } - : {}), - ...(nativeMcpServer - ? { - PAPERCLIP_NATIVE_MCP_NAME: nativeMcpServer.name, - PAPERCLIP_NATIVE_MCP_URL: nativeMcpServer.url, - PAPERCLIP_NATIVE_MCP_TOKEN: nativeMcpServer.token, - } - : {}), - ...(providerTraceCapture - ? { - PAPERCLIP_PROVIDER_TRACE_PATH: - providerTraceCapture.path, - PAPERCLIP_PROVIDER_TRACE_MAX_BYTES: String( - PROVIDER_TRACE_MAX_BYTES, - ), - } - : {}), - }, - runnerExecutionTarget: executionTarget, - runnerIngressAuthorized: isRunnerIngressAuthorized( - nativeRuntimeResolution, + onEvent: onAdapterEvent, + preparationSpans: nativeRunnerPreparationSpans, + // Bootstrap with executable/home discovery while keeping + // configured provider values and the server-selected + // workspace boundary authoritative. + managedGitHub: !useHostGitHub && githubSelection.configured, + managedAiCredentialIdentity: managedAiRuntime?.identity, + managedAiCredentialHome: managedAiRuntime ? String((managedAiRuntime.config.env as Record).CODEX_HOME) : undefined, + runnerEnvironment: { + ...buildNativeProviderEnvironment( + adapterEnv, + process.env, + executionWorkspace.cwd, ), - runnerPublicUrl: - runtimeEnv.PAPERCLIP_RUNNER_PUBLIC_URL?.trim() || null, - runnerCaBundlePath: - runtimeEnv.PAPERCLIP_RUNNER_CA_BUNDLE_PATH?.trim() || - null, - runnerRemoteBinaryPath: - runtimeEnv.PAPERCLIP_RUNNER_REMOTE_BINARY_PATH?.trim() || - null, - runnerRemoteCodexPath: - runtimeEnv.PAPERCLIP_RUNNER_REMOTE_CODEX_PATH?.trim() || - null, - runnerRemoteCodexNpmSpec: - runtimeEnv.PAPERCLIP_RUNNER_REMOTE_CODEX_NPM_SPEC?.trim() || - null, - runnerRemoteProviderPackPath: - runtimeEnv.PAPERCLIP_RUNNER_REMOTE_PROVIDER_PACK_PATH?.trim() || - null, - stopTaskForReassignment: async (target) => { - await settleLiveRunnerGoalBeforeInterrupt(db, target); - if (!target.runId) return; - const prior = await getRun(target.runId); - if (!prior || prior.companyId !== target.companyId || prior.agentId !== target.agentId) { - throw conflict("Reassignment run binding changed"); - } - const stopped = await cancelRunInternal(target.runId, "Cancelled for task reassignment", { - errorCode: "issue_reassigned", suppressImmediateRecovery: true, - resultJson: { reassignmentStopConfirmed: true }, - }); - if (stopped && ["running", "queued", "scheduled_retry"].includes(stopped.status)) { - throw conflict("The previous run did not stop; reassignment was not applied"); - } - }, - enqueueWakeup, - onSpawn: async (meta) => { - markDispatchStarted(); - await persistRunProcessMetadata(run.id, meta); - }, - }), - ); - if (!guardedDispatch.dispatched) return; - nativeDispatchStarted = true; - adapterResult = await guardedDispatch.resultPromise; - } finally { - await nativeGitHubBridge?.stop(); - } + ...(nativeMcpServer + ? { + PAPERCLIP_NATIVE_MCP_NAME: nativeMcpServer.name, + PAPERCLIP_NATIVE_MCP_URL: nativeMcpServer.url, + PAPERCLIP_NATIVE_MCP_TOKEN: nativeMcpServer.token, + } + : {}), + ...(providerTraceCapture + ? { + PAPERCLIP_PROVIDER_TRACE_PATH: + providerTraceCapture.path, + PAPERCLIP_PROVIDER_TRACE_MAX_BYTES: String( + PROVIDER_TRACE_MAX_BYTES, + ), + } + : {}), + }, + runnerExecutionTarget: executionTarget, + runnerIngressAuthorized: isRunnerIngressAuthorized( + nativeRuntimeResolution, + ), + runnerPublicUrl: + runtimeEnv.PAPERCLIP_RUNNER_PUBLIC_URL?.trim() || null, + runnerCaBundlePath: + runtimeEnv.PAPERCLIP_RUNNER_CA_BUNDLE_PATH?.trim() || + null, + runnerRemoteBinaryPath: + runtimeEnv.PAPERCLIP_RUNNER_REMOTE_BINARY_PATH?.trim() || + null, + runnerRemoteCodexPath: + runtimeEnv.PAPERCLIP_RUNNER_REMOTE_CODEX_PATH?.trim() || + null, + runnerRemoteCodexNpmSpec: + runtimeEnv.PAPERCLIP_RUNNER_REMOTE_CODEX_NPM_SPEC?.trim() || + null, + runnerRemoteProviderPackPath: + runtimeEnv.PAPERCLIP_RUNNER_REMOTE_PROVIDER_PACK_PATH?.trim() || + null, + stopTaskForReassignment: async (target) => { + await settleLiveRunnerGoalBeforeInterrupt(db, target); + if (!target.runId) return; + const prior = await getRun(target.runId); + if (!prior || prior.companyId !== target.companyId || prior.agentId !== target.agentId) { + throw conflict("Reassignment run binding changed"); + } + const stopped = await cancelRunInternal(target.runId, "Cancelled for task reassignment", { + errorCode: "issue_reassigned", suppressImmediateRecovery: true, + resultJson: { reassignmentStopConfirmed: true }, + }); + if (stopped && ["running", "queued", "scheduled_retry"].includes(stopped.status)) { + throw conflict("The previous run did not stop; reassignment was not applied"); + } + }, + enqueueWakeup, + onSpawn: async (meta) => { + markDispatchStarted(); + await persistRunProcessMetadata(run.id, meta); + }, + }), + ); + if (!guardedDispatch.dispatched) return; + nativeDispatchStarted = true; + adapterResult = await guardedDispatch.resultPromise; } else { const interactionId = readNonEmptyString(context.interactionId); const legacyQuestionResponse = diff --git a/server/src/services/native-runtime/native-github-access.test.ts b/server/src/services/native-runtime/native-github-access.test.ts new file mode 100644 index 00000000000..e6c1aaea539 --- /dev/null +++ b/server/src/services/native-runtime/native-github-access.test.ts @@ -0,0 +1,128 @@ +import { afterEach, expect, it, vi } from "vitest"; +import { spawn, execFile } from "node:child_process"; +import { mkdtemp, mkdir, readFile, rm, writeFile, access } from "node:fs/promises"; +import { promisify } from "node:util"; +import type { CommandManagedRuntimeRunner } from "@paperclipai/adapter-utils/command-managed-runtime"; +import { tmpdir } from "node:os"; +import path from "node:path"; +import { createInterface } from "node:readline"; +import { createNativeGitHubAccess, type NativeGitHubAccess } from "./native-github-access.js"; +const scope = { companyId: "company-a", agentId: "agent-a", issueId: "task-a" }; +const run = (runId: string) => ({ ...scope, runId }); +const cleanups: (() => Promise)[] = []; +afterEach(async () => { for (const cleanup of cleanups.splice(0).reverse()) await cleanup(); }); +async function broker(resolveCredentials = vi.fn(async (binding: ReturnType) => ({ + status: "available", env: { GH_TOKEN: `fixture-${binding.runId}` }, +})), remote = false, bridgeUnavailable = false, onLog?: (stream: "stdout" | "stderr", chunk: string) => Promise) { + const root = await mkdtemp(path.join(tmpdir(), "native-github-reuse-")); + cleanups.push(() => rm(root, { recursive: true, force: true })); + const bin = path.join(root, "real-bin"); + await mkdir(bin); + await writeFile(path.join(bin, "gh"), `#!${process.execPath}\nprocess.stdout.write(process.env.GH_TOKEN || 'anonymous');`, { mode: 0o700 }); + const execute: CommandManagedRuntimeRunner["execute"] = async (input) => { + const startedAt = new Date().toISOString(); + const child = promisify(execFile)(input.command, input.args ?? [], { + cwd: input.cwd ?? root, env: { ...process.env, ...input.env }, timeout: 15_000, maxBuffer: 4 * 1024 * 1024, + }); + child.child.stdin?.on("error", () => undefined); + child.child.stdin?.end(input.stdin ?? ""); + const result = await child; + return { ...result, exitCode: 0, signal: null, timedOut: false, pid: null, startedAt }; + }; + const target = remote ? { kind: "remote" as const, transport: "sandbox" as const, providerKey: "test", + remoteCwd: root, runner: { execute }, streamRunLogs: false } : null; + const result = await createNativeGitHubAccess({ scope, target, cwd: root, + env: { PATH: `${bin}:${path.dirname(process.execPath)}:/usr/bin:/bin`, PAPERCLIP_API_KEY: "old-run-api-key" }, resolveCredentials, onLog }, + bridgeUnavailable ? async () => { throw new Error("fixture bridge unavailable"); } : undefined); + cleanups.push(result.stop); + return { ...result, root, resolveCredentials }; +} +function request(broker: NativeGitHubAccess, extra: RequestInit = {}, endpoint = "/runtime-tools/github/credentials") { + return fetch(broker.env.PAPERCLIP_GITHUB_BROKER_URL + endpoint, { + method: "POST", body: "{}", headers: { authorization: `Bearer ${broker.env.PAPERCLIP_GITHUB_BRIDGE_TOKEN}`, "content-type": "application/json" }, ...extra, + }); +} +it.each([false, true])("keeps one live parent and its original launcher environment across two authorized runs (callback bridge: %s)", async (remote) => { + const b = await broker(undefined, remote); + const parent = spawn(process.execPath, ["-e", ` + const {execFile}=require('node:child_process'); + require('node:readline').createInterface({input:process.stdin}).on('line', () => { + execFile('gh', [], (error, stdout, stderr) => console.log(JSON.stringify({pid:process.pid, value:stdout, error:error?.message, stderr}))); + }); + `], { env: { ...process.env, ...b.env, PAPERCLIP_API_KEY: "stale-api-key" }, stdio: ["pipe", "pipe", "pipe"] }); + cleanups.push(async () => { const exited = new Promise(resolve => parent.once("exit", () => resolve())); parent.kill(); await exited; }); + const lines = createInterface({ input: parent.stdout }); + const operation = () => new Promise<{ pid: number; value: string; stderr: string }>(resolve => { + lines.once("line", line => resolve(JSON.parse(line))); parent.stdin.write("run\n"); + }); + expect((await request(b)).status).toBe(403); + const releaseA = b.activate(run("run-a")); + const a = await operation(); + expect(a.value).toBe("fixture-run-a"); + releaseA(); + expect((await operation()).value).toBe("anonymous"); + const releaseB = b.activate(run("run-b")); + releaseA(); // A's delayed cleanup cannot revoke B. + const second = await operation(); + expect(second.value).toBe("fixture-run-b"); + expect(second.pid).toBe(a.pid); + expect(b.resolveCredentials.mock.calls.map(([binding]) => binding.runId)).toEqual(["run-a", "run-b"]); + expect(await readFile(path.join(b.env.PAPERCLIP_GITHUB_LAUNCHER_DIR, "gh"), "utf8")).not.toContain(b.env.PAPERCLIP_GITHUB_BROKER_TOKEN); + releaseB(); await b.stop(); + await expect(access(b.env.PAPERCLIP_GITHUB_LAUNCHER_DIR)).rejects.toThrow(); +}, 45_000); +it("rejects other scopes, concurrent bindings, browser requests and forged authority", async () => { + const b = await broker(); + for (const field of ["companyId", "agentId", "issueId"] as const) { + expect(() => b.activate({ ...run("a"), [field]: "other" })).toThrow("scope mismatch"); + } + const release = b.activate(run("a")); + expect(() => b.activate(run("b"))).toThrow("busy"); + const rejectedHeaders: Record[] = [{ authorization: "Bearer wrong" }, { authorization: "é".repeat(71) }, + { authorization: `Bearer ${b.env.PAPERCLIP_GITHUB_BRIDGE_TOKEN}`, origin: "https://browser.test" }]; + for (const headers of rejectedHeaders) { + expect((await request(b, { headers })).status).toBe(403); + } + expect((await request(b, {}, "/api/companies")).status).toBe(404); + expect((await request(b, { method: "GET", body: undefined })).status).toBe(404); + const response = await request(b, { body: JSON.stringify({ runId: "b", responsibleUserId: "other" }) }); + expect(response.status).toBe(200); + expect(response.headers.get("cache-control")).toBe("no-store"); + expect(b.resolveCredentials).toHaveBeenCalledExactlyOnceWith(run("a")); + release(); await b.stop(); + expect(() => b.activate(run("b"))).toThrow("closed"); +}); +it("does not release an in-flight credential after its run has been replaced", async () => { + let complete!: (value: { status: string; env: { GH_TOKEN: string } }) => void; + const resolver = vi.fn(() => new Promise<{ status: string; env: { GH_TOKEN: string } }>(resolve => { complete = resolve; })); + const b = await broker(resolver); + const release = b.activate(run("a")); + const pending = request(b); + await vi.waitFor(() => expect(resolver).toHaveBeenCalledOnce()); + release(); b.activate(run("b")); + complete({ status: "available", env: { GH_TOKEN: "must-not-escape" } }); + const response = await pending; + expect(response.status).toBe(403); + expect(await response.text()).not.toContain("must-not-escape"); +}); +it.each([403, 409, 500])("preserves denial/steering without leaking errors (%s)", async (status) => { + const b = await broker(vi.fn(async () => { throw Object.assign(new Error("secret-value"), { status }); })); + b.activate(run("a")); + const response = await request(b); + expect(response.status).toBe(status === 500 ? 503 : status); + expect(await response.text()).not.toContain("secret-value"); +}); + +it.each([false, true])("keeps anonymous launchers usable when the remote broker cannot start (logging fails: %s)", async (loggingFails) => { + const onLog = vi.fn(async () => { if (loggingFails) throw new Error("fixture log sink unavailable"); }); + const b = await broker(undefined, true, true, onLog); + expect(onLog).toHaveBeenCalledWith("stderr", expect.stringContaining("continuing without managed GitHub access")); + b.activate(run("a")); + expect(b.ready).toBe(false); + expect(b.env.PAPERCLIP_GITHUB_BROKER_TOKEN).toBe(""); + const result = await promisify(execFile)(path.join(b.env.PAPERCLIP_GITHUB_LAUNCHER_DIR, "gh"), [], { + env: { ...process.env, ...b.env, GH_TOKEN: "ambient-must-not-leak" }, + }); + expect(result.stdout).toBe("anonymous"); + expect(b.resolveCredentials).not.toHaveBeenCalled(); +}); diff --git a/server/src/services/native-runtime/native-github-access.ts b/server/src/services/native-runtime/native-github-access.ts new file mode 100644 index 00000000000..f0245291f41 --- /dev/null +++ b/server/src/services/native-runtime/native-github-access.ts @@ -0,0 +1,146 @@ +import { randomBytes, randomUUID, timingSafeEqual } from "node:crypto"; +import { createServer } from "node:http"; +import path from "node:path"; +import { + cleanupGitHubOperationLaunchers, + prepareGitHubOperationLaunchers, + startAdapterExecutionTargetPaperclipBridge, +} from "@paperclipai/adapter-utils/execution-target"; +import { githubBrokerEnvironment } from "@paperclipai/adapter-utils/github-launcher"; + +type Binding = { companyId: string; agentId: string; issueId: string; runId: string }; +type LauncherInput = Parameters[0]; +export type NativeGitHubAccess = Awaited>; + +/** A process-lifetime transport, with authority only while its controller owns a run. + * No run token or GitHub credential is stored in the provider's environment/files. + * The resolver still checks the active run and current identity/grants per operation. + */ +export async function createNativeGitHubAccess(input: { + scope: Omit; + target: LauncherInput["target"]; + cwd: string; + env: NodeJS.ProcessEnv; + resolveCredentials: (binding: Binding) => Promise; + onLog?: (stream: "stdout" | "stderr", chunk: string) => Promise; +}, startBridge = startAdapterExecutionTargetPaperclipBridge) { + const token = randomBytes(32).toString("hex"); + const location = { runId: `native-session-${randomUUID()}`, target: input.target }; + let active: Binding | null = null; + let stopped = false; + let ready = true; + let stopping: Promise | undefined; + let bridge: Awaited> = null; + const server = createServer(async (req, res) => { + res.setHeader("Cache-Control", "no-store"); + res.setHeader("Content-Type", "application/json"); + req.resume(); + const reply = (status: number, body: unknown) => { + res.writeHead(status); + res.end(JSON.stringify(body)); + }; + const bearer = req.headers.authorization ?? ""; + const expected = `Bearer ${token}`; + if (req.headers.origin || req.headers.cookie || req.headers["sec-fetch-site"] || + Buffer.byteLength(bearer) !== Buffer.byteLength(expected) || + !timingSafeEqual(Buffer.from(bearer), Buffer.from(expected))) { + reply(403, { error: "GitHub session authentication required" }); + return; + } + if (req.method !== "POST" || req.url !== "/runtime-tools/github/credentials") { + reply(404, { error: "Unknown GitHub session operation" }); + return; + } + const binding = active; + if (!binding || stopped) { + reply(403, { error: "No active GitHub run" }); + return; + } + try { + // Bind at receipt; body/headers cannot select a run or responsible user. + const result = await input.resolveCredentials({ ...binding }); + if (active !== binding || stopped) { + reply(403, { error: "GitHub run ended during credential acquisition" }); + return; + } + reply(200, result); + } catch (error) { + const status = (error as { status?: number })?.status; + // Never leak secret-store/provider errors. Preserve steering's retry signal. + reply(status === 409 ? 409 : status === 403 ? 403 : 503, + { error: "GitHub credentials unavailable" }); + } + }); + server.requestTimeout = 15_000; + server.headersTimeout = 10_000; + const stop = () => stopping ??= (async () => { + stopped = true; + active = null; + server.closeAllConnections(); + const results = await Promise.allSettled([ + bridge?.stop(), + new Promise(resolve => server.close(() => resolve())), + cleanupGitHubOperationLaunchers(location), + ]); + if (results.some(result => result.status === "rejected")) { + await input.onLog?.("stderr", "[paperclip] GitHub session cleanup incomplete.\n").catch(() => undefined); + } + })(); + try { + await new Promise((resolve, reject) => { + server.once("error", reject); + server.listen(0, "127.0.0.1", () => { server.off("error", reject); resolve(); }); + }); + const address = server.address(); + if (!address || typeof address === "string") throw new Error("GitHub broker did not listen"); + const url = `http://127.0.0.1:${address.port}`; + try { + bridge = await startBridge({ + ...location, + runtimeRootDir: input.target?.kind === "remote" + ? path.posix.join(input.target.remoteCwd, ".paperclip-runtime", "github", location.runId) + : null, + adapterKey: "native-github", + hostApiToken: token, + hostApiUrl: url, + onLog: input.onLog, + }); + if (input.target?.kind === "remote" && !bridge) { + throw new Error("GitHub session requires a remote callback bridge"); + } + } catch { + // GitHub remains optional. Stage anonymous wrappers for this session; + // the supervisor retries transport setup on the next run. + ready = false; + await new Promise(resolve => server.close(() => resolve())); + await input.onLog?.("stderr", "[paperclip] GitHub runtime transport unavailable; continuing without managed GitHub access.\n").catch(() => undefined); + } + const env = await prepareGitHubOperationLaunchers({ + ...location, cwd: input.cwd, + env: { + ...githubBrokerEnvironment({ PATH: input.env.PATH }, { + url: ready ? bridge?.env.PAPERCLIP_API_URL ?? url : "", + token: ready ? bridge?.env.PAPERCLIP_API_KEY ?? token : "", + }), + // Never retain an old run's bridge authentication override. + PAPERCLIP_GITHUB_BRIDGE_TOKEN: ready ? bridge?.env.PAPERCLIP_API_KEY ?? token : "", + }, + }); + return { + env, + ready, + activate(binding: Binding) { + if (stopped || active) throw new Error("GitHub session is closed or busy"); + if (binding.companyId !== input.scope.companyId || binding.agentId !== input.scope.agentId || + binding.issueId !== input.scope.issueId) throw new Error("GitHub session scope mismatch"); + const owner = { ...binding }; + active = owner; + return () => { if (active === owner) active = null; }; + }, + stop, + }; + } catch (error) { + await stop().catch(() => undefined); + throw error; + } +} diff --git a/server/src/services/native-runtime/native-session-executor.test.ts b/server/src/services/native-runtime/native-session-executor.test.ts index cab3d6de20d..a6e79f988f1 100644 --- a/server/src/services/native-runtime/native-session-executor.test.ts +++ b/server/src/services/native-runtime/native-session-executor.test.ts @@ -53,6 +53,15 @@ import { buildNativeHeartbeatPreparationSpans } from "./native-run-trace.js"; import { NativeRunnerOwnershipUnverifiedError } from "./native-runner-ownership.js"; import type { AdapterRuntimeEvent } from "../../adapters/index.js"; +const githubAccess = vi.hoisted(() => ({ + activate: vi.fn((_binding: { runId: string }) => vi.fn()), + stop: vi.fn(async () => undefined), + create: vi.fn(), +})); +vi.mock("./native-github-access.js", () => ({ + createNativeGitHubAccess: githubAccess.create, +})); + type BackendFactoryOptions = { runnerInstanceId?: string; acpxRuntimeDirectory?: string; @@ -6150,6 +6159,10 @@ describe("native warm session supervision", () => { it.each( [ + ...[false, true].flatMap((local) => [true, false].map(brokerReady => ({ + firstBroker: true, secondBroker: true, projectless: false, local, brokerReady, + firstMode: "managed", secondMode: "managed", managedGitHub: true, + }))), ...[ { firstBroker: false, secondBroker: false }, { firstBroker: false, secondBroker: true }, @@ -6213,10 +6226,12 @@ describe("native warm session supervision", () => { firstNetwork: "disabled", secondNetwork: "disabled", checkpointContract: "valid", + managedGitHub: false, + brokerReady: true, ...scenario, })), )( - "verifies a live warm owner before refreshing run authority (broker: $firstBroker -> $secondBroker, projectless: $projectless, local: $local, auth: $firstMode -> $secondMode, network: $firstNetwork -> $secondNetwork, checkpoint: $checkpointContract)", + "verifies a live warm owner before refreshing run authority (broker: $firstBroker -> $secondBroker, projectless: $projectless, local: $local, auth: $firstMode -> $secondMode, network: $firstNetwork -> $secondNetwork, checkpoint: $checkpointContract, managed access: $managedGitHub, broker ready: $brokerReady)", async ({ firstBroker, secondBroker, @@ -6227,10 +6242,18 @@ describe("native warm session supervision", () => { firstNetwork, secondNetwork, checkpointContract, + managedGitHub, + brokerReady, }) => { + githubAccess.create.mockReset().mockResolvedValue({ + env: { PAPERCLIP_GITHUB_BROKER_TOKEN: "stable-session-capability" }, + ready: brokerReady, + activate: githubAccess.activate.mockReset().mockImplementation(() => vi.fn()), + stop: githubAccess.stop.mockReset().mockResolvedValue(undefined), + }); const replacesProvider = - firstBroker || - secondBroker || + !brokerReady || + (!managedGitHub && (firstBroker || secondBroker)) || firstMode !== secondMode || firstNetwork !== secondNetwork; const stateBase = await mkdtemp( @@ -6337,7 +6360,7 @@ describe("native warm session supervision", () => { // the undefined value when a live owner is reused without a load. expect(options.persistedSession).toBeNull(); } - } else { + } else if (!managedGitHub) { expect(options.existingSession).toBe(firstSession); expect(options.persistedSession).toBeUndefined(); } @@ -6357,6 +6380,7 @@ describe("native warm session supervision", () => { }, runnerInstanceId: "runner-runnerd-warm", useRunnerd: true, + managedGitHub, runnerExecutionTarget: remoteTarget, }); if (projectless && replacesProvider) { @@ -6451,8 +6475,21 @@ describe("native warm session supervision", () => { }, runnerInstanceId: "runner-runnerd-warm", useRunnerd: true, + managedGitHub, runnerExecutionTarget: remoteTarget, }); + if (managedGitHub) { + expect(state.execute.mock.calls[1]?.[0].existingSession).toBe(brokerReady ? firstSession : undefined); + expect(githubAccess.create).toHaveBeenCalledTimes(brokerReady ? 1 : 2); + expect(githubAccess.activate.mock.calls.map(([binding]) => binding.runId)) + .toEqual([first.binding.runId, second.binding.runId]); + // Replacement fixture publishes no new provider handle: both the old + // owner and the unclaimed replacement broker must be cleaned up. + expect(githubAccess.stop).toHaveBeenCalledTimes(brokerReady ? 0 : 2); + for (const activation of githubAccess.activate.mock.results) { + expect(activation.value).toHaveBeenCalledOnce(); + } + } if (replacesProvider) { expect(firstClose).toHaveBeenCalledOnce(); expect(firstClose).toHaveBeenCalledWith({ @@ -6469,6 +6506,7 @@ describe("native warm session supervision", () => { timeout: 1_500, }, ); + if (managedGitHub) expect(githubAccess.stop).toHaveBeenCalledOnce(); } } finally { if (previousStateDirectory === undefined) { diff --git a/server/src/services/native-runtime/native-session-executor.ts b/server/src/services/native-runtime/native-session-executor.ts index 41b0262134a..5f350aaa7bc 100644 --- a/server/src/services/native-runtime/native-session-executor.ts +++ b/server/src/services/native-runtime/native-session-executor.ts @@ -1,3 +1,5 @@ +import { createNativeGitHubAccess, type NativeGitHubAccess } from "./native-github-access.js"; +import { resolveGitHubOperationCredentials } from "../github-operation-credentials.js"; import { bindManagedNativeCredentialTurn, completeManagedNativeCredentialTurn } from "./managed-native-credentials.js"; import { createLocalNativeQuestionBridge } from "./local-native-question-bridge.js"; import { readVerifiedRemoteWorkspaceFile } from "./remote-deliverable-file.js"; @@ -376,6 +378,7 @@ function clearNativeRuntimeRequestResolutions(runId: string): void { type WarmNativeSession = { managedAiCredentialIdentity?: string; credentialRunId?: string; + githubAccess?: NativeGitHubAccess; githubAuthenticationMode?: string; networkAccess: boolean; session: NativeSession; @@ -389,6 +392,13 @@ type WarmNativeSession = { lastActivityAt: string; }; +async function closeWarmNativeSession(entry: WarmNativeSession, reason: string) { + // Revoke before awaiting process retirement/checkpoint IO. + const stopping = entry.githubAccess?.stop(); + try { await entry.session.close({ reason }); } + finally { await stopping; } +} + const warmNativeSessions = new Map(); // Closing a remote owner saves its checkpoint asynchronously. A new turn must // not inspect or quarantine that owner's state until the save has finished. @@ -444,7 +454,7 @@ async function closeIdleWarmNativeSessions(input: { // adopt a session whose transport is already shutting down. warmNativeSessions.delete(sessionId); const closing = Promise.resolve().then(() => - entry.session.close({ reason: input.reason }), + closeWarmNativeSession(entry, input.reason), ); closingWarmNativeSessions.set(sessionId, closing); try { @@ -5825,9 +5835,8 @@ async function releaseWarmNativeSession( if (entry.idleTimer !== null) clearTimeout(entry.idleTimer); if (failed || entry.closeOnReleaseReason !== undefined) { warmNativeSessions.delete(sessionId); - const closing = entry.session.close({ - reason: entry.closeOnReleaseReason ?? "warm native session failed", - }); + const closing = closeWarmNativeSession(entry, + entry.closeOnReleaseReason ?? "warm native session failed"); // Restart checkpointing is required to restore this successful session. // Surface failure instead of reporting a clean release without authority. if (entry.closeOnReleaseReason !== undefined) await closing; @@ -5840,8 +5849,7 @@ async function releaseWarmNativeSession( // is the idle timer's ownership fence across a later warm acquisition. if (current !== entry || current.busy) return; warmNativeSessions.delete(sessionId); - void current.session - .close({ reason: "warm native session idle timeout" }) + void closeWarmNativeSession(current, "warm native session idle timeout") .catch(() => undefined); }, idleTimeoutMs); entry.idleTimer.unref(); @@ -7063,6 +7071,8 @@ export async function executePaperclipNativeSession(input: { sessionGoalControl?: NativeSessionGoalControl | null; resumeSessionGoalHeartbeat?: boolean; preparationSpans?: NativeRunHistoricalSpan[]; + /** Use a session-owned GitHub broker, rebound only after run ownership is acquired. */ + managedGitHub?: boolean; /** Resolved adapter env; the runner transport applies a provider allowlist before spawn. */ runnerEnvironment?: NodeJS.ProcessEnv; /** Private grant materialization; never a user-configured host path. */ @@ -7913,14 +7923,16 @@ async function executePaperclipNativeSessionWithinScope( ); let existingWarmSession: NativeSession | undefined; let managedCredentialSession: NativeSession | undefined; + let githubAccess: NativeGitHubAccess | undefined; + let releaseGitHubRun: (() => void) | undefined; let persistedWarmSession: PersistedNativeSession | null | undefined; if (warmSessionId !== null && warmConfigDigest !== null) { const entry = warmNativeSessions.get(warmSessionId); if (entry) { - // Run-scoped broker capabilities must rotate with the process, while the - // settled provider checkpoint retains the conversation across runs. + // Old run-scoped environments still require process replacement. A + // session-owned broker can change run authority without replacing it. const hasBrokerCapability = Boolean( - input.runnerEnvironment?.PAPERCLIP_GITHUB_BROKER_TOKEN, + !input.managedGitHub && input.runnerEnvironment?.PAPERCLIP_GITHUB_BROKER_TOKEN, ); const credentialRunChanged = Boolean(entry.credentialRunId) !== hasBrokerCapability || @@ -7930,6 +7942,8 @@ async function executePaperclipNativeSessionWithinScope( entry.configDigest !== warmConfigDigest || entry.managedAiCredentialIdentity !== input.managedAiCredentialIdentity || credentialRunChanged || + Boolean(entry.githubAccess) !== Boolean(input.managedGitHub) || + entry.githubAccess?.ready === false || entry.githubAuthenticationMode !== input.runnerEnvironment?.PAPERCLIP_GITHUB_AUTH_MODE || entry.networkAccess !== @@ -7939,9 +7953,7 @@ async function executePaperclipNativeSessionWithinScope( if (entry.busy) throw new Error("native_session_supervisor_busy"); if (entry.idleTimer !== null) clearTimeout(entry.idleTimer); warmNativeSessions.delete(warmSessionId); - await entry.session.close({ - reason: "warm native session configuration changed", - }); + await closeWarmNativeSession(entry, "warm native session configuration changed"); persistedWarmSession = loadWarmNativeCheckpoint( input.execution, warmConfigDigest, @@ -7956,6 +7968,7 @@ async function executePaperclipNativeSessionWithinScope( if (entry.idleTimer !== null) clearTimeout(entry.idleTimer); entry.idleTimer = null; existingWarmSession = entry.session; + githubAccess = entry.githubAccess; } } else { persistedWarmSession = loadWarmNativeCheckpoint( @@ -8068,6 +8081,18 @@ async function executePaperclipNativeSessionWithinScope( controller, }); try { + if (input.managedGitHub) { + githubAccess ??= await createNativeGitHubAccess({ + scope: input.execution.binding, + target: input.runnerExecutionTarget, + cwd: input.execution.workspace.cwd, + env: input.runnerEnvironment ?? process.env, + resolveCredentials: (binding) => resolveGitHubOperationCredentials(input.db, binding), + onLog: input.onLog, + }); + releaseGitHubRun = githubAccess.activate(input.execution.binding); + input = { ...input, runnerEnvironment: { ...input.runnerEnvironment, ...githubAccess.env } }; + } const expectedCurrentWakeComments = await resolveCurrentWakeCommentsBinding( input.db, input.execution.binding, @@ -8269,7 +8294,8 @@ async function executePaperclipNativeSessionWithinScope( networkAccess: input.runnerEnvironment ?.PAPERCLIP_RUNNER_NETWORK_ACCESS === "enabled", - credentialRunId: input.runnerEnvironment + githubAccess, + credentialRunId: !input.managedGitHub && input.runnerEnvironment ?.PAPERCLIP_GITHUB_BROKER_TOKEN ? input.execution.binding.runId : undefined, @@ -8823,6 +8849,13 @@ async function executePaperclipNativeSessionWithinScope( if (ownershipUnverified) throw new NativeRunnerOwnershipUnverifiedError(); if (protocolIntegrityFailure !== null) throw protocolIntegrityFailure; } + } finally { + releaseGitHubRun?.(); + // A startup failure or onSession(null) must not leak a transport. A warm + // owner retains only the inactive broker until its normal retirement. + if (githubAccess && (!warmSessionId || warmNativeSessions.get(warmSessionId)?.githubAccess !== githubAccess)) { + await githubAccess.stop(); + } } if ( planSynchronizations.length === 0 &&