fix: normalize a single backslash so protectedFilePatterns work on Windows - #593
Open
LHMQ878 wants to merge 1 commit into
Open
fix: normalize a single backslash so protectedFilePatterns work on Windows#593LHMQ878 wants to merge 1 commit into
LHMQ878 wants to merge 1 commit into
Conversation
…ndows `normalizePath` searched for `"\\\\"`, which in source is the two-character string `\`. A real Windows path contains single separators, so nothing was replaced and the normalisation was inert on the only platform it exists for. `protectedFilePatterns` therefore protected a file on POSIX and silently failed to protect the same file on Windows, at all four `isFilePathProtected` call sites (protected-content, sweep, deduplication, purge-errors). Patterns that name a directory or an exact file did nothing; only patterns like `**/*.ts` appeared to work, because `**` compiles to `.*` and spans backslashes anyway. Search for the single backslash instead. Separators are converted rather than stripped, so `*` still compiles to `[^/]*` and does not cross a directory boundary. Because both the path and the pattern are normalised, a pattern written with Windows separators now works too. Adds tests/protected-patterns.test.ts; the module had no test file.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Fixes #592
What was wrong
normalizePathsearched for"\\\\"— in source, the two-character string\\. A path from the filesystem contains single separators, so nothing was ever replaced. The function exists solely to make Windows paths comparable to forward-slash glob patterns, and it was inert on the only platform that needs it.protectedFilePatternsconsequently protected a file on POSIX and silently failed to protect the same file on Windows:C:/repo/src/config/secrets.tsC:\repo\src\config\secrets.ts**/secrets.tstruefalse**/config/*.tstruefalseC:/repo/src/**truefalse**/*.tstruetrueThat last row is why this survived:
**compiles to.*, which spans backslashes, so a user who tests with**/*.tssees protection working. Every pattern that names a directory or an exact file did nothing.isFilePathProtectedis the guard that keeps configured files out of pruning, consulted fromcompress/protected-content.ts:138,commands/sweep.ts:184/:201,strategies/deduplication.ts:60, andstrategies/purge-errors.ts:62. On Windows all of them sawfalsefor paths the user explicitly asked to protect. The failure was silent: config parses, schema validates, nothing logs.What changed
One character.
replaceAlltakes a string, not a regex, so no regex-level escaping belongs there.Because
normalizePathis applied to the pattern as well as the path, a pattern written with Windows separators now also works — which is what a user copying a path out of Explorer naturally produces.What deliberately does not change: separators are converted, not stripped.
*compiles to[^/]*and must not cross a directory boundary, sosrc/*still does not matchsrc\config\secrets.ts. A fix that removed backslashes instead of translating them would break that quietly, so it has a test of its own.Verification
npm test: 95 pass / 0 fail (was 87 — exactly +8, the new tests).lib/protected-patterns.tsreverted tomasterand the new tests kept: 5 fail / 90 pass. The three tests that pass either way are the platform-neutral ones (tool-name matching, empty pattern, regex metacharacters) — they are there to pin that this change does not widen matching beyond separators.npm run typecheck: clean.prettier --checkon both files: clean.Tests added
tests/protected-patterns.test.ts— the module had no test file. 8 tests:*still does not cross a normalised separator — the property a strip-instead-of-convert fix would breakisFilePathProtectedend-to-end throughgetFilePathsFromParameters("read", …), which is the shape the call sites actually passfalsemultiedit(nestededits[].filePath) andapply_patch(paths embedded inpatchText) on Windows, since each carries paths in its own shapeisToolNameProtectedunchanged, and empty-pattern / regex-metacharacter handling inmatchesGlobThe tests build the backslash with
String.fromCharCode(92)rather than writing a literal. The bug being pinned was an escaping mistake in a string literal, so the tests avoid depending on one.