fix(browse): NTFS ACL hardening for Windows state files via icacls (complements v1.24)

[scarson]

Summary

gstack's ~/.gstack/ state directory holds bearer tokens (.auth.json), canary tokens, agent queue contents (with prompt history), session state, security-decision logs, and saved cookie bundles — all written with { mode: 0o600 } or appended with the same. On Windows, those mode bits are a silent no-op — Node's fs module doesn't translate POSIX modes to NTFS ACLs, and inherited ACLs leave every "restricted" file readable by other principals on the machine.

Refresh of #1121 (closed in favor of this PR), rebased against current main and extended to cover terminal-agent.ts + tunnel-denial-log.ts (both new since v1.4). Orthogonal to v1.24.0.0's binary-resolution work but complementary at the write side: v1.24's bin/gstack-paths resolves ~/.gstack/ correctly across plugin / global / local installs (#1252); this PR ensures files written into those resolved paths actually get the POSIX 0o600 semantic translated to NTFS via icacls.

Empirical proof

Fresh file written on Windows 11 with fs.writeFileSync(p, 'secret', { mode: 0o600 }), then inspected via icacls:

=== BEFORE restrictFilePermissions ===
...\acl-proof.txt  S-1-5-21-810533980-956579296-1443678346-1568416717:(I)(M,DC)
                   Artemis\CodexSandboxUsers:(I)(M,DC)
                   S-1-5-21-572213208-1484666115-665933948-1070144907:(I)(M,DC)
                   NT AUTHORITY\SYSTEM:(I)(F)
                   BUILTIN\Administrators:(I)(F)
                   ARTEMIS\Sam:(I)(F)

Six ACEs — the one I actually want (ARTEMIS\Sam) is the LAST of six. The { mode: 0o600 } hint did nothing. After applying this PR's helper:

=== AFTER restrictFilePermissions ===
...\acl-proof.txt  ARTEMIS\Sam:(F)

One ACE. POSIX 0o600 semantic translated correctly.

Threat model

The "inherited by 6 principals" state is merely aesthetic on a single-user laptop where all those principals map back to the same human. It's a real leak on:

• Self-hosted CI runners (GitHub Actions, GitLab, Jenkins agents). The runner's service account runs as a different user on the same Windows box; under inherited ACLs, that account can read the developer's canary token, session data, and prompt history. This is the scenario I'd prioritize — common setup, cross-user leak, no user action required for the attack.
• Shared development machines (agencies, studios, lab environments).
• Multi-tenant servers / shared hosts.
• Compromised processes running as the same user. Same-user isolation isn't a defense POSIX offers either, so this is neutral — but the first three cases aren't.

The agent queue contains the canary token, which is the secret that backstops prompt-injection defense. Letting another user read that defeats one of the security classifier's core layers.

The fix

A new browse/src/file-permissions.ts (158 LOC) with two primitives and three drop-in wrappers:

export function restrictFilePermissions(path: string): void      // chmod 0o600 | icacls <user>:(F) /inheritance:r
export function restrictDirectoryPermissions(path: string): void // chmod 0o700 | icacls <user>:(OI)(CI)(F) /inheritance:r

export function writeSecureFile(path, data): void                // writeFileSync + restrictFile
export function appendSecureFile(path, data): void               // appendFileSync + restrictFile on first create
export function mkdirSecure(path): void                          // mkdirSync recursive + restrictDir

(OI)(CI) on directories means new child files (object inherit) and subdirs (container inherit) inherit the single-user-full ACL — important because any fs.writeFileSync(...) inside an mkdirSecure-created dir will land already-restricted without further work. This is what makes the async fsp.appendFile calls in tunnel-denial-log.ts correct without needing an async helper variant.

19 call sites converted across 9 source files (extends the original PR by terminal-agent.ts + tunnel-denial-log.ts, both new since the original was written in April):

