Skip to content

Commit 2ae5137

Browse files
TheGreatAxiosTheGreatAxios
authored
Merge pull request #1539 from corbitsdev/cl-10822-secret-prefilter-on-main
perf(plugins): skip secret scrub passes without a secret shape
2 parents 0e22649 + 049cc05 commit 2ae5137

2 files changed

Lines changed: 263 additions & 0 deletions

File tree

‎src/plugins/tool-result-secret-scrub.test.ts‎

Lines changed: 167 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1,8 +1,10 @@
11
import { describe, test, expect } from "bun:test";
22
import {
33
CREDENTIAL_REDACTION,
4+
countExpensiveSecretScrubPasses,
45
scrubSecretShapedContent,
56
scrubSecretShapedValue,
7+
secretScrubPatternHits,
68
} from "./tool-result-secret-scrub.js";
79
import { toolResultSecretScrubPlugin } from "./tool-result-secret-scrub-plugin.js";
810
import { resultTruncationPlugin } from "./result-truncation-plugin.js";
@@ -462,3 +464,168 @@ describe("toolResultSecretScrubPlugin", () => {
462464
expect(result.content.split(CREDENTIAL_REDACTION).length - 1).toBe(1);
463465
});
464466
});
467+
468+
const SENTINEL = "zzzsecretvalue";
469+
470+
function expectEnteredAndRedacted(text: string): void {
471+
expect(secretScrubPatternHits(text)).toBe(true);
472+
const passes = countExpensiveSecretScrubPasses(() => {
473+
const out = scrubSecretShapedContent(text);
474+
expect(out).not.toContain(SENTINEL);
475+
expect(out).toContain(CREDENTIAL_REDACTION);
476+
});
477+
expect(passes).toBe(1);
478+
}
479+
480+
function expectEnteredUnchanged(text: string): void {
481+
expect(secretScrubPatternHits(text)).toBe(true);
482+
const passes = countExpensiveSecretScrubPasses(() => {
483+
expect(scrubSecretShapedContent(text)).toBe(text);
484+
});
485+
expect(passes).toBe(1);
486+
}
487+
488+
function expectSkipped(text: string): void {
489+
const passes = countExpensiveSecretScrubPasses(() => {
490+
expect(scrubSecretShapedContent(text)).toBe(text);
491+
});
492+
expect(passes).toBe(0);
493+
expect(secretScrubPatternHits(text)).toBe(false);
494+
}
495+
496+
describe("secret scrub prefilter", () => {
497+
test("a large clean fixture does not enter the scrub passes", () => {
498+
const clean = "The quick brown fox jumps over the lazy dog. ".repeat(5000);
499+
expectSkipped(clean);
500+
const secret = `sk-${"a".repeat(6)}${SENTINEL}`;
501+
expectEnteredAndRedacted(`${clean}${secret}`);
502+
});
503+
504+
test("prefilter state does not skip a later secret", () => {
505+
const first = `sk-${"a".repeat(6)}${SENTINEL}`;
506+
const second = `ghp_${"b".repeat(22)}${SENTINEL}`;
507+
const passes = countExpensiveSecretScrubPasses(() => {
508+
expect(scrubSecretShapedContent(first)).not.toContain(SENTINEL);
509+
expect(scrubSecretShapedContent(second)).not.toContain(SENTINEL);
510+
});
511+
expect(passes).toBe(2);
512+
});
513+
514+
test("every scrub pattern still redacts and enters the passes", () => {
515+
const pem = (kind: string) => {
516+
const label = kind.length === 0 ? "PRIVATE KEY" : `${kind} PRIVATE KEY`;
517+
return `-----BEGIN ${label}-----\n${SENTINEL}\n-----END ${label}-----`;
518+
};
519+
for (const kind of ["", "RSA", "EC", "OPENSSH", "ENCRYPTED"]) {
520+
expectEnteredAndRedacted(pem(kind));
521+
}
522+
523+
expectEnteredAndRedacted(`prefix sk-${"a".repeat(6)}${SENTINEL} tail`);
524+
expectEnteredAndRedacted(`ghp_${"a".repeat(22)}${SENTINEL}`);
525+
expectEnteredAndRedacted(`gho_${"a".repeat(22)}${SENTINEL}`);
526+
for (const flavor of ["b", "a", "p", "r", "s"]) {
527+
expectEnteredAndRedacted(`xox${flavor}-${SENTINEL}`);
528+
}
529+
const akia = `AKIA${"A".repeat(16)}`;
530+
expect(secretScrubPatternHits(akia)).toBe(true);
531+
const akiaPasses = countExpensiveSecretScrubPasses(() => {
532+
expect(scrubSecretShapedContent(akia)).not.toContain(akia);
533+
});
534+
expect(akiaPasses).toBe(1);
535+
expectEnteredAndRedacted(`Bearer ${"a".repeat(6)}${SENTINEL}`);
536+
expectEnteredAndRedacted(`Bearer\t${"a".repeat(6)}${SENTINEL}`);
537+
expectEnteredAndRedacted(`Basic ${SENTINEL}`);
538+
expectEnteredAndRedacted(`Basic\n${SENTINEL}==`);
539+
540+
const queryKeys = [
541+
"apikey",
542+
"api_key",
543+
"api-key",
544+
"ApiKey",
545+
"accesstoken",
546+
"access_token",
547+
"access-token",
548+
"AccessToken",
549+
"token",
550+
"TOKEN",
551+
"password",
552+
"secret",
553+
"credential",
554+
];
555+
const markers = [
556+
"api",
557+
"secret",
558+
"private",
559+
"access",
560+
"auth",
561+
"token",
562+
"password",
563+
"credential",
564+
"openai",
565+
];
566+
for (const prefix of ["", "my_", "a1_", "my_app_"]) {
567+
for (const marker of markers) {
568+
queryKeys.push(`${prefix}${marker}_key`);
569+
queryKeys.push(`${prefix}${marker}_key`.toUpperCase());
570+
}
571+
}
572+
for (const key of queryKeys) {
573+
expectEnteredAndRedacted(
574+
`GET https://provider.invalid/v1?${key}=${SENTINEL}&format=json`,
575+
);
576+
expectEnteredAndRedacted(`{"${key}":"${SENTINEL}"}`);
577+
expectEnteredAndRedacted(`{ "${key}" : "${SENTINEL}" }`);
578+
}
579+
expectEnteredAndRedacted(`{"authorization":"${SENTINEL}"}`);
580+
expectEnteredAndRedacted(`{ "Authorization" : "${SENTINEL}" }`);
581+
582+
expectEnteredAndRedacted(`API_KEY=${SENTINEL}`);
583+
expectEnteredAndRedacted(`STRIPE_KEY=${SENTINEL}`);
584+
expectEnteredAndRedacted(`GITHUB_KEY=${SENTINEL}`);
585+
expectEnteredAndRedacted(`OPENAI_KEY=${SENTINEL}`);
586+
expectEnteredAndRedacted(`./app/.env:3:TOKEN=${SENTINEL}`);
587+
expectEnteredAndRedacted(`\nPASSWORD=${SENTINEL}`);
588+
expectEnteredAndRedacted(`note:PRIVATE=${SENTINEL}`);
589+
expectEnteredAndRedacted(`CREDENTIAL=${SENTINEL}`);
590+
expectEnteredAndRedacted(`AUTH=${SENTINEL}`);
591+
expectEnteredAndRedacted(`SECRET=${SENTINEL}`);
592+
expectEnteredAndRedacted(`OPENAI_KEY=proj-OPENAI_KEY-${SENTINEL}`);
593+
expectEnteredAndRedacted(
594+
`DATABASE_URL=postgres://admin:${SENTINEL}@db.internal:5432/appdb`,
595+
);
596+
expectEnteredAndRedacted(
597+
`DATABASE_URL=Server=db.internal;Password=${SENTINEL};`,
598+
);
599+
expectEnteredAndRedacted(
600+
`FOO_URI=postgres://admin:${SENTINEL}@db.internal/app`,
601+
);
602+
expectEnteredAndRedacted(`FOO_DSN=Password=${SENTINEL}`);
603+
expectEnteredAndRedacted(`FOO_CONNECTION_STRING=Password=${SENTINEL}`);
604+
});
605+
606+
test("env matches that are not secrets still enter the passes", () => {
607+
expectEnteredUnchanged("MONKEY=banana");
608+
expectEnteredUnchanged("AB=cd");
609+
expectEnteredUnchanged("DATABASE_URL=sqlite:///local.db");
610+
expectEnteredUnchanged(
611+
"REGISTRY_URL=https://registry.npmjs.org/@scope/pkg",
612+
);
613+
expectEnteredUnchanged("DOCS_URL=https://medium.com/@octocat/hello");
614+
expectEnteredUnchanged(
615+
"SUPPORT_URL=https://example.com/help?email=dev@example.com",
616+
);
617+
});
618+
619+
test("prose and storage keys do not enter the scrub passes", () => {
620+
expectSkipped(
621+
"See docs/authentication.md for how API keys are issued and rotated. No secrets in this paragraph.",
622+
);
623+
expectSkipped(
624+
"GET https://provider.invalid/v1?sort_key=created_at&partition_key=tenant&format=json",
625+
);
626+
expectSkipped(
627+
'{"partition_key":"tenant","sort_key":2,"primary_key":["id"],"cache_key":[1],"prompt_cache_key":"sess"}',
628+
);
629+
expectSkipped('{"MONKEY": "banana", "key": "ctrl+s"}');
630+
});
631+
});

