fix(agent): refuse windows device names in resolve_path

[akushonkamen]

Summary

On Windows, the file tools took a DOS device name for an ordinary workspace file:
write_file with path="nul" sent the content to the NUL device and answered
success for a file that never existed, diff and file_change included. The same
holds for the rest of the reserved set under Windows' naming rules: a first
extension and trailing spaces are stripped before the device comparison, so nul.md
and con are devices too, and on Windows 10 and earlier the whole set is reserved
in every directory.

Root cause: resolve_path (raven/agent/tools/filesystem.py:57-76) joins the name
onto the workspace and calls Path.resolve(). On Windows that is ntpath.realpath,
which maps only a path that is exactly nul to \\.\NUL and hands <workspace>\nul
back unchanged, so the allowed_dirs prefix check then accepts it, and the file has
no reserved-name check anywhere. CPython tracks the general "device name inside a
longer path" gap as issue 95486.

Fix: right after expanduser, resolve_path runs a pure string guard on the final
segment - strip the first extension and any trailing spaces, uppercase, and compare
against the DOS reserved set (CON, PRN, AUX, NUL, CONIN$, CONOUT$, COM0-9, LPT0-9,
matching CPython 3.13's os.path.isreserved device names). On a match it raises
PermissionError with a remedy, the same refusal a path outside the allowed
directories gets. The guard is gated on _WINDOWS (snapshotted from
os.name == "nt" at import), and every consumer reaches it through this single
choke point - the five file tools, deliver_files, the RPC file viewer and the turn
attachments - so one check covers them all. Off Windows nothing changes: a file
called nul stays a legal name, and the new tests cover both sides of the gate.

This fix was prepared with AI assistance; attribution follows the repo's AGENTS.md
convention via the commit's Co-authored-by trailer.

Type

• Fix
• Feature
• Docs
• CI / tooling
• Refactor
• Other

Verification

All commands run with timeout, on this branch's head with a clean tree, against
the repo .venv.

• Red first (tree before the fix): inline timeout 60 .venv/bin/python calling
  resolve_path for nul / nul.md / con.md / report.md -> all four resolved
  as ordinary paths (device names accepted); pytest tests/test_filesystem_tools.py
  -> ImportError (_is_device_name absent).
• Green, gate on (os.name == "nt" simulated): same inline script -> nul,
  nul.md and con.md raise PermissionError, report.md resolves normally.
• Gate off (POSIX default): same inline script -> all four names resolve as before.
• File: timeout 120 .venv/bin/python -m pytest tests/test_filesystem_tools.py -q -p no:cacheprovider -> 54 passed.
• Related subset: timeout 300 .venv/bin/python -m pytest tests/ -k "filesystem or file_tool or fence" -q -p no:cacheprovider -> 281 passed, 7 skipped.
• Direct consumers of resolve_path (5 test files): 288 passed, 1 failed
  (test_a_host_without_libreoffice_says_so; reproduced identical red on a clean
  tree -> pre-existing environment failure).
• Full suite: timeout 590 .venv/bin/python -m pytest tests -q --ignore=tests/manual -p no:cacheprovider -> 27973 passed, 128 skipped, 4 failed (a2a interop /
  rpc_files libreoffice / simulation suite / simulation scenario; each reproduced
  identical red on clean origin/main -> pre-existing environment failures, not new
  regressions).
• Lint: timeout 120 uvx ruff@latest check raven/ -> All checks passed;
  uv run ruff format applied (touches the added lines only), ruff check -> All
  checks passed.
• Re-run at PR time (main unchanged since the branch was cut, so the numbers
  above stand): timeout 120 .venv/bin/python -m pytest tests/test_filesystem_tools.py -q -p no:cacheprovider -> 54 passed (2.78s).

Not run: a real Windows end-to-end pass (development host is macOS, no Windows
environment available). The refusal is gated on _WINDOWS and was verified at the
logic level on both sides of the gate; the Windows-side behaviour rests on the
CPython ntpath source (read at 3.13) and issue 95486, not on a live run.

• Relevant tests pass locally
• Relevant lint / type checks pass locally
• User-facing docs or screenshots are updated when needed

No docs change: no document describes the file tools' path handling at this level.

Risk

User-visible change is Windows-only: the file tools now refuse a reserved device
name in the final path segment instead of silently opening the device. Off Windows
the gate is off and behaviour is unchanged.

• Security impact considered

The change closes a silent write-to-device hole (content discarded, fake success
receipt); it adds a refusal, removes no capability, and cannot widen any path.

• Backward compatibility considered

POSIX filenames like nul or nul.md stay legal; on Windows such names never
addressed a real file through these tools, so no working workflow breaks.

• Rollback path is clear for risky changes

Rollback is a revert of the single commit; the guard is one string check at one
fence point with no persistent state.

Related Issues

Fixes #895

[gloryfromca]

Blocking: the reserved-device guard must cover every path component and all Windows COM/LPT aliases.

I found one incomplete case, marked inline.

Coverage: I reviewed the full github/main...HEAD diff, the issue and PR history, every direct resolve_path caller, the unchanged target implementation, backward compatibility of the Windows-only gate, and the added tests for weakening or false-green behavior. I also checked the repository rules in AGENTS.md and the architecture/domain routing in CONTEXT-MAP.md; the shared resolver remains the correct architectural choke point.

Verification: uv sync --extra dev completed; uv run pytest tests/test_filesystem_tools.py -q passed with 54 tests; git diff --check github/main...HEAD passed. A direct predicate probe confirmed that nul/report.md, COM¹.txt, and LPT²/log.txt are not caught. No live Windows run was available.

[gloryfromca]

Blocking: scan every path component and include Windows' superscript COM/LPT aliases.

This checks only p.name, so nul/report.md passes even though Windows applies its naming rules to directory components too. The predicate also returns false for COM¹.txt and LPT²; Microsoft documents those aliases as reserved, and CPython 3.13's ntpath.isreserved both scans all components and includes superscript 1/2/3 in its COM/LPT set. Consequently the central fence still accepts reserved device paths, and a final COM¹.txt can address a serial device instead of the reported file. The current subdir/nul test covers only the opposite component order. Please cover both gaps and add regressions for them.

[LivXue]

Windows compatibility review: three findings reproduced on native Windows 11 (build 26300), Python 3.13.12.

Scope: both changed files and their filesystem/search, delivery, viewer and attachment callers at head ddb81af819703299d2111defade49fbc038ea3d1, compared with base 4acc63cebd0752652194f9e4989628da29a8a25d, in isolated detached checkouts. These observations add native evidence beyond the existing component/superscript-alias discussion.

1. P1 - NUL: still produces a successful write receipt without a file. filesystem.py:73, resolver:88-93. The predicate compares NUL: literally, so it passes; native resolution then returns <workspace>\NUL, after the only device check. With allowed_dirs=(workspace,), the real WriteFileTool.execute(path="NUL:", content=payload) returned Successfully wrote 28 bytes, a diff and a file_change, while directory enumeration remained empty. Bare nul was correctly refused at head; an ordinary file round-tripped. The base also accepted NUL:: this is an unclosed instance of the original data-loss defect, not a newly introduced write failure. Account for colon device spellings before I/O and ensure normalization cannot introduce an unchecked device name; add this real write-level regression.

2. P2 - legitimate com0.txt and lpt0.txt files become unreadable/unwritable. filesystem.py:66-68. range(10) adds zero-numbered names. Native exclusive file creation produced real directory entries for both names, and CPython's ntpath.isreserved returned false. The base's real read/write tools read and updated both; this head rejected both as devices. An ordinary-name control worked on both revisions. Match the documented device set instead of widening it to zero: Microsoft's naming rules list COM/LPT 1-9 and the superscript aliases. Keep positive and negative Windows file-I/O controls.

3. P2 - the new no-write assertion fails on Windows even when the guard works. test_filesystem_tools.py:328. In a fresh empty directory, (workspace / "nul").exists() already returns true for the device. The tool returned the expected refusal, then this added assertion failed. Check directory entries or verify that the write operation was not reached; probing the device's existence cannot prove that no file was created.

Verification used PYTHONUTF8=1, UV_NO_SYNC=1 and isolated user/Raven homes after uv sync --frozen --extra dev:

uv run pytest tests/test_filesystem_tools.py -n 0 -x --tb=short --basetemp "$reviewRunDir/pytest-filesystem"
# 52 passed, 1 failed at line 328; stopped before the last test.
uv run pytest tests/test_filesystem_tools.py::test_a_read_of_a_device_name_answers_an_error_not_a_missing_file -n 0 -x --tb=short --basetemp "$runDir/pytest-read"
# Remaining test: 1 passed.
uv run python ../probe_device_paths.py
# Native resolver: NUL: accepted and normalized to NUL; bare nul/nul.md refused.
uv run python ../probe_write_device.py --source head
uv run python ../probe_write_device.py --source base
# Findings 1's real write/control results on both exact revisions.
uv run python ../probe_ordinary_names.py --source head
uv run python ../probe_ordinary_names.py --source base
# Findings 2-3: regular COM0/LPT0 files; NUL exists before any write.
git diff --check 4acc63cebd0752652194f9e4989628da29a8a25d...ddb81af819703299d2111defade49fbc038ea3d1
# Passed.

The scratch probes import the actual checked-out modules and use real file tools; source paths were verified. Serial/console aliases were checked only as strings, without opening hardware. No Windows 10, remote UNC share, full repository suite or live viewer session was tested. Non-failing shared-setup idle warnings occurred in both pytest runs.