demo: windows-smoke catches garrytan#1121 with test only (src file missing)

Author: scarson

.github/workflows/windows-smoke.yml

@@ -0,0 +1,94 @@
+# Windows Smoke CI — Phase 1 of the phased rollout in docs/designs/WINDOWS_CI.md
+#
+# Answers one question per run: "does the code path through a Windows-critical
+# module actually run on Windows." That's deliberately a lower bar than "does
+# every test pass" — it catches the class of bugs where Linux/macOS CI runs
+# green but a Windows user immediately hits ENOENT / "browse binary not found"
+# / silent mislocations of ~/.gstack/ state.
+#
+# Coverage catch list (see RFC for full reasoning):
+#   - Build fails to produce .exe on Windows              (catches #1013 / #1024)
+#   - Binary-resolution probes wrong filename             (catches #1118 / #1094)
+#   - Shebang bash script spawn fails                     (catches #1119)
+#   - Sensitive files written without ACL restriction     (catches #1121)
+#   - { mode: 0o600 } silently ignored on Windows         (catches Pre-#1121 state)
+#
+# Miss: #1120-style home-directory fallback — no direct unit test. RFC
+# proposes adding one as a follow-on.
+name: windows-smoke
+on:
+  pull_request:
+    branches: [main]
+    paths:
+      - 'browse/**'
+      - 'make-pdf/**'
+      - 'design/**'
+      - 'scripts/**'
+      - 'bin/**'
+      - 'package.json'
+      - 'bun.lockb'
+      - '.github/workflows/windows-smoke.yml'
+  push:
+    branches: [main]
+    paths:
+      - 'browse/**'
+      - 'make-pdf/**'
+      - 'design/**'
+      - 'scripts/**'
+      - 'bin/**'
+      - 'package.json'
+      - 'bun.lockb'
+  workflow_dispatch:
+
+concurrency:
+  group: windows-smoke-${{ github.head_ref || github.ref }}
+  cancel-in-progress: true
+
+jobs:
+  smoke:
+    runs-on: windows-latest
+    timeout-minutes: 10
+    steps:
+      - uses: actions/checkout@v4
+
+      - uses: oven-sh/setup-bun@v2
+        with:
+          bun-version: latest
+
+      - name: Install dependencies
+        run: bun install --frozen-lockfile
+
+      - name: Build binaries
+        run: bun run build
+
+      - name: Assert Windows binary layout
+        shell: pwsh
+        run: |
+          $missing = @()
+          foreach ($p in @(
+            'browse/dist/browse.exe',
+            'browse/dist/find-browse.exe',
+            'browse/dist/server-node.mjs',
+            'make-pdf/dist/pdf.exe',
+            'design/dist/design.exe'
+          )) { if (-not (Test-Path $p)) { $missing += $p } }
+          if ($missing.Count -gt 0) {
+            Write-Error "Missing build artifacts: $($missing -join ', ')"
+            exit 1
+          }
+
+      - name: Smoke-test binaries
+        run: |
+          ./browse/dist/browse.exe --version
+          ./make-pdf/dist/pdf.exe --version
+          ./design/dist/design.exe --version
+
+      - name: Windows-specific unit tests
+        run: |
+          bun test browse/test/security.test.ts
+          bun test browse/test/file-permissions.test.ts
+          bun test make-pdf/test/browseClient.test.ts
+          bun test make-pdf/test/pdftotext.test.ts
+
+      - name: make-pdf render smoke
+        run: bun test make-pdf/test/render.test.ts

browse/test/file-permissions.test.ts

