mirror of
https://github.com/garrytan/gstack.git
synced 2026-08-31 18:30:39 +02:00
Merge remote-tracking branch 'origin/main' into perth-v2
# Conflicts: # CLAUDE.md # package.json
This commit is contained in:
+7
-1
@@ -192,7 +192,13 @@ If `NEEDS_SETUP`:
|
||||
BUN_INSTALL_SHA="bab8acfb046aac8c72407bdcce903957665d655d7acaa3e11c7c4616beae68dd"
|
||||
tmpfile=$(mktemp)
|
||||
curl -fsSL "https://bun.sh/install" -o "$tmpfile"
|
||||
actual_sha=$(shasum -a 256 "$tmpfile" | awk '{print $1}')
|
||||
# shasum is macOS/perl; coreutils-only Linux ships sha256sum instead —
|
||||
# resolve whichever exists so the verify never fails on a missing tool.
|
||||
if command -v sha256sum >/dev/null 2>&1; then
|
||||
actual_sha=$(sha256sum "$tmpfile" | awk '{print $1}')
|
||||
else
|
||||
actual_sha=$(shasum -a 256 "$tmpfile" | awk '{print $1}')
|
||||
fi
|
||||
if [ "$actual_sha" != "$BUN_INSTALL_SHA" ]; then
|
||||
echo "ERROR: bun install script checksum mismatch" >&2
|
||||
echo " expected: $BUN_INSTALL_SHA" >&2
|
||||
|
||||
@@ -135,6 +135,21 @@ export function restrictFilePermissions(filePath: string): void {
|
||||
* POSIX: `fs.chmodSync(path, 0o700)`. Idempotent if the dir was already
|
||||
* created with `{ mode: 0o700 }`.
|
||||
*
|
||||
* Owner-only hardening is for directories gstack creates and owns. Two
|
||||
* pre-existing shapes must never be chmodded, so the POSIX branch refuses
|
||||
* them:
|
||||
*
|
||||
* - A directory owned by another user. On most hosts the chmod just
|
||||
* fails EPERM, but run as root (or with CAP_FOWNER — containers, CI
|
||||
* sandboxes) it SUCCEEDS and takes the directory away from its owner.
|
||||
* - A shared sticky world-writable directory (`/tmp`, `/var/tmp`).
|
||||
* Callers land here when a state file is configured directly inside
|
||||
* the system temp dir (`BROWSE_STATE_FILE=/tmp/foo.json` makes
|
||||
* `path.dirname()` derive `/tmp` as the state dir). A 0o700 `/tmp`
|
||||
* locks every other process on the machine out of it: access(2)-based
|
||||
* checks (`fs.existsSync`) return EACCES→false machine-wide, even
|
||||
* while stat keeps working for capability-holding processes.
|
||||
*
|
||||
* Windows: `icacls /inheritance:r /grant:r <user>:(OI)(CI)(F)`. The
|
||||
* `(OI)(CI)` flags make new files (OI = object inherit) and subdirs
|
||||
* (CI = container inherit) inherit the single-user-full ACL — important
|
||||
@@ -176,7 +191,78 @@ export function restrictDirectoryPermissions(dirPath: string): void {
|
||||
}
|
||||
return;
|
||||
}
|
||||
try { fs.chmodSync(dirPath, 0o700); } catch { /* best-effort */ }
|
||||
try {
|
||||
// fd-anchored check-then-act: statSync + chmodSync resolve the path twice,
|
||||
// so a symlink swapped in between would let the chmod land on a directory
|
||||
// the check never saw (exactly the CAP_FOWNER hosts this guard exists
|
||||
// for). O_NOFOLLOW refuses a symlinked state dir outright; fstat + fchmod
|
||||
// pin both the check and the act to the same inode.
|
||||
const fd = fs.openSync(
|
||||
dirPath,
|
||||
fs.constants.O_RDONLY | fs.constants.O_DIRECTORY | fs.constants.O_NOFOLLOW,
|
||||
);
|
||||
try {
|
||||
if (!shouldHardenDir(fs.fstatSync(fd))) {
|
||||
warnUnhardenedDirOnce(dirPath);
|
||||
return;
|
||||
}
|
||||
fs.fchmodSync(fd, 0o700);
|
||||
} finally {
|
||||
fs.closeSync(fd);
|
||||
}
|
||||
} catch (err: any) {
|
||||
// The fd path refuses two shapes the old chmodSync handled, and a silent
|
||||
// skip here would quietly lose hardening (the module's own rule is that
|
||||
// refusals are never silent):
|
||||
// ELOOP/ENOTDIR — dirPath is a symlink (dotfiles-managed ~/.gstack via
|
||||
// stow/chezmoi). Never follow it blind; warn so the operator knows.
|
||||
// EACCES — an OWNED dir stuck without the read bit (mode 0300/0000)
|
||||
// can't be opened but could always be repaired by plain chmod. Keep
|
||||
// that self-repair: re-check ownership via lstat (no link follow) and
|
||||
// chmod only a real, owned, non-shared directory.
|
||||
if (err?.code === 'EACCES') {
|
||||
try {
|
||||
const st = fs.lstatSync(dirPath);
|
||||
if (st.isDirectory() && shouldHardenDir(st)) {
|
||||
fs.chmodSync(dirPath, 0o700);
|
||||
return;
|
||||
}
|
||||
} catch { /* fall through to the warning */ }
|
||||
}
|
||||
if (err?.code === 'ELOOP' || err?.code === 'ENOTDIR' || err?.code === 'EACCES') {
|
||||
warnUnhardenedDirOnce(dirPath);
|
||||
}
|
||||
/* anything else: best-effort, matching the old behavior */
|
||||
}
|
||||
}
|
||||
|
||||
/** Owner-only hardening applies to a real dir we own that isn't shared. */
|
||||
function shouldHardenDir(st: fs.Stats): boolean {
|
||||
// chmod authorization is judged on the EFFECTIVE uid; fall back to the
|
||||
// real uid where geteuid is unavailable (Windows — unreachable here).
|
||||
const uid = process.geteuid?.() ?? process.getuid?.();
|
||||
if (st.uid !== uid) return false;
|
||||
// sticky + world-writable = shared temp dir (/tmp, /var/tmp) — never ours
|
||||
// to restrict. Running as root, treat ANY world-writable dir as shared:
|
||||
// root "owns" docker/CI volume mounts (0777, no sticky bit) that other
|
||||
// users depend on, and a 0700 there locks them all out.
|
||||
if ((st.mode & 0o1002) === 0o1002) return false;
|
||||
if (uid === 0 && (st.mode & 0o002) === 0o002) return false;
|
||||
return true;
|
||||
}
|
||||
|
||||
// The POSIX refusal must not be silent (the Windows branch already warns when
|
||||
// icacls misses its hardening target): an operator who points state at a
|
||||
// shared or foreign-owned directory should learn it was left unhardened.
|
||||
const unhardenedWarned = new Set<string>();
|
||||
function warnUnhardenedDirOnce(dirPath: string): void {
|
||||
if (unhardenedWarned.has(dirPath)) return;
|
||||
unhardenedWarned.add(dirPath);
|
||||
// biome-ignore lint/suspicious/noConsole: intentional user-facing warning
|
||||
console.warn(
|
||||
`[gstack] directory ${dirPath} is shared, symlinked, or owned by another user; ` +
|
||||
'gstack left its permissions untouched — use a private, non-symlinked directory for owner-only hardening',
|
||||
);
|
||||
}
|
||||
|
||||
/**
|
||||
@@ -239,7 +325,10 @@ export function repairBrokenDacl(dirPath: string): void {
|
||||
/**
|
||||
* `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.
|
||||
* Safe to call on an existing directory — re-applies the ACL idempotently,
|
||||
* except on directories gstack doesn't own (another user's dir, or a shared
|
||||
* sticky world-writable dir like `/tmp`), which are left untouched — see
|
||||
* `restrictDirectoryPermissions`.
|
||||
*
|
||||
* Windows: after applying the restricted ACL, verifies the directory is
|
||||
* still listable by this process and repairs a broken DACL (#1605) if not.
|
||||
|
||||
@@ -1,4 +1,5 @@
|
||||
import { describe, test, expect } from 'bun:test';
|
||||
import { canRevokeWrites } from '../../test/helpers/fs-caps';
|
||||
import { resolveConfig, ensureStateDir, readVersionHash, getGitRoot, getRemoteSlug, resolveGstackHome, resolveChromiumProfile, cleanSingletonLocks } from '../src/config';
|
||||
import * as fs from 'fs';
|
||||
import * as path from 'path';
|
||||
@@ -129,6 +130,7 @@ describe('config', () => {
|
||||
});
|
||||
|
||||
test('logs warning to browse-server.log on non-ENOENT gitignore error', () => {
|
||||
if (!canRevokeWrites()) return; // chmod is advisory here (win32, root, DAC-override containers)
|
||||
const tmpDir = path.join(os.tmpdir(), `browse-gitignore-test-${Date.now()}`);
|
||||
fs.mkdirSync(tmpDir, { recursive: true });
|
||||
// Create a read-only .gitignore (no .gstack/ entry → would try to append)
|
||||
|
||||
@@ -77,6 +77,34 @@ describe('restrictDirectoryPermissions', () => {
|
||||
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');
|
||||
@@ -205,6 +233,23 @@ describe('mkdirSecure', () => {
|
||||
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
|
||||
@@ -254,3 +299,27 @@ describe('repairBrokenDacl', () => {
|
||||
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 });
|
||||
}
|
||||
});
|
||||
|
||||
|
||||
@@ -115,7 +115,7 @@ describe('validateOutputPath — symlink resolution', () => {
|
||||
it('blocks symlink inside /tmp pointing outside safe dirs', () => {
|
||||
const linkPath = join(tmpdir(), 'test-output-symlink-' + Date.now() + '.png');
|
||||
try {
|
||||
symlinkSync('/etc/crontab', linkPath);
|
||||
symlinkSync('/etc/passwd', linkPath); // /etc/passwd exists on every Unix — /etc/crontab is absent on Amazon Linux/Fedora minimal;
|
||||
expect(() => validateOutputPath(linkPath)).toThrow(/Path must be within/);
|
||||
} finally {
|
||||
try { unlinkSync(linkPath); } catch {}
|
||||
|
||||
@@ -14,6 +14,7 @@
|
||||
*/
|
||||
|
||||
import { describe, test, expect, afterAll } from 'bun:test';
|
||||
import { canRevokeWrites } from '../../test/helpers/fs-caps';
|
||||
import * as fs from 'fs';
|
||||
// Default (CJS) export — its properties are mutable in Bun, unlike the frozen
|
||||
// `* as fs` namespace, and mutations propagate to cli.ts's own fs import.
|
||||
@@ -41,7 +42,7 @@ describe('acquireServerLock (#1084 error honesty)', () => {
|
||||
});
|
||||
|
||||
test('EACCES throws ServerLockError with the real errno — NOT phantom contention', () => {
|
||||
if (process.platform === 'win32' || process.getuid?.() === 0) return; // chmod semantics differ
|
||||
if (!canRevokeWrites()) return; // chmod is advisory here (win32, root, DAC-override containers)
|
||||
const rodir = path.join(tmpRoot, 'rodir');
|
||||
fs.mkdirSync(rodir, { recursive: true });
|
||||
fs.chmodSync(rodir, 0o500); // r-x: open('wx') inside fails EACCES
|
||||
|
||||
@@ -16,6 +16,7 @@
|
||||
*/
|
||||
|
||||
import { describe, test, expect, afterAll } from 'bun:test';
|
||||
import { canRevokeWrites } from '../../test/helpers/fs-caps';
|
||||
import * as fs from 'fs';
|
||||
import * as os from 'os';
|
||||
import * as path from 'path';
|
||||
@@ -128,7 +129,7 @@ describe('session-persist units', () => {
|
||||
|
||||
test('a failed snapshot write preserves the previous good snapshot', async () => {
|
||||
// chmod-based read-only dirs don't bind on Windows or when running as root.
|
||||
if (process.platform === 'win32' || process.getuid?.() === 0) return;
|
||||
if (!canRevokeWrites()) return; // chmod is advisory here (win32, root, DAC-override containers)
|
||||
const dir = path.join(tmpRoot, 'ro');
|
||||
fs.mkdirSync(dir);
|
||||
const file = path.join(dir, 'session-state.json');
|
||||
|
||||
Reference in New Issue
Block a user