fix(file-tools): normalize separators for POSIX sensitive-path denylist on Windows

[andrexibiza]

Related #76246 #76247 #78565 #78658 #78806

Note

Topology adjudicated on 2026-08-19: closed as a duplicate of #76247 after exact file-level comparison. Both PRs change the same two files and their complete patches are byte-for-byte identical. #76247 is the older implementation and owns the full review/fix history; this branch's test-import commit preserves @fangliquanflq credit with Co-authored-by. The surviving path is #76247.

This work fixes #76246 only. #78565 is a separate git-worktree .git managed-state corruption class and remains open. Its current class-closing implementation is open PR #78806; closed PR #78652 is the unmerged pointer-file foundation subsumed into that branch. This PR is not part of #78565.

Summary

On Windows hosts, the file-tool sensitive-path write guard fails open for POSIX system
targets (/etc/..., /boot/..., /var/run/docker.sock): ntpath/Path rewriting
produces backslash forms that no longer match the /-prefixed denylist, while the shell
layer can later restore the POSIX path and attempt the write. Native Windows local
backends are also case-insensitive and ignore trailing dots/spaces in ordinary path
components, so Git Bash-equivalent spellings (/Etc/hosts, /Etc./hosts) bypass the
guard.

Fix

tools/file_tools.py — _check_sensitive_path compares denylist entries against
slash-normalized forms (_posix_form_for_sensitive_check); on native Windows local
backends the comparison additionally strips Win32 trailing spaces/dots and casefolds
(_casefold_sensitive_posix_paths), matching the sink's actual resolution semantics.
Container and remote POSIX backends stay case-sensitive. Hermes config.yaml identity
checks use the same canonicalization so mixed-case and trailing-dot spellings cannot
bypass the approval-configuration guard.

Verification

The promised tests are present on this branch in
tests/tools/test_file_write_safety.py. They cover:

• backslash rewriting for prefix and exact denylist entries;
• native-Windows case variants and trailing-dot/space aliases;
• write_file and V4A patch refusal before file operations are acquired;
• Hermes-config canonical, mixed-case, and trailing-dot aliases;
• safe lookalikes and nearby paths;
• POSIX/container case-sensitive negative controls.

Current head: 40efdc90061d7abb3c030cd9327178cf9a6ced6b.

• GitHub Actions CI run 31094380438: success.
• Native Windows 11 evidence on unchanged production commit 2d33a223: the current-main
  bypass reproduced; protected aliases blocked on the branch; focused run 14 passed,
  62 deselected; broader file-tool run 73 passed with three documented pre-existing
  Windows harness failures.
• Surviving duplicate fix(tools): stop Windows hosts from allowing write_file into /etc and docker.sock #76247 head 3e0d31c40ff0523214d8a7f820b3a91f17c551f7
  also has successful CI run 30874010869.

Links

• Fixes [Bug]: Windows hosts allow write_file into /etc and docker.sock despite sensitive-path guard #76246
• Surviving implementation: fix(tools): stop Windows hosts from allowing write_file into /etc and docker.sock #76247
• Out-of-scope NTFS 8.3 identity follow-up: security(tools): guard Hermes config writes against Windows 8.3 aliases #78658
• Separate worktree managed-state class: fix: write_file and patch tools silently destroy git worktree .git files #78565 / fix(file-safety): refuse writes into git-managed state under .git directories #78806

[andrexibiza]

Status check from the worktree-honesty class review — this PR is yours and stays yours; coordination only.

Findings: (1) CI is RED on the current head (runs 30930891109 + 30931082237 both failed; job detail not exposed via API) — needs a green run before it can merge. (2) Dedup flag: open PR #76247 covers the same separator/casefold normalization — verify overlap before both merge. (3) It composes with #78806 (different hunks of _check_sensitive_path; small trivially-resolvable conflict at the hermes-config block boundary expected). (4) It fixes sibling issue #76246, NOT the #78565 cluster — fine, but the body should say so.

Want me to run the failing CI jobs locally and post the root cause? Credit: the Windows separator normalization approach here is the right fix for #76246 — the class review confirms it.

[monerostar]

Native Win11 verification (monerostar)

Host: Windows 11 build 26200, sys.platform=win32, Python 3.11.15 (Hermes venv). PR head 2d33a223.

Bug reproduces on current main (fail-open)

Swapped in upstream/main tools/file_tools.py on this host. _check_sensitive_path ALLOWs every POSIX denylist target because os.path.normpath rewrites them to backslash form that no longer matches /-prefixed denylist entries:

input	os.path.normpath	main
/etc/hosts	\etc\hosts	ALLOW
\etc\hosts	\etc\hosts	ALLOW
/Etc/hosts	\Etc\hosts	ALLOW
/Etc./hosts	\Etc.\hosts	ALLOW
/var/run/docker.sock	\var\run\docker.sock	ALLOW
\boot\grub\grub.cfg	\boot\grub\grub.cfg	ALLOW
%TEMP%\hermes-safe-write.txt	(native temp)	ALLOW (expected)

So the write guard is currently a no-op for these spellings on native Windows local.

PR head closes the hole

Same cases under this branch (TERMINAL_ENV=local, casefold_local=True):

