Skip to content

(file-panel): refuse 8.3 short names and \?\ paths into credential directories (#390) - #399

Merged
devsuitup merged 4 commits into
mainfrom
fix/390-sensitive-path-windows
Oct 2, 2026
Merged

devsuitup merged 4 commits into
mainfrom
fix/390-sensitive-path-windows

Conversation

@devsuitup

Copy link
Copy Markdown
Owner

Closes #390

What changed

The sensitive-path denylist (isSensitivePath, and its async twin isSensitivePathAsync from #389) tested the literal path and the JS fs.realpath result. On Windows that misses two spellings of a credential location:

  • an 8.3 short name for a denylisted directory (SSH~1\id_rsa): the JS realpath walker keeps the short name;
  • a \\?\ path through a junction into a denylisted directory: the JS realpath throws EISDIR, resolveOnDisk returned null and only the literal was tested.

Both guards now share one candidate builder (sensitiveCandidates / sensitiveCandidatesAsync in resolve-path-on-disk.js) and match the denylist against:

  • the literal path and the same path with a \\?\ / \\.\ prefix removed (\\?\UNC\h\s becomes \\h\s);
  • the JS realpath and the native realpath (fs.realpath.native, which expands short names). Both are tried and every result is matched, because they diverge in each direction;
  • for a path that does not exist yet (save of a new file), the deepest existing ancestor resolved the same two ways, with the missing tail re-attached;
  • fail closed: a resolution error other than ENOENT/ENOTDIR that yields no path at all makes the path sensitive.

resolveOnDisk / resolveOnDiskAsync are unchanged (the allowlist and containment checks keep comparing against the JS realpath). Rationale is in .ai/contexts/ipc-bridge.md, "Sensitive-path candidates".

Tests

New test/sensitive-path-windows.test.js:

  • pure tests of stripExtendedPrefix (run on every platform);
  • win32: short-named .ssh for an existing and a not-yet-created file, a \\?\ path through a junction, a \\?\ path with a short name, a \\?\ path when only the JS realpath can resolve it, fail-closed on EACCES, and two negatives (ordinary file, missing file under an ordinary directory). Each is asserted on both guards.
  • The 8.3 cases create the fixture and read its short name via Scripting.FileSystemObject; they skip with a stated reason only if the volume generates no short names (it does here).

Before the fix 8 of the 11 original tests failed (stripExtendedPrefix is not a function, and the short-name / \\?\ cases returned false). After: 12/12, plus the related files (resolve-path-on-disk-async, ipc-path-validator, terminal-path-target, terminal-path-links, read-file-for-panel-bounds, viewer-save-guard).

Mutations run (each reverted):

  • sync guard uses JS realpath only: both short-name tests red;
  • async guard uses JS realpath only: three short-name tests red;
  • no ancestor walk: the not-yet-created short-name test red;
  • EACCES classed as "missing": the fail-closed test red;
  • no prefix stripping: stays green. The path is still refused because the JS realpath fails with EISDIR and fail-closed catches it, so stripping is defence in depth here; it is covered by the pure tests only.

Not verified

  • Only measured on a local Windows 11 NTFS volume; windows-2022 CI is the check for the rest.
  • A dangling symlink is still resolved by neither realpath (existing documented gap).

…rectories

The sensitive-path denylist tested the literal path and the JS realpath,
which keeps 8.3 short names and fails with EISDIR on a \?\ path through a
junction. Both guards now share one candidate builder that also tries the
native realpath, strips the extended prefix, resolves the deepest existing
ancestor of a missing path, and treats an unexplained resolution failure
as sensitive.

Closes #390
@devsuitup

Copy link
Copy Markdown
Owner Author

Reviewing 5ae2d26 (adversarial review in progress).

@devsuitup

Copy link
Copy Markdown
Owner Author

Adversarial review at 5ae2d26: no bypass found. Refused by both guards on this machine (real 8.3 names, junctions): 8.3 at every segment, \?\, \.\, //?/, \?\UNC\localhost\C$, \localhost\C$, \?\GLOBALROOT\Device\…, ::$DATA, case variants, not-yet-existing save paths; sync and async agree. Being fixed: (1) a JS-realpath success + native-realpath EPERM leaves only the 8.3 spelling as candidate, so fail-closed does not hold in the mixed case (reproduced with an ACL deny); (2) trailing-dot/space spellings pass and open-path → shell.openPath normalises them (pre-existing, outside the panel); (3) the ancestor walk is quadratic — 15.4 ms vs 0.85 ms for a missing file 6 levels deep on the sync path used per .md file.

…dots, walk ancestors once

Review follow-up for #390. A failure of either realpath (other than
ENOENT/ENOTDIR) now marks the path sensitive even when the other one
resolved. On win32 a candidate with trailing dots and spaces removed from
each segment is matched too, since the shell opens the real file. The
missing-path ancestor walk uses one lstat per level and resolves only the
first existing ancestor instead of two realpaths per level.

Refs #390
@devsuitup

Copy link
Copy Markdown
Owner Author

Reviewing b6561ff (adversarial review in progress).

@devsuitup

Copy link
Copy Markdown
Owner Author

Re-review at b6561ff: changes needed. Blocker, reproduced: the trailing-dot/space candidate is skipped for every prefixed path, including \.\ — which Win32 normalises — so \.\C:\…\.ssh.\id_rsa (also //./…/.git-credentials. and \.\UNC\localhost\C$\…) is read by fs while both guards answer not sensitive. The strip will be skipped only for \?\. The four earlier points hold: mixed realpath EPERM is now sensitive in both guards, trailing-dot spellings (plain, UNC, Volume GUID, GLOBALROOT) are refused, the walk is bounded (2.8 ms vs 13.3 ms for a missing file 6 deep, sync), no false positive on ordinary files; full bypass sweep still refused.

Win32 normalises the \.\ device prefix; only \?\ skips normalisation.
The trailing-dot candidate was skipped for any stripped prefix, so
\.\C:\...\.ssh.\id_rsa passed both guards. Skip it only for \?\.

Refs #390
@devsuitup

Copy link
Copy Markdown
Owner Author

Re-review at 0ff8915: the blocker is closed — the trailing-dot/space strip is now skipped only for a verbatim \?\ prefix (/^[\/]{2}\?[\/]/), so \.\C:\…\.ssh.\id_rsa, //./C:/…/.git-credentials. and \.\UNC\localhost\C$\…\.ssh.\id_rsa are trimmed and refused; the three spellings are pinned (red before, green after) with a negative for \?\. Ready to merge once CI is green on 0ff8915.

@devsuitup
devsuitup merged commit 25f5fa6 into main Oct 2, 2026
10 checks passed
@devsuitup
devsuitup deleted the fix/390-sensitive-path-windows branch October 2, 2026 10:00
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.

(file-panel): the sensitive-path guard misses 8.3 short names and \?\ paths on Windows

1 participant