Skip to content

Commit adf2cd9

Browse files
committed
test(agentsessions): the newline case asserts the halves survive, not just the newline
Raised by CodeRabbit. The comment said the halves "must NOT become the secret" and nothing checked it. The newline assertion alone passes just as well if the matcher spans the newline and redacts both halves as one, because the separator would survive inside a "[REDACTED]" that ate the text around it. Both halves are now named, and so is the absence of any redaction at all. The failure this guards against is over-redaction: a credential cannot contain a raw newline, so treating a newline-split pair as one destroys legitimate transcript content while protecting nothing. It is also what stops stripControl being widened to strip newlines, which would make the NUL case in the test above pass for the wrong reason. Measured, to pin the three-way distinction: newline -> "before sk-ant-api03-\nAAAA... after" not redacted, halves intact joined -> "before [REDACTED] after" NUL -> "before [REDACTED] after" stripped, so it rejoins Mutation-checked: removing the '\t'/'\n' exemption from stripControl fires all four assertions. Origin-Session: local-13d543 | Claude Code | 2 prompts Origin-Snapshot: a939509c08a8
1 parent 638ef36 commit adf2cd9

1 file changed

Lines changed: 21 additions & 1 deletion

File tree

‎internal/agentsessions/redaction_order_test.go‎

Lines changed: 21 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -95,8 +95,28 @@ func TestRedactKeepsTheSurroundingText(t *testing.T) {
9595
}
9696
// A newline separates rather than rejoins, so the halves must NOT become the
9797
// secret, and the newline itself is legitimate transcript content.
98-
split := redact("before sk-ant-api03-\n" + strings.Repeat("A", 24) + " after")
98+
//
99+
// THE COMMENT ABOVE WAS THE ONLY THING ASSERTING THE FIRST HALF OF THAT. The
100+
// newline check alone passes just as well if the matcher spans the newline
101+
// and redacts both halves as one secret — the separator would survive inside
102+
// a "[REDACTED]" that ate the text around it. Both halves are named here, and
103+
// so is the absence of any redaction at all, because the failure this guards
104+
// against is over-redaction: a credential cannot contain a raw newline, so
105+
// treating a newline-split pair as one destroys legitimate transcript content
106+
// while protecting nothing. It is also what stops stripControl being widened
107+
// to strip newlines, which would make the NUL case above pass for the wrong
108+
// reason.
109+
halves := []string{"sk-ant-api03-", strings.Repeat("A", 24)}
110+
split := redact("before " + halves[0] + "\n" + halves[1] + " after")
99111
if !strings.Contains(split, "\n") {
100112
t.Errorf("a newline was stripped from transcript text: %q", split)
101113
}
114+
for _, half := range halves {
115+
if !strings.Contains(split, half) {
116+
t.Errorf("a newline-separated half %q was consumed as part of a secret, leaving %q", half, split)
117+
}
118+
}
119+
if strings.Contains(split, "[REDACTED]") {
120+
t.Errorf("two halves separated by a newline were redacted as one secret: %q", split)
121+
}
102122
}

0 commit comments

Comments
 (0)