From f3144b17ca4676309ca425718cb09fcfd372ff60 Mon Sep 17 00:00:00 2001 From: nickyleach <331803+nickyleach@users.noreply.github.com> Date: Wed, 16 Sep 2026 07:31:37 +0000 Subject: [PATCH] fix(server): mask a registered bearer by fingerprint on every registry read The redaction registry only replaced the exact stored value of a run's own registrations. A bearer minted on one run and pasted into another run's or issue's text stayed unmasked, and a row swept clear of its material could no longer resolve any plain text to match against. Add a second pass to the three read functions. It finds each JWT-shaped candidate in the value, hashes it, and checks the hash against the company's registry. A match needs no decryption, because the candidate found in the text already is the plain text, so a row whose material the sweep cleared still masks. The lookup scopes to the company only, so it also covers a value pasted across runs and issues. It batches its query over the whole candidate set instead of querying once per run or per candidate. Co-authored-by: Paperclip --- .../run-secret-redactions-migration.test.ts | 9 ++ .../__tests__/run-secret-redaction.test.ts | 88 +++++++++++- server/src/services/run-secret-redaction.ts | 126 +++++++++++++++++- 3 files changed, 217 insertions(+), 6 deletions(-) diff --git a/packages/db/src/run-secret-redactions-migration.test.ts b/packages/db/src/run-secret-redactions-migration.test.ts index f19c632843d..7e6045d9a9a 100644 --- a/packages/db/src/run-secret-redactions-migration.test.ts +++ b/packages/db/src/run-secret-redactions-migration.test.ts @@ -35,6 +35,15 @@ d("run_secret_redactions migration", () => { const planText = plan.map((r) => Object.values(r)[0]).join("\n"); expect(planText).toContain("run_secret_redactions_company_fingerprint_run_uq"); + // A candidate-fingerprint lookup issues an `IN` list over many digests in + // one query, not one query per digest. Prove that shape also uses the + // index, not only the single-digest shape above. + const inListPlan = await sql.unsafe( + "EXPLAIN SELECT id FROM run_secret_redactions WHERE company_id = '00000000-0000-0000-0000-000000000001' AND fingerprint_sha256 IN ('x', 'y', 'z')", + ); + const inListPlanText = inListPlan.map((r) => Object.values(r)[0]).join("\n"); + expect(inListPlanText).toContain("run_secret_redactions_company_fingerprint_run_uq"); + const runLookupPlan = await sql.unsafe( "EXPLAIN SELECT id FROM run_secret_redactions WHERE company_id = '00000000-0000-0000-0000-000000000001' AND run_id = '00000000-0000-0000-0000-000000000002'", ); diff --git a/server/src/__tests__/run-secret-redaction.test.ts b/server/src/__tests__/run-secret-redaction.test.ts index 74f4c875e5b..bbb8f22c936 100644 --- a/server/src/__tests__/run-secret-redaction.test.ts +++ b/server/src/__tests__/run-secret-redaction.test.ts @@ -1,6 +1,6 @@ import { createHash, randomUUID } from "node:crypto"; import { and, eq } from "drizzle-orm"; -import { afterAll, afterEach, beforeAll, describe, expect, it } from "vitest"; +import { afterAll, afterEach, beforeAll, describe, expect, it, vi } from "vitest"; import type { Db } from "@paperclipai/db"; import { agents, @@ -335,4 +335,90 @@ describeEmbeddedPostgres("run secret redaction registry (company-scoped table)", expect(row.material).not.toBeNull(); }); }); + + describe("fingerprint match masking", () => { + // Shapes a JWT-like candidate: three non-empty base64url segments joined + // by two dots, the same shape a run bearer has. + function jwtShaped(seed: string): string { + const segment = (label: string) => Buffer.from(`${label}-${seed}`).toString("base64url"); + return `${segment("header")}.${segment("payload")}.${segment("sig")}`; + } + + it("masks a bearer registered on one issue's run when it is pasted into a different issue's text", async () => { + const { companyId, agentId, runId: runOnIssueX } = await seedRun(); + await db.update(heartbeatRuns).set({ contextSnapshot: { issueId: "issue-x" } }) + .where(eq(heartbeatRuns.id, runOnIssueX)); + const runOnIssueY = randomUUID(); + await db.insert(heartbeatRuns).values({ + id: runOnIssueY, companyId, agentId, status: "running", contextSnapshot: { issueId: "issue-y" }, + }); + const registry = createRunSecretRedactionRegistry(db); + const bearer = jwtShaped("cross-issue"); + await registry.register(companyId, runOnIssueX, bearer, farFutureRedactionExpiry()); + + const result = await registry.redactForIssue(companyId, "issue-y", `pasted ${bearer} here`); + + expect(result).toBe(`pasted ${REDACTED_EVENT_VALUE} here`); + }); + + it("still masks a bearer after the expiry sweep has cleared its row's material", async () => { + const { companyId, runId } = await seedRun(); + const registry = createRunSecretRedactionRegistry(db); + const bearer = jwtShaped("fingerprint-only"); + await registry.register(companyId, runId, bearer, new Date(0)); + + await createRunSecretRedactionReaper(db).sweep(); + + const [row] = await rowsForRun(companyId, runId); + expect(row.material).toBeNull(); + expect(await registry.redactForRun(companyId, runId, `still has ${bearer}`)) + .toBe(`still has ${REDACTED_EVENT_VALUE}`); + }); + + it("never matches a fingerprint registered by another company", async () => { + const companyA = await seedRun(); + const companyB = await seedRun(); + const registry = createRunSecretRedactionRegistry(db); + const bearer = jwtShaped("cross-company"); + await registry.register(companyA.companyId, companyA.runId, bearer, farFutureRedactionExpiry()); + + expect(await registry.redactForRun(companyB.companyId, companyB.runId, `token ${bearer} here`)) + .toBe(`token ${bearer} here`); + }); + + it("leaves an unregistered JWT-shaped string and a literal environment variable reference unchanged", async () => { + const { companyId, runId } = await seedRun(); + const registry = createRunSecretRedactionRegistry(db); + const unregistered = jwtShaped("never-registered"); + + expect(await registry.redactForRun(companyId, runId, `${unregistered} and $PAPERCLIP_API_KEY`)) + .toBe(`${unregistered} and $PAPERCLIP_API_KEY`); + }); + + it("issues a bounded number of queries for a batch of many runs and still applies each run's own value pass", async () => { + const { companyId, agentId } = await seedRun(); + const registry = createRunSecretRedactionRegistry(db); + const runIds = await Promise.all(Array.from({ length: 20 }, async () => { + const id = randomUUID(); + await db.insert(heartbeatRuns).values({ id, companyId, agentId, status: "running", contextSnapshot: {} }); + return id; + })); + const sharedBearer = jwtShaped("batch-shared"); + await registry.register(companyId, runIds[0]!, sharedBearer, farFutureRedactionExpiry()); + await registry.register(companyId, runIds[1]!, "own-value-secret", farFutureRedactionExpiry()); + + const selectSpy = vi.spyOn(db, "select"); + const runs = runIds.map((id, index) => ({ + id, + text: index === 1 ? `has own-value-secret and ${sharedBearer}` : `only ${sharedBearer}`, + })); + + const redacted = await registry.redactForRuns(companyId, runs); + + expect(selectSpy.mock.calls.length).toBeLessThan(runIds.length); + expect(redacted.every((run) => !run.text.includes(sharedBearer))).toBe(true); + expect(redacted[1]!.text).not.toContain("own-value-secret"); + selectSpy.mockRestore(); + }); + }); }); diff --git a/server/src/services/run-secret-redaction.ts b/server/src/services/run-secret-redaction.ts index bd6f08f4e42..d050c4c2cd4 100644 --- a/server/src/services/run-secret-redaction.ts +++ b/server/src/services/run-secret-redaction.ts @@ -43,6 +43,56 @@ function redactText(input: string, values: string[]) { ); } +// A JWT-shaped candidate: three non-empty base64url segments joined by two +// dot characters. A registered run bearer that appears in text is the plain +// text of that bearer, so this pass hashes each candidate it finds and looks +// up the hash. It never decrypts stored material to find a match. +const JWT_CANDIDATE_RE = /[A-Za-z0-9_-]+\.[A-Za-z0-9_-]+\.[A-Za-z0-9_-]+/g; + +// A caller may supply a batch of many runs, or one run with a large payload. +// This bounds each fingerprint lookup to a fixed number of query round trips +// instead of one round trip per candidate. +const FINGERPRINT_QUERY_BATCH_SIZE = 1000; + +function collectJwtCandidates(value: unknown, into: Set): void { + if (typeof value === "string") { + for (const match of value.matchAll(JWT_CANDIDATE_RE)) into.add(match[0]); + return; + } + if (Array.isArray(value)) { + for (const item of value) collectJwtCandidates(item, into); + return; + } + if (value instanceof Date) return; + const record = asRecord(value); + if (!record) return; + for (const [key, entry] of Object.entries(record)) { + if (key === REGISTRY_KEY) continue; + collectJwtCandidates(entry, into); + } +} + +// Runs after the value pass has already rebuilt the object graph and +// stripped the legacy registry key, so this pass does not need to repeat +// that filter. It only replaces a candidate substring whose fingerprint +// matched; every other character stays exactly as the value pass left it. +function replaceMatchedCandidates(input: T, matched: Set): T { + if (matched.size === 0) return input; + if (typeof input === "string") { + return input.replace( + JWT_CANDIDATE_RE, + (candidate) => matched.has(candidate) ? REDACTED_EVENT_VALUE : candidate, + ) as T; + } + if (Array.isArray(input)) return input.map((item) => replaceMatchedCandidates(item, matched)) as T; + if (input instanceof Date) return input; + const record = asRecord(input); + if (!record) return input; + return Object.fromEntries( + Object.entries(record).map(([key, value]) => [key, replaceMatchedCandidates(value, matched)]), + ) as T; +} + // A proposed or bound company secret value carries no token-style expiry // claim the way a run bearer does, and the pre-move registry masked such a // value for as long as its run row existed. This far horizon keeps that same @@ -146,6 +196,67 @@ export function createRunSecretRedactionRegistry(db: Db) { return resolveEntries(entries, new Map()); } + // Looks up a company-scoped set of digests against the registry, in query + // batches bounded to `FINGERPRINT_QUERY_BATCH_SIZE`. The lookup filters on + // `company_id` only, so a registered value masks wherever it appears in + // that company's text, not only in the run or issue text that registered + // it. It never filters on `run_id`, and it never matches a row of another + // company. + async function matchedFingerprints(companyId: string, digests: string[]): Promise> { + const matched = new Set(); + for (let start = 0; start < digests.length; start += FINGERPRINT_QUERY_BATCH_SIZE) { + const batch = digests.slice(start, start + FINGERPRINT_QUERY_BATCH_SIZE); + const rows = await db.select({ fingerprintSha256: runSecretRedactions.fingerprintSha256 }) + .from(runSecretRedactions) + .where(and( + eq(runSecretRedactions.companyId, companyId), + inArray(runSecretRedactions.fingerprintSha256, batch), + )); + for (const row of rows) matched.add(row.fingerprintSha256); + } + return matched; + } + + // Walks a value for JWT-shaped candidates, hashes each distinct candidate + // once, and masks only the candidates whose hash the registry holds. A + // fingerprint-only row (its material cleared by the expiry sweep) still + // masks here, because this pass needs no decrypted plain text: the + // candidate found in the value already is the plain text. + async function maskByFingerprint(companyId: string, value: T): Promise { + const candidates = new Set(); + collectJwtCandidates(value, candidates); + if (candidates.size === 0) return value; + const digestByCandidate = new Map( + [...candidates].map((candidate) => [candidate, createHash("sha256").update(candidate).digest("hex")] as const), + ); + const digests = [...new Set(digestByCandidate.values())]; + const matched = await matchedFingerprints(companyId, digests); + if (matched.size === 0) return value; + const matchedCandidates = new Set( + [...digestByCandidate.entries()].filter(([, digest]) => matched.has(digest)).map(([candidate]) => candidate), + ); + return replaceMatchedCandidates(value, matchedCandidates); + } + + // The batch form of `maskByFingerprint`: it collects the candidates of the + // whole batch of values first, then issues one bounded set of queries for + // the whole batch, instead of one query per value. + async function maskManyByFingerprint(companyId: string, values: T[]): Promise { + const candidates = new Set(); + for (const value of values) collectJwtCandidates(value, candidates); + if (candidates.size === 0) return values; + const digestByCandidate = new Map( + [...candidates].map((candidate) => [candidate, createHash("sha256").update(candidate).digest("hex")] as const), + ); + const digests = [...new Set(digestByCandidate.values())]; + const matched = await matchedFingerprints(companyId, digests); + if (matched.size === 0) return values; + const matchedCandidates = new Set( + [...digestByCandidate.entries()].filter(([, digest]) => matched.has(digest)).map(([candidate]) => candidate), + ); + return values.map((value) => replaceMatchedCandidates(value, matchedCandidates)); + } + return { // `expiresAt` must be the real expiry of the value, not a guess. For a // run bearer, this is the token's own JWT `exp`. The later sweep clears @@ -197,11 +308,16 @@ export function createRunSecretRedactionRegistry(db: Db) { const entries = [...(legacyByRun.get(runId) ?? []), ...tableEntries(tableByRun.get(runId) ?? [])]; return [runId, await resolveEntries(entries, resolved)] as const; }))); - return runs.map((run) => redactRegisteredSecretValues(run, valuesByRun.get(run.id) ?? [])); + const valuePass = runs.map((run) => redactRegisteredSecretValues(run, valuesByRun.get(run.id) ?? [])); + return maskManyByFingerprint(companyId, valuePass); + }, + redactForRun: async (companyId: string, runId: string, value: T): Promise => { + const valuePass = redactRegisteredSecretValues(value, await valuesForRun(companyId, runId)); + return maskByFingerprint(companyId, valuePass); + }, + redactForIssue: async (companyId: string, issueId: string, value: T): Promise => { + const valuePass = redactRegisteredSecretValues(value, await valuesForIssue(companyId, issueId)); + return maskByFingerprint(companyId, valuePass); }, - redactForRun: async (companyId: string, runId: string, value: T): Promise => - redactRegisteredSecretValues(value, await valuesForRun(companyId, runId)), - redactForIssue: async (companyId: string, issueId: string, value: T): Promise => - redactRegisteredSecretValues(value, await valuesForIssue(companyId, issueId)), }; }