fix: normalize single Windows path separators in protectedFilePatterns

devlog/2026-09-16_fix-windows-path-normalization/REQ.md

@@ -0,0 +1,54 @@
+# REQ - Fix Windows path separator normalization in protectedFilePatterns
+
+- Task ID: `2026-09-16_fix-windows-path-normalization`
+- Home Repo: `opencode-acp`
+- Created: 2026-09-16
+- Status: InProgress
+- Priority: P1
+- Owner: ranxianglei
+- References: https://github.com/ranxianglei/opencode-acp/issues/402 ; upstream DCP commit 5f8f33b
+
+## 1. Background & Problem Statement
+
+- **Context**: `protectedFilePatterns` protects files from pruning by matching tool-parameter paths against glob patterns. `lib/protected-patterns.ts` normalizes path separators before glob matching so that Windows paths work.
+- **Current behavior (symptom)**: On Windows, ordinary single-backslash paths (`C:\repo\src\secrets.ts`) bypass configured protection. Only patterns like `**/*.ts` accidentally worked, because `*` compiles to `[^/]*` which spans backslashes freely.
+- **Expected behavior**: A documented-style pattern such as `**/secrets.ts` matches the same file whether the tool parameter uses `/` or `\` separators; patterns written with Windows separators also work.
+- **Impact**: Silent security/privacy bypass — files the user explicitly protected on Windows were unprotected at all `isFilePathProtected` call sites (protected-content, sweep, deduplication, purge-errors).
+
+## 2. Reproduction
+
+- **Environment**: Node 22+, any OS (pure string logic)
+- **Minimal reproduction steps**:
+  1) Configure `"protectedFilePatterns": ["**/secrets.ts"]`
+  2) Tool call with `filePath: "C:\\repo\\src\\secrets.ts"` (single backslashes at runtime)
+  3) `isFilePathProtected` returns `false` — bug confirmed by new regression tests failing against pre-fix code
+- **Relevant configuration**: `protectedFilePatterns` (root config key)
+
+## 3. Constraints & Non-Goals
+
+- **Constraints**:
+  - Backward compatibility: POSIX paths and existing forward-slash patterns must behave identically; both input path and pattern are normalized on both sides, so cross-style matching stays consistent.
+  - Performance requirements: none (single-string replaceAll per call)
+  - Resource limits: none
+- **Non-Goals**: no changes to glob semantics (`*` still `[^/]*`, does not cross separators); no changes to other call sites.
+
+## 4. Acceptance Criteria
+
+- **Correctness**:
+  - [x] `normalizePath` converts each single backslash to `/`
+  - [x] Windows path + forward-slash pattern → protected
+  - [x] Forward-slash path + Windows-separator pattern → protected
+  - [x] `*` does not cross a Windows separator (no over-matching)
+  - [x] read / multiedit / apply_patch parameter shapes all covered
+- **Performance / Stability**:
+  - [x] Full test suite passes
+- **Regression**:
+  - [x] New test file `tests/protected-patterns.test.ts` added (ported from upstream DCP commit 5f8f33b) and passing
+
+## 5. Proposed Approach
+
+- **Affected modules & entry files**:
+  - `lib/protected-patterns.ts` — one-line fix in `normalizePath` (+ explanatory comment)
+  - `tests/protected-patterns.test.ts` — new regression suite
+- **Risks**: Literal doubled backslashes (`C:\\Users`) become `C://Users` after normalization — but since both path and pattern are normalized identically, matching consistency is preserved; cross-style double-vs-single mismatches are contrived edge cases.
+- **Rollback strategy**: Revert the single commit; no state format or API changes.

devlog/2026-09-16_fix-windows-path-normalization/WORKLOG.md

