mirror of
https://github.com/paperclipai/paperclip.git
synced 2026-10-30 13:20:54 +01:00
fix(server): mint the run bearer through a register-first wrapper
The run bearer minted for a run must reach the redaction registry before any code can use it, so a later read of run or issue text can mask it. Add mintAndRegisterRunBearer(), the only allowed caller of createLocalAgentJwt. It mints, then registers the token with the run secret redaction registry before returning it. A registration failure propagates and returns no token, so the caller aborts before it injects a bearer into an adapter or a tool call. Route all three mint sites through the wrapper: the legacy adapter dispatch path and the two native-runtime tool paths (create_project and call_api). A source-scanning test pins createLocalAgentJwt to have no caller outside the wrapper and the unit tests. Co-Authored-By: Paperclip <noreply@paperclip.ing>
This commit is contained in:
1 parent
ae06329971
commit
74b3e024eb
7 files changed
+581
-12
No files matched your search
@@ -1,15 +1,16 @@
|
||||
import { randomUUID } from "node:crypto";
|
||||
import { mkdtemp, readFile } from "node:fs/promises";
|
||||
import { createHash, randomUUID } from "node:crypto";
|
||||
import { access, mkdtemp, readFile } from "node:fs/promises";
|
||||
import { tmpdir } from "node:os";
|
||||
import { join } from "node:path";
|
||||
import { and, eq, sql } from "drizzle-orm";
|
||||
import { afterAll, afterEach, beforeAll, describe, expect, it } from "vitest";
|
||||
import { afterAll, afterEach, beforeAll, describe, expect, it, vi } from "vitest";
|
||||
import {
|
||||
agents,
|
||||
companies,
|
||||
createDb,
|
||||
environmentLeases,
|
||||
environments,
|
||||
heartbeatRuns,
|
||||
} from "@paperclipai/db";
|
||||
import {
|
||||
getEmbeddedPostgresTestSupport,
|
||||
@@ -17,6 +18,32 @@ import {
|
||||
} from "./helpers/embedded-postgres.js";
|
||||
import { heartbeatService } from "../services/heartbeat.ts";
|
||||
|
||||
// D8.1 legacy heartbeat coverage. `forceRunBearerRegistrationFailure` lets one
|
||||
// test make the real redaction registry's `register()` reject, so the legacy
|
||||
// mint site (server/src/services/heartbeat.ts, guarded by
|
||||
// `nativeRuntimeResolution.kind === "legacy" && adapter.supportsLocalAgentJwt`)
|
||||
// can be proven to abort dispatch before the adapter process launches. Every
|
||||
// other test in this file keeps the real registry.
|
||||
const forceRunBearerRegistrationFailure = vi.hoisted(() => ({ current: false }));
|
||||
vi.mock("../services/run-secret-redaction.js", async (importOriginal) => {
|
||||
const actual = await importOriginal<typeof import("../services/run-secret-redaction.js")>();
|
||||
return {
|
||||
...actual,
|
||||
createRunSecretRedactionRegistry: (db: Parameters<typeof actual.createRunSecretRedactionRegistry>[0]) => {
|
||||
const real = actual.createRunSecretRedactionRegistry(db);
|
||||
return {
|
||||
...real,
|
||||
register: async (...args: Parameters<typeof real.register>) => {
|
||||
if (forceRunBearerRegistrationFailure.current) {
|
||||
throw new Error("Heartbeat run redaction registration failed (forced for test)");
|
||||
}
|
||||
return real.register(...args);
|
||||
},
|
||||
};
|
||||
},
|
||||
};
|
||||
});
|
||||
|
||||
const embeddedPostgresSupport = await getEmbeddedPostgresTestSupport();
|
||||
const describeEmbeddedPostgres = embeddedPostgresSupport.supported ? describe : describe.skip;
|
||||
|
||||
@@ -75,6 +102,7 @@ describeEmbeddedPostgres("heartbeat local environment lifecycle", () => {
|
||||
}, 20_000);
|
||||
|
||||
afterEach(async () => {
|
||||
forceRunBearerRegistrationFailure.current = false;
|
||||
// A run reaches its terminal status before finalizeRun finishes writing
|
||||
// its trailing lifecycle events and side effects (see the comment on
|
||||
// drainActiveRunExecutions in heartbeat.ts). Drain those in-flight writes
|
||||
@@ -224,4 +252,112 @@ describeEmbeddedPostgres("heartbeat local environment lifecycle", () => {
|
||||
});
|
||||
expect(captured.apiUrl).toEqual(expect.stringMatching(/^https?:\/\//));
|
||||
});
|
||||
|
||||
// D8.1 — mint and register at the legacy adapter mint site
|
||||
// (server/src/services/heartbeat.ts, guarded by
|
||||
// `nativeRuntimeResolution.kind === "legacy" && adapter.supportsLocalAgentJwt`).
|
||||
it("registers the minted run bearer against the correct company and run for the legacy adapter path", async () => {
|
||||
const companyId = randomUUID();
|
||||
const agentId = randomUUID();
|
||||
const issuePrefix = `T${companyId.replace(/-/g, "").slice(0, 6).toUpperCase()}`;
|
||||
const tempDir = await mkdtemp(join(tmpdir(), "paperclip-process-register-"));
|
||||
const tokenPath = join(tempDir, "token.txt");
|
||||
|
||||
await db.insert(companies).values({
|
||||
id: companyId,
|
||||
name: "Paperclip",
|
||||
issuePrefix,
|
||||
requireBoardApprovalForNewAgents: false,
|
||||
defaultResponsibleUserId: "responsible-user",
|
||||
});
|
||||
|
||||
await db.insert(agents).values({
|
||||
id: agentId,
|
||||
companyId,
|
||||
name: "ProcessAgent",
|
||||
role: "engineer",
|
||||
status: "idle",
|
||||
adapterType: "process",
|
||||
adapterConfig: {
|
||||
command: process.execPath,
|
||||
args: [
|
||||
"-e",
|
||||
[
|
||||
"const fs = require('node:fs');",
|
||||
`fs.writeFileSync(${JSON.stringify(tokenPath)}, process.env.PAPERCLIP_API_KEY ?? "");`,
|
||||
].join(" "),
|
||||
],
|
||||
},
|
||||
runtimeConfig: {},
|
||||
permissions: {},
|
||||
});
|
||||
|
||||
const queued = await heartbeat.invoke(agentId, "on_demand", {}, "manual");
|
||||
expect(queued).not.toBeNull();
|
||||
|
||||
const finished = await waitForRunToFinish(heartbeat, queued!.id);
|
||||
expect(finished?.status).toBe("succeeded");
|
||||
|
||||
const injectedToken = await readFile(tokenPath, "utf8");
|
||||
expect(injectedToken.length).toBeGreaterThan(0);
|
||||
|
||||
const [row] = await db
|
||||
.select({ contextSnapshot: heartbeatRuns.contextSnapshot })
|
||||
.from(heartbeatRuns)
|
||||
.where(and(eq(heartbeatRuns.id, queued!.id), eq(heartbeatRuns.companyId, companyId)));
|
||||
const entries = (row?.contextSnapshot as { paperclipSecretRedactions?: Array<{ fingerprintSha256: string }> } | undefined)
|
||||
?.paperclipSecretRedactions ?? [];
|
||||
expect(entries.map((entry) => entry.fingerprintSha256))
|
||||
.toContain(createHash("sha256").update(injectedToken).digest("hex"));
|
||||
});
|
||||
|
||||
it("aborts before the adapter process launches when registration fails for the legacy adapter path", async () => {
|
||||
const companyId = randomUUID();
|
||||
const agentId = randomUUID();
|
||||
const issuePrefix = `T${companyId.replace(/-/g, "").slice(0, 6).toUpperCase()}`;
|
||||
const tempDir = await mkdtemp(join(tmpdir(), "paperclip-process-abort-"));
|
||||
const markerPath = join(tempDir, "adapter-launched.marker");
|
||||
|
||||
await db.insert(companies).values({
|
||||
id: companyId,
|
||||
name: "Paperclip",
|
||||
issuePrefix,
|
||||
requireBoardApprovalForNewAgents: false,
|
||||
defaultResponsibleUserId: "responsible-user",
|
||||
});
|
||||
|
||||
await db.insert(agents).values({
|
||||
id: agentId,
|
||||
companyId,
|
||||
name: "ProcessAgent",
|
||||
role: "engineer",
|
||||
status: "idle",
|
||||
adapterType: "process",
|
||||
adapterConfig: {
|
||||
command: process.execPath,
|
||||
// A registration failure must abort before this process ever spawns.
|
||||
// If it did spawn, this line alone would create the marker file.
|
||||
args: ["-e", `require('node:fs').writeFileSync(${JSON.stringify(markerPath)}, "launched")`],
|
||||
},
|
||||
runtimeConfig: {},
|
||||
permissions: {},
|
||||
});
|
||||
|
||||
forceRunBearerRegistrationFailure.current = true;
|
||||
const queued = await heartbeat.invoke(agentId, "on_demand", {}, "manual");
|
||||
expect(queued).not.toBeNull();
|
||||
|
||||
const finished = await waitForRunToFinish(heartbeat, queued!.id);
|
||||
expect(finished?.status).not.toBe("succeeded");
|
||||
|
||||
await expect(access(markerPath)).rejects.toThrow();
|
||||
|
||||
const [row] = await db
|
||||
.select({ contextSnapshot: heartbeatRuns.contextSnapshot })
|
||||
.from(heartbeatRuns)
|
||||
.where(and(eq(heartbeatRuns.id, queued!.id), eq(heartbeatRuns.companyId, companyId)));
|
||||
const entries = (row?.contextSnapshot as { paperclipSecretRedactions?: unknown[] } | undefined)
|
||||
?.paperclipSecretRedactions ?? [];
|
||||
expect(entries).toEqual([]);
|
||||
});
|
||||
});
|
||||
@@ -0,0 +1,127 @@
|
||||
import { mkdirSync, mkdtempSync, readFileSync, readdirSync, rmSync, writeFileSync } from "node:fs";
|
||||
import os from "node:os";
|
||||
import path from "node:path";
|
||||
import { fileURLToPath } from "node:url";
|
||||
import { describe, expect, it } from "vitest";
|
||||
|
||||
// `createLocalAgentJwt` (server/src/agent-auth-jwt.ts) mints a run bearer but
|
||||
// does not register it with the redaction registry. A direct call site mints
|
||||
// a bearer that no later read path can mask. `mintAndRegisterRunBearer`
|
||||
// (server/src/services/run-bearer.ts) is the only call path that mints and
|
||||
// registers together, so every production caller must go through it.
|
||||
//
|
||||
// This scan reads the repository source and reports each production line
|
||||
// that calls `createLocalAgentJwt` outside the wrapper file. It follows the
|
||||
// source-scanning style of `cli-invocation-safety.test.ts`.
|
||||
|
||||
const here = path.dirname(fileURLToPath(import.meta.url));
|
||||
const srcRoot = path.resolve(here, "..");
|
||||
|
||||
const DEFINITION_FILE = "agent-auth-jwt.ts";
|
||||
const WRAPPER_FILE = path.join("services", "run-bearer.ts");
|
||||
|
||||
const CALL_PATTERN = /\bcreateLocalAgentJwt\s*\(/;
|
||||
const DECLARATION_PATTERN = /\bfunction\s+createLocalAgentJwt\s*\(/;
|
||||
|
||||
function isTestFile(relPath: string): boolean {
|
||||
return relPath.includes("__tests__/") || /\.(test|spec)\.[tj]sx?$/.test(relPath);
|
||||
}
|
||||
|
||||
function listSourceFiles(rootDir: string): string[] {
|
||||
const found: string[] = [];
|
||||
|
||||
function walk(absDir: string, relDir: string): void {
|
||||
for (const entry of readdirSync(absDir, { withFileTypes: true })) {
|
||||
if (entry.isSymbolicLink()) continue;
|
||||
const relPath = relDir ? `${relDir}/${entry.name}` : entry.name;
|
||||
if (entry.isDirectory()) {
|
||||
if (entry.name === "node_modules" || entry.name === "dist") continue;
|
||||
walk(path.join(absDir, entry.name), relPath);
|
||||
continue;
|
||||
}
|
||||
if (!/\.(ts|tsx)$/.test(entry.name)) continue;
|
||||
found.push(relPath);
|
||||
}
|
||||
}
|
||||
|
||||
walk(rootDir, "");
|
||||
return found;
|
||||
}
|
||||
|
||||
// Return one `path:line` entry for each production line that calls
|
||||
// `createLocalAgentJwt` outside the wrapper file. The function definition
|
||||
// line and every unit test file are exempt.
|
||||
function findMintSiteOffenders(rootDir: string): string[] {
|
||||
const offenders: string[] = [];
|
||||
for (const relPath of listSourceFiles(rootDir)) {
|
||||
const normalized = relPath.split(path.sep).join("/");
|
||||
if (normalized === DEFINITION_FILE) continue;
|
||||
if (normalized === WRAPPER_FILE.split(path.sep).join("/")) continue;
|
||||
if (isTestFile(normalized)) continue;
|
||||
const text = readFileSync(path.join(rootDir, relPath), "utf8");
|
||||
text.split("\n").forEach((line, index) => {
|
||||
if (CALL_PATTERN.test(line) && !DECLARATION_PATTERN.test(line)) {
|
||||
offenders.push(`${normalized}:${index + 1}`);
|
||||
}
|
||||
});
|
||||
}
|
||||
return offenders;
|
||||
}
|
||||
|
||||
describe("createLocalAgentJwt mint-site pinning", () => {
|
||||
it("has no caller in production source outside the register-first wrapper", () => {
|
||||
const offenders = findMintSiteOffenders(srcRoot);
|
||||
expect(
|
||||
offenders,
|
||||
"createLocalAgentJwt must be called only from server/src/services/run-bearer.ts. " +
|
||||
`A direct caller mints a bearer that the redaction registry never learns about:\n${offenders.join("\n")}`,
|
||||
).toEqual([]);
|
||||
});
|
||||
|
||||
it("fails on a tree that still holds a direct call outside the wrapper", () => {
|
||||
// Prove the scanner can fail. Build a fixture tree with a production file
|
||||
// that calls createLocalAgentJwt directly, mirroring the old direct-call
|
||||
// shape this task replaced at all three mint sites.
|
||||
const root = mkdtempSync(path.join(os.tmpdir(), "run-bearer-pinning-"));
|
||||
try {
|
||||
const offenderDir = path.join(root, "services", "native-runtime");
|
||||
mkdirSync(offenderDir, { recursive: true });
|
||||
writeFileSync(
|
||||
path.join(offenderDir, "paperclip-runner-tool-authority.ts"),
|
||||
[
|
||||
'import { createLocalAgentJwt } from "../../agent-auth-jwt.js";',
|
||||
"",
|
||||
"const token = createLocalAgentJwt(agentId, companyId, adapterType, runId, responsibleUserId);",
|
||||
"",
|
||||
].join("\n"),
|
||||
);
|
||||
const offenders = findMintSiteOffenders(root);
|
||||
expect(offenders).toEqual(["services/native-runtime/paperclip-runner-tool-authority.ts:3"]);
|
||||
} finally {
|
||||
rmSync(root, { recursive: true, force: true });
|
||||
}
|
||||
});
|
||||
|
||||
it("exempts the wrapper file and unit tests from the scan", () => {
|
||||
const root = mkdtempSync(path.join(os.tmpdir(), "run-bearer-pinning-allow-"));
|
||||
try {
|
||||
mkdirSync(path.join(root, "services"), { recursive: true });
|
||||
writeFileSync(
|
||||
path.join(root, "services", "run-bearer.ts"),
|
||||
'const token = createLocalAgentJwt(agentId, companyId, adapterType, runId, responsibleUserId, keyScope);\n',
|
||||
);
|
||||
mkdirSync(path.join(root, "__tests__"), { recursive: true });
|
||||
writeFileSync(
|
||||
path.join(root, "__tests__", "run-bearer.test.ts"),
|
||||
'const token = createLocalAgentJwt("agent-1", "company-1", "claude_local", "run-1");\n',
|
||||
);
|
||||
writeFileSync(
|
||||
path.join(root, "agent-auth-jwt.ts"),
|
||||
"export function createLocalAgentJwt(agentId, companyId, adapterType, runId) {}\n",
|
||||
);
|
||||
expect(findMintSiteOffenders(root)).toEqual([]);
|
||||
} finally {
|
||||
rmSync(root, { recursive: true, force: true });
|
||||
}
|
||||
});
|
||||
});
|
||||
@@ -1,8 +1,10 @@
|
||||
import { createHash } from "node:crypto";
|
||||
import { beforeEach, describe, expect, it, vi } from "vitest";
|
||||
import { REDACTED_EVENT_VALUE } from "../redaction.js";
|
||||
import type { Db } from "@paperclipai/db";
|
||||
import { PgDialect } from "drizzle-orm/pg-core";
|
||||
import { createRunSecretRedactionRegistry, redactRegisteredSecretValues } from "../services/run-secret-redaction.js";
|
||||
import { mintAndRegisterRunBearer } from "../services/run-bearer.js";
|
||||
|
||||
const secret = "q2a-exact-secret-value";
|
||||
|
||||
@@ -79,8 +81,18 @@ describe("registered run secret redaction", () => {
|
||||
});
|
||||
});
|
||||
|
||||
const { resolveVersion } = vi.hoisted(() => ({ resolveVersion: vi.fn(async ({ material }) => material.value as string) }));
|
||||
vi.mock("../secrets/provider-registry.js", () => ({ getSecretProvider: () => ({ resolveVersion }) }));
|
||||
const { resolveVersion, createSecret } = vi.hoisted(() => ({
|
||||
resolveVersion: vi.fn(async ({ material }) => material.value as string),
|
||||
createSecret: vi.fn(async ({ value }: { value: string }) => ({
|
||||
material: { value },
|
||||
valueSha256: createHash("sha256").update(value).digest("hex"),
|
||||
externalRef: null,
|
||||
})),
|
||||
}));
|
||||
vi.mock("../secrets/provider-registry.js", () => ({ getSecretProvider: () => ({ resolveVersion, createSecret }) }));
|
||||
|
||||
const { mintLocalAgentJwt } = vi.hoisted(() => ({ mintLocalAgentJwt: vi.fn() }));
|
||||
vi.mock("../agent-auth-jwt.js", () => ({ createLocalAgentJwt: mintLocalAgentJwt }));
|
||||
|
||||
describe("batched run secret redaction", () => {
|
||||
beforeEach(() => { resolveVersion.mockClear(); });
|
||||
@@ -125,3 +137,99 @@ describe("batched run secret redaction", () => {
|
||||
await expect(registry.redactForRuns("company", [{ id: "a", text: secret }])).rejects.toThrow("unavailable");
|
||||
});
|
||||
});
|
||||
|
||||
// D8.1 — mint and register. `mintAndRegisterRunBearer` (server/src/services/run-bearer.js)
|
||||
// is the wrapper every mint site must call instead of `createLocalAgentJwt`
|
||||
// directly, so a minted bearer always reaches the redaction registry before a
|
||||
// caller can use it.
|
||||
describe("mintAndRegisterRunBearer", () => {
|
||||
const companyId = "company-1";
|
||||
const runId = "run-1";
|
||||
|
||||
beforeEach(() => {
|
||||
mintLocalAgentJwt.mockReset();
|
||||
createSecret.mockClear();
|
||||
});
|
||||
|
||||
// A fake `Db` that only implements the `transaction` shape `register()`
|
||||
// needs: a locked select of the run row, then an update of its
|
||||
// `contextSnapshot`. `row` is the current row seen inside the transaction,
|
||||
// or `null` to exercise the missing-run failure path.
|
||||
function fakeDb(row: { contextSnapshot: unknown } | null) {
|
||||
const selectWhere = vi.fn(() => ({ for: () => Promise.resolve(row ? [row] : []) }));
|
||||
const updateWhere = vi.fn(async () => {});
|
||||
const tx = {
|
||||
select: () => ({ from: () => ({ where: selectWhere }) }),
|
||||
update: () => ({ set: (values: unknown) => ({ where: (predicate: unknown) => updateWhere(values, predicate) }) }),
|
||||
};
|
||||
const db = { transaction: async (callback: (tx: unknown) => Promise<unknown>) => callback(tx) };
|
||||
return { db: db as unknown as Db, selectWhere, updateWhere };
|
||||
}
|
||||
|
||||
it("registers the minted bearer against the given company and run, then returns it", async () => {
|
||||
const token = "minted-run-bearer-token";
|
||||
mintLocalAgentJwt.mockReturnValue(token);
|
||||
const { db, selectWhere, updateWhere } = fakeDb({ contextSnapshot: {} });
|
||||
|
||||
const result = await mintAndRegisterRunBearer(db, "agent-1", companyId, "claude_local", runId, "user-1");
|
||||
|
||||
expect(result).toBe(token);
|
||||
expect(selectWhere).toHaveBeenCalledTimes(1);
|
||||
const dialect = new PgDialect();
|
||||
const selectPredicate = dialect.sqlToQuery(selectWhere.mock.calls[0][0] as Parameters<typeof dialect.sqlToQuery>[0]);
|
||||
expect(selectPredicate.params).toContain(companyId);
|
||||
expect(selectPredicate.params).toContain(runId);
|
||||
|
||||
expect(updateWhere).toHaveBeenCalledTimes(1);
|
||||
const [values] = updateWhere.mock.calls[0] as [{ contextSnapshot: { paperclipSecretRedactions: Array<{ fingerprintSha256: string }> } }, unknown];
|
||||
expect(values.contextSnapshot.paperclipSecretRedactions).toHaveLength(1);
|
||||
expect(values.contextSnapshot.paperclipSecretRedactions[0].fingerprintSha256)
|
||||
.toBe(createHash("sha256").update(token).digest("hex"));
|
||||
expect(createSecret).toHaveBeenCalledWith({ value: token });
|
||||
});
|
||||
|
||||
it("passes the sixth scope argument through to the mint call", async () => {
|
||||
mintLocalAgentJwt.mockReturnValue("token-with-scope");
|
||||
const { db } = fakeDb({ contextSnapshot: {} });
|
||||
const scope = { kind: "skill_test" as const, issueId: "issue-1" };
|
||||
|
||||
await mintAndRegisterRunBearer(db, "agent-1", companyId, "claude_local", runId, "user-1", scope);
|
||||
|
||||
expect(mintLocalAgentJwt).toHaveBeenCalledWith("agent-1", companyId, "claude_local", runId, "user-1", scope);
|
||||
});
|
||||
|
||||
it("returns null and registers nothing on the null-mint configuration path", async () => {
|
||||
mintLocalAgentJwt.mockReturnValue(null);
|
||||
const { db, selectWhere, updateWhere } = fakeDb({ contextSnapshot: {} });
|
||||
|
||||
const result = await mintAndRegisterRunBearer(db, "agent-1", companyId, "claude_local", runId, "user-1");
|
||||
|
||||
expect(result).toBeNull();
|
||||
expect(selectWhere).not.toHaveBeenCalled();
|
||||
expect(updateWhere).not.toHaveBeenCalled();
|
||||
});
|
||||
|
||||
it("propagates a registration failure and returns no token", async () => {
|
||||
mintLocalAgentJwt.mockReturnValue("token-that-cannot-register");
|
||||
// `row: null` reproduces the registry's own missing-run failure
|
||||
// (server/src/services/run-secret-redaction.ts:103).
|
||||
const { db, updateWhere } = fakeDb(null);
|
||||
|
||||
await expect(
|
||||
mintAndRegisterRunBearer(db, "agent-1", companyId, "claude_local", runId, "user-1"),
|
||||
).rejects.toThrow("Heartbeat run redaction registration failed");
|
||||
expect(updateWhere).not.toHaveBeenCalled();
|
||||
});
|
||||
|
||||
it("never surfaces the minted token text in the registration failure", async () => {
|
||||
mintLocalAgentJwt.mockReturnValue("must-not-leak-in-error-token");
|
||||
const { db } = fakeDb(null);
|
||||
|
||||
await expect(
|
||||
mintAndRegisterRunBearer(db, "agent-1", companyId, "claude_local", runId, "user-1"),
|
||||
).rejects.toSatisfy((error: unknown) => {
|
||||
const message = error instanceof Error ? error.message : String(error);
|
||||
return !message.includes("must-not-leak-in-error-token");
|
||||
});
|
||||
});
|
||||
});
|
||||
Reference in new issue
Block a user