From 5ae2d2640d7450c36cb8763c39d170c90df18d48 Mon Sep 17 00:00:00 2001 From: Jean-Baptiste Date: Thu, 1 Oct 2026 20:23:30 +0200 Subject: [PATCH 1/3] (file-panel): refuse 8.3 short names and \?\ paths into credential directories 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 --- .ai/contexts/ipc-bridge.md | 10 ++ CHANGELOG.md | 3 + ipc-path-validator.js | 29 ++---- resolve-path-on-disk.js | 90 +++++++++++++++++- test/sensitive-path-windows.test.js | 139 ++++++++++++++++++++++++++++ 5 files changed, 250 insertions(+), 21 deletions(-) create mode 100644 test/sensitive-path-windows.test.js diff --git a/.ai/contexts/ipc-bridge.md b/.ai/contexts/ipc-bridge.md index e6988ff2..d3e6cee9 100644 --- a/.ai/contexts/ipc-bridge.md +++ b/.ai/contexts/ipc-bridge.md @@ -187,6 +187,16 @@ Every handler that takes a renderer-supplied path or derives a spawn location fr | `git-changes-save` | `isSafeRepoRelativePath` + the same `resolveTargetInsideRepo`, plus a version token that must still match the bytes on disk; the write runs on the path the guard returned, never on a re-derived one | shape + disk-resolved containment + denylist — **the only write handler in the app whose entire input is a relative path from the renderer**, so containment is the guard, not an afterthought; `save-file-for-panel` next to it has none (it takes an absolute path and checks only `isSensitivePath`) and is not the precedent to copy here | | `git-changes-diff` | `isSafeGitPath`, or `isSafeNoIndexPath` + containment when `untracked` (`git-changes-runner.js`) | a git pathspec relative to an arbitrary (possibly remote) cwd; see `.ai/contexts/changes-view.md` ("Quoting rule") for why this is a denylist, not an allowlist. The untracked variant is a real filesystem operand of `git diff --no-index`, which has no repository-boundary check of its own: on top of the syntactic guard it is resolved with `realpath`/`stat` against the resolved cwd (local) or checked against `git ls-files --others` (remote), git receives the guard's operand rather than the caller's, and the returned diff must name that same path in its `diff --git` line — see "Untracked files" in the same doc | +### Sensitive-path candidates + +`isSensitivePath` and `isSensitivePathAsync` (`ipc-path-validator.js`) share one candidate builder, `sensitiveCandidates` / `sensitiveCandidatesAsync` (`resolve-path-on-disk.js`), and test the denylist against every spelling it returns: + +- the literal path, and the same path with a Windows `\\?\` / `\\.\` prefix removed (`\\?\UNC\host\share` becomes `\\host\share`); +- the JS realpath **and** the native realpath. They diverge on Windows: the JS walker keeps 8.3 short names (`SSH~1` for `.ssh`) and throws `EISDIR` on a `\\?\` path through a junction, while `fs.realpath.native` expands the short name and resolves the junction. Either one alone misses a spelling the other catches, so both are tried and every result is matched; +- when nothing exists at the path yet (a file about to be saved), the deepest existing ancestor resolved the same two ways, with the missing tail re-attached — otherwise `SSH~1\new-key` would pass because there is nothing to resolve. + +A resolution that fails for a reason other than `ENOENT`/`ENOTDIR` (`EACCES`, `EPERM`, `ELOOP`, …) and yields no path at all is treated as sensitive: the path may exist behind a spelling we could not resolve. `resolveOnDisk` / `resolveOnDiskAsync` are unchanged and stay on the JS realpath (the allowlist and containment checks compare against it). The tests are in `test/sensitive-path-windows.test.js`; the 8.3 cases skip, with a stated reason, on a volume that generates no short names. + ### Non-obvious behaviors - **`preload.js` is the *single* surface the renderer sees**. If you add `ipcMain.handle('xyz', ...)` but forget to add `xyz: () => ipcRenderer.invoke('xyz')` in preload, the renderer can't call it. Symptom: `window.api.xyz is not a function`. diff --git a/CHANGELOG.md b/CHANGELOG.md index 97dae2a5..3331edd3 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -4,6 +4,9 @@ What changes for you in each release of Switchboard. How to write an entry: [doc ## Unreleased +### Fixed +- On Windows, the file panel no longer opens or saves a credential file (such as one under `.ssh`) through its 8.3 short name or a `\\?\` path. (#390) + ## v0.0.86 — 2026-10-01 ### New diff --git a/ipc-path-validator.js b/ipc-path-validator.js index bf1e89b0..dbd2f931 100644 --- a/ipc-path-validator.js +++ b/ipc-path-validator.js @@ -28,7 +28,7 @@ const os = require('os'); const path = require('path'); -const { resolveOnDisk, resolveOnDiskAsync } = require('./resolve-path-on-disk'); +const { resolveOnDisk, sensitiveCandidates, sensitiveCandidatesAsync } = require('./resolve-path-on-disk'); const CLAUDE_DIR = path.join(os.homedir(), '.claude'); @@ -61,28 +61,17 @@ const SENSITIVE_PATH_PATTERNS = [ * @returns {boolean} */ function isSensitivePath(filePath) { - const resolved = path.resolve(filePath); - if (SENSITIVE_PATH_PATTERNS.some(pattern => pattern.test(resolved))) return true; - - // Also test the on-disk real path: a symlink (e.g. a "notes" directory that - // is actually a link to ~/.ssh) makes the literal string look harmless - // while the file it opens is not. Skipped when nothing exists there yet — - // see the file header. - const real = resolveOnDisk(resolved); - if (real && real !== resolved) { - return SENSITIVE_PATH_PATTERNS.some(pattern => pattern.test(real)); - } - return false; + const { paths, unresolved } = sensitiveCandidates(filePath); + return unresolved || matchesDenylist(paths); } async function isSensitivePathAsync(filePath) { - const resolved = path.resolve(filePath); - if (SENSITIVE_PATH_PATTERNS.some(pattern => pattern.test(resolved))) return true; - const real = await resolveOnDiskAsync(resolved); - if (real && real !== resolved) { - return SENSITIVE_PATH_PATTERNS.some(pattern => pattern.test(real)); - } - return false; + const { paths, unresolved } = await sensitiveCandidatesAsync(filePath); + return unresolved || matchesDenylist(paths); +} + +function matchesDenylist(paths) { + return paths.some(candidate => SENSITIVE_PATH_PATTERNS.some(pattern => pattern.test(candidate))); } /** diff --git a/resolve-path-on-disk.js b/resolve-path-on-disk.js index e1a351a9..5dd97f8b 100644 --- a/resolve-path-on-disk.js +++ b/resolve-path-on-disk.js @@ -57,6 +57,94 @@ async function resolveOnDiskAsync(filePath) { } } +const EXTENDED_UNC = /^[\\/]{2}[?.][\\/]UNC[\\/]/i; +const EXTENDED_DRIVE = /^[\\/]{2}[?.][\\/](?=[A-Za-z]:)/; +const MISSING = new Set(['ENOENT', 'ENOTDIR']); + +// see .ai/contexts/ipc-bridge.md, "Sensitive-path candidates" +function stripExtendedPrefix(p) { + if (typeof p !== 'string') return p; + if (EXTENDED_UNC.test(p)) return '\\\\' + p.replace(EXTENDED_UNC, ''); + return p.replace(EXTENDED_DRIVE, ''); +} + +function attempt(fn, arg) { + try { return { real: fn(arg) }; } catch (e) { return { code: e && e.code }; } +} + +function attemptAsync(fn, arg) { + return new Promise((resolve) => { + try { + fn(arg, (e, real) => resolve(e ? { code: e.code } : { real })); + } catch (e) { + resolve({ code: e && e.code }); + } + }); +} + +function ancestorsOf(p) { + const out = []; + let dir = path.dirname(p); + let prev = p; + while (dir !== prev) { + out.push(dir); + prev = dir; + dir = path.dirname(dir); + } + return out; +} + +function collect(literal, results) { + const paths = [literal]; + for (const r of results) if (r.real) paths.push(r.real); + const resolved = paths.length > 1; + const missing = !resolved && results.every((r) => MISSING.has(r.code)); + return { paths, resolved, missing }; +} + +// see .ai/contexts/ipc-bridge.md, "Sensitive-path candidates" +function sensitiveCandidates(filePath) { + const literal = path.resolve(stripExtendedPrefix(filePath)); + const paths = new Set([path.resolve(filePath), literal]); + const first = collect(literal, [attempt(fs.realpathSync, literal), attempt(fs.realpathSync.native, literal)]); + first.paths.forEach((p) => paths.add(p)); + if (first.resolved) return { paths: [...paths], unresolved: false }; + if (!first.missing) return { paths: [...paths], unresolved: true }; + for (const dir of ancestorsOf(literal)) { + const r = collect(dir, [attempt(fs.realpathSync, dir), attempt(fs.realpathSync.native, dir)]); + if (r.resolved) { + const tail = path.relative(dir, literal); + r.paths.slice(1).forEach((p) => paths.add(path.join(p, tail))); + return { paths: [...paths], unresolved: false }; + } + if (!r.missing) return { paths: [...paths], unresolved: true }; + } + return { paths: [...paths], unresolved: false }; +} + +async function sensitiveCandidatesAsync(filePath) { + const literal = path.resolve(stripExtendedPrefix(filePath)); + const paths = new Set([path.resolve(filePath), literal]); + const both = async (p) => collect(p, [ + await attemptAsync(fs.realpath, p), + await attemptAsync(fs.realpath.native, p), + ]); + const first = await both(literal); + first.paths.forEach((p) => paths.add(p)); + if (first.resolved) return { paths: [...paths], unresolved: false }; + if (!first.missing) return { paths: [...paths], unresolved: true }; + for (const dir of ancestorsOf(literal)) { + const r = await both(dir); + if (r.resolved) { + const tail = path.relative(dir, literal); + r.paths.slice(1).forEach((p) => paths.add(path.join(p, tail))); + return { paths: [...paths], unresolved: false }; + } + if (!r.missing) return { paths: [...paths], unresolved: true }; + } + return { paths: [...paths], unresolved: false }; +} + /** * True when `child` is `parent` itself or lies beneath it. * @@ -76,4 +164,4 @@ function isInsideDir(child, parent) { return c === p || c.startsWith(p + path.sep); } -module.exports = { resolveOnDisk, resolveOnDiskAsync, isInsideDir }; +module.exports = { resolveOnDisk, resolveOnDiskAsync, isInsideDir, stripExtendedPrefix, sensitiveCandidates, sensitiveCandidatesAsync }; diff --git a/test/sensitive-path-windows.test.js b/test/sensitive-path-windows.test.js new file mode 100644 index 00000000..f7758161 --- /dev/null +++ b/test/sensitive-path-windows.test.js @@ -0,0 +1,139 @@ +'use strict'; + +const test = require('node:test'); +const assert = require('node:assert/strict'); +const fs = require('node:fs'); +const os = require('node:os'); +const path = require('node:path'); +const { execFileSync } = require('node:child_process'); + +const { stripExtendedPrefix } = require('../resolve-path-on-disk'); +const { isSensitivePath, isSensitivePathAsync } = require('../ipc-path-validator'); + +const BS = String.fromCharCode(92); +const EXT = BS + BS + '?' + BS; +const DEV = BS + BS + '.' + BS; + +test('stripExtendedPrefix drops the \\\\?\\ and \\\\.\\ drive prefixes', () => { + assert.strictEqual(stripExtendedPrefix(EXT + 'C:' + BS + 'a' + BS + 'b'), 'C:' + BS + 'a' + BS + 'b'); + assert.strictEqual(stripExtendedPrefix(DEV + 'C:' + BS + 'a'), 'C:' + BS + 'a'); +}); + +test('stripExtendedPrefix turns \\\\?\\UNC\\host\\share into \\\\host\\share, whatever the case', () => { + assert.strictEqual(stripExtendedPrefix(EXT + 'UNC' + BS + 'h' + BS + 's' + BS + 'a'), BS + BS + 'h' + BS + 's' + BS + 'a'); + assert.strictEqual(stripExtendedPrefix(EXT + 'unc' + BS + 'h' + BS + 's'), BS + BS + 'h' + BS + 's'); +}); + +test('stripExtendedPrefix leaves ordinary, relative and plain UNC paths alone', () => { + for (const p of ['C:' + BS + 'a', 'a' + BS + 'b', BS + BS + 'h' + BS + 's' + BS + 'a', '/home/u/.ssh/k', '']) { + assert.strictEqual(stripExtendedPrefix(p), p); + } +}); + +const win = process.platform === 'win32'; + +function shortPathOf(dir) { + const out = execFileSync('powershell', [ + '-NoProfile', '-Command', + '(New-Object -ComObject Scripting.FileSystemObject).GetFolder($env:SB_P).ShortPath', + ], { encoding: 'utf8', env: { ...process.env, SB_P: dir }, timeout: 30000 }); + return out.trim(); +} + +const root = fs.mkdtempSync(path.join(os.tmpdir(), 'sb-sens-')); +const realRoot = fs.realpathSync.native(root); +const sshDir = path.join(realRoot, '.ssh'); +fs.mkdirSync(sshDir); +fs.writeFileSync(path.join(sshDir, 'id_rsa'), 'key\n'); + +let shortSsh = null; +let junction = null; +if (win) { + try { + const s = shortPathOf(sshDir); + if (s && s.toLowerCase() !== sshDir.toLowerCase() && fs.existsSync(path.join(s, 'id_rsa'))) shortSsh = s; + } catch {} + const j = path.join(realRoot, 'notes'); + try { fs.symlinkSync(sshDir, j, 'junction'); junction = j; } catch {} +} + +test.after(() => { + if (junction) fs.rmdirSync(junction); + fs.rmSync(root, { recursive: true, force: true }); +}); + +const bothGuards = async (p) => [isSensitivePath(p), await isSensitivePathAsync(p)]; + +test('a plain file next to the credential directory stays allowed', async () => { + const ok = path.join(realRoot, 'plain.txt'); + fs.writeFileSync(ok, 'x\n'); + assert.deepStrictEqual(await bothGuards(ok), [false, false]); +}); + +test('an 8.3 short name for a credential directory is refused by both guards', + { skip: !win ? 'win32 only' : !shortSsh && 'this volume generates no 8.3 short names' }, + async () => { + assert.deepStrictEqual(await bothGuards(path.join(shortSsh, 'id_rsa')), [true, true]); + }); + +test('an 8.3 short name for a credential directory is refused for a file not created yet', + { skip: !win ? 'win32 only' : !shortSsh && 'this volume generates no 8.3 short names' }, + async () => { + assert.deepStrictEqual(await bothGuards(path.join(shortSsh, 'not-yet')), [true, true]); + }); + +test('a \\\\?\\ path through a junction into a credential directory is refused by both guards', + { skip: !win ? 'win32 only' : !junction && 'cannot create a junction here' }, + async () => { + assert.deepStrictEqual(await bothGuards(EXT + path.join(junction, 'id_rsa')), [true, true]); + }); + +test('a \\\\?\\ path through a junction is still refused when only the JS realpath can resolve it', + { skip: !win ? 'win32 only' : !junction && 'cannot create a junction here' }, + async () => { + const nativeSync = fs.realpathSync.native; + const nativeAsync = fs.realpath.native; + const missing = () => { throw Object.assign(new Error('missing'), { code: 'ENOENT' }); }; + fs.realpathSync.native = missing; + fs.realpath.native = (p, cb) => cb(Object.assign(new Error('missing'), { code: 'ENOENT' })); + try { + assert.deepStrictEqual(await bothGuards(EXT + path.join(junction, 'id_rsa')), [true, true]); + } finally { + fs.realpathSync.native = nativeSync; + fs.realpath.native = nativeAsync; + } + }); + +test('a \\\\?\\ path with an 8.3 credential directory is refused by both guards', + { skip: !win ? 'win32 only' : !shortSsh && 'this volume generates no 8.3 short names' }, + async () => { + assert.deepStrictEqual(await bothGuards(EXT + path.join(shortSsh, 'id_rsa')), [true, true]); + }); + +test('a \\\\?\\ path to an ordinary file is not refused', { skip: !win && 'win32 only' }, async () => { + assert.deepStrictEqual(await bothGuards(EXT + path.join(realRoot, 'plain.txt')), [false, false]); +}); + +test('a realpath failure other than "does not exist" fails closed in both guards', { skip: !win && 'win32 only' }, async () => { + const target = path.join(realRoot, 'plain.txt'); + const fail = () => { throw Object.assign(new Error('denied'), { code: 'EACCES' }); }; + const realSync = fs.realpathSync; + const nativeSync = fs.realpathSync.native; + const realAsync = fs.realpath; + const nativeAsync = fs.realpath.native; + const failCb = (p, cb) => cb(Object.assign(new Error('denied'), { code: 'EACCES' })); + fs.realpathSync = Object.assign(fail, { native: fail }); + fs.realpath = Object.assign(failCb, { native: failCb }); + try { + assert.strictEqual(isSensitivePath(target), true); + assert.strictEqual(await isSensitivePathAsync(target), true); + } finally { + fs.realpathSync = Object.assign(realSync, { native: nativeSync }); + fs.realpath = Object.assign(realAsync, { native: nativeAsync }); + } +}); + +test('a missing path under an ordinary directory is not refused for failing to resolve', { skip: !win && 'win32 only' }, async () => { + const missing = path.join(realRoot, 'nowhere', 'file.txt'); + assert.deepStrictEqual(await bothGuards(missing), [false, false]); +}); From b6561ffae4dbc829fa131d56c708e98e46b6be83 Mon Sep 17 00:00:00 2001 From: Jean-Baptiste Date: Thu, 1 Oct 2026 22:00:26 +0200 Subject: [PATCH 2/3] (file-panel): fail closed on a mixed realpath failure, trim trailing 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 --- .ai/contexts/ipc-bridge.md | 5 +- resolve-path-on-disk.js | 104 +++++++++++++++------------- test/sensitive-path-windows.test.js | 40 +++++++++++ 3 files changed, 100 insertions(+), 49 deletions(-) diff --git a/.ai/contexts/ipc-bridge.md b/.ai/contexts/ipc-bridge.md index d3e6cee9..b3590edf 100644 --- a/.ai/contexts/ipc-bridge.md +++ b/.ai/contexts/ipc-bridge.md @@ -193,9 +193,10 @@ Every handler that takes a renderer-supplied path or derives a spawn location fr - the literal path, and the same path with a Windows `\\?\` / `\\.\` prefix removed (`\\?\UNC\host\share` becomes `\\host\share`); - the JS realpath **and** the native realpath. They diverge on Windows: the JS walker keeps 8.3 short names (`SSH~1` for `.ssh`) and throws `EISDIR` on a `\\?\` path through a junction, while `fs.realpath.native` expands the short name and resolves the junction. Either one alone misses a spelling the other catches, so both are tried and every result is matched; -- when nothing exists at the path yet (a file about to be saved), the deepest existing ancestor resolved the same two ways, with the missing tail re-attached — otherwise `SSH~1\new-key` would pass because there is nothing to resolve. +- on win32, the same path with trailing dots and spaces removed from each segment (`.ssh.\id_rsa`, `.netrc `): Win32 normalisation drops them, so `shell.openPath` reaches the real file while the literal string matches nothing. Not applied to `\?\` paths, which Win32 does not normalise; +- when nothing exists at the path yet (a file about to be saved), the first existing ancestor — found with one `lstat` per level going up, then resolved the two ways — with the missing tail re-attached, otherwise `SSH~1\new-key` would pass because there is nothing to resolve. -A resolution that fails for a reason other than `ENOENT`/`ENOTDIR` (`EACCES`, `EPERM`, `ELOOP`, …) and yields no path at all is treated as sensitive: the path may exist behind a spelling we could not resolve. `resolveOnDisk` / `resolveOnDiskAsync` are unchanged and stay on the JS realpath (the allowlist and containment checks compare against it). The tests are in `test/sensitive-path-windows.test.js`; the 8.3 cases skip, with a stated reason, on a volume that generates no short names. +Fail closed: if **either** realpath fails for a reason other than `ENOENT`/`ENOTDIR` (`EACCES`, `EPERM`, `ELOOP`, ...), or an ancestor cannot be `lstat`ed, the path is treated as sensitive even when the other realpath succeeded: the one that failed may have been the one that sees the credential location. `resolveOnDisk` / `resolveOnDiskAsync` are unchanged and stay on the JS realpath (the allowlist and containment checks compare against it). The tests are in `test/sensitive-path-windows.test.js`; the 8.3 cases skip, with a stated reason, on a volume that generates no short names. ### Non-obvious behaviors diff --git a/resolve-path-on-disk.js b/resolve-path-on-disk.js index 5dd97f8b..6b6d29c5 100644 --- a/resolve-path-on-disk.js +++ b/resolve-path-on-disk.js @@ -82,67 +82,77 @@ function attemptAsync(fn, arg) { }); } -function ancestorsOf(p) { - const out = []; - let dir = path.dirname(p); - let prev = p; - while (dir !== prev) { - out.push(dir); - prev = dir; - dir = path.dirname(dir); +function stripTrailingDotsAndSpaces(p) { + const root = path.parse(p).root; + const rest = p.slice(root.length).split(path.sep).map((seg) => seg.replace(/[. ]+$/, '')); + return root + rest.join(path.sep); +} + +function literalsOf(filePath) { + const literal = path.resolve(stripExtendedPrefix(filePath)); + const out = [literal]; + if (process.platform === 'win32' && stripExtendedPrefix(filePath) === filePath) { + const trimmed = stripTrailingDotsAndSpaces(literal); + if (trimmed !== literal) out.push(trimmed); } return out; } -function collect(literal, results) { - const paths = [literal]; - for (const r of results) if (r.real) paths.push(r.real); - const resolved = paths.length > 1; - const missing = !resolved && results.every((r) => MISSING.has(r.code)); - return { paths, resolved, missing }; +const failed = (results) => results.some((r) => !r.real && !MISSING.has(r.code)); + +function addResolved(paths, results, tail) { + for (const r of results) if (r.real) paths.add(tail ? path.join(r.real, tail) : r.real); } // see .ai/contexts/ipc-bridge.md, "Sensitive-path candidates" function sensitiveCandidates(filePath) { - const literal = path.resolve(stripExtendedPrefix(filePath)); - const paths = new Set([path.resolve(filePath), literal]); - const first = collect(literal, [attempt(fs.realpathSync, literal), attempt(fs.realpathSync.native, literal)]); - first.paths.forEach((p) => paths.add(p)); - if (first.resolved) return { paths: [...paths], unresolved: false }; - if (!first.missing) return { paths: [...paths], unresolved: true }; - for (const dir of ancestorsOf(literal)) { - const r = collect(dir, [attempt(fs.realpathSync, dir), attempt(fs.realpathSync.native, dir)]); - if (r.resolved) { - const tail = path.relative(dir, literal); - r.paths.slice(1).forEach((p) => paths.add(path.join(p, tail))); - return { paths: [...paths], unresolved: false }; + const paths = new Set([path.resolve(filePath)]); + let unresolved = false; + for (const literal of literalsOf(filePath)) { + paths.add(literal); + const first = [attempt(fs.realpathSync, literal), attempt(fs.realpathSync.native, literal)]; + if (failed(first)) { unresolved = true; continue; } + if (first.some((r) => r.real)) { addResolved(paths, first); continue; } + let dir = literal; + for (;;) { + const parent = path.dirname(dir); + if (parent === dir) break; + dir = parent; + const st = attempt(fs.lstatSync, dir); + if (st.code && MISSING.has(st.code)) continue; + if (st.code) { unresolved = true; break; } + const results = [attempt(fs.realpathSync, dir), attempt(fs.realpathSync.native, dir)]; + if (failed(results)) unresolved = true; + else addResolved(paths, results, path.relative(dir, literal)); + break; } - if (!r.missing) return { paths: [...paths], unresolved: true }; } - return { paths: [...paths], unresolved: false }; + return { paths: [...paths], unresolved }; } async function sensitiveCandidatesAsync(filePath) { - const literal = path.resolve(stripExtendedPrefix(filePath)); - const paths = new Set([path.resolve(filePath), literal]); - const both = async (p) => collect(p, [ - await attemptAsync(fs.realpath, p), - await attemptAsync(fs.realpath.native, p), - ]); - const first = await both(literal); - first.paths.forEach((p) => paths.add(p)); - if (first.resolved) return { paths: [...paths], unresolved: false }; - if (!first.missing) return { paths: [...paths], unresolved: true }; - for (const dir of ancestorsOf(literal)) { - const r = await both(dir); - if (r.resolved) { - const tail = path.relative(dir, literal); - r.paths.slice(1).forEach((p) => paths.add(path.join(p, tail))); - return { paths: [...paths], unresolved: false }; + const paths = new Set([path.resolve(filePath)]); + let unresolved = false; + for (const literal of literalsOf(filePath)) { + paths.add(literal); + const first = [await attemptAsync(fs.realpath, literal), await attemptAsync(fs.realpath.native, literal)]; + if (failed(first)) { unresolved = true; continue; } + if (first.some((r) => r.real)) { addResolved(paths, first); continue; } + let dir = literal; + for (;;) { + const parent = path.dirname(dir); + if (parent === dir) break; + dir = parent; + const st = await attemptAsync(fs.lstat, dir); + if (st.code && MISSING.has(st.code)) continue; + if (st.code) { unresolved = true; break; } + const results = [await attemptAsync(fs.realpath, dir), await attemptAsync(fs.realpath.native, dir)]; + if (failed(results)) unresolved = true; + else addResolved(paths, results, path.relative(dir, literal)); + break; } - if (!r.missing) return { paths: [...paths], unresolved: true }; } - return { paths: [...paths], unresolved: false }; + return { paths: [...paths], unresolved }; } /** @@ -164,4 +174,4 @@ function isInsideDir(child, parent) { return c === p || c.startsWith(p + path.sep); } -module.exports = { resolveOnDisk, resolveOnDiskAsync, isInsideDir, stripExtendedPrefix, sensitiveCandidates, sensitiveCandidatesAsync }; +module.exports = { resolveOnDisk, resolveOnDiskAsync, isInsideDir, stripExtendedPrefix, stripTrailingDotsAndSpaces, sensitiveCandidates, sensitiveCandidatesAsync }; diff --git a/test/sensitive-path-windows.test.js b/test/sensitive-path-windows.test.js index f7758161..8d7b7136 100644 --- a/test/sensitive-path-windows.test.js +++ b/test/sensitive-path-windows.test.js @@ -137,3 +137,43 @@ test('a missing path under an ordinary directory is not refused for failing to r const missing = path.join(realRoot, 'nowhere', 'file.txt'); assert.deepStrictEqual(await bothGuards(missing), [false, false]); }); + +test('a failure of one realpath fails closed even when the other resolves', { skip: !win && 'win32 only' }, async () => { + const target = path.join(realRoot, 'plain.txt'); + const nativeSync = fs.realpathSync.native; + const nativeAsync = fs.realpath.native; + fs.realpathSync.native = () => { throw Object.assign(new Error('denied'), { code: 'EPERM' }); }; + fs.realpath.native = (p, cb) => cb(Object.assign(new Error('denied'), { code: 'EPERM' })); + try { + assert.deepStrictEqual(await bothGuards(target), [true, true]); + } finally { + fs.realpathSync.native = nativeSync; + fs.realpath.native = nativeAsync; + } +}); + +test('trailing dots and spaces on a credential name are refused by both guards', { skip: !win && 'win32 only' }, async () => { + for (const name of [path.join('.ssh.', 'id_rsa'), '.git-credentials.', '.netrc ', path.join('.ssh. ', 'x')]) { + assert.deepStrictEqual(await bothGuards(path.join(realRoot, name)), [true, true], name); + } +}); + +test('stripTrailingDotsAndSpaces trims each segment and keeps the root', { skip: !win && 'win32 only' }, () => { + const { stripTrailingDotsAndSpaces } = require('../resolve-path-on-disk'); + assert.strictEqual(stripTrailingDotsAndSpaces('C:' + BS + 'a. ' + BS + 'b.' + BS + 'c '), 'C:' + BS + 'a' + BS + 'b' + BS + 'c'); +}); + +test('a missing file is resolved with a bounded number of realpath calls', { skip: !win && 'win32 only' }, () => { + const real = fs.realpathSync; + const nativeSync = real.native; + let calls = 0; + const count = (fn) => Object.assign((...a) => { calls++; return fn(...a); }, { native: nativeSync }); + fs.realpathSync = count(real); + fs.realpathSync.native = (...a) => { calls++; return nativeSync(...a); }; + try { + isSensitivePath(path.join(realRoot, 'a', 'b', 'c', 'd', 'e', 'f', 'file.txt')); + } finally { + fs.realpathSync = Object.assign(real, { native: nativeSync }); + } + assert.ok(calls <= 6, 'realpath calls: ' + calls); +}); From 0ff8915644bc7f536138093bea3d732435802f79 Mon Sep 17 00:00:00 2001 From: Jean-Baptiste Date: Thu, 1 Oct 2026 23:05:44 +0200 Subject: [PATCH 3/3] (file-panel): trim trailing dots behind a \.\ prefix too 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 --- .ai/contexts/ipc-bridge.md | 2 +- resolve-path-on-disk.js | 3 ++- test/sensitive-path-windows.test.js | 18 ++++++++++++++++++ 3 files changed, 21 insertions(+), 2 deletions(-) diff --git a/.ai/contexts/ipc-bridge.md b/.ai/contexts/ipc-bridge.md index b3590edf..2e1aab5a 100644 --- a/.ai/contexts/ipc-bridge.md +++ b/.ai/contexts/ipc-bridge.md @@ -193,7 +193,7 @@ Every handler that takes a renderer-supplied path or derives a spawn location fr - the literal path, and the same path with a Windows `\\?\` / `\\.\` prefix removed (`\\?\UNC\host\share` becomes `\\host\share`); - the JS realpath **and** the native realpath. They diverge on Windows: the JS walker keeps 8.3 short names (`SSH~1` for `.ssh`) and throws `EISDIR` on a `\\?\` path through a junction, while `fs.realpath.native` expands the short name and resolves the junction. Either one alone misses a spelling the other catches, so both are tried and every result is matched; -- on win32, the same path with trailing dots and spaces removed from each segment (`.ssh.\id_rsa`, `.netrc `): Win32 normalisation drops them, so `shell.openPath` reaches the real file while the literal string matches nothing. Not applied to `\?\` paths, which Win32 does not normalise; +- on win32, the same path with trailing dots and spaces removed from each segment (`.ssh.\id_rsa`, `.netrc `): Win32 normalisation drops them, so `shell.openPath` reaches the real file while the literal string matches nothing. Not applied to `\\?\` paths, which Win32 does not normalise (the `\\.\` device prefix is normalised, so those are trimmed); - when nothing exists at the path yet (a file about to be saved), the first existing ancestor — found with one `lstat` per level going up, then resolved the two ways — with the missing tail re-attached, otherwise `SSH~1\new-key` would pass because there is nothing to resolve. Fail closed: if **either** realpath fails for a reason other than `ENOENT`/`ENOTDIR` (`EACCES`, `EPERM`, `ELOOP`, ...), or an ancestor cannot be `lstat`ed, the path is treated as sensitive even when the other realpath succeeded: the one that failed may have been the one that sees the credential location. `resolveOnDisk` / `resolveOnDiskAsync` are unchanged and stay on the JS realpath (the allowlist and containment checks compare against it). The tests are in `test/sensitive-path-windows.test.js`; the 8.3 cases skip, with a stated reason, on a volume that generates no short names. diff --git a/resolve-path-on-disk.js b/resolve-path-on-disk.js index 6b6d29c5..43570145 100644 --- a/resolve-path-on-disk.js +++ b/resolve-path-on-disk.js @@ -59,6 +59,7 @@ async function resolveOnDiskAsync(filePath) { const EXTENDED_UNC = /^[\\/]{2}[?.][\\/]UNC[\\/]/i; const EXTENDED_DRIVE = /^[\\/]{2}[?.][\\/](?=[A-Za-z]:)/; +const VERBATIM_PREFIX = /^[\\/]{2}\?[\\/]/; const MISSING = new Set(['ENOENT', 'ENOTDIR']); // see .ai/contexts/ipc-bridge.md, "Sensitive-path candidates" @@ -91,7 +92,7 @@ function stripTrailingDotsAndSpaces(p) { function literalsOf(filePath) { const literal = path.resolve(stripExtendedPrefix(filePath)); const out = [literal]; - if (process.platform === 'win32' && stripExtendedPrefix(filePath) === filePath) { + if (process.platform === 'win32' && !VERBATIM_PREFIX.test(filePath)) { const trimmed = stripTrailingDotsAndSpaces(literal); if (trimmed !== literal) out.push(trimmed); } diff --git a/test/sensitive-path-windows.test.js b/test/sensitive-path-windows.test.js index 8d7b7136..101a3c69 100644 --- a/test/sensitive-path-windows.test.js +++ b/test/sensitive-path-windows.test.js @@ -177,3 +177,21 @@ test('a missing file is resolved with a bounded number of realpath calls', { ski } assert.ok(calls <= 6, 'realpath calls: ' + calls); }); + +test('trailing dots on a credential name are refused behind a \\.\ prefix, which Win32 still normalises', { skip: !win && 'win32 only' }, async () => { + const drive = realRoot.slice(0, 2); + const rest = realRoot.slice(2); + const spellings = [ + DEV + path.join(realRoot, '.ssh.', 'id_rsa'), + '//./' + path.join(realRoot, '.git-credentials.').split(BS).join('/'), + DEV + 'UNC' + BS + 'localhost' + BS + drive[0] + '$' + path.join(rest, '.ssh.', 'id_rsa'), + ]; + for (const p of spellings) { + assert.deepStrictEqual(await bothGuards(p), [true, true], p); + } +}); + +test('trailing dots are not trimmed behind a \\?\ prefix, which Win32 does not normalise', { skip: !win && 'win32 only' }, async () => { + const p = EXT + path.join(realRoot, 'notes.'); + assert.deepStrictEqual(await bothGuards(p), [false, false]); +});