‎src/plugins/tool-result-secret-scrub.ts‎

Lines changed: 96 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1,3 +1,5 @@
1+
import { AsyncLocalStorage } from "node:async_hooks";
2+
13
// Scrub secret-shaped substrings before untrusted text reaches a transcript or
24
// terminal. Complements secret-guard (path denylist) for secrets that surface
35
// in tool output and upstream diagnostics.
@@ -93,7 +95,101 @@ function replaceAll(
9395
return text.replace(pattern, replacement);
9496
}
9597

98+
// One scan before the full-text passes. Each alternative drops constraints
99+
// from exactly one pattern above, so any match of those patterns is a match
100+
// here. Extra matches are safe: they only pay for the passes. /i is required
101+
// for query and JSON case-folding; it also over-matches the case-sensitive
102+
// patterns. Not global — RegExp.test would otherwise keep lastIndex.
103+
//
104+
// PEM: every match contains "PRIVATE KEY-----".
105+
// API keys: sk- / ghp_ / gho_ / xox[baprs]- / AKIA / Bearer\s / Basic\s
106+
// (\b and minimum lengths are stricter than these literals).
107+
// Query: the key is glued to "=". access[_-]?token always contains "token=".
108+
// Marker prefixes sit in front of "{marker}_key=", so they cannot hide it.
109+
// JSON: the same keys are glued to a closing quote, optional space, and ":".
110+
// access[_-]?token therefore contains token"\s*:. authorization is JSON-only.
111+
// Prefixed marker keys still contain {marker}_key"\s*:.
112+
// ENV: every match contains (?:^|:)[A-Z][A-Z0-9_]+= with /m. The value length
113+
// check is dropped. /i makes this alternative match lowercase too.
114+
const SECRET_SHAPE_PREFILTER = new RegExp(
115+
[
116+
"PRIVATE KEY-----",
117+
"sk-",
118+
"ghp_",
119+
"gho_",
120+
String.raw`xox[baprs]-`,
121+
"AKIA",
122+
String.raw`Bearer\s`,
123+
String.raw`Basic\s`,
124+
"api[-_]?key=",
125+
"token=",
126+
"password=",
127+
"secret=",
128+
"credential=",
129+
"secret_key=",
130+
"private_key=",
131+
"access_key=",
132+
"auth_key=",
133+
"token_key=",
134+
"password_key=",
135+
"credential_key=",
136+
"openai_key=",
137+
String.raw`api[-_]?key"\s*:`,
138+
String.raw`token"\s*:`,
139+
String.raw`password"\s*:`,
140+
String.raw`secret"\s*:`,
141+
String.raw`credential"\s*:`,
142+
String.raw`authorization"\s*:`,
143+
String.raw`secret_key"\s*:`,
144+
String.raw`private_key"\s*:`,
145+
String.raw`access_key"\s*:`,
146+
String.raw`auth_key"\s*:`,
147+
String.raw`token_key"\s*:`,
148+
String.raw`password_key"\s*:`,
149+
String.raw`credential_key"\s*:`,
150+
String.raw`openai_key"\s*:`,
151+
String.raw`(?:^|:)[A-Z][A-Z0-9_]+=`,
152+
].join("|"),
153+
"im",
154+
);
155+
156+
const expensivePassCounter = new AsyncLocalStorage<{ count: number }>();
157+
158+
function noteExpensiveSecretScrubPass(): void {
159+
const counter = expensivePassCounter.getStore();
160+
if (counter !== undefined) counter.count += 1;
161+
}
162+
163+
/** Runs `run` and reports entries into the full-text scrub passes. */
164+
export function countExpensiveSecretScrubPasses(run: () => void): number {
165+
const counter = { count: 0 };
166+
expensivePassCounter.run(counter, run);
167+
return counter.count;
168+
}
169+
170+
function patternHits(pattern: RegExp, text: string): boolean {
171+
pattern.lastIndex = 0;
172+
try {
173+
return pattern.test(text);
174+
} finally {
175+
pattern.lastIndex = 0;
176+
}
177+
}
178+
179+
/** True when any scrub pattern matches. Test seam for the prefilter. */
180+
export function secretScrubPatternHits(text: string): boolean {
181+
return (
182+
patternHits(PEM_BLOCK, text) ||
183+
API_KEY_PATTERNS.some((pattern) => patternHits(pattern, text)) ||
184+
patternHits(SECRET_QUERY_PARAMETER, text) ||
185+
patternHits(JSON_CREDENTIAL_FIELD, text) ||
186+
patternHits(ENV_ASSIGNMENT, text)
187+
);
188+
}
189+
96190
export function scrubSecretShapedContent(text: string): string {
191+
if (!SECRET_SHAPE_PREFILTER.test(text)) return text;
192+
noteExpensiveSecretScrubPass();
97193
let result = text;
98194

99195
result = replaceAll(result, PEM_BLOCK, CREDENTIAL_REDACTION);

0 commit comments

Comments
 (0)