input	PR head
/etc/hosts, \etc\hosts	BLOCK
/Etc/hosts, /Etc./hosts, /ETC/HOSTS	BLOCK (casefold + trailing-dot strip)
/var/run/docker.sock, \var\run\docker.sock	BLOCK
/boot/grub/grub.cfg, \boot\grub\grub.cfg	BLOCK
/tmp/safe_file.txt	ALLOW
%TEMP%\hermes-safe-write.txt	ALLOW

Helper sanity: \etc\hosts → posix /etc/hosts; /Etc./hosts → casefold /etc/hosts.

Tests

pytest tests/tools/test_file_write_safety.py tests/tools/test_file_tools.py \
  -k "sensitive or Sensitive or hosts or boot or private" -o addopts=
→ 14 passed, 62 deselected

Broader file-tool run: 73 passed, 3 failed — all pre-existing Windows suite noise unrelated to this diff (0o600 mode bits; mock expects /tmp/... vs actual \tmp\...).

Notes for maintainers

1. Sibling: open #76247 covers the same fail-open class and ships tests/tools/test_file_write_safety.py coverage. This PR is a smaller standalone production-only hunk (+63/−9, 1 file). Prefer merging one; the later should drop duplicate logic. Live evidence here supports the behavior, not a preference between the two.
2. Commit message claims “Regression tests cover …”; this branch does not add tests (only tools/file_tools.py). Either pull the #76247 tests forward or drop that claim.
3. CI on this PR currently only shows skipped label-rerun jobs — no full required check rollup visible from here. Worth a push/re-run so “All required checks pass” lights up.
4. Could not leave a formal Approve (fork collaborator scope) — evidence comment only.

Verdict: fix is real and high-confidence on native Win11; good candidate once CI is green and the #76247 overlap is resolved.

[andrexibiza]

Fixed in 40efdc9. Thanks for the native Win11 verification, monerostar — that evidence table is exactly what this class of bug needs.

Addressed points:

1. Missing regression tests (your note 2): The commit message claimed coverage the branch didn't have — corrected. Pulled TestCheckSensitivePathWindowsHostSemantics (432 lines) forward from fix(tools): stop Windows hosts from allowing write_file into /etc and docker.sock #76247, verbatim, with fangliquanflq credited via Co-authored-by. It covers backslash normalization, Win32 casefold, trailing-dot/space aliases, and Hermes-config identity checks across _check_sensitive_path, write_file_tool, and patch_tool.
2. CI rollup (your note 3): Fresh push triggered the full required-check set — all green: 12/12 Python test slices, ruff (blocking), Windows footguns (blocking), uv.lock, check-attribution.
3. fix(tools): stop Windows hosts from allowing write_file into /etc and docker.sock #76247 overlap (your note 1): Agreed — the PR body already states the later merge drops duplicate hunks; both branches now carry the same test class so the merge conflict is a trivial one-way resolution.

Local validation (Windows, git-bash, via scripts/run_tests.sh):

• test_file_write_safety.py sensitive-path selection: 33 passed
• Full file: 55 passed, 1 failed — the failure is TestAtomicWrite::test_patch_routes_through_atomic_write, pre-existing Windows suite noise (mock expects /tmp/... vs actual \\tmp\\...) that reproduces identically on the untouched PR head; unrelated to this diff.
• git diff --check clean; Ruff clean.

Local native-Win11 evidence for the casefold path (matching your table): /Etc/hosts, /Etc./hosts, /ETC/HOSTS, \\etc\\hosts all BLOCK on TERMINAL_ENV=local; /tmp/safe_file.txt and %TEMP%\\hermes-safe-write.txt ALLOW.

[egilewski]

suggesting changes

• [P1] Windows sensitive-path matching remains lexical and bypassable through target aliases
  The guard can be bypassed by links, junctions, short names, or extended namespaces. Canonicalize or reject those aliases before comparison and pass the same canonical identity to the write sink; add native-Windows regression coverage for write and patch.
• [P2] Nearby-safe regression coverage needs an isolated safe fixture
  Move the fixture outside protected project configuration data (or isolate unrelated policy checks) and retain the successful-write assertion.

Security evidence:

• trust boundary: Model-controlled path strings enter write and patch operations; the sensitive-path guard is the pre-sink boundary.
• source/sink/invariant: The change normalizes separators and native-Windows case and trailing-dot/space aliases before matching sensitive prefixes, exact sockets, and configuration identity; both write paths invoke the guard before file operations.
• current-main reproduction: Backslash-form resolution can evade POSIX denylist comparisons; normalized forms close that bypass, while target-identity aliases remain unresolved.
• PR-head or patch-replay validation: The changed helper, call sites, and regression assertions cover separators, case, trailing aliases, write, patch, and configuration identity.
• positive/negative cases: Coverage includes blocked sensitive targets, exact sockets, safe lookalikes, backend case sensitivity, write and patch refusal, and configuration identity.
• residual bypass search: Lexical resolution does not establish target identity for Windows aliases, and sink behavior follows links, leaving a bypass gap.
• reviewer validation: Source review confirmed guard ordering, patch-header coverage, and sink behavior.

Review setup: I reviewed a run-owned local rebase or patch replay against current GitHub main because the submitted branch is stale or conflicted; this does not mean the submitted branch itself merges cleanly.

Not checked:

• Native Windows execution
• Full repository test suite

Signed: GPT-5.6-luna-max in Codex