fix: mask sibling secrets by byte span in serialized scan snippets (iss-65)

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
 

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)
 }
 

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)
+	}
+}