diff --git a/CHANGELOG.md b/CHANGELOG.md index 819a2c92..4985c7dd 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -145,6 +145,13 @@ called out in a **Breaking** section. closed rather than inheriting the committer-editable in-tree list. The `gate_lockstep` workflow parser no longer mistakes a nested `with: name:` for a step name. (The receipt-gate remains disabled outside release time.) +- **Secret-scanner serialisation hardening** (iss-65). A serialized scan finding's + snippet now masks *every* secret on its source line, not only the finding's own + token — two secrets sharing a line (a minified `.env`, collapsed JSON) no longer + leak each other into the `abcd launch --json` report. The content sniff no longer + misclassifies a valid UTF-8 file as binary when a multibyte rune straddles the + 8 KB boundary (which would have skipped scanning it), and a bundle file that + cannot be read is now surfaced in `unscanned` rather than silently dropped. ## [v0.1.0] - 2026-07-07 diff --git a/internal/adapter/scanner/scanner.go b/internal/adapter/scanner/scanner.go index afcc04d7..c3b4f371 100644 --- a/internal/adapter/scanner/scanner.go +++ b/internal/adapter/scanner/scanner.go @@ -246,10 +246,110 @@ func ScanText(text string, id Identity, patterns []Pattern, id2sev map[string]Se } } } + sealSnippets(findings) sortFindings(findings) return findings } +// sealSnippets masks EVERY finding's matched token out of the shared source line +// each finding on that line carries, so a serialized snippet cannot leak a +// sibling secret found on the same line. Finding.MarshalJSON only masks a +// finding's OWN token; without this, two secrets on one line each leak the other. +// Findings on one line always come from the same ScanText call (same file), so +// grouping by line number is sufficient. Masking is by BYTE SPAN (Column is the +// 1-based byte offset, len(Matched) the length), not substring: two matches that +// partially overlap — where a substring rewrite would leave the shorter match's +// non-overlapping tail raw — are both masked span-exactly. +func sealSnippets(findings []Finding) { + byLine := map[int][]int{} + for i := range findings { + byLine[findings[i].Line] = append(byLine[findings[i].Line], i) + } + for _, idxs := range byLine { + if len(idxs) < 2 { + continue // a lone finding: MarshalJSON already masks its own token + } + src := "" + for _, i := range idxs { + if findings[i].line != "" { + src = findings[i].line + break + } + } + if src == "" { + continue + } + sealed := sealLine(src, findings, idxs) + for _, i := range idxs { + findings[i].line = sealed + } + } +} + +// sealLine masks the byte spans of every finding in idxs out of src. A byte +// covered by exactly one span shows that token's head/tail fingerprint; a byte in +// an OVERLAP of two or more spans is forced to '*' (so no token's raw bytes leak +// through a partner's fingerprint window); an uncovered byte is kept raw. Byte +// spans are authoritative, so overlapping matches cannot leak. O(len(src) + spans). +func sealLine(src string, findings []Finding, idxs []int) string { + b := []byte(src) + n := len(b) + if n == 0 { + return src + } + cov := make([]int, n+1) // difference array → per-byte coverage count in O(n+k) + spans := make([]span, 0, len(idxs)) + for _, i := range idxs { + start := findings[i].Column - 1 + end := start + len(findings[i].Matched) + if start < 0 { + start = 0 + } + if end > n { + end = n + } + if start >= end { + continue + } + spans = append(spans, span{start, end}) + cov[start]++ + cov[end]-- + } + out := make([]byte, n) + copy(out, b) + // Star the middle of each span, keeping its head/tail fingerprint bytes. + for _, s := range spans { + fingerprintSpan(out, b, s.start, s.end) + } + // Any byte covered by two or more spans is an overlap: force '*' regardless of + // a fingerprint head/tail char, so no token's raw bytes survive in an overlap. + run := 0 + for i := 0; i < n; i++ { + run += cov[i] + if run >= 2 { + out[i] = '*' + } + } + return string(out) +} + +// fingerprintSpan masks out[start:end] to a token fingerprint: the first keepHead +// and last keepTail bytes are kept (enough to triage which credential), the middle +// starred; a short span is fully starred. It is the byte-level mirror of +// maskSecret so a byte span can be masked in place. +func fingerprintSpan(out, src []byte, start, end int) { + const keepHead, keepTail, fingerprintBelow = 3, 2, 16 + length := end - start + for j := start; j < end; j++ { + k := j - start + if length >= fingerprintBelow && (k < keepHead || k >= length-keepTail) { + out[j] = src[j] // keep head/tail raw — the fingerprint + } else { + out[j] = '*' + } + } +} + // ScanBundle scans the resolved content of every bundle file, reading // ResolvedPath and reporting under LogicalPath. Binary/oversized/skip-listed // files are skipped via the extension/filename sets and a null-byte + UTF-8 @@ -266,7 +366,11 @@ func (s *Scanner) ScanBundle(files []BundleFile) (ScanResult, error) { } data, err := os.ReadFile(f.ResolvedPath) if err != nil { - continue // unreadable individual file is skipped, not fatal + // An unreadable file is skipped, not fatal — but surfaced in Unscanned + // with the same visibility as a binary-skipped file, so a read-skipped + // file cannot silently vanish from the bundle's coverage. + res.Unscanned = append(res.Unscanned, f.LogicalPath) + continue } if !isText(data) { res.Unscanned = append(res.Unscanned, f.LogicalPath) @@ -323,18 +427,29 @@ func path_base(p string) string { return p } -// isText sniffs the first 8KB: a null byte or invalid UTF-8 means binary. +// isText sniffs the first 8KB: a null byte or invalid UTF-8 means binary. When +// the file is longer than the sniff window the cut can land mid-rune; a dangling +// partial trailing rune (at most UTFMax-1 bytes) is trimmed before validating, so +// a valid multibyte file whose rune straddles the boundary is not misread as +// binary. A genuinely invalid encoding still fails. func isText(data []byte) bool { const sniff = 8192 chunk := data + truncated := false if len(chunk) > sniff { chunk = chunk[:sniff] + truncated = true } for _, b := range chunk { if b == 0 { return false } } + if truncated { + for i := 0; i < utf8.UTFMax-1 && len(chunk) > 0 && !utf8.Valid(chunk); i++ { + chunk = chunk[:len(chunk)-1] + } + } return utf8.Valid(chunk) } diff --git a/internal/adapter/scanner/scanner_test.go b/internal/adapter/scanner/scanner_test.go index 3474e7d5..41bc6886 100644 --- a/internal/adapter/scanner/scanner_test.go +++ b/internal/adapter/scanner/scanner_test.go @@ -417,3 +417,104 @@ func TestUnscannedBinaryRecorded(t *testing.T) { t.Errorf("unscannable binary file must be recorded, not silently dropped: %+v", res.Unscanned) } } + +// TestSerializedFindingRedactsSiblingSecret (iss-65 C14/C17, the BLOCK) plants +// TWO distinct secrets on one line. Each finding stores the whole line; the +// serialized snippet of finding A must not leak finding B's raw token, and vice +// versa — masking only a finding's own token leaves every sibling secret verbatim. +func TestSerializedFindingRedactsSiblingSecret(t *testing.T) { + a := "ghp_" + strings.Repeat("A", 40) + b := "ghp_" + strings.Repeat("B", 40) + line := "gh=" + a + " other=" + b // both on one line, minified-env style + + findings := ScanText(line, Identity{}, DefaultPatterns(), DefaultIdentitySeverities(), "commands/x.md") + if len(findings) < 2 { + t.Fatalf("both planted PATs must be caught: %+v", findings) + } + blob, err := json.Marshal(findings) + if err != nil { + t.Fatal(err) + } + js := string(blob) + for _, tok := range []string{a, b} { + if strings.Contains(js, tok) { + t.Errorf("serialized result leaks a sibling secret verbatim %q: %s", tok, js) + } + // No >=6-char window of either raw token may survive in any snippet. + for i := 0; i+6 <= len(tok); i++ { + if w := tok[i : i+6]; strings.Contains(js, w) { + t.Errorf("serialized result leaks a raw token window %q: %s", w, js) + } + } + } +} + +// TestSerializedFindingRedactsOverlappingSecret (iss-65, the overlap variant of +// the BLOCK) plants two secrets whose matches PARTIALLY OVERLAP on one line: a +// greedy sk-ant- key that runs into a following JWT, both detected. Substring +// masking (longest-first) destroys the shorter match's substring and leaves its +// non-overlapping tail raw; only byte-span masking closes it. No >=6-char raw +// window of either token may survive in the serialized snippet. +func TestSerializedFindingRedactsOverlappingSecret(t *testing.T) { + key := "sk-ant-" + strings.Repeat("X", 34) + jwt := "eyJABCDEFGHIJ.KLMNOPQRST.UVWXYZ0123456" + line := key + "-" + jwt // the `-` lets the key run into the JWT head; both fire + + findings := ScanText(line, Identity{}, DefaultPatterns(), DefaultIdentitySeverities(), "commands/x.md") + if len(findings) < 2 { + t.Fatalf("both an anthropic key and a JWT must be caught on the overlapping line: %+v", findings) + } + blob, err := json.Marshal(findings) + if err != nil { + t.Fatal(err) + } + js := string(blob) + // The JWT payload/signature segments are the sensitive part — assert no raw + // >=6-char window of either token survives anywhere in the serialized result. + for _, tok := range []string{key, jwt} { + for i := 0; i+6 <= len(tok); i++ { + if w := tok[i : i+6]; strings.Contains(js, w) { + t.Errorf("serialized snippet leaks a raw token window %q from an overlapping match: %s", w, js) + } + } + } +} + +// TestIsTextMultibyteRuneAtSniffBoundary (iss-65 C15) proves a valid UTF-8 file +// whose multibyte rune straddles the 8192-byte sniff cap is NOT misclassified as +// binary. The cut lands mid-rune, so a naive utf8.Valid(chunk[:8192]) fails. +func TestIsTextMultibyteRuneAtSniffBoundary(t *testing.T) { + // '€' is 3 bytes (E2 82 AC) starting at index 8190, so the 8192 cut splits it. + data := []byte(strings.Repeat("a", 8190) + "€" + strings.Repeat("z", 100)) + if !isText(data) { + t.Fatal("a valid UTF-8 file with a rune straddling the sniff cap must read as text, not binary") + } + // A genuinely invalid encoding (a lone continuation byte early on) is still binary. + bad := append([]byte("head "), 0x82) + bad = append(bad, []byte(" tail")...) + if isText(bad) { + t.Error("genuinely invalid UTF-8 must still read as binary") + } +} + +// TestUnscannedUnreadableRecorded (iss-65 C18) proves a bundle file that cannot +// be read is surfaced in Unscanned, with the same visibility as a binary-skipped +// file, rather than silently dropped. +func TestUnscannedUnreadableRecorded(t *testing.T) { + root := t.TempDir() + sc, err := New(root) + if err != nil { + t.Fatal(err) + } + // ResolvedPath points at a file that does not exist → os.ReadFile errors. + res, _ := sc.ScanBundle([]BundleFile{{LogicalPath: "commands/gone.md", ResolvedPath: filepath.Join(root, "nope", "gone.md")}}) + found := false + for _, p := range res.Unscanned { + if p == "commands/gone.md" { + found = true + } + } + if !found { + t.Errorf("an unreadable bundle file must be recorded in Unscanned, not silently dropped: %+v", res.Unscanned) + } +}