Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
9 changes: 5 additions & 4 deletions internal/githubreview/sanitize.go
Original file line number Diff line number Diff line change
Expand Up @@ -27,7 +27,7 @@ func SanitizeRemoteText(value string, limit int) TextEvidence {
limit = MaxTextBytes
}
originalBytes := len([]byte(value))
clean := outputsafe.Sanitize([]byte(value))
clean, inputRedacted := outputsafe.SanitizeWithRedaction([]byte(value))
clean = remoteImage.ReplaceAllString(clean, "[remote image omitted]")
clean = remoteLink.ReplaceAllString(clean, "$1 [remote link omitted]")
clean = remoteHTMLTag.ReplaceAllString(clean, "[remote HTML omitted]")
Expand All @@ -37,16 +37,17 @@ func SanitizeRemoteText(value string, limit int) TextEvidence {
clean = strings.TrimSpace(clean)
clean, truncated := truncateUTF8Bytes(clean, limit)
redacted := redact.String(clean)
if redacted != clean {
lateRedacted := redacted != clean
if lateRedacted {
clean = redacted
}
// A redaction marker can grow the text. Re-apply the hard byte boundary.
clean, secondTruncation := truncateUTF8Bytes(clean, limit)
truncated = truncated || secondTruncation || originalBytes > len([]byte(clean))
truncated = truncated || secondTruncation
sum := Fingerprint("remote-text", clean)
return TextEvidence{Text: clean, SHA256: sum, OriginalBytes: originalBytes,
StoredBytes: len([]byte(clean)), Truncated: truncated,
Redacted: redacted != outputsafe.Sanitize([]byte(value)), Untrusted: true}
Redacted: inputRedacted || lateRedacted, Untrusted: true}
}

func EmptyTextEvidence() TextEvidence {
Expand Down
41 changes: 41 additions & 0 deletions internal/githubreview/sanitize_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -23,6 +23,47 @@ func TestSanitizeRemoteTextMakesRemoteSyntaxInertAndBounded(t *testing.T) {
}
}

func TestSanitizeRemoteTextRedactedFlagTracksSecretRedaction(t *testing.T) {
token := "ghp_abcdefghijklmnopqrstuvwxyz0123456789"
for _, test := range []struct {
name string
value string
redacted bool
}{
{name: "token only", value: token, redacted: true},
{name: "inert link", value: "see [docs](https://example.com/doc)", redacted: false},
{name: "oversized plain text", value: strings.Repeat("a", 200), redacted: false},
} {
evidence := SanitizeRemoteText(test.value, 100)
if evidence.Redacted != test.redacted {
t.Errorf("%s: Redacted=%v, want %v (%#v)", test.name, evidence.Redacted, test.redacted, evidence)
}
}
if evidence := SanitizeRemoteText(token, 100); !strings.Contains(evidence.Text, "[REDACTED:github-token]") {
t.Fatalf("token evidence lost its redaction marker: %#v", evidence)
}
}

// Masking a credential makes the stored evidence shorter than its input, but no
// byte bound was applied. OriginalBytes/StoredBytes already carry that difference,
// so Truncated must stay reserved for the boundary actually being reached — with
// and without the second pass that re-applies it after a growing marker.
func TestSanitizeRemoteTextTruncatedFlagTracksTheByteBound(t *testing.T) {
token := "ghp_abcdefghijklmnopqrstuvwxyz0123456789"
masked := SanitizeRemoteText("github="+token, 4096)
if masked.Truncated || !masked.Redacted || masked.StoredBytes >= masked.OriginalBytes {
t.Errorf("in-bounds redaction reported truncation: %#v", masked)
}
bounded := SanitizeRemoteText(strings.Repeat("界", 100), 40)
if !bounded.Truncated {
t.Errorf("byte bound was not reported: %#v", bounded)
}
grown := SanitizeRemoteText(strings.Repeat("a", 20)+token+strings.Repeat("b", 20), 40)
if !grown.Truncated {
t.Errorf("redaction growth past the byte bound was not reported: %#v", grown)
}
}

func TestSanitizeRemoteTextPreservesUTF8Boundary(t *testing.T) {
evidence := SanitizeRemoteText(strings.Repeat("界", 10), 10)
if !evidence.Truncated || !strings.HasPrefix(strings.Repeat("界", 10), evidence.Text) ||
Expand Down
11 changes: 10 additions & 1 deletion internal/outputsafe/stream.go
Original file line number Diff line number Diff line change
Expand Up @@ -367,6 +367,15 @@ func (s *Stream) consume(value rune, output *strings.Builder) {

// Sanitize is the one-shot form used for already bounded output.
func Sanitize(data []byte) string {
text, _ := SanitizeWithRedaction(data)
return text
}

// SanitizeWithRedaction is Sanitize plus whether secret redaction changed the
// text, so callers can tell a redaction apart from control-character removal.
func SanitizeWithRedaction(data []byte) (string, bool) {
stream := &Stream{}
return redact.String(stream.Feed(data) + stream.Flush())
inert := stream.Feed(data) + stream.Flush()
redacted := redact.String(inert)
return redacted, redacted != inert
}
14 changes: 14 additions & 0 deletions internal/outputsafe/stream_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -31,6 +31,20 @@ func TestSanitizeRemovesC1UnicodeControlsAndRedactsSecrets(t *testing.T) {
}
}

func TestSanitizeWithRedactionSeparatesRedactionFromControlStripping(t *testing.T) {
secret := []byte("before\u009b31mafter token=secret-value-1234567890")
got, redacted := SanitizeWithRedaction(secret)
if !redacted || got != Sanitize(secret) {
t.Fatalf("redaction was not reported apart from stripping: %q %v", got, redacted)
}
// Removing control characters shortens the text but is not a secret redaction.
plain := []byte("before\u009b31mafter\u009dtitle")
got, redacted = SanitizeWithRedaction(plain)
if redacted || got != Sanitize(plain) {
t.Fatalf("control stripping must not report redaction: %q %v", got, redacted)
}
}

func TestRedactingStreamCarriesSecretAndPrivateKeyStateAcrossChunks(t *testing.T) {
stream := &RedactingStream{}
parts := []string{
Expand Down
Loading