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

.ai/contexts/ipc-bridge.md

@@ -187,6 +187,17 @@ 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;
+- 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.
+
 ### 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`.

CHANGELOG.md

@@ -4,6 +4,8 @@ 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)
 ### New
 - A session's Changes panel also lists the changes in the worktrees its subagents are working in, under a header naming the agent and its branch. Those rows open as read-only diffs; a subagent that works in the session's own directory adds nothing. (#303)
 - A live session on a remote host that is not open in a terminal has a Send a prompt… button on its row: type a text and it is written to the running session as a new prompt, without attaching. It needs `ncat` or an OpenBSD `nc` on the host, and is refused for a Windows host. The dialog says "Sent": the session's own status shows whether it picked the prompt up. (#219)

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)));
 }
 
 /**

resolve-path-on-disk.js

@@ -57,6 +57,105 @@ 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"
+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 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' && !VERBATIM_PREFIX.test(filePath)) {
+    const trimmed = stripTrailingDotsAndSpaces(literal);
+    if (trimmed !== literal) out.push(trimmed);
+  }
+  return out;
+}
+
+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 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;
+    }
+  }
+  return { paths: [...paths], unresolved };
+}
+
+async function sensitiveCandidatesAsync(filePath) {
+  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;
+    }
+  }
+  return { paths: [...paths], unresolved };
+}
+
 /**
  * True when `child` is `parent` itself or lies beneath it.
  *
@@ -76,4 +175,4 @@ function isInsideDir(child, parent) {
   return c === p || c.startsWith(p + path.sep);
 }
 
-module.exports = { resolveOnDisk, resolveOnDiskAsync, isInsideDir };
+module.exports = { resolveOnDisk, resolveOnDiskAsync, isInsideDir, stripExtendedPrefix, stripTrailingDotsAndSpaces, sensitiveCandidates, sensitiveCandidatesAsync };

test/sensitive-path-windows.test.js

@@ -0,0 +1,197 @@
+'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]);
+});
+
+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);
+});
+
+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]);
+});