Skip to content

Commit 07e0f10

Browse files
committed
fix(duplicatedcontent): hash rootDir in corpusIndexKey
Pass 2 of code review on PR #732 found that corpusIndexKey joined rootDir into the cache key with the same 0x1f/0x1e control-byte separators used for include/exclude, but unlike those (YAML-parsed strings that cannot contain these bytes), rootDir is a raw filesystem path — most filesystems permit any byte but NUL and '/' in a path component. A pathological directory name containing a literal 0x1f/0x1e could bleed into the fields that follow it, colliding two different scopes onto the same RunCache.DuplicateContentIndex slot. Hash rootDir to a fixed-length hex digest before writing it into the key: the digest's length and alphabet (0-9a-f) never depend on rootDir's content, so it can never absorb or be absorbed by an adjacent field. TestCorpusIndexKey_RootDirControlBytesCannotBleedIntoOtherFields pins this — confirmed red against the prior unhashed scheme (the fixed-length assertion failed: 11 vs 26 chars for two different-length rootDirs) before the fix.
1 parent 886c970 commit 07e0f10

2 files changed

Lines changed: 37 additions & 3 deletions

File tree

internal/rules/duplicatedcontent/rule.go

Lines changed: 9 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -393,13 +393,19 @@ func (r *Rule) corpusIndex(f *lint.File, corpus fs.FS, minChars int, sharedRoot
393393
// besides the corpus FS and selfName (selfName is intentionally
394394
// excluded — see corpusIndex). 0x1f/0x1e separators avoid collisions
395395
// between adjacent fields the way config.EffectiveSignature's key
396-
// does; rule settings are YAML-parsed strings, so they cannot contain
397-
// these control bytes.
396+
// does; rule settings (include/exclude) are YAML-parsed strings, so
397+
// they cannot contain these control bytes. rootDir is not YAML-parsed
398+
// — it is a filesystem path, and most filesystems permit any byte but
399+
// NUL and '/' in a path component — so it is hashed to a fixed-length
400+
// hex string first: a pathological directory name that happened to
401+
// contain a literal 0x1f/0x1e could otherwise bleed into the fields
402+
// that follow it and collide two different scopes onto one cache slot.
398403
func corpusIndexKey(
399404
rootDir string, maxBytes int64, minChars int, stripFrontMatter bool, include, exclude []string,
400405
) string {
401406
var b strings.Builder
402-
b.WriteString(rootDir)
407+
rootSum := sha256.Sum256([]byte(rootDir))
408+
b.WriteString(hex.EncodeToString(rootSum[:]))
403409
b.WriteByte(0x1f)
404410
b.WriteString(strconv.FormatInt(maxBytes, 10))
405411
b.WriteByte(0x1f)

internal/rules/duplicatedcontent/rule_test.go

Lines changed: 28 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1034,6 +1034,34 @@ func TestCorpusIndexKey_DistinctInputsProduceDistinctKeys(t *testing.T) {
10341034
assert.Equal(t, a, b, "identical inputs must produce identical keys")
10351035
}
10361036

1037+
// TestCorpusIndexKey_RootDirControlBytesCannotBleedIntoOtherFields pins
1038+
// the fix for a review finding: unlike include/exclude (YAML-parsed
1039+
// strings that cannot contain 0x1f/0x1e), rootDir is a raw filesystem
1040+
// path — most filesystems permit any byte but NUL and '/' in a path
1041+
// component, including the key's own 0x1f/0x1e separators. Writing
1042+
// rootDir raw into the key would let a pathological directory name
1043+
// bleed into the fields that follow it. Hashing rootDir to a
1044+
// fixed-length hex digest first closes that: the digest is always the
1045+
// same length and alphabet (0-9a-f) regardless of rootDir's byte
1046+
// content, so it can never absorb or be absorbed by an adjacent field.
1047+
func TestCorpusIndexKey_RootDirControlBytesCannotBleedIntoOtherFields(t *testing.T) {
1048+
poisoned := "root" + string(rune(0x1f)) + "999" + string(rune(0x1f)) + "sneaky" + string(rune(0x1e))
1049+
1050+
// A pathological rootDir must still produce a key where every
1051+
// other field keeps distinguishing power — changing maxBytes with
1052+
// the exact same poisoned rootDir must still change the key.
1053+
a := corpusIndexKey(poisoned, 1, 200, false, nil, nil)
1054+
b := corpusIndexKey(poisoned, 2, 200, false, nil, nil)
1055+
assert.NotEqual(t, a, b, "maxBytes must still distinguish keys when rootDir embeds the key's own separator bytes")
1056+
1057+
// The hashed rootDir segment (64 hex chars) has fixed length
1058+
// regardless of the raw rootDir's length or content.
1059+
short := corpusIndexKey("x", 1, 200, false, nil, nil)
1060+
long := corpusIndexKey(poisoned, 1, 200, false, nil, nil)
1061+
assert.Equal(t, len(short), len(long),
1062+
"the hashed rootDir segment must be a fixed length independent of the raw rootDir's length")
1063+
}
1064+
10371065
func newLintFileWithRoot(t *testing.T, path, root string) *lint.File {
10381066
t.Helper()
10391067
data, err := os.ReadFile(path)

0 commit comments

Comments
 (0)