@@ -0,0 +1,148 @@
+/**
+ * Unit tests for browse/src/file-permissions.ts
+ *
+ * Strategy:
+ *   - POSIX assertions check fs.statSync.mode bits directly (cheap, reliable,
+ *     runs on every CI config).
+ *   - Windows assertions don't check ACLs (that'd require parsing icacls
+ *     output, which is brittle across Windows versions / locales). Instead
+ *     we verify the helper doesn't throw and the file ends up accessible
+ *     to the current user — the "doesn't crash, file still usable"
+ *     contract the callers rely on.
+ */
+
+import { afterEach, beforeEach, describe, expect, test } from 'bun:test';
+import * as fs from 'fs';
+import * as os from 'os';
+import * as path from 'path';
+
+import {
+  restrictFilePermissions,
+  restrictDirectoryPermissions,
+  writeSecureFile,
+  appendSecureFile,
+  mkdirSecure,
+  __resetWarnedForTests,
+} from '../src/file-permissions';
+
+let tmpDir: string;
+
+beforeEach(() => {
+  tmpDir = fs.mkdtempSync(path.join(os.tmpdir(), 'file-perms-'));
+  __resetWarnedForTests();
+});
+
+afterEach(() => {
+  try { fs.rmSync(tmpDir, { recursive: true, force: true }); } catch { /* best-effort */ }
+});
+
+describe('restrictFilePermissions', () => {
+  test('on POSIX, sets file mode to 0o600', () => {
+    if (process.platform === 'win32') return;
+    const p = path.join(tmpDir, 'secret');
+    fs.writeFileSync(p, 'token');
+    fs.chmodSync(p, 0o644); // start world-readable to prove the call mutates it
+    restrictFilePermissions(p);
+    expect(fs.statSync(p).mode & 0o777).toBe(0o600);
+  });
+
+  test('on Windows, does not throw on an existing file', () => {
+    if (process.platform !== 'win32') return;
+    const p = path.join(tmpDir, 'secret');
+    fs.writeFileSync(p, 'token');
+    expect(() => restrictFilePermissions(p)).not.toThrow();
+    // File remains readable by the caller — core contract.
+    expect(fs.readFileSync(p, 'utf8')).toBe('token');
+  });
+
+  test('on Windows, does not throw when icacls fails (bad path)', () => {
+    if (process.platform !== 'win32') return;
+    // icacls emits an error for a nonexistent path; helper must swallow.
+    expect(() => restrictFilePermissions(path.join(tmpDir, 'nonexistent'))).not.toThrow();
+  });
+});
+
+describe('restrictDirectoryPermissions', () => {
+  test('on POSIX, sets directory mode to 0o700', () => {
+    if (process.platform === 'win32') return;
+    const d = path.join(tmpDir, 'subdir');
+    fs.mkdirSync(d, { mode: 0o755 });
+    restrictDirectoryPermissions(d);
+    expect(fs.statSync(d).mode & 0o777).toBe(0o700);
+  });
+
+  test('on Windows, does not throw on an existing directory', () => {
+    if (process.platform !== 'win32') return;
+    const d = path.join(tmpDir, 'subdir');
+    fs.mkdirSync(d);
+    expect(() => restrictDirectoryPermissions(d)).not.toThrow();
+  });
+});
+
+describe('writeSecureFile', () => {
+  test('writes the payload and restricts permissions atomically', () => {
+    const p = path.join(tmpDir, 'data');
+    writeSecureFile(p, 'hello');
+    expect(fs.readFileSync(p, 'utf8')).toBe('hello');
+    if (process.platform !== 'win32') {
+      expect(fs.statSync(p).mode & 0o777).toBe(0o600);
+    }
+  });
+
+  test('accepts Buffer payloads', () => {
+    const p = path.join(tmpDir, 'buffer');
+    writeSecureFile(p, Buffer.from([0xde, 0xad, 0xbe, 0xef]));
+    const out = fs.readFileSync(p);
+    expect(out.length).toBe(4);
+    expect(out[0]).toBe(0xde);
+  });
+
+  test('overwrites existing file', () => {
+    const p = path.join(tmpDir, 'existing');
+    fs.writeFileSync(p, 'old', { mode: 0o644 });
+    writeSecureFile(p, 'new');
+    expect(fs.readFileSync(p, 'utf8')).toBe('new');
+  });
+});
+
+describe('appendSecureFile', () => {
+  test('appends to a new file and sets owner-only permissions', () => {
+    const p = path.join(tmpDir, 'log');
+    appendSecureFile(p, 'line1\n');
+    expect(fs.readFileSync(p, 'utf8')).toBe('line1\n');
+    if (process.platform !== 'win32') {
+      expect(fs.statSync(p).mode & 0o777).toBe(0o600);
+    }
+  });
+
+  test('appends without re-applying ACL on subsequent writes', () => {
+    const p = path.join(tmpDir, 'log');
+    appendSecureFile(p, 'line1\n');
+    appendSecureFile(p, 'line2\n');
+    expect(fs.readFileSync(p, 'utf8')).toBe('line1\nline2\n');
+  });
+});
+
+describe('mkdirSecure', () => {
+  test('creates directory with owner-only mode (POSIX)', () => {
+    if (process.platform === 'win32') return;
+    const d = path.join(tmpDir, 'nested', 'deep');
+    mkdirSecure(d);
+    expect(fs.statSync(d).isDirectory()).toBe(true);
+    expect(fs.statSync(d).mode & 0o777).toBe(0o700);
+  });
+
+  test('is idempotent — safe to call on existing directory', () => {
+    const d = path.join(tmpDir, 'dir');
+    mkdirSecure(d);
+    expect(() => mkdirSecure(d)).not.toThrow();
+  });
+
+  test('recursive behavior: creates intermediate directories', () => {
+    const d = path.join(tmpDir, 'a', 'b', 'c');
+    mkdirSecure(d);
+    expect(fs.existsSync(path.join(tmpDir, 'a'))).toBe(true);
+    expect(fs.existsSync(path.join(tmpDir, 'a', 'b'))).toBe(true);
+    expect(fs.existsSync(d)).toBe(true);
+  });
+});