File	Sites
browse/src/server.ts	5
browse/src/security.ts	4
browse/src/terminal-agent.ts	8
browse/src/security-classifier.ts	2
browse/src/browser-skill-write.ts	2
browse/src/browser-manager.ts (.auth.json — bearer token)	2
browse/src/cli.ts	1
browse/src/config.ts	1
browse/src/meta-commands.ts	2
browse/src/tunnel-denial-log.ts	1

Error handling

icacls failures (nonexistent path, missing icacls.exe, hardened environments that block it) log a one-shot warning to stderr and proceed. Once-per-process gating prevents log spam if the condition persists. Filesystem stays functional; the file just ends up with inherited ACLs.

Design notes — inviting pushback

This PR introduces a new pattern to gstack (shell-out to icacls). Three judgment calls worth your read:

1. icacls vs. an npm library. I used execFileSync('icacls', ...) — the Win32 native tool that ships in System32 on every Windows install since 7. Alternative is something like win-permissions (typed wrapper around icacls). I'd lean shell-out (one fewer dependency, no npm-supply-chain exposure for a security-critical function), but happy to swap.

2. ACL scope: just the user, or user + SYSTEM? The helper grants ONLY the current user — matches POSIX 0o600 exactly. An argument for adding SYSTEM: backup / antivirus services run as SYSTEM and may need read access. An argument against: gstack state files are small, ephemeral, and recreated on each session. I went with "just user" to be deliberately strict; happy to relax to SYSTEM:(F),<user>:(F) if you prefer.

3. Graceful degradation on icacls failure. Log once and continue. Alternatives: silent (lose audit trail), log per failure (spammy), hard-fail (crash the server on a security hardening failure — wrong). Once-per-process compromise seemed right.

What changed from #1121

• Coverage extended to terminal-agent.ts (8 sites) and tunnel-denial-log.ts (1 site). Both files are new since the original PR's branch point; the original would have left them as silent NTFS-no-op gaps. Without these, every PTY-backed terminal session and every tunnel denial event leaves state in an inherited ACL — exactly the surfaces the threat model cares about.
• No async helper variant added. The (OI)(CI) inheritance flags on the parent dir mean async appends inherit the restricted ACL automatically, which is cleaner than a separate mkdirSecureAsync.
• Rebased against current main — sidebar-agent.ts call sites got consolidated/moved during v1.20-v1.26, so the original PR's literal diffs against those files no longer apply.

Test plan

• bun test browse/test/file-permissions.test.ts — 13 pass, 0 fail (POSIX mode-bit assertions, Windows no-throw, mkdir idempotence, recursive creation, Buffer payloads, append-creates-then-reapplies-once semantics)
• bun test browse/test/security.test.ts — 38 pass, 0 fail (existing security suite + the new tests added in companion PR fix(browse): bash.exe wrap for telemetry on Windows (v1.24 extension) #1306; the converted writeFileSync/appendFileSync/mkdirSync sites in security.ts integrate cleanly)
• Empirical icacls before/after on a real file — 6 ACEs → 1 ACE
• bun build typecheck on all 9 modified files — clean (server.ts has a pre-existing playwright-core/electron resolution issue unrelated to this PR)
• Ubuntu/macOS CI pass — relying on the existing matrix. POSIX branch is bit-identical (fs.chmodSync(path, 0o6XX) matches the inline { mode: 0o6XX } it replaces).

What this PR doesn't do

• Fix non-gstack-owned state files. ~/.gstack/ngrok.env is created by the user (we only read it), so we can't restrict its ACL at write time.
• Retroactively re-ACL existing files. If a user has gstack state on disk from an earlier install where mode: 0o600 was a no-op, those files keep their original broad ACL until gstack next writes them. A one-shot migration could scan ~/.gstack/ and run icacls on everything — out of scope; happy to add if you want it.
• Change the POSIX behavior. Everything on Linux/macOS is bit-identical to what shipped.

🤖 Generated with Claude Code

---

^{Need help on this PR? Tag @codesmith with what you need.}

• Let Codesmith autofix CI failures and bot reviews