@@ -0,0 +1,66 @@
+# WORKLOG - Fix Windows path separator normalization in protectedFilePatterns
+
+- Task ID: `2026-09-16_fix-windows-path-normalization`
+- Home Repo: `opencode-acp`
+- Status: Done
+- Updated: 2026-09-16 01:50
+
+## 1. Summary
+
+- **What was done**: Fixed `normalizePath()` in `lib/protected-patterns.ts` to replace each single backslash with `/` instead of only doubled backslashes; added `tests/protected-patterns.test.ts` regression suite (ported from upstream DCP commit 5f8f33b).
+- **Why**: `"\\\\"` in source is the two-character string `\\`, so the normalization was a no-op on real Windows paths (single separators), silently bypassing `protectedFilePatterns`.
+- **Behavior / compatibility changes**: Yes — Windows-separator paths/patterns now match as intended. POSIX behavior unchanged (both sides normalized identically).
+- **Risk level**: Low
+
+## 2. Change Log
+
+### Commits
+
+| Commit | Description |
+|--------|-------------|
+| TBD | fix: normalize single Windows path separators in protectedFilePatterns |
+
+### Key Files
+
+- `lib/protected-patterns.ts` — `normalizePath`: `replaceAll("\\\\", "/")` → `replaceAll("\\", "/")` (+ comment explaining the escape-level bug so it isn't reintroduced)
+- `tests/protected-patterns.test.ts` — NEW: 8 tests covering matchesGlob Windows separators, pattern-side normalization, wildcard boundary semantics, read/multiedit/apply_patch parameter shapes, isToolNameProtected non-regression, regex metacharacter escaping
+
+## 3. Design & Implementation Notes
+
+- **Entry point / key function**: `normalizePath()` (private) used by `matchesGlob()` for both input path and pattern.
+- **Key logic explanation**: Separators are *converted*, not stripped, so `*` still compiles to `[^/]*` and cannot cross a directory boundary. Tests build backslashes via `String.fromCharCode(92)` so a future editor/linter reformatting can't reintroduce an escaping mistake — the exact class of bug being pinned.
+
+## 4. Testing & Verification
+
+### Build & Test Commands
+
+```sh
+npm run typecheck
+node --import tsx --test tests/protected-patterns.test.ts
+node --import tsx --test tests/*.test.ts
+npm run format:check
+```
+
+### Test Coverage
+
+- New/modified test files: `tests/protected-patterns.test.ts` (new)
+- Key scenarios verified: Windows path vs forward-slash pattern; forward-slash path vs Windows pattern; `*` does not cross `\`; read/multiedit/apply_patch shapes; unmatched paths still false; tool-name protection unaffected; regex metachars escaped literally.
+- Bug-reproduction check: new tests must FAIL against pre-fix code (verified before commit).
+
+### Results
+
+- **PASS/FAIL**: PASS
+- Typecheck: clean (`tsc --noEmit`)
+- New suite: 8/8 pass on fixed code; 5/8 FAIL on pre-fix code (bug-reproduction verified via temporary stash of the fix)
+- Full suite: 1271 tests, 1271 pass, 0 fail
+- Prettier: changed files clean (`npx prettier --check` on both files). Note: repo-wide `npm run format:check` reports 452 pre-existing unformatted files unrelated to this change (left untouched to keep the diff minimal).
+
+## 5. Risk Assessment & Rollback
+
+- **Risk points**: Literal doubled-backslash inputs become double slashes after normalization; both sides normalize identically so matching stays consistent — contrived edge case only.
+- **Rollback method**: Revert the single commit; no state format, config schema, or API changes.
+- **Compatibility notes**: No persisted-state or schema impact.
+
+## 6. Lessons Learned
+
+- String-literal escaping bugs are invisible to casual review (`"\\\\"` reads like "a backslash"); pinning such bugs requires tests that avoid hand-written escapes (char-code construction) plus a source comment explaining why the literal looks the way it does.

lib/protected-patterns.ts

@@ -1,5 +1,9 @@
 function normalizePath(input: string): string {
-    return input.replaceAll("\\\\", "/")
+    // A single backslash. In source, "\\" is the one-character string; the
+    // previous "\\\\" was a *two*-character string, so it only ever matched a
+    // doubled separator -- which a real Windows path does not contain. The
+    // normalisation was therefore a no-op on the only platform that needs it.
+    return input.replaceAll("\\", "/")
 }
 
 function escapeRegExpChar(ch: string): string {

tests/protected-patterns.test.ts

@@ -0,0 +1,104 @@
+import assert from "node:assert/strict"
+import test from "node:test"
+import {
+    getFilePathsFromParameters,
+    isFilePathProtected,
+    isToolNameProtected,
+    matchesGlob,
+} from "../lib/protected-patterns"
+
+// A single backslash, built from its char code so the intent survives any later
+// reformatting of this file. The bug being pinned here was precisely an escaping
+// mistake in a string literal, so the tests avoid writing one by hand.
+const BS = String.fromCharCode(92)
+const winPath = (...segments: string[]) => segments.join(BS)
+
+test("matchesGlob treats a Windows separator as a path separator", () => {
+    // `protectedFilePatterns` is documented with forward slashes ("**/*.config.ts"),
+    // but on Windows the tool parameters carry backslashes. Both sides are
+    // normalised, so the same pattern must match either spelling of the same file.
+    const path = winPath("C:", "repo", "src", "config", "secrets.ts")
+
+    assert.equal(matchesGlob(path, "**/secrets.ts"), true)
+    assert.equal(matchesGlob(path, "**/config/*.ts"), true)
+    assert.equal(matchesGlob(path, "C:/repo/src/**"), true)
+    assert.equal(matchesGlob(path, "**/*.ts"), true)
+})
+
+test("matchesGlob accepts a pattern written with Windows separators", () => {
+    // Normalisation applies to the pattern too, so a user who copies a path out of
+    // Explorer and uses it as a pattern gets the same result as the documented form.
+    const pattern = winPath("**", "config", "*.ts")
+
+    assert.equal(matchesGlob("C:/repo/src/config/secrets.ts", pattern), true)
+    assert.equal(matchesGlob(winPath("C:", "repo", "src", "config", "secrets.ts"), pattern), true)
+})
+
+test("a single-segment wildcard still does not cross a Windows separator", () => {
+    // `*` is defined as "[^/]*". Normalisation must convert separators rather than
+    // erase them, or `*` would silently start matching across directories.
+    assert.equal(matchesGlob(winPath("src", "config", "secrets.ts"), "src/*"), false)
+    assert.equal(matchesGlob(winPath("src", "config", "secrets.ts"), "src/*/*.ts"), true)
+    assert.equal(matchesGlob(winPath("src", "secrets.ts"), "src/*"), true)
+})
+
+test("isFilePathProtected protects a Windows path the user configured", () => {
+    // The end-to-end shape: a `read` tool call on Windows, checked against the
+    // documented pattern style. This is the assertion that failed before the fix --
+    // the file was protected on POSIX and unprotected on Windows.
+    const patterns = ["**/secrets.ts", "**/.env"]
+
+    for (const path of [
+        "C:/repo/src/config/secrets.ts",
+        winPath("C:", "repo", "src", "config", "secrets.ts"),
+    ]) {
+        const paths = getFilePathsFromParameters("read", { filePath: path })
+        assert.equal(isFilePathProtected(paths, patterns), true, `not protected: ${path}`)
+    }
+})
+
+test("isFilePathProtected still returns false for a genuinely unmatched path", () => {
+    // The fix must widen matching only for separators, not for anything else.
+    const paths = getFilePathsFromParameters("read", {
+        filePath: winPath("C:", "repo", "src", "main.ts"),
+    })
+
+    assert.equal(isFilePathProtected(paths, ["**/secrets.ts"]), false)
+    assert.equal(isFilePathProtected(paths, []), false)
+    assert.equal(isFilePathProtected([], ["**/*.ts"]), false)
+})
+
+test("multiedit and apply_patch paths are protected on Windows too", () => {
+    // These two tools carry paths in shapes of their own, so they need their own
+    // coverage: a nested `edits` array and paths embedded in patch text.
+    const patterns = ["**/secrets.ts"]
+
+    const multiedit = getFilePathsFromParameters("multiedit", {
+        filePath: winPath("src", "main.ts"),
+        edits: [{ filePath: winPath("src", "config", "secrets.ts") }],
+    })
+    assert.equal(isFilePathProtected(multiedit, patterns), true)
+
+    const patch = getFilePathsFromParameters("apply_patch", {
+        patchText: `*** Update File: ${winPath("src", "config", "secrets.ts")}\n@@\n-a\n+b\n`,
+    })
+    assert.equal(isFilePathProtected(patch, patterns), true)
+})
+
+test("isToolNameProtected is unaffected by separator normalisation", () => {
+    // Tool names contain no separators; this pins that the shared helper does not
+    // change their behaviour.
+    assert.equal(isToolNameProtected("bash", ["bash"]), true)
+    assert.equal(isToolNameProtected("bash", ["ba*"]), true)
+    assert.equal(isToolNameProtected("bash", ["read"]), false)
+    assert.equal(isToolNameProtected("bash", []), false)
+    assert.equal(isToolNameProtected("", ["bash"]), false)
+})
+
+test("matchesGlob rejects an empty pattern and handles regex metacharacters", () => {
+    // Pattern text is interpolated into a RegExp, so characters that mean something
+    // there must be matched literally.
+    assert.equal(matchesGlob("a/b.ts", ""), false)
+    assert.equal(matchesGlob("src/a+b(1).ts", "src/a+b(1).ts"), true)
+    assert.equal(matchesGlob("src/axb1.ts", "src/a+b(1).ts"), false)
+})