mirror of
https://github.com/garrytan/gstack.git
synced 2026-08-31 02:10:51 +02:00
* fix(browse): never chmod shared, symlinked, or foreign-owned dirs to 0700 restrictDirectoryPermissions unconditionally chmodded its target. On hosts where the process holds CAP_FOWNER (Docker as root, CI sandboxes) that chmod SUCCEEDS on root-owned /tmp whenever a state file is configured there (BROWSE_STATE_FILE=/tmp/x.json derives stateDir=/tmp), and a 0700 /tmp breaks access(2)-based checks machine-wide for every other process. The POSIX branch now refuses shared sticky dirs, world-writable mounts under root, foreign-owned dirs, and symlinked state dirs; refusals warn once per process instead of failing silent; owned-but-unreadable dirs keep their chmod self-repair; and the check-then-act race is closed with fd-anchored O_NOFOLLOW + fstat/fchmod on a single inode. Regression tests cover the sticky-dir, foreign-uid, mkdirSecure-reapply, and symlinked-dir shapes. * fix: hash with sha256sum before shasum on Linux (config slugs + setup verify) shasum is perl/macOS; coreutils-only Linux ships sha256sum. Two call sites hard-coded shasum: gstack-config's sha8_of/sha16 (so resolve-user-slug exited 127 for any Linux user with a git email, the Layer-3 fallback) and the generated bun-installer checksum snippet in the browse/qa NEEDS_SETUP flow (spurious "checksum mismatch" on the same distros). Both now resolve sha256sum first and fall back to shasum -a 256. New shim-PATH tests pin BOTH hasher branches of sha8_of to a known vector and cover the sha8->sha16 collision escalation end to end. * feat(contract): Aside is the recommended driver for third-party web actions The Third-Party Web Actions contract (ship, spec, office-hours, land-and-deploy, setup-deploy) now names the Aside AI browser as the recommended driver: it acts across the user's real logged-in sessions, which is what vendor-dashboard moments need. Supersedes the v1.65.0.0 de-Aside stance by explicit user directive (2026-08-27). Detection is a runtime probe (command -v + aside --version under a portable gtimeout/timeout/bare guard; nonzero exit = not detected). Consent options render per detection state with Aside recommended and the first-party stack ($B headed + handoff, GStack Browser) as the universal fallback. Absent on macOS, the contract mentions the aside.com download (macOS 15+) once per task; gstack never runs an installer and binary presence is never consent. Drive discipline: step-wise over whole-task delegation, vendor confirm mode on, vendor skill/--help text scoped to operational syntax only, secrets minimized (autofill / human-used copy buttons), Apple credential creation never a drive target in any skill, failure path quotes redacted errors and falls back only with fresh consent. test/third-party-actions.test.ts pins every load-bearing sentence (21 tests) plus repo-wide tripwires: an aside command allowlist (--version/--help only, code spans AND prose) and a ban on Aside installer invocations across all generated docs. Budget ratchet fixture and carve skeleton ceilings refreshed in this commit per the ratchet protocol. * chore: regenerate remaining browse-setup snippet consumers The sha256sum-first checksum fallback in the generated NEEDS_SETUP snippet renders into every browse-consuming skill, not just browse/qa. Mechanical regen of the other ten consumers; no template changes here. * test: consent-gate E2E suite + functional fs-capability probes Five hermetic gate-tier E2E cases (tpa-present / absent-linux / broken / absent-darwin / apple-ban) drive the real contract section through claude -p with PATH shims for aside and uname; the absent cases filter any REAL aside binary out of the child PATH and assert absence with Bun.which before spawning, so dev machines cannot leak into detection. Registered per-case in E2E_TOUCHFILES/E2E_TIERS with template-level deps (ship/SKILL.md.tmpl, gen-skill-docs.ts) and added to the evals.yml matrix with tier: gate. eval:bg:periodic's detach timeout rises to 36000s for the grown periodic shard census (floor-enforced by test/eval-detach-timeout-floor.test.ts); CLAUDE.md doc updated to match. test/helpers/fs-caps.ts adds canRevokeWrites/canRevokeReads functional probes; 13 chmod-based tests swap their uid-0-only guards for the probes so suites skip honestly on CAP_DAC_OVERRIDE containers (this sandbox: uid 1000 with full caps) instead of asserting revocations the kernel ignores. path-validation's symlink test targets /etc/passwd (exists everywhere; /etc/crontab is absent on Amazon Linux). * docs: file the Aside follow-ups in TODOS Phase-2 QA logged-in-evidence path (P3), a hostile-vendor-skill E2E for the contract's override sentence (P2), and fd-anchoring the file-level permission writes to match the directory hardening (P3). * chore: bump version and changelog (v1.72.0.0) Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> * docs: update project documentation for v1.72.0.0 docs/skills.md: Third-Party Web Actions subsection under /ship (Aside recommended driver, consent rules, credential boundaries). BROWSER.md: "Aside and third-party drives" subsection under Real-browser mode + ToC entry, including the no-gstack-side-audit-trail caveat (ship adversarial finding 12). TODOS.md: mark the finding-12 doc note done. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> * docs: apply cross-model doc review fixes for v1.72.0.0 docs/skills.md: restore the /ship closing line above the new subsection. BROWSER.md: ToC label matches the heading; BROWSE_STATE_FILE env row documents the new dir-hardening refusal + one-time warning. CHANGELOG: correct the hasher precedence wording (sha256sum first, shasum fallback) and the fs-caps count (14 test files, verified against the diff). Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> * docs: close cross-model doc-review gaps for v1.72.0.0 setup's manual bun-verify instruction gets the same sha256sum-first fallback the automated snippet got (coreutils-only Linux); BROWSER.md's BROWSE_STATE_FILE row now lists the under-root world-writable refusal; test-cost ceilings in CLAUDE.md/CONTRIBUTING.md updated for the five new gate E2E cases (~$4.20 E2E / ~$4.35 evals). Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> * fix(test): gate the symlink-refusal test to POSIX and drop the umask assumption The symlink regression test exercised the POSIX O_NOFOLLOW branch but ran on Windows, where restrictDirectoryPermissions takes the icacls branch and stat has no POSIX modes (0o666 always) — windows-free-tests failed on mode 493 vs 438. Early-return on win32 like every sibling test in the file, and assert the target's mode is UNCHANGED (captured post-mkdir) instead of hardcoding 0o755, which a strict umask would also break. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> --------- Co-authored-by: Claude Fable 5 <noreply@anthropic.com>
280 lines
11 KiB
TypeScript
280 lines
11 KiB
TypeScript
/**
|
|
* 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,
|
|
repairBrokenDacl,
|
|
__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 POSIX, leaves a shared sticky world-writable dir untouched', () => {
|
|
if (process.platform === 'win32') return;
|
|
// Simulates /tmp: sticky + world-writable. Hardening a shared dir to
|
|
// 0o700 locks every other user on the machine out of it, so the helper
|
|
// must refuse — even when the caller owns the dir (root / CAP_FOWNER
|
|
// hosts are where the chmod would actually succeed).
|
|
const d = path.join(tmpDir, 'shared-tmp');
|
|
fs.mkdirSync(d);
|
|
// System chmod, not fs.chmodSync: Bun masks the sticky bit off chmod/
|
|
// mkdir modes, so 0o1777 through the fs API lands as 0o777.
|
|
Bun.spawnSync(['chmod', '1777', d]);
|
|
expect(fs.statSync(d).mode & 0o7777).toBe(0o1777); // fixture took
|
|
restrictDirectoryPermissions(d);
|
|
expect(fs.statSync(d).mode & 0o7777).toBe(0o1777);
|
|
});
|
|
|
|
test('on POSIX, leaves a directory owned by another user untouched', () => {
|
|
if (process.platform === 'win32') return;
|
|
const d = path.join(tmpDir, 'foreign');
|
|
fs.mkdirSync(d, { mode: 0o755 });
|
|
// Only constructible where chown to a foreign uid succeeds (root /
|
|
// CAP_CHOWN — containers, CI sandboxes). Elsewhere the chown throws
|
|
// and there's nothing to assert; bail.
|
|
try { fs.chownSync(d, process.getuid!() + 1, fs.statSync(d).gid); } catch { return; }
|
|
restrictDirectoryPermissions(d);
|
|
expect(fs.statSync(d).mode & 0o777).toBe(0o755);
|
|
});
|
|
|
|
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();
|
|
});
|
|
|
|
test('on Windows, the directory stays usable by the calling process', () => {
|
|
if (process.platform !== 'win32') return;
|
|
const d = path.join(tmpDir, 'still-usable');
|
|
fs.mkdirSync(d);
|
|
fs.writeFileSync(path.join(d, 'before'), 'x');
|
|
|
|
restrictDirectoryPermissions(d);
|
|
|
|
// Regression: an unqualified username passed to icacls can resolve to
|
|
// the machine SID rather than the user account. Combined with
|
|
// /inheritance:r that leaves a directory whose only ACE matches nobody,
|
|
// so the process that just "secured" it can no longer enumerate or
|
|
// write to it. icacls still reports success, so a not-toThrow assertion
|
|
// sails straight past it — hence these access checks.
|
|
expect(() => fs.readdirSync(d)).not.toThrow();
|
|
expect(fs.readdirSync(d)).toContain('before');
|
|
expect(() => fs.writeFileSync(path.join(d, 'after'), 'y')).not.toThrow();
|
|
expect(fs.readFileSync(path.join(d, 'after'), 'utf8')).toBe('y');
|
|
});
|
|
});
|
|
|
|
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('does not chmod a pre-existing shared sticky dir (the /tmp state-dir case)', () => {
|
|
if (process.platform === 'win32') return;
|
|
// BROWSE_STATE_FILE=/tmp/foo.json derives stateDir=/tmp, and every
|
|
// daemon boot runs mkdirSecure(stateDir). The mkdir is a no-op on the
|
|
// existing dir; the permission re-apply must be one too — chmodding the
|
|
// real /tmp to 0o700 breaks fs.existsSync (access(2) → EACCES) for
|
|
// every process on the machine until something restores 1777.
|
|
const d = path.join(tmpDir, 'shared-tmp');
|
|
fs.mkdirSync(d);
|
|
// System chmod: Bun's fs API masks the sticky bit off modes (see the
|
|
// restrictDirectoryPermissions sticky-dir test).
|
|
Bun.spawnSync(['chmod', '1777', d]);
|
|
expect(fs.statSync(d).mode & 0o7777).toBe(0o1777); // fixture took
|
|
mkdirSecure(d);
|
|
expect(fs.statSync(d).mode & 0o7777).toBe(0o1777);
|
|
});
|
|
|
|
test('on Windows, the created directory stays usable by the caller', () => {
|
|
if (process.platform !== 'win32') return;
|
|
// The state-dir path that broke: mkdirSecure() creates .gstack/, hardens
|
|
// it, and the very next thing the daemon does is write a lockfile inside.
|
|
const d = path.join(tmpDir, 'state', '.gstack');
|
|
mkdirSecure(d);
|
|
expect(() => fs.writeFileSync(path.join(d, 'browse.json.lock'), '1')).not.toThrow();
|
|
expect(fs.readdirSync(d)).toContain('browse.json.lock');
|
|
});
|
|
|
|
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);
|
|
});
|
|
|
|
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();
|
|
});
|
|
});
|
|
// Symlinked state dir (dotfiles-managed ~/.gstack via stow/chezmoi): the
|
|
// fd-anchored path refuses to follow it (O_NOFOLLOW) — the refusal must warn,
|
|
// never throw, and never chmod the symlink target. POSIX-branch behavior:
|
|
// Windows takes the icacls branch and has no POSIX modes (stat reports 0o666),
|
|
// so gate like the sibling tests above.
|
|
test('restrictDirectoryPermissions warns and skips a symlinked dir without throwing', () => {
|
|
if (process.platform === 'win32') return;
|
|
const base = fs.mkdtempSync(path.join(os.tmpdir(), 'fp-symlink-'));
|
|
const target = path.join(base, 'real');
|
|
const link = path.join(base, 'link');
|
|
fs.mkdirSync(target, { mode: 0o755 });
|
|
fs.symlinkSync(target, link);
|
|
// Capture the actual post-umask mode rather than assuming 0o755 — a strict
|
|
// umask (077) would legitimately yield 0o700 at creation time.
|
|
const modeBefore = fs.statSync(target).mode & 0o777;
|
|
try {
|
|
expect(() => restrictDirectoryPermissions(link)).not.toThrow();
|
|
// Target permissions untouched — the link was never followed.
|
|
expect(fs.statSync(target).mode & 0o777).toBe(modeBefore);
|
|
} finally {
|
|
fs.rmSync(base, { recursive: true, force: true });
|
|
}
|
|
});
|
|
|