diff --git a/report/publish-braintrust.ts b/report/publish-braintrust.ts index 02b7d24..9360f6f 100644 --- a/report/publish-braintrust.ts +++ b/report/publish-braintrust.ts @@ -46,6 +46,8 @@ interface AtifStep { metrics?: Record; } +const WITHHELD = "[WITHHELD]"; + interface BraintrustEvent { id: string; span_id: string; @@ -492,10 +494,45 @@ export function buildExperimentEvents( events.push(...atifEvents(trial, rowId)); } } - for (const event of events) { + const withheld = { count: 0 }; + const safeEvents = events.map( + (event) => withholdUnsafe(event, withheld) as BraintrustEvent, + ); + if (withheld.count > 0) { + console.warn( + `Withheld ${withheld.count} values that failed the redaction check`, + ); + } + for (const event of safeEvents) { assertSafeToPublish(event, `event ${event.id}`); } - return events; + return safeEvents; +} + +// Replace any string the redaction check rejects, so one unexpected value +// costs that value rather than the whole experiment. +function withholdUnsafe(value: unknown, withheld: { count: number }): unknown { + if (typeof value === "string") { + try { + assertSafeToPublish(value); + return value; + } catch { + withheld.count += 1; + return WITHHELD; + } + } + if (Array.isArray(value)) { + return value.map((entry) => withholdUnsafe(entry, withheld)); + } + if (value !== null && typeof value === "object") { + return Object.fromEntries( + Object.entries(value as Record).map(([key, entry]) => [ + key, + withholdUnsafe(entry, withheld), + ]), + ); + } + return value; } function experimentMetadata(arms: BenchmarkArm[]): Record { diff --git a/report/redact.ts b/report/redact.ts index 6547773..8dbd0e5 100644 --- a/report/redact.ts +++ b/report/redact.ts @@ -111,6 +111,8 @@ function skipSnapshotAttributes(text: string, start: number): number { return cursor; } +const METHOD_CALL = /^[A-Za-z_$][\w$]*(?:\.[A-Za-z_$][\w$]*)+\(/; + function sensitiveAssignments(text: string): SensitiveAssignment[] { const assignments: SensitiveAssignment[] = []; for (let separator = 0; separator < text.length; separator += 1) { @@ -144,6 +146,9 @@ function sensitiveAssignments(text: string): SensitiveAssignment[] { let end = valueCursor; if (quote === '"' || quote === "'") { + // A quote followed by a line break closes a label such as + // `"Email address:"`; the field has no value on this line. + if (lineEndsAfter(text, valueCursor + slashCount + 1)) continue; let openingSlashCount = 0; for (let cursor = keyStart - 2; text[cursor] === "\\"; cursor -= 1) { openingSlashCount += 1; @@ -152,9 +157,6 @@ function sensitiveAssignments(text: string): SensitiveAssignment[] { !keyHasClosingQuote && text[keyStart - 1] === quote && openingSlashCount === slashCount; - if (quotedLabel && lineEndsAfter(text, valueCursor + slashCount + 1)) { - continue; - } if (quotedLabel) { valueCursor = skipSnapshotAttributes( text, @@ -217,6 +219,8 @@ function sensitiveAssignments(text: string): SensitiveAssignment[] { } end += 1; } + // Code such as `cookie=page.getByRole(...)` is not a credential. + if (METHOD_CALL.test(text.slice(start))) continue; separator = Math.max(separator, end - 1); } @@ -550,32 +554,24 @@ function assertTypedCallsRedacted(value: string): void { function assertSensitiveAssignmentsRedacted(value: string): void { const assignmentStart = - /(?:^|\\[nrt]|[^A-Za-z0-9_-])(?:\\*["'])?([A-Za-z0-9_-]+)(?:\\*["'])?\s*[:=]\s*/gi; + /(?:^|\\[nrt]|[^A-Za-z0-9_-])(\\*["'])?([A-Za-z0-9_-]+)(\\*["'])?\s*[:=]\s*/gi; for (const match of value.matchAll(assignmentStart)) { - if (!sensitiveField(match[1])) continue; + const [, openingQuote, key, closingQuote] = match; + if (!sensitiveField(key)) continue; const remainderStart = (match.index ?? 0) + match[0].length; const remainder = value.slice(remainderStart); const valueQuote = remainder.match(/^(\\*)(["'])/); - const separator = Math.max( - match[0].lastIndexOf(":"), - match[0].lastIndexOf("="), - ); + // Like `"Password:"`: the separator sits inside a quoted label, and the + // quote after it closes the label rather than opening a value. const quotedLabel = - valueQuote && - [...match[0].slice(0, separator)].filter( - (character) => character === valueQuote[2], - ).length % - 2 === - 1; + valueQuote && !closingQuote && openingQuote?.endsWith(valueQuote[2]); if ( - quotedLabel && - lineEndsAfter( - value, - (match.index ?? 0) + match[0].length + valueQuote[0].length, - ) + valueQuote && + lineEndsAfter(value, remainderStart + valueQuote[0].length) ) { continue; } + if (!valueQuote && METHOD_CALL.test(remainder)) continue; let unquoted = remainder.replace(/^(?:\\*["'])?/, ""); if (quotedLabel && valueQuote) { let valueStart = skipSnapshotAttributes( diff --git a/report/results.test.ts b/report/results.test.ts index 67064ce..451e7ce 100644 --- a/report/results.test.ts +++ b/report/results.test.ts @@ -869,3 +869,45 @@ describe("Braintrust redaction", () => { } }); }); + +describe("redaction false positives", () => { + test("keeps labels, method calls, and quoted labels with values safe", () => { + const label = String.raw`- text: \"Email address:\"\n - textbox`; + const call = "const cookie=page.getByRole('button',{name:'Accept'});"; + for (const source of [ + label, + '- text: "Email address:"\n - textbox', + call, + ]) { + expect(redactString(source)).toBe(source); + expect(collectSensitiveValues(source)).toEqual([]); + expect(() => assertSafeToPublish(source)).not.toThrow(); + } + + const quoted = "{name:'Account password':'Hunter2Pass'}"; + const redacted = redactString(quoted); + expect(redacted).not.toContain("Hunter2Pass"); + expect(() => assertSafeToPublish(redacted)).not.toThrow(); + expect(() => assertSafeToPublish(quoted)).toThrow("sensitive field value"); + }); + + test("withholds unsafe values instead of failing the experiment", () => { + const root = fixture(); + const trajectoryPath = join( + root, + "task-one__abc", + "steps/run/agent/trajectory.json", + ); + const trajectory = JSON.parse(readFileSync(trajectoryPath, "utf8")); + trajectory.steps[0].message = 'password: [REDACTED]"StillVisible"'; + writeFileSync(trajectoryPath, JSON.stringify(trajectory)); + + const events = buildExperimentEvents( + [readBenchmarkArm({ name: "candidate", path: root })], + "test-experiment", + ); + const serialized = JSON.stringify(events); + expect(serialized).not.toContain("StillVisible"); + expect(serialized).toContain("[WITHHELD]"); + }); +});