fix(browse): self-repair broken Windows DACLs on state dirs (#1605)

icacls '/inheritance:r /grant:r' can partially fail on localized or domain
accounts: inheritance strips but the user grant doesn't resolve, leaving a
machine-SID-only DACL the owner can't even list — the sidebar/PTY failure
chain in #1605, caused by the very hardening call meant to protect the dir.
mkdirSecure now verifies listability after hardening (a real readdir —
fs.accessSync doesn't consult NTFS ACLs) and repairs via icacls /reset,
re-hardens, and if hardening breaks access again leaves inherited ACLs:
functional-but-unhardened beats hardened-but-unusable. The icacls calls
carry windowsHide (#1835's last two sites) and the fork's static spawn-hide
tripwire lands here, pinning every covered site. file-permissions.test.ts
is already in the windows-free-tests curated shard, so the DACL contract
executes on windows-latest.

Ported from time-attack/gstack (GStack 2).

Co-authored-by: Sina Matian <sina@time-attack.dev>
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>

browse/src/file-permissions.ts

@@ -71,7 +71,7 @@ export function restrictFilePermissions(filePath: string): void {
       execFileSync(
         'icacls',
         [filePath, '/inheritance:r', '/grant:r', `${user}:(F)`],
-        { stdio: 'ignore' },
+        { stdio: 'ignore', windowsHide: true },
       );
     } catch (err) {
       warnIcaclsFailure(filePath, err);
@@ -101,7 +101,7 @@ export function restrictDirectoryPermissions(dirPath: string): void {
       execFileSync(
         'icacls',
         [dirPath, '/inheritance:r', '/grant:r', `${user}:(OI)(CI)(F)`],
-        { stdio: 'ignore' },
+        { stdio: 'ignore', windowsHide: true },
       );
     } catch (err) {
       warnIcaclsFailure(dirPath, err);
@@ -139,14 +139,54 @@ export function appendSecureFile(
   if (!existed) restrictFilePermissions(filePath);
 }
 
+/**
+ * Windows only: probe whether the current process can actually list the
+ * directory. `fs.accessSync` doesn't consult NTFS ACLs on Windows, so a
+ * real readdir is the only honest check.
+ */
+function canListDir(dirPath: string): boolean {
+  try { fs.readdirSync(dirPath); return true; } catch { return false; }
+}
+
+/**
+ * Windows only: repair a broken DACL on a state directory (#1605).
+ *
+ * `icacls /inheritance:r /grant:r <user>:(F)` is a single command, but the
+ * two halves can partially fail: inheritance gets stripped while the user
+ * grant doesn't resolve (localized account names, domain accounts, roaming
+ * profiles). The result is a DACL with no usable ACE — often just a machine
+ * SID — and the client can't read its own state files. `/reset` restores
+ * inherited ACLs from the parent, making the directory functional again.
+ * Functional-but-unhardened beats hardened-but-unusable.
+ */
+export function repairBrokenDacl(dirPath: string): void {
+  if (process.platform !== 'win32') return;
+  try {
+    execFileSync('icacls', [dirPath, '/reset', '/T', '/C', '/Q'], { stdio: 'ignore', windowsHide: true });
+  } catch (err) {
+    warnIcaclsFailure(dirPath, err);
+  }
+}
+
 /**
  * `mkdir -p` with owner-only directory permissions, cross-platform.
  * Replaces `fs.mkdirSync(path, { recursive: true, mode: 0o700 })` + Windows ACL.
  * Safe to call on an existing directory — re-applies the ACL idempotently.
+ *
+ * Windows: after applying the restricted ACL, verifies the directory is
+ * still listable by this process and repairs a broken DACL (#1605) if not.
  */
 export function mkdirSecure(dirPath: string): void {
   fs.mkdirSync(dirPath, { recursive: true, mode: 0o700 });
   restrictDirectoryPermissions(dirPath);
+  if (process.platform === 'win32' && !canListDir(dirPath)) {
+    repairBrokenDacl(dirPath);
+    restrictDirectoryPermissions(dirPath);
+    // If re-hardening broke access again, reset once more and leave the
+    // directory with inherited ACLs — the client must be able to read
+    // its own state.
+    if (!canListDir(dirPath)) repairBrokenDacl(dirPath);
+  }
 }
 
 /**

browse/test/file-permissions.test.ts

@@ -22,6 +22,7 @@ import {
   writeSecureFile,
   appendSecureFile,
   mkdirSecure,
+  repairBrokenDacl,
   __resetWarnedForTests,
 } from '../src/file-permissions';
 
@@ -145,4 +146,35 @@ describe('mkdirSecure', () => {
     expect(fs.existsSync(path.join(tmpDir, 'a', 'b'))).toBe(true);
     expect(fs.existsSync(d)).toBe(true);
   });
+
+  test('created directory is listable by the creating process', () => {
+    // #1605 contract: whatever ACL hardening happens, the client must be
+    // able to read its own state dir immediately after creation.
+    const d = path.join(tmpDir, 'state');
+    mkdirSecure(d);
+    fs.writeFileSync(path.join(d, 'browse.json'), '{}');
+    expect(fs.readdirSync(d)).toContain('browse.json');
+  });
+});
+
+describe('repairBrokenDacl', () => {
+  test('is a no-op on non-Windows platforms', () => {
+    if (process.platform === 'win32') return;
+    const d = path.join(tmpDir, 'dir');
+    fs.mkdirSync(d);
+    expect(() => repairBrokenDacl(d)).not.toThrow();
+  });
+
+  test('on Windows, does not throw and directory stays listable', () => {
+    if (process.platform !== 'win32') return;
+    const d = path.join(tmpDir, 'dir');
+    fs.mkdirSync(d);
+    expect(() => repairBrokenDacl(d)).not.toThrow();
+    expect(() => fs.readdirSync(d)).not.toThrow();
+  });
+
+  test('on Windows, swallows icacls failure on a nonexistent path', () => {
+    if (process.platform !== 'win32') return;
+    expect(() => repairBrokenDacl(path.join(tmpDir, 'nonexistent'))).not.toThrow();
+  });
 });

browse/test/windows-spawn-hide.test.ts

@@ -0,0 +1,55 @@
+/**
+ * Static tripwire for #1835: child spawns reachable on Windows must pass
+ * windowsHide, or every daemon relaunch / taskkill / icacls / powershell
+ * invocation flashes a black console window (and can steal focus).
+ *
+ * Source-level, same style as server-auth.test.ts / cdp-session-cleanup.test.ts:
+ * cheap, deterministic, runs on every platform.
+ */
+
+import { describe, expect, test } from 'bun:test';
+import * as fs from 'fs';
+import * as path from 'path';
+
+const SRC = (f: string) => fs.readFileSync(path.join(import.meta.dir, '../src', f), 'utf-8');
+
+/** Every occurrence of `needle` in `src` must have `windowsHide` within the
+ * next `window` chars (the spawn's options object). */
+function expectHideNearEvery(src: string, needle: string, window = 400): void {
+  let idx = src.indexOf(needle);
+  expect(idx).toBeGreaterThanOrEqual(0);
+  while (idx !== -1) {
+    const slice = src.slice(idx, idx + window);
+    expect(slice).toMatch(/windowsHide:\s*true/);
+    idx = src.indexOf(needle, idx + needle.length);
+  }
+}
+
+describe('windowsHide on Windows-reachable spawns (#1835)', () => {
+  test('daemon launch paths in cli.ts pass windowsHide', () => {
+    const cli = SRC('cli.ts');
+    // Installed path: node -e launcher — both the outer spawnSync and the
+    // inner detached daemon spawn (inside the launcher code string).
+    expect(cli).toContain('detached:true,windowsHide:true');
+    expectHideNearEvery(cli, "'-e', launcherCode]");
+    // Dev fallback: detached bun spawn.
+    expectHideNearEvery(cli, "nodeSpawn('bun'");
+    // taskkill (killServer).
+    expectHideNearEvery(cli, "'taskkill'");
+  });
+
+  test('Windows-only process probes pass windowsHide', () => {
+    // tasklist in isProcessAlive — runs in polling loops.
+    expectHideNearEvery(SRC('error-handling.ts'), "'tasklist'");
+    // powershell DPAPI + tasklist in cookie import.
+    const cookie = SRC('cookie-import-browser.ts');
+    expectHideNearEvery(cookie, "'powershell'");
+    expectHideNearEvery(cookie, "'tasklist'");
+  });
+
+  test('icacls calls in file-permissions.ts pass windowsHide', () => {
+    const perms = SRC('file-permissions.ts');
+    expect((perms.match(/'icacls'/g) || []).length).toBeGreaterThanOrEqual(3);
+    expectHideNearEvery(perms, "'icacls'");
+  });
+});