From 38d08847b8d3d5a78bf864c355868d5420b8a2fb Mon Sep 17 00:00:00 2001 From: Dotta <34892728+cryppadotta@users.noreply.github.com> Date: Tue, 27 Oct 2026 20:57:19 -0500 Subject: [PATCH] fix(runner): keep OpenRouter upstream keys in the parent proxy (#17949) ## Thinking Path > - Paperclip manages agents and their work. > - Paperclip Runner starts OpenCode with the selected provider and model. > - The native OpenRouter route passed the reusable upstream key into the child environment. > - A persisted-state scan found that key in an OpenCode database WAL. The exact write operation is not known. > - This pull request reuses the existing parent-held provider proxy for that route. > - The child receives a temporary loopback credential instead of the upstream key. ## Linked Issues or Issue Description **What happened?** The native OpenRouter path passed `OPENROUTER_API_KEY` into the OpenCode child. A real Product E2E persisted-state scan detected the key in the child database WAL. The scan does not establish which operation wrote it. **Expected behavior** The runner keeps the reusable upstream key in its own process. OpenCode can call only the selected model through the session proxy. **Steps to reproduce** 1. Start the OpenCode driver with an explicit or ambient synthetic OpenRouter key. 2. Inspect the fake child configuration, environment, and persisted provider values. 3. Send a request through the real loopback proxy and verify the upstream authorization and exact model. **Paperclip version or commit** The regression is present on merged master `813e808af462b6a6f9db38b6791e3bfe8465b51e`. This PR is rebased onto merged master `d40f9ac377`. **Deployment mode** Built from source. The finding came from an AWS Product E2E job. The regression tests use local synthetic servers and a fake child, with no provider calls. Related issue search found #16643 about general inherited environment scope. This change addresses the narrower native OpenRouter path. No duplicate custody PR was found. This PR must remain unmerged until separately approved; it does not change the eval baseline or qualify a new live result. ## What Changed - Reuse the existing parent-held proxy for native `openrouter/` models. - Remove the upstream key from the child environment and retain it in diagnostic redaction. - Keep the selected provider and exact model. Reject alternate-routing fields before the upstream request. Use a session-lived loopback token. - Preserve the effective explicit or ambient proxy bypass list when adding loopback entries. - Add explicit and ambient key regressions, upstream routing checks, and child persistence checks. - Document the boundary and its limits. ## Verification - Final rebased head: full OpenCode driver suite, 64 synthetic tests passed, and runner source TypeScript check passed. Original author run had 61 passing tests before the new master tests. - The two new explicit and ambient regressions fail on the old implementation. - Independent source review passed. The two new synthetic regressions also passed independently. A separately filtered legacy pair hit fixture selection/chmod ordering and is not claimed as a passing run. - Tests use the real driver and proxy with a fake OpenCode child and synthetic upstream transport. They check key exclusion, exact URL/model/authentication, wrong-model and alternate-routing rejection, proxy bypass preservation, redirect rejection, and proxy shutdown. - The two advisory regressions failed before the correction: fallback routing reached upstream, and ambient NO_PROXY was lost. The full 64-test driver suite and source TypeScript check pass after the correction. - No real OpenCode SDK/provider qualification was run. The original WAL write mechanism remains unknown. - Repo-wide typecheck, test:run, and build were not run locally because local native/DB/model execution is excluded. Normal PR CI passed on head `3e1903272ce65b92055ea70f8dc586d9aa5e0205`: typecheck/release registry, general and serialized suites, runner static/Rust/Vitest checks, E2E shards, build, and canary dry run. Greptile is 5/5 on that head. Paperclip Review is neutral with no blocking findings; its two earlier findings are fixed and resolved. ## Risks The native OpenRouter route now uses the existing OpenAI-compatible proxy transport. Synthetic tests do not prove every live SDK response shape. The loopback token remains a credential, is redacted, and expires with the session. This change does not claim isolation of every child secret or persisted value. ## Model Used OpenAI GPT-6.1 Sol (`gpt-6.1-sol`) assisted source investigation, implementation, synthetic tests, and review with tool use. No provider model call was used for verification. ## 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 --- packages/paperclip-runner/README.md | 7 ++ .../opencode/opencode-server-driver.test.ts | 91 ++++++++++++++++++- .../opencode/opencode-server-driver.ts | 24 +++-- .../test/fixtures/fake-opencode-server.mjs | 5 + 4 files changed, 116 insertions(+), 11 deletions(-) diff --git a/packages/paperclip-runner/README.md b/packages/paperclip-runner/README.md index 488c97584b3..4003cb8da6f 100644 --- a/packages/paperclip-runner/README.md +++ b/packages/paperclip-runner/README.md @@ -388,6 +388,13 @@ support disabling reasoning; other runner providers reject an explicit selection before starting work. Native sessions expose support through the `perTurnReasoning` capability. +For the native `openrouter/` route, the parent-held provider proxy retains the +upstream OpenRouter key. The child config keeps the selected `openrouter` provider +and exact model, using a random loopback token restricted to that model's chat +completions endpoint. The proxy and token expire when the session closes. The +local token remains a credential and is redacted from diagnostics; this does not +claim isolation for every child credential or persisted value. + OpenCode's proxy sends agent instructions once per turn through the system prompt. The task envelope retains task-specific constraints and the completion contract, without repeating the system instructions. This applies to all users of the diff --git a/packages/paperclip-runner/src/drivers/opencode/opencode-server-driver.test.ts b/packages/paperclip-runner/src/drivers/opencode/opencode-server-driver.test.ts index c8e2ed1c292..5f171927977 100644 --- a/packages/paperclip-runner/src/drivers/opencode/opencode-server-driver.test.ts +++ b/packages/paperclip-runner/src/drivers/opencode/opencode-server-driver.test.ts @@ -19,7 +19,7 @@ import { EventEmitter } from "node:events"; import { promisify } from "node:util"; import { getCACertificates } from "node:tls"; import { basename, dirname, join, resolve } from "node:path"; -import { afterAll, afterEach, beforeAll, describe, expect, it } from "vitest"; +import { afterAll, afterEach, beforeAll, describe, expect, it, vi } from "vitest"; import { NATIVE_RUNTIME_ASSET_SCHEMA, @@ -37,6 +37,18 @@ import { openCodeServerDriverInternals, } from "./opencode-server-driver.js"; +const upstreamTransport = vi.hoisted(() => ({ endpoint: "", urls: [] as string[] })); +vi.mock("node:https", async importOriginal => { + const actual = await importOriginal(); + const http = await import("node:http"); + return { ...actual, request: (...args: Parameters) => { + if (!upstreamTransport.endpoint) return actual.request(...args); + upstreamTransport.urls.push(String(args[0])); + const options = args[1] as import("node:http").RequestOptions; + return http.request(upstreamTransport.endpoint, { ...options, agent: undefined }, args[2]); + } }; +}); + const roots: string[] = []; let fixtureRoot: string | undefined; let fixture: string; @@ -704,6 +716,81 @@ describe("OpenCodeServerDriver", () => { } }); + it.each(["explicit", "ambient"])("keeps OpenRouter upstream credentials out of the child while preserving exact provider/model routing (%s)", async source => { + await chmod(fixture, 0o755); + const key = "synthetic-upstream-openrouter-key"; + const model = "deepseek/deepseek-v4.1-flash-1017"; + const received: Array<{ authorization?: string; body: unknown; referer?: string }> = []; + let redirect = false; + const upstream = createHttpServer((request, response) => { + void (async () => { + const chunks = []; + for await (const chunk of request) chunks.push(Buffer.from(chunk)); + received.push({ authorization: request.headers.authorization, body: JSON.parse(Buffer.concat(chunks).toString("utf8")), referer: request.headers["http-referer"] as string }); + if (redirect) response.writeHead(302, { location: "https://must-not-follow.invalid/" }).end(); + else response.writeHead(200, { "content-type": "text/event-stream" }).end("data: [DONE]\n\n"); + })().catch(() => response.destroy()); + }); + await new Promise(resolve => upstream.listen(0, "127.0.0.1", resolve)); + const address = upstream.address(); + if (!address || typeof address === "string") throw new Error("Missing fixture address"); + upstreamTransport.endpoint = `http://127.0.0.1:${address.port}/chat/completions`; + upstreamTransport.urls = []; + const root = await mkdtemp(join(tmpdir(), "paperclip-openrouter-custody-")); + roots.push(root); + let session: Awaited> | undefined; + try { + if (source === "ambient") { + vi.stubEnv("OPENROUTER_API_KEY", key); + vi.stubEnv("no_proxy", undefined); + vi.stubEnv("NO_PROXY", "custody-bypass.invalid"); + } + const driver = new OpenCodeServerDriver({ model: `openrouter/${model}`, runtimeDirectory: root, command: fixture, + ...(source === "explicit" ? { environment: { PATH: process.env.PATH, OPENROUTER_API_KEY: key, NO_PROXY: "custody-bypass.invalid" } } : {}) }); + session = await driver.openSession({ runId: "custody", normalizedSessionId: "custody", workingDirectory: root }); + const configText = await readFile(join(root, "custody", "config", "opencode", "opencode.json"), "utf8"); + const config = JSON.parse(configText); + expect(config.model).toBe(`openrouter/${model}`); + expect(config.small_model).toBe(`openrouter/${model}`); + expect(Object.keys(config.provider)).toEqual(["openrouter"]); + expect(Object.keys(config.provider.openrouter.models)).toEqual([model]); + const { baseURL, apiKey } = config.provider.openrouter.options; + expect(configText).not.toContain(key); + const environmentText = await readFile(join(root, "custody", "data", "fake-environment.json"), "utf8"); + expect(JSON.parse(environmentText).keys).not.toContain("OPENROUTER_API_KEY"); + expect(environmentText).not.toContain(key); + expect(JSON.parse(environmentText).noProxy).toBe("custody-bypass.invalid,127.0.0.1,localhost"); + const persisted = await readFile(join(root, "custody", "data", "fake-provider-custody.json"), "utf8"); + expect(persisted).not.toContain(key); + expect(JSON.parse(persisted)).toMatchObject({ providerOptions: { baseURL, apiKey } }); + expect(baseURL).toMatch(/^http:\/\/127\.0\.0\.1:\d+\/v1$/); + expect(apiKey).not.toBe(key); + const url = `${baseURL}/chat/completions`; + const headers = { Authorization: `Bearer ${apiKey}`, "Content-Type": "application/json" }; + const payload = { model, messages: [{ role: "user", content: "synthetic" }] }; + expect((await fetch(url, { method: "POST", headers, body: JSON.stringify({ ...payload, model: "wrong-model" }) })).status).toBe(400); + expect(received).toHaveLength(0); + for (const [field, value] of Object.entries({ models: ["wrong-model"], route: "fallback", plugins: [{ id: "web" }], transforms: ["middle-out"] })) { + expect((await fetch(url, { method: "POST", headers, body: JSON.stringify({ ...payload, [field]: value }) })).status).toBe(400); + expect(received).toHaveLength(0); + } + expect((await fetch(url, { method: "POST", headers, body: JSON.stringify(payload) })).status).toBe(200); + expect(received).toEqual([{ authorization: `Bearer ${key}`, body: payload, referer: "https://paperclip.ing" }]); + expect(upstreamTransport.urls).toEqual(["https://openrouter.ai/api/v1/chat/completions"]); + redirect = true; + expect((await fetch(url, { method: "POST", headers, body: JSON.stringify(payload) })).status).toBe(502); + expect(upstreamTransport.urls).toHaveLength(2); + await session.close({ reason: "test" }); + session = undefined; + await expect(fetch(url, { method: "POST", headers, body: JSON.stringify(payload) })).rejects.toThrow(); + } finally { + await session?.close({ reason: "test" }); + upstreamTransport.endpoint = ""; + vi.unstubAllEnvs(); + await new Promise(resolve => { upstream.close(() => resolve()); upstream.closeAllConnections(); }); + } + }); + it.each([["HTTP_PROXY", "file"], ["ALL_PROXY", "directory"]] as const)("uses the runtime's %s, %s trust, and NO_PROXY without changing global transport", async (proxySetting, trust) => { const requests: string[] = []; const outgoingProxy = createHttpServer((request, response) => { @@ -910,7 +997,7 @@ describe("OpenCodeServerDriver", () => { "utf8", ), ); - expect(environment.keys).toContain("OPENROUTER_API_KEY"); + expect(environment.keys).not.toContain("OPENROUTER_API_KEY"); expect(environment.keys).not.toContain("PAPERCLIP_API_KEY"); expect(environment.keys).not.toContain("UNRELATED_SECRET"); expect(environment.keys).not.toContain("PAPERCLIP_PROVIDER_TRACE_PATH"); diff --git a/packages/paperclip-runner/src/drivers/opencode/opencode-server-driver.ts b/packages/paperclip-runner/src/drivers/opencode/opencode-server-driver.ts index 4505b29d687..825812a0d09 100644 --- a/packages/paperclip-runner/src/drivers/opencode/opencode-server-driver.ts +++ b/packages/paperclip-runner/src/drivers/opencode/opencode-server-driver.ts @@ -2216,7 +2216,8 @@ async function startOpenCodeProviderProxy(baseUrl: string, key: string, model: s let payload: Record; try { payload = JSON.parse(body.toString("utf8")); } catch { response.writeHead(400).end(); return; } - if (!payload || typeof payload !== "object" || payload.model !== model) { + if (!payload || typeof payload !== "object" || payload.model !== model || + ["models", "route", "plugins", "transforms"].some(field => Object.hasOwn(payload, field))) { response.writeHead(400).end(); return; } @@ -2323,14 +2324,15 @@ async function startRuntime(input: { const identityKey = input.options.environment?.PAPERCLIP_AGENT_PRIVATE_KEY; const identityValues = identityKey ? [identityKey, JSON.stringify(identityKey).slice(1, -1), ...identityKey.split(/\r?\n/).filter(line => line && !line.startsWith("-----"))] : []; + const providerEnvironment = input.options.environment ?? process.env; const sensitiveValues = [ ...identityValues, password, authHeader, bridge.secret, assignedMcp?.token, - input.options.environment?.OPENROUTER_API_KEY, - input.options.environment?.PAPERCLIP_AI_PROVIDER_KEY, + providerEnvironment.OPENROUTER_API_KEY, + providerEnvironment.PAPERCLIP_AI_PROVIDER_KEY, ].filter((value): value is string => Boolean(value)); input.trace?.addSensitiveValues(sensitiveValues); const instructionRoot = @@ -2346,8 +2348,13 @@ async function startRuntime(input: { if (instructionRoot) externalDirectories[`${instructionRoot}/**`] = "allow"; const [modelProvider, ...modelIdParts] = input.options.model.split("/"); const providerModelId = modelIdParts.join("/"); - const providerProxy = modelProvider === "paperclip" && input.options.environment?.PAPERCLIP_AI_PROVIDER_URL - ? await startOpenCodeProviderProxy(input.options.environment.PAPERCLIP_AI_PROVIDER_URL, input.options.environment.PAPERCLIP_AI_PROVIDER_KEY ?? "", providerModelId, input.options.environment).catch(async error => { + const providerRoute = modelProvider === "openrouter" + ? { baseUrl: "https://openrouter.ai/api/v1", key: providerEnvironment.OPENROUTER_API_KEY ?? "" } + : modelProvider === "paperclip" && input.options.environment?.PAPERCLIP_AI_PROVIDER_URL + ? { baseUrl: input.options.environment.PAPERCLIP_AI_PROVIDER_URL, key: input.options.environment.PAPERCLIP_AI_PROVIDER_KEY ?? "" } + : null; + const providerProxy = providerRoute + ? await startOpenCodeProviderProxy(providerRoute.baseUrl, providerRoute.key, providerModelId, providerEnvironment).catch(async error => { await bridge.close().catch(() => {}); await removeIsolatedHome(isolatedHome).catch(() => {}); throw error; @@ -2397,8 +2404,9 @@ async function startRuntime(input: { ...(modelProvider === "openrouter" ? { options: { headers: OPENROUTER_ATTRIBUTION_HEADERS } } : {}), ...(providerProxy ? { npm: "@ai-sdk/openai-compatible", - name: "Paperclip connection", + name: modelProvider === "openrouter" ? "OpenRouter" : "Paperclip connection", options: { + ...(modelProvider === "openrouter" ? { headers: OPENROUTER_ATTRIBUTION_HEADERS } : {}), baseURL: providerProxy.baseURL, apiKey: providerProxy.token, }, @@ -2470,7 +2478,7 @@ async function startRuntime(input: { OPENCODE_SERVER_USERNAME: username, OPENCODE_SERVER_PASSWORD: password, ...performanceDiagnostics?.environment, - ...(providerProxy ? { NO_PROXY: [input.options.environment?.no_proxy ?? input.options.environment?.NO_PROXY, "127.0.0.1", "localhost"].filter(Boolean).join(",") } : {}), + ...(providerProxy ? { NO_PROXY: [providerEnvironment.no_proxy ?? providerEnvironment.NO_PROXY, "127.0.0.1", "localhost"].filter(Boolean).join(",") } : {}), }, ); const isolateProcessGroup = input.options.isolateProcessGroup ?? true; @@ -2960,7 +2968,6 @@ function sanitizedEnvironment( "ALL_PROXY", "SSL_CERT_FILE", "SSL_CERT_DIR", - "OPENROUTER_API_KEY", ]; const result: NodeJS.ProcessEnv = {}; const candidate = source ?? process.env; @@ -2982,7 +2989,6 @@ function sanitizedEnvironmentKeys(): string[] { "ALL_PROXY", "SSL_CERT_FILE", "SSL_CERT_DIR", - "OPENROUTER_API_KEY", ]; } diff --git a/packages/paperclip-runner/test/fixtures/fake-opencode-server.mjs b/packages/paperclip-runner/test/fixtures/fake-opencode-server.mjs index 412d602171a..cde9d5fbc87 100644 --- a/packages/paperclip-runner/test/fixtures/fake-opencode-server.mjs +++ b/packages/paperclip-runner/test/fixtures/fake-opencode-server.mjs @@ -93,10 +93,15 @@ let pendingPermission = await readFile( let permissionStyle = "v2"; await mkdir(process.env.XDG_DATA_HOME, { recursive: true }); +await writeFile(join(process.env.XDG_DATA_HOME, "fake-provider-custody.json"), JSON.stringify({ + openrouterKey: process.env.OPENROUTER_API_KEY, + providerOptions: runtimeConfig.provider?.openrouter?.options, +})); await writeFile( join(process.env.XDG_DATA_HOME, "fake-environment.json"), JSON.stringify({ keys: Object.keys(process.env).sort(), + noProxy: process.env.NO_PROXY, home: process.env.HOME, configHome: process.env.XDG_CONFIG_HOME, projectConfigDisabled: process.env.OPENCODE_DISABLE_PROJECT_CONFIG,