diff --git a/server/src/__tests__/issue-comment-redaction.test.ts b/server/src/__tests__/issue-comment-redaction.test.ts index e32ddd00f48..4c092d36e0d 100644 --- a/server/src/__tests__/issue-comment-redaction.test.ts +++ b/server/src/__tests__/issue-comment-redaction.test.ts @@ -591,10 +591,9 @@ describeEmbeddedPostgres("deleted issue comment redaction", () => { ).toEqual([]); }); - // Write-time bearer redaction (PAP-6528). Step C already proves the - // read-mask side of this fingerprint registry in - // run-secret-redaction.test.ts; these tests cover the write side: - // addComment() must redact a registered run bearer out of an + // Write-time bearer redaction. run-secret-redaction.test.ts already proves + // the read-mask side of this fingerprint registry; these tests cover the + // write side: addComment() must redact a registered run bearer out of an // agent-authored body before the row is written. async function seedAgentRun(companyId: string) { const agentId = randomUUID(); @@ -624,7 +623,7 @@ describeEmbeddedPostgres("deleted issue comment redaction", () => { return `${header}.${payload}.${signature}`; } - it("redacts a registered run bearer from an agent-authored comment body before it is written (PAP-6528)", async () => { + it("redacts a registered run bearer from an agent-authored comment body before it is written", async () => { const { companyId, issueId } = await seedIssue(); const { agentId, runId } = await seedAgentRun(companyId); const bearer = jwtShapedBearer("comment-agent"); @@ -644,7 +643,7 @@ describeEmbeddedPostgres("deleted issue comment redaction", () => { expect(stored?.body).toBe(`Authorization: Bearer ${REDACTED_EVENT_VALUE}`); }); - it("does not redact a human-authored comment body carrying the same bearer shape (out of scope for PAP-6528)", async () => { + it("does not redact a human-authored comment body carrying the same bearer shape", async () => { const { companyId, issueId } = await seedIssue(); const { runId } = await seedAgentRun(companyId); const bearer = jwtShapedBearer("comment-human"); diff --git a/server/src/__tests__/issues-service.test.ts b/server/src/__tests__/issues-service.test.ts index 88d7a716e81..dc0deba3294 100644 --- a/server/src/__tests__/issues-service.test.ts +++ b/server/src/__tests__/issues-service.test.ts @@ -7228,11 +7228,10 @@ describeEmbeddedPostgres("issueService.addComment createdByRunId", () => { }); }); -// Write-time redaction of an agent-authored issue description (PAP-6528). -// `create` and `update` are the only two writers of `issues.description` -// (the child-create helper and the accepted-plan-decomposition path both -// funnel through `create`), so covering these two methods covers every -// writer. +// Write-time redaction of an agent-authored issue description. `create` and +// `update` are the only two writers of `issues.description` (the +// child-create helper and the accepted-plan-decomposition path both funnel +// through `create`), so covering these two methods covers every writer. describeEmbeddedPostgres("issueService write-time run bearer redaction", () => { let db!: ReturnType; let svc!: ReturnType; diff --git a/server/src/__tests__/run-secret-redaction.test.ts b/server/src/__tests__/run-secret-redaction.test.ts index 8d0d8acd313..062e53866e9 100644 --- a/server/src/__tests__/run-secret-redaction.test.ts +++ b/server/src/__tests__/run-secret-redaction.test.ts @@ -387,7 +387,7 @@ describeEmbeddedPostgres("run secret redaction registry (company-scoped table)", .toBe(`token ${bearer} here`); }); - it("masks a bearer whose chain a dotted prefix word joins into a longer run (PAP-6528)", async () => { + it("masks a bearer whose chain a dotted prefix word joins into a longer run", async () => { // Before the fix, the scanner took only the first three dot-joined // segments of a chain. "paperclip.local." then offered // "paperclip.local.
" as the sole candidate, and the real @@ -403,6 +403,32 @@ describeEmbeddedPostgres("run secret redaction registry (company-scoped table)", expect(result).toBe(`paperclip.local.${REDACTED_EVENT_VALUE}`); }); + it("masks a bearer glued to a preceding word by a hyphen and keeps the word's text before the match", async () => { + // Before the fix, the scanner found only whole dot-joined segments. + // The hyphen glued the word "run-" onto the bearer's first segment, + // so the bearer's own first segment was never a segment of the + // chain, and the digest lookup never ran on it. + const { companyId, runId } = await seedRun(); + const registry = createRunSecretRedactionRegistry(db); + const bearer = jwtShaped("hyphen-glued"); + await registry.register(companyId, runId, bearer, farFutureRedactionExpiry()); + + const result = await registry.redactForRun(companyId, runId, `run-${bearer} finished`); + + expect(result).toBe(`run-${REDACTED_EVENT_VALUE} finished`); + }); + + it("masks a bearer glued to a preceding word by an underscore and keeps the word's text before the match", async () => { + const { companyId, runId } = await seedRun(); + const registry = createRunSecretRedactionRegistry(db); + const bearer = jwtShaped("underscore-glued"); + await registry.register(companyId, runId, bearer, farFutureRedactionExpiry()); + + const result = await registry.redactForRun(companyId, runId, `tok_${bearer} finished`); + + expect(result).toBe(`tok_${REDACTED_EVENT_VALUE} finished`); + }); + it("leaves an unregistered JWT-shaped string and a literal environment variable reference unchanged", async () => { const { companyId, runId } = await seedRun(); const registry = createRunSecretRedactionRegistry(db); @@ -440,9 +466,9 @@ describeEmbeddedPostgres("run secret redaction registry (company-scoped table)", }); // The write-time redactor for an agent-authored comment body or issue - // description (PAP-6528). Step C already proves the cross-issue paste and - // the fingerprint-only row above; these tests cover the two write-time - // arms and the text a false positive must never touch. + // description. The read mask tests above already prove the cross-issue + // paste and the fingerprint-only row cases; these tests cover the two + // write-time arms and the text a false positive must never touch. describe("redactAuthoredRunBearer (write-time redaction)", () => { function jwtShaped(seed: string): string { const segment = (label: string) => Buffer.from(`${label}-${seed}`).toString("base64url"); @@ -479,6 +505,28 @@ describeEmbeddedPostgres("run secret redaction registry (company-scoped table)", expect(result).toBe(`Authorization: Bearer ${REDACTED_EVENT_VALUE}`); }); + it("redacts a registered bearer glued to a preceding word by a hyphen, keeping the word's text before the match", async () => { + const { companyId, runId } = await seedRun(); + const registry = createRunSecretRedactionRegistry(db); + const bearer = jwtShaped("write-hyphen-glued"); + await registry.register(companyId, runId, bearer, farFutureRedactionExpiry()); + + const result = await redactAuthoredRunBearer(db, companyId, `see run-${bearer} in the log`); + + expect(result).toBe(`see run-${REDACTED_EVENT_VALUE} in the log`); + }); + + it("redacts a registered bearer glued to a preceding word by an underscore, keeping the word's text before the match", async () => { + const { companyId, runId } = await seedRun(); + const registry = createRunSecretRedactionRegistry(db); + const bearer = jwtShaped("write-underscore-glued"); + await registry.register(companyId, runId, bearer, farFutureRedactionExpiry()); + + const result = await redactAuthoredRunBearer(db, companyId, `see tok_${bearer} in the log`); + + expect(result).toBe(`see tok_${REDACTED_EVENT_VALUE} in the log`); + }); + it("leaves an unexpanded environment variable, its brace form, and the exact incident-report line unchanged", async () => { const companyId = randomUUID(); @@ -501,7 +549,7 @@ describeEmbeddedPostgres("run secret redaction registry (company-scoped table)", .resolves.toBe(REDACTED_EVENT_VALUE); }); - it("leaves an ordinary dotted identifier and a file name unchanged (PAP-6528 D4)", async () => { + it("leaves an ordinary dotted identifier and a file name unchanged", async () => { const companyId = randomUUID(); await expect(redactAuthoredRunBearer(db, companyId, "com.example.Foo.bar")) diff --git a/server/src/services/run-secret-redaction.ts b/server/src/services/run-secret-redaction.ts index b1dd6f2986e..59d1c6acb7a 100644 --- a/server/src/services/run-secret-redaction.ts +++ b/server/src/services/run-secret-redaction.ts @@ -60,21 +60,63 @@ const JWT_CHAIN_RE = /[A-Za-z0-9_-]+(?:\.[A-Za-z0-9_-]+){2,}/g; // instead of one round trip per candidate. const FINGERPRINT_QUERY_BATCH_SIZE = 1000; +// The base64url alphabet includes the hyphen and the underscore, so a word +// right before a bearer that ends in one of those characters (for example +// "run-") joins into the chain's first segment with no dot to mark +// the join. This returns every offset right after a hyphen or an underscore +// inside `segment`, so a caller can try the text starting there as an +// alternate first segment. The count is bounded by the segment's own +// length, at most one extra offset per delimiter character in it. +function offsetsAfterDelimiter(segment: string): number[] { + const offsets: number[] = []; + for (let index = 0; index < segment.length - 1; index += 1) { + if (segment[index] === "-" || segment[index] === "_") offsets.push(index + 1); + } + return offsets; +} + // Every window of three consecutive segments inside one maximal chain. A // dotted word right before a bearer (for example "paperclip.local.") // joins into one longer chain, so the bearer's own three segments are not // always the chain's first three. Offering every window, not only the // first, closes that gap. No window is dropped here: a later pass bounds // its database lookups, but this candidate set itself stays unbounded. +// +// Only the chain's own first segment can carry a glued, undelimited prefix: +// every later segment starts right after a literal dot, a real boundary +// nothing can glue across. So the extra windows below only ever replace +// segment zero, each with the text starting after one of its internal +// hyphens or underscores. The added candidate count is bounded by the +// length of the first segment, not by the number of windows already found. function jwtWindowsInChain(chain: string): string[] { const segments = chain.split("."); const windows: string[] = []; for (let start = 0; start + 3 <= segments.length; start += 1) { windows.push(segments.slice(start, start + 3).join(".")); } + if (segments.length >= 3) { + const firstSegment = segments[0]!; + for (const offset of offsetsAfterDelimiter(firstSegment)) { + windows.push([firstSegment.slice(offset), segments[1], segments[2]].join(".")); + } + } return windows; } +// Finds a match for a candidate that starts inside the chain's first +// segment, after one of its hyphens or underscores. Returns the literal +// text to keep in front of the match (for example "run-"), so a caller can +// splice the redaction marker in without disturbing that prefix. +function firstSegmentSpliceMatch(segments: string[], matched: Set): string | null { + if (segments.length < 3) return null; + const firstSegment = segments[0]!; + for (const offset of offsetsAfterDelimiter(firstSegment)) { + const candidate = [firstSegment.slice(offset), segments[1], segments[2]].join("."); + if (matched.has(candidate)) return firstSegment.slice(0, offset); + } + return null; +} + // The one candidate scanner. Both the read mask below and the write-time // redactor in `redactAuthoredRunBearer` call this, so a shape fix here // applies to both paths at once. @@ -106,6 +148,11 @@ function collectJwtCandidates(value: unknown, into: Set): void { // left to right and consuming three segments at a time on a match. Every // other character of the chain, including a dotted prefix such as // "paperclip.local.", stays exactly as it was. +// +// At index zero this also tries a match that starts inside the first +// segment, after one of its hyphens or underscores (the "run-" +// shape). The text before that offset, for example "run-", is kept byte for +// byte and only the matched suffix becomes the redaction marker. function replaceJwtWindowsInText(text: string, matched: Set): string { if (matched.size === 0) return text; return text.replace(JWT_CHAIN_RE, (chain) => { @@ -119,10 +166,22 @@ function replaceJwtWindowsInText(text: string, matched: Set): string { if (window && matched.has(window)) { tokens.push(REDACTED_EVENT_VALUE); index += 3; - } else { - tokens.push(segments[index]!); - index += 1; + continue; } + if (index === 0) { + const keptPrefix = firstSegmentSpliceMatch(segments, matched); + if (keptPrefix !== null) { + // One token, not two: `keptPrefix` and the marker sit inside the + // same original segment, with no dot between them. Pushing them + // separately would let the trailing `tokens.join(".")` insert a + // dot that was never in the source text. + tokens.push(keptPrefix + REDACTED_EVENT_VALUE); + index += 3; + continue; + } + } + tokens.push(segments[index]!); + index += 1; } return tokens.join("."); });