mirror of
https://github.com/paperclipai/paperclip.git
synced 2026-10-24 18:19:05 +02:00
## Thinking Path > - Paperclip manages AI agents and their authorized connections. > - Native OpenCode agents use managed Git and GitHub launchers for shell work. > - Each operation must obtain credentials from the active run's broker. > - Two OpenCode environment filters dropped that broker binding before the shell started. > - The shell therefore ran anonymously even when connector tools had authorized access. > - This pull request preserves the explicit broker environment through both filters. > - The benefit is working GitHub shell access with current identity and revocation checks. ## Linked Issues or Issue Description **What happened?** Native OpenCode shell commands reported `GitHub capability_missing` despite an authorized GitHub connection. Both runner startup and provider startup dropped the controller's broker settings. Connector MCP calls used a separate path and could still succeed. **Expected behavior** A managed shell operation obtains credentials for the current run. A later operation uses the new identity after rotation and loses access after revocation. Ambient host credentials do not supply authority. **Steps to reproduce** 1. Start the native GitHub broker with an active run binding. 2. Pass its environment through native OpenCode startup. 3. Run `gh api user` from the provider shell. 4. Observe anonymous access before this fix. **Paperclip version or commit** Reproduced on master `6943904bad` before this change. **Deployment mode** Native Paperclip Runner with OpenCode, including remote execution. Related: #16401 fixes the separate oversized OpenCode SSE failure and is already merged. The same live-driver regression was verified locally against its exact pre-fix source and current source. This PR does not duplicate that fix. Searches found no open PR for this native broker propagation defect; #15309 concerns the legacy OpenCode adapter. ## What Changed - Use the existing GitHub credential environment projection at both OpenCode launch boundaries. - Preserve explicit controller authority while excluding an ambient host broker. - Exercise the real launcher and broker from an OpenCode child process, directly and through runner projection. - Check operation-time identity rotation, revocation, and exclusion of controller API credentials and persistent GitHub tokens. - Document the managed OpenCode shell contract. ## Verification - Red: both new `native-github-access.test.ts` cases returned `anonymous` instead of `fixture-a` before production changes. - Green: both cases pass after the fix, including `fixture-a` to `fixture-b` rotation and anonymous operation after revocation. - Full native GitHub broker suite: 18 tests pass. - OpenCode, provider-event, and configured-environment suites: 141 tests pass across eight files. - Separate SSE red/green verification: the existing live-driver regression times out on `fd057f6ac4^` and passes with merged #16401. The full OpenCode group also passes after restoration. - Full repository build and typecheck pass locally. All CI jobs pass for `23d734293f6a5c6d6128cae0c8773661816daf9a`. Final GitHub rollup: 55 successful checks/statuses, two skipped checks, and one neutral check; none pending or failing. Greptile gave that commit 5/5 with no findings or unresolved threads. - Paperclip Review spent over 23 minutes preparing its runtime without a provider assessment. Its run was cancelled for recovery and ended neutral (review execution did not complete). This neutral result is not a code assessment. No check policy or branch protection was changed. - The full local Vitest attempt first hit embedded PostgreSQL startup failures. Running the unchanged `initdb --version` and `postgres --version` independently showed missing native library symlinks. The dependency had been installed with scripts disabled. Running its bundled `hydrate-symlinks.js` restored startup; all 15 affected native-session cases and all 112 AI connection tests then passed without application code changes. The original full-suite attempt was stopped after the setup failure and its log is retained. A complete local suite pass is not claimed. - The broker and executable are local fixtures. No live GitHub account, credential change, or test PR is required by these regressions. ## Risks The change affects credential delivery into OpenCode processes. It reuses the existing bounded GitHub projection and accepts broker authority only from explicit input. Each shell operation still uses the normal broker, scope, identity, and revocation rules. Unrelated controller secrets remain excluded. There are no database or permission-policy changes. ## Model Used OpenAI `gpt-6.1-sol`, through Codex, with xhigh reasoning and repository tools/code execution. The session does not report its context-window size. ## 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 the focused regression and affected suites locally and they pass; the full local attempt and setup failure are disclosed above - [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 (Paperclip Review is neutral/incomplete, as disclosed above) - [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>
261 lines
15 KiB
TypeScript
261 lines
15 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 { CommandManagedDuplexChannel, CommandManagedRuntimeRunner } from "@paperclipai/adapter-utils/command-managed-runtime";
|
|
import { startAdapterExecutionTargetPaperclipBridge } from "@paperclipai/adapter-utils/execution-target";
|
|
import { tmpdir } from "node:os";
|
|
import path from "node:path";
|
|
import { createInterface } from "node:readline";
|
|
import { createNativeGitHubAccess, type NativeGitHubAccess } from "./native-github-access.js";
|
|
import { OpenCodeServerDriver } from "@paperclipai/paperclip-runner";
|
|
import { createCapabilityRunnerdProviderEnvironment } from "@paperclipai/paperclip-runner/live";
|
|
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(); });
|
|
function localDuplexChannel(command: readonly string[], cwd: string): CommandManagedDuplexChannel {
|
|
const child = spawn(command[0]!, command.slice(1), { cwd, stdio: ["pipe", "pipe", "pipe"] });
|
|
const buffered: Buffer[] = [];
|
|
let dataListener: ((chunk: Uint8Array) => void) | undefined;
|
|
let exitListener: ((exit: { exitCode: number | null }) => void) | undefined;
|
|
let exit: { exitCode: number | null } | undefined;
|
|
child.stdin.on("error", () => undefined);
|
|
child.stderr.resume();
|
|
child.stdout.on("data", (chunk: Buffer) => dataListener ? dataListener(chunk) : buffered.push(chunk));
|
|
const closed = new Promise<void>(resolve => child.once("close", code => {
|
|
exit = { exitCode: code };
|
|
exitListener?.(exit);
|
|
resolve();
|
|
}));
|
|
return {
|
|
write: data => { child.stdin.write(data); },
|
|
onData: listener => { dataListener = listener; for (const chunk of buffered.splice(0)) listener(chunk); },
|
|
onExit: listener => { exitListener = listener; if (exit) listener(exit); },
|
|
stop: () => { child.kill(); },
|
|
close: async () => { child.kill(); await closed; },
|
|
};
|
|
}
|
|
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>,
|
|
duplex?: { enabled?: boolean; capability: boolean; unavailable?: boolean }) {
|
|
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 = vi.fn<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 openDuplexChannel = vi.fn<NonNullable<CommandManagedRuntimeRunner["openDuplexChannel"]>>(async input => {
|
|
if (duplex?.unavailable) throw new Error("fixture channel unavailable");
|
|
return localDuplexChannel(input.command, root);
|
|
});
|
|
const target = remote ? { kind: "remote" as const, transport: "sandbox" as const, providerKey: "test",
|
|
remoteCwd: root, runner: { execute, ...(duplex ? { openDuplexChannel } : {}) }, streamRunLogs: false,
|
|
...(duplex ? { enableSandboxDuplexBridge: duplex.enabled, effectiveCapabilities: {
|
|
reusableLeases: true, nativeSyncIn: false, nativeSyncOut: false, persistentProcessSessions: false,
|
|
independentControlCommands: true, incrementalSessionOutput: false, concurrentSyncOperations: false,
|
|
duplexCommandStream: duplex.capability, runnerWebSocketIngress: false,
|
|
} } : {}) } : null;
|
|
const bridgeModes: (string | undefined)[] = [];
|
|
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"); } : async input => {
|
|
const bridge = await startAdapterExecutionTargetPaperclipBridge(input);
|
|
bridgeModes.push(bridge?.env.PAPERCLIP_API_BRIDGE_MODE);
|
|
return bridge;
|
|
});
|
|
cleanups.push(result.stop);
|
|
return Object.assign(result, { root, resolveCredentials, execute, openDuplexChannel, bridgeModes });
|
|
}
|
|
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])("resolves managed GitHub credentials from OpenCode shell operations (runner projection: %s)", async throughRunner => {
|
|
const b = await broker();
|
|
const command = path.join(b.root, "fake-opencode-server.mjs");
|
|
const fixture = new URL("../../../../packages/paperclip-runner/test/fixtures/fake-opencode-server.mjs", import.meta.url);
|
|
await writeFile(command, await readFile(fixture), { mode: 0o700 });
|
|
const source = { ...b.env, OPENROUTER_API_KEY: "fixture-provider-key", PAPERCLIP_API_KEY: "controller-must-not-cross" };
|
|
const environment = throughRunner ? createCapabilityRunnerdProviderEnvironment({
|
|
provider: "opencode",
|
|
options: { environment: source, stateDirectory: b.root },
|
|
identity: { runnerInstanceId: "runner-a", environmentLeaseId: "lease-a", runId: "a", normalizedSessionId: "github", turnId: "turn-a", itemId: "item-a" },
|
|
codexHome: path.join(b.root, "codex-home"),
|
|
runtimeContextPath: path.join(b.root, "context.json"),
|
|
hasRuntimeContext: false,
|
|
}) : source;
|
|
const driver = new OpenCodeServerDriver({
|
|
model: "openrouter/z-ai/glm-5.3", runtimeDirectory: b.root, command, environment,
|
|
});
|
|
const session = await driver.openSession({ runId: "a", normalizedSessionId: "github", workingDirectory: b.root });
|
|
cleanups.push(() => session.close({ reason: "test complete" }));
|
|
const operate = async () => {
|
|
await session.startTurn({ message: { role: "user", text: "managed-github-operation" } });
|
|
for await (const event of session.events()) {
|
|
if (event.eventType === "turn.completed") break;
|
|
if (event.eventType === "turn.failed") throw new Error("OpenCode fixture turn failed");
|
|
}
|
|
return JSON.parse(await readFile(path.join(b.root, "github", "data", "fake-github-operation.json"), "utf8"));
|
|
};
|
|
const releaseA = b.activate(run("a"));
|
|
expect(await operate()).toMatchObject({ stdout: "fixture-a", error: null });
|
|
releaseA();
|
|
const releaseB = b.activate(run("b"));
|
|
expect(await operate()).toMatchObject({ stdout: "fixture-b", error: null });
|
|
releaseB();
|
|
expect(await operate()).toMatchObject({ stdout: "anonymous", error: null });
|
|
expect(b.resolveCredentials.mock.calls.map(([binding]) => binding.runId)).toEqual(["a", "b"]);
|
|
const childEnvironment = JSON.parse(await readFile(path.join(b.root, "github", "data", "fake-environment.json"), "utf8"));
|
|
expect(childEnvironment.keys).not.toContain("PAPERCLIP_API_KEY");
|
|
expect(childEnvironment.githubTokenPresent).toBe(false);
|
|
}, 30000);
|
|
it("uses the selected duplex transport across credential rotation without file polling", async () => {
|
|
const b = await broker(undefined, true, false, undefined, { enabled: true, capability: true });
|
|
expect(b.bridgeModes).toEqual(["http2_v1"]);
|
|
expect(b.openDuplexChannel).toHaveBeenCalledOnce();
|
|
const commandsAfterStartup = b.execute.mock.calls.length;
|
|
expect((await request(b)).status).toBe(403);
|
|
const releaseA = b.activate(run("a"));
|
|
expect(await (await request(b)).json()).toMatchObject({ env: { GH_TOKEN: "fixture-a" } });
|
|
releaseA();
|
|
expect((await request(b)).status).toBe(403);
|
|
const releaseB = b.activate(run("b"));
|
|
releaseA();
|
|
expect(await (await request(b)).json()).toMatchObject({ env: { GH_TOKEN: "fixture-b" } });
|
|
expect(b.resolveCredentials.mock.calls.map(([binding]) => binding.runId)).toEqual(["a", "b"]);
|
|
expect(b.execute.mock.calls).toHaveLength(commandsAfterStartup);
|
|
releaseB();
|
|
await b.stop();
|
|
expect(() => b.activate(run("c"))).toThrow("closed");
|
|
});
|
|
it("marks a lost HTTP/2 bridge unavailable before the next warm admission", async () => {
|
|
const b = await broker(undefined, true, false, undefined, { enabled: true, capability: true });
|
|
const release = b.activate(run("a"));
|
|
expect((await request(b)).status).toBe(200);
|
|
release();
|
|
expect(b.ready).toBe(true);
|
|
const channel = await b.openDuplexChannel.mock.results[0]!.value;
|
|
channel.stop();
|
|
await vi.waitFor(() => expect(b.ready).toBe(false), { timeout: 2000 });
|
|
});
|
|
it("closes a healthy HTTP/2 bridge without reporting a lost active run", async () => {
|
|
const onLog = vi.fn(async (_stream: "stdout" | "stderr", _chunk: string) => undefined);
|
|
const b = await broker(undefined, true, false, onLog, { enabled: true, capability: true });
|
|
const release = b.activate(run("a"));
|
|
expect((await request(b)).status).toBe(200);
|
|
release();
|
|
await b.stop();
|
|
expect(b.ready).toBe(false);
|
|
expect(onLog.mock.calls.filter(([, chunk]) => chunk.includes("channel lost"))).toEqual([]);
|
|
});
|
|
it.each([
|
|
{ enabled: false, capability: true, unavailable: false, opens: 0 },
|
|
{ enabled: undefined, capability: true, unavailable: false, opens: 0 },
|
|
{ enabled: true, capability: false, unavailable: false, opens: 0 },
|
|
{ enabled: true, capability: true, unavailable: true, opens: 1 },
|
|
])("retains authorized file fallback with duplex settings %j", async options => {
|
|
const b = await broker(undefined, true, false, undefined, options);
|
|
expect(b.bridgeModes).toEqual(["queue_v1"]);
|
|
expect(b.openDuplexChannel).toHaveBeenCalledTimes(options.opens);
|
|
const release = b.activate(run("fallback"));
|
|
expect(await (await request(b)).json()).toMatchObject({ env: { GH_TOKEN: "fixture-fallback" } });
|
|
release();
|
|
expect((await request(b)).status).toBe(403);
|
|
});
|
|
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();
|
|
});
|