mirror of
https://github.com/paperclipai/paperclip.git
synced 2026-10-21 00:05:49 +02:00
fix(adapter-utils): accept the bridge token in X-API-Key (#12785)
## Thinking Path > - Paperclip is the open source app people use to manage AI agents for work > - An agent can run inside a sandbox. The sandbox callback bridge is the loopback gateway that lets the agent call the Paperclip API from inside that sandbox > - The gateway authenticates each local call with a per-run bridge token. It read that token from the `Authorization: Bearer <token>` header only > - The run hands the agent that token in `PAPERCLIP_API_KEY`, next to `PAPERCLIP_API_URL` (`execution-target.ts`). The token is named an API key, and the public Paperclip API documents that an API key goes in the `X-API-Key` header > - An agent that follows the documentation therefore sends `X-API-Key` first, and its first call fails with 401 `Invalid bridge token.` > - The agent then retries with a bearer header and the same token. The run still succeeds, but it wastes a turn and it can leave a confused agent trying unrelated repairs > - This pull request makes both generated gateways read the bridge token from `X-API-Key` when the request carries no bearer token > - The benefit is that a documented header convention stops producing a needless failure, at no cost to the token check ## Linked Issues or Issue Description No existing issue found. I searched the repository for open pull requests matching `x-api-key` and `bridge token`. Nothing addresses the in-sandbox gateway header convention. The closest neighbours are unrelated: #12632 (Composio `mcp.headers.x-api-key`) and #12120 (the HTTP/2 transport for this bridge, already merged). The problem follows `bug_report.yml`: **What happened?** An agent running in a sandbox calls the Paperclip API through the loopback callback bridge. When it sends the credential as `X-API-Key: <bridge token>`, the gateway answers `401` with `{"error":"Invalid bridge token."}`. The token in that header is the correct per-run bridge token. **Expected behavior** The gateway accepts the correct per-run bridge token in either header convention. The public API documents `X-API-Key`, so an agent that follows the documentation succeeds on its first call. **Steps to reproduce** 1. Start a run whose agent executes in a sandbox execution target, so the callback bridge is active. 2. The run exports `PAPERCLIP_API_URL` (the loopback bridge origin) and `PAPERCLIP_API_KEY` (the bridge token) into the sandbox environment. 3. From inside the sandbox, call the bridge with that key in the documented header: ```bash curl -i -H "X-API-Key: $PAPERCLIP_API_KEY" "$PAPERCLIP_API_URL/api/agents/me" ``` 4. The response is `401 {"error":"Invalid bridge token."}`. 5. Repeat the same call with `-H "Authorization: Bearer $PAPERCLIP_API_KEY"`. The response is `200`. **Paperclip version or commit** Branched from `master` at `06c0e883f`. **Deployment mode** Self-hosted Kubernetes, with the agent in a sandbox execution target and a local model behind an OpenAI-compatible endpoint. **Agent adapter(s) involved** Not adapter-specific (core bug). Observed with a local-model adapter on a sandboxed lane. Any agent that follows the public API documentation sends this header. **Additional context** The variable name is the trigger. The run calls the credential `PAPERCLIP_API_KEY`, and an agent that has read the public API documentation knows an API key goes in `X-API-Key`. Paperclip's own in-prompt guidance uses `Authorization: Bearer $PAPERCLIP_API_KEY`, so the two conventions sit side by side. Across 12 observed runs on that lane, the agent's first bridge call used `X-API-Key` in 6 runs. Every one of those runs recovered by retrying with a bearer header and the same token. In some runs the agent first tried unrelated repairs, such as writing a test comment, before it changed the header. ## What Changed - Add one `readBridgeToken(req)` helper to the generated in-sandbox gateway source. The helper returns the bearer token from `Authorization` when the header carries one. It otherwise returns the `x-api-key` header value, or an empty string. - Use that helper in both generated gateway request handlers: the file-mode queue gateway and the HTTP/2 gateway. Both previously repeated the same two lines of bearer parsing. - The token still goes to the same `timingSafeEqual` comparison. The change does not alter the comparison, the token, or its lifetime. - Extend the file-mode round-trip test to cover an `X-API-Key` request with the right token, an `X-API-Key` request with a wrong token, and the fact that the header never reaches the host. - Add one HTTP/2-path test that covers the same three cases, and that proves a bearer token still wins when a request carries both headers. ## Verification Commands run on this branch: ``` node_modules/.bin/vitest run packages/adapter-utils/src/sandbox-callback-bridge.test.ts Test Files 1 passed (1) Tests 52 passed (52) cd packages/adapter-utils && ../../node_modules/.bin/tsc --noEmit 0 errors in the two files this pull request touches ``` Both new tests are real regression guards. With the test file in place and the source change reverted, each one fails with `expected 401 to be 200`: the file-mode round-trip test and the HTTP/2 test. Both pass with the source change applied. A reviewer can confirm the security properties by reading two things. First, `tokensMatch` in the generated source is unchanged, so the constant-time comparison against the same per-run token is unchanged. Second, `allowedHeaders` in the same source is a closed allowlist of `accept`, `content-type`, `if-match`, and `if-none-match`. `normalizeHeaders` drops every other header, so neither `authorization` nor `x-api-key` leaves the sandbox on either transport. The independent host-side check in `http2-bridge-server.ts` is untouched, because each gateway sets `authorization: Bearer <token>` on the request it forwards. **ROADMAP.md check:** I grepped `ROADMAP.md` for `bridge`, `x-api-key`, `redaction`, and `sandbox callback`. There were no matches. This is a small bug fix in an existing subsystem, so it does not overlap planned core work. **Duplicate search:** I searched pull requests in this repository for `x-api-key` and `bridge token`. No open or merged pull request changes how the in-sandbox gateway reads the token. ## Risks Low risk. - The change widens which header the gateway reads. It does not widen what the gateway accepts. A request still needs the exact per-run bridge token, and it still passes the same constant-time comparison. - A bearer token keeps priority. A request that carries both headers behaves exactly as it did before, which a test now pins. - A caller that presents a non-bearer `Authorization` header and a valid `X-API-Key` header now succeeds instead of failing. This is intentional: the bridge only ever issues bearer tokens, so no bridge caller relied on that failure. - The header allowlist already excluded `x-api-key`, so no forwarded request changes shape. - No migration, no configuration change, and no behavior change for a caller that already sends a bearer token. ## Model Used Claude Opus 5 (`claude-opus-5`), extended thinking at high effort — verify before merge. ## 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 — no documentation describes the bridge's header convention, so there is nothing to update - [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 --- <sub><em>**[@superbiche](https://github.com/superbiche)** · contributor · drafted with Claude Opus 5, reviewed before posting; the voice and decisions are mine.</em></sub>
This commit is contained in:
1 parent
67a9d5cc37
commit
5b15de9568
2 files changed
+113
-6
No files matched your search
@@ -320,6 +320,42 @@ describe("sandbox callback bridge", () => {
|
||||
expect(seenRequests[0]?.headers.authorization).toBeUndefined();
|
||||
expect(seenRequests[0]?.headers["x-paperclip-run-id"]).toBeUndefined();
|
||||
|
||||
// The public Paperclip API documents the "X-API-Key" header, so an agent in
|
||||
// the sandbox sends that header before it sends "Authorization: Bearer".
|
||||
// The gateway accepts the same per-run token through either header.
|
||||
const apiKeyResponse = await fetch(`${bridge.baseUrl}/api/agents/me`, {
|
||||
headers: {
|
||||
"x-api-key": bridgeToken,
|
||||
accept: "application/json",
|
||||
},
|
||||
});
|
||||
expect(apiKeyResponse.status).toBe(200);
|
||||
await expect(apiKeyResponse.json()).resolves.toMatchObject({
|
||||
ok: true,
|
||||
method: "GET",
|
||||
path: "/api/agents/me",
|
||||
});
|
||||
|
||||
// A wrong token in the same header still fails closed.
|
||||
const apiKeyDeniedResponse = await fetch(`${bridge.baseUrl}/api/agents/me`, {
|
||||
headers: {
|
||||
"x-api-key": "wrong-token",
|
||||
},
|
||||
});
|
||||
expect(apiKeyDeniedResponse.status).toBe(401);
|
||||
await expect(apiKeyDeniedResponse.json()).resolves.toMatchObject({
|
||||
error: "Invalid bridge token.",
|
||||
});
|
||||
|
||||
// The header allowlist strips the credential header, so the token never
|
||||
// leaves the sandbox with the forwarded request.
|
||||
expect(seenRequests).toHaveLength(2);
|
||||
expect(seenRequests[1]).toMatchObject({
|
||||
method: "GET",
|
||||
path: "/api/agents/me",
|
||||
});
|
||||
expect(seenRequests[1]?.headers["x-api-key"]).toBeUndefined();
|
||||
|
||||
const githubResponse = await fetch(`${bridge.baseUrl}/runtime-tools/github/credentials`, {
|
||||
method: "POST",
|
||||
headers: {
|
||||
@@ -331,11 +367,11 @@ describe("sandbox callback bridge", () => {
|
||||
});
|
||||
expect(githubResponse.status).toBe(200);
|
||||
await githubResponse.arrayBuffer();
|
||||
expect(seenRequests[1]).toMatchObject({
|
||||
expect(seenRequests[2]).toMatchObject({
|
||||
method: "POST", path: "/runtime-tools/github/credentials", body: "{}",
|
||||
headers: { "x-paperclip-github-capability": "test-run-scoped-capability" },
|
||||
});
|
||||
expect(seenRequests[1]?.headers.authorization).toBeUndefined();
|
||||
expect(seenRequests[2]?.headers.authorization).toBeUndefined();
|
||||
|
||||
const feedbackBody = JSON.stringify({ kind: "complaint", body: "😭".repeat(5000), idempotencyKey: "bridge-feedback" });
|
||||
const feedbackResponse = await fetch(`${bridge.baseUrl}/api/companies/company-1/agent-commentary`, {
|
||||
@@ -3754,6 +3790,61 @@ describe("sandbox callback bridge", () => {
|
||||
expect(typeof seenRequests[0]?.body).toBe("string");
|
||||
expect(seenRequests[0]?.body).toBe(requestBodyText);
|
||||
});
|
||||
|
||||
it("accepts the bridge token in X-API-Key on the HTTP/2 path and strips the header", async () => {
|
||||
// Both generated gateways share one token reader, so the HTTP/2 transport
|
||||
// must accept "X-API-Key" exactly as the file-mode transport does. The
|
||||
// header allowlist must still strip the credential before the gateway
|
||||
// forwards the request to the host.
|
||||
const bridgeToken = createSandboxCallbackBridgeToken();
|
||||
const seenHeaders: Array<Record<string, string>> = [];
|
||||
const gateway = await startHttp2GatewayForTest({
|
||||
bridgeToken,
|
||||
forwardRequest: async (request) => {
|
||||
seenHeaders.push(request.headers);
|
||||
return {
|
||||
status: 200,
|
||||
headers: { "content-type": "application/json" },
|
||||
body: Buffer.from(JSON.stringify({ ok: true }), "utf8"),
|
||||
};
|
||||
},
|
||||
});
|
||||
|
||||
const response = await fetch(`${gateway.baseUrl}/api/agents/me`, {
|
||||
headers: {
|
||||
"x-api-key": bridgeToken,
|
||||
accept: "application/json",
|
||||
},
|
||||
});
|
||||
expect(response.status).toBe(200);
|
||||
await expect(response.json()).resolves.toMatchObject({ ok: true });
|
||||
expect(seenHeaders).toHaveLength(1);
|
||||
expect(seenHeaders[0]?.["x-api-key"]).toBeUndefined();
|
||||
|
||||
const deniedResponse = await fetch(`${gateway.baseUrl}/api/agents/me`, {
|
||||
headers: {
|
||||
"x-api-key": "wrong-token",
|
||||
},
|
||||
});
|
||||
expect(deniedResponse.status).toBe(401);
|
||||
await expect(deniedResponse.json()).resolves.toMatchObject({
|
||||
error: "Invalid bridge token.",
|
||||
});
|
||||
// The rejected request never opened a stream to the host.
|
||||
expect(seenHeaders).toHaveLength(1);
|
||||
|
||||
// A bearer token still works and still wins when both headers are present.
|
||||
const bearerResponse = await fetch(`${gateway.baseUrl}/api/agents/me`, {
|
||||
headers: {
|
||||
authorization: `Bearer ${bridgeToken}`,
|
||||
"x-api-key": "wrong-token",
|
||||
},
|
||||
});
|
||||
expect(bearerResponse.status).toBe(200);
|
||||
expect(seenHeaders).toHaveLength(2);
|
||||
expect(seenHeaders[1]?.authorization).toBeUndefined();
|
||||
expect(seenHeaders[1]?.["x-api-key"]).toBeUndefined();
|
||||
}, 15_000);
|
||||
});
|
||||
|
||||
interface EmbeddedBridgeProcessBodyLedger {
|
||||
|
||||
@@ -2407,6 +2407,24 @@ function tokensMatch(received) {
|
||||
return timingSafeEqual(expected, actual);
|
||||
}
|
||||
|
||||
// Read the bridge token a local caller presented. The gateway accepts two
|
||||
// header conventions for the same token. "Authorization: Bearer <token>" is the
|
||||
// bridge convention. "X-API-Key: <token>" is the convention the public Paperclip
|
||||
// API documents, so an agent that follows those docs sends it first. Both
|
||||
// headers carry the same per-run token and both reach the same constant-time
|
||||
// comparison in tokensMatch, so the second header removes a needless 401 without
|
||||
// widening what the gateway trusts. The header allowlist strips both
|
||||
// headers before the gateway forwards a request, so neither one leaves the
|
||||
// sandbox. Authorization wins when a caller presents a bearer token in it.
|
||||
function readBridgeToken(req) {
|
||||
const auth = req.headers.authorization || "";
|
||||
if (auth.startsWith("Bearer ")) {
|
||||
return auth.slice("Bearer ".length);
|
||||
}
|
||||
const apiKey = req.headers["x-api-key"];
|
||||
return typeof apiKey === "string" ? apiKey : "";
|
||||
}
|
||||
|
||||
function writeJsonResponse(res, status, body) {
|
||||
res.statusCode = status;
|
||||
res.setHeader("content-type", "application/json");
|
||||
@@ -2484,8 +2502,7 @@ async function runFileGateway() {
|
||||
let envelopeReservation = 0;
|
||||
let dispatched = false;
|
||||
try {
|
||||
const auth = req.headers.authorization || "";
|
||||
const receivedToken = auth.startsWith("Bearer ") ? auth.slice("Bearer ".length) : "";
|
||||
const receivedToken = readBridgeToken(req);
|
||||
if (!tokensMatch(receivedToken)) {
|
||||
writeJsonResponse(res, 401, { error: "Invalid bridge token." });
|
||||
return;
|
||||
@@ -2768,8 +2785,7 @@ function runHttp2Gateway() {
|
||||
// deadline timeout all reach the same finally.
|
||||
let releaseBodyReservation = null;
|
||||
try {
|
||||
const auth = req.headers.authorization || "";
|
||||
const receivedToken = auth.startsWith("Bearer ") ? auth.slice("Bearer ".length) : "";
|
||||
const receivedToken = readBridgeToken(req);
|
||||
if (!tokensMatch(receivedToken)) {
|
||||
writeJsonResponse(res, 401, { error: "Invalid bridge token." });
|
||||
return;
|
||||
|
||||
Reference in new issue
Block a user