mirror of
https://github.com/paperclipai/paperclip.git
synced 2026-10-21 16:27:18 +02:00
## Thinking Path > - Paperclip manages AI agents and their work. > - The native Runner keeps a live provider process between task turns. > - Managed GitHub access used a token tied to one run. > - A new run forced Paperclip to replace that process to replace its token. > - This PR gives the session a stable credential transport and binds each operation to the active run. > - The agent can keep its process while Paperclip checks current identity and grants. ## Linked Issues or Issue Description Follow-up to #13738. Related credential-rotation work: #11770 and #8208 use process replacement for other adapter credentials; this change applies to managed GitHub access in the native Runner. **What happened?** A configured GitHub connection forced a warm native provider process to close at each new run. The saved conversation survived, but the live process did not. **Expected behavior** Keep the warm provider process. Resolve GitHub access for the current run when each command starts. Deny access while idle or after the run ends. **Steps to reproduce** 1. Configure managed GitHub access for a native Runner agent with a warm session. 2. Complete a turn, then send another message to the same task. 3. Observe the provider process close with the reason `warm native session configuration changed`. **Paperclip version or commit** Reproduced on master `8326e33ad`. Rebased onto `e3d8fb087` before submission. **Deployment mode** Local and remote native execution, including the sandbox callback bridge. ## What Changed - Move configured native GitHub transport and launcher ownership from the run to the provider session. - Bind the broker only after the executor acquires session ownership. Clear that binding when the run exits. - Keep the shared live-run, identity, grant, and trust-policy checks for each credential request. - Reject wrong scopes, idle requests, and credential responses that arrive after their run binding changes. - Retire transport and launcher files with the provider session. Keep anonymous commands available if bridge startup fails. - Add red/green executor tests, real subprocess and callback-bridge tests, and database checks. Update the runtime documentation. ## Verification - Before the fix, both new local and remote warm-session reuse tests failed. - After the fix, 435 targeted tests passed across the executor, broker, launcher, token, and database suites. - A real long-lived test process kept the same PID and original environment across two runs, including through the production callback bridge on local test processes. - Server typecheck and TypeScript compilation passed. - Full workspace typecheck and build passed. Server typecheck passed again after the review fix. - The fallback-logging regression failed before the fix; all 9 broker tests pass afterward. - The exact chat sidebar browser scenario passed locally. The initial CI timeout showed failed Vite module downloads; all eight browser shards pass on the latest commit. - All 53 latest-head checks passed, including the full CI test matrix and security checks (two unrelated conditional checks skipped). - The duplicate full local test run was stopped after CI passed; it is not claimed as a completed local pass. Targeted local tests, workspace typecheck/build, and the browser scenario passed. - Greptile reviewed the latest commit at 5/5 with no unresolved findings. - No fresh paid provider or Daytona campaign has run for this change. ## Risks - The broker now lives as long as the provider session. Tests cover idle denial, late cleanup, late responses, shutdown, and failed startup. - Its in-memory authority does not survive a controller restart. Existing checkpoint and process-recovery rules still apply. - Raw GitHub credentials remain confined to individual command processes. The session transport token cannot select a different task, agent, company, or run. - No database migration or public API change. ## Model Used OpenAI Codex, GPT-6, with reasoning, terminal tools, and code execution. The exact serving model ID and context-window size are 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 <noreply@paperclip.ing>
129 lines
7.8 KiB
TypeScript
129 lines
7.8 KiB
TypeScript
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<unknown>)[] = [];
|
|
afterEach(async () => { for (const cleanup of cleanups.splice(0).reverse()) await cleanup(); });
|
|
async function broker(resolveCredentials = vi.fn(async (binding: ReturnType<typeof run>) => ({
|
|
status: "available", env: { GH_TOKEN: `fixture-${binding.runId}` },
|
|
})), remote = false, bridgeUnavailable = false, onLog?: (stream: "stdout" | "stderr", chunk: string) => Promise<void>) {
|
|
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<void>(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<string, string>[] = [{ 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();
|
|
});
|