mirror of
https://github.com/paperclipai/paperclip.git
synced 2026-10-28 20:21:50 +01:00
fix(server): mask a bearer glued to a neighbour by a hyphen or underscore
The candidate scanner found a maximal chain of base64url segments joined by dots, then offered every window of three consecutive segments in the chain. A neighbour word ending in a hyphen or underscore joined into the chain's first segment with no dot to mark the join, so the bearer's own first segment was never one of the chain's segments and the digest lookup never ran on it. The scanner now also offers a candidate that starts right after a hyphen or an underscore inside the chain's first segment. Only the first segment of a chain can carry this kind of glued prefix, since every later segment starts right after a literal dot. The extra candidate count per chain is bounded by the length of the first segment. The replacement now splices a match that starts inside a segment, so the literal text in front of it survives byte for byte. Also remove the internal tracker references the four preceding commits on this branch left in test names and comments. Co-authored-by: Paperclip <noreply@paperclip.ing>
This commit is contained in:
1 parent
ba465523ea
commit
8fbc49de8b
4 files changed
+124
-19
No files matched your search
@@ -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");
|
||||
|
||||
@@ -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<typeof createDb>;
|
||||
let svc!: ReturnType<typeof issueService>;
|
||||
|
||||
@@ -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.<bearer>" then offered
|
||||
// "paperclip.local.<header>" 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"))
|
||||
|
||||
@@ -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-<bearer>") 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.<bearer>")
|
||||
// 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>): 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<string>): 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-<bearer>"
|
||||
// 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>): 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>): 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(".");
|
||||
});
|
||||
|
||||
Reference in new issue
Block a user