Skip to content

Dedupe identical target paths and case-fold nesting in directory scans - #189

Open
mazen-salah wants to merge 2 commits into
betterleaks:mainfrom
mazen-salah:fix-remove-nested-paths
Open

Dedupe identical target paths and case-fold nesting in directory scans#189
mazen-salah wants to merge 2 commits into
betterleaks:mainfrom
mazen-salah:fix-remove-nested-paths

Conversation

@mazen-salah

Copy link
Copy Markdown
Contributor

removeNestedPaths is meant to drop overlapping/duplicate scan targets, but two identical paths both survived (the self-comparison was skipped), producing duplicate findings; and the child-of check was case-sensitive, so nesting was missed on case-insensitive filesystems (Windows).

Track kept paths to drop exact duplicates and normalize the comparison key (lowercased on Windows). Adds tests.

Two identical target paths both survived (the self-comparison was skipped),
producing duplicate findings the function is meant to prevent; and the
child-of check was case-sensitive, so nesting was missed on case-insensitive
filesystems (Windows). Track kept paths to drop exact duplicates and
normalize the comparison key (lowercased on Windows). Adds tests.
@greptile-apps

greptile-apps Bot commented Jun 8, 2026

Copy link
Copy Markdown
Contributor

Greptile Summary

Fixes two bugs in removeNestedPaths: identical paths both survived because self-comparison was skipped (now caught via a seen map), and nesting was missed on Windows due to case-sensitive comparison (now normalized via pathKey). Tests for both scenarios are added.

  • The seen map correctly deduplicates only kept (non-nested) paths; duplicate nested paths still re-run the inner loop, but path lists are user-supplied and small so this is harmless.
  • pathKey lowercases only on runtime.GOOS == "windows", leaving macOS APFS (case-insensitive by default) unhandled.

Confidence Score: 4/5

Safe to merge; the dedup and Windows case-fold logic are both correct, with macOS case-insensitivity left as a gap.

The core logic is sound and the tests validate it. The only gap is that pathKey doesn't lowercase on macOS even though APFS is case-insensitive by default, so users on Mac could still see duplicate findings from case-variant paths — the same class of bug the PR set out to fix.

cmd/directory.go — specifically the pathKey function and its OS guard.

Important Files Changed

Filename Overview
cmd/directory.go Dedup + case-fold logic is correct for Windows; pathKey skips macOS (also case-insensitive by default on APFS)
cmd/directory_test.go New tests cover dedup and Windows case-folding; non-Windows branch is correctly guarded

Reviews (1): Last reviewed commit: "dedupe identical paths and case-fold nes..." | Re-trigger Greptile

Comment thread cmd/directory.go
Comment on lines +139 to +144
func pathKey(p string) string {
if runtime.GOOS == "windows" {
return strings.ToLower(p)
}
return p
}

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

P2 macOS with APFS (the default since 2017) is case-insensitive, so the nesting check will still miss "Foo" vs "foo/sub" there. runtime.GOOS == "windows" is correct for Windows, but the same issue exists on macOS for most users. Consider extending the condition, or detecting case-sensitivity at runtime via a stat-based probe.

Suggested change
func pathKey(p string) string {
if runtime.GOOS == "windows" {
return strings.ToLower(p)
}
return p
}
func pathKey(p string) string {
if runtime.GOOS == "windows" || runtime.GOOS == "darwin" {
return strings.ToLower(p)
}
return p
}

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant