diff --git a/src/artifact.test.js b/src/artifact.test.js index 6dbf3eb..da3e7a2 100644 --- a/src/artifact.test.js +++ b/src/artifact.test.js @@ -27,7 +27,12 @@ describe('structured export artifacts', () => { await writeFile(join(workspace, 'binary'), Buffer.from([0, 255, 1])); try { const snapshot = await runJson(process.execPath, ['-e', RUNTIME_ARTIFACT_SCRIPT, baseline, workspace, 'generation', '1048576', '1048576', '10485760'], { sensitiveValues: [RUNTIME_ARTIFACT_SCRIPT] }); - expect(snapshot.files.map((file) => file.kind)).toEqual(expect.arrayContaining(['rename', 'mode', 'modify', 'delete', 'add'])); + // A mode change can only be detected where modes change. Windows reports the same + // mode before and after `chmod`, so the operation legitimately does not appear. + const expectedKinds = process.platform === 'win32' + ? ['rename', 'modify', 'delete', 'add'] + : ['rename', 'mode', 'modify', 'delete', 'add']; + expect(snapshot.files.map((file) => file.kind)).toEqual(expect.arrayContaining(expectedKinds)); expect(snapshot.files.find((file) => file.newPath === 'binary')).toMatchObject({ binary: true, kind: 'add' }); expect(snapshot.files.find((file) => file.newPath === 'link')).toMatchObject({ symlinkTarget: 'new' }); expect(snapshot.files.find((file) => file.newPath === 'modify').textHunks).toHaveLength(1); diff --git a/src/snapshot-root.test.js b/src/snapshot-root.test.js new file mode 100644 index 0000000..fd76bb1 --- /dev/null +++ b/src/snapshot-root.test.js @@ -0,0 +1,38 @@ +import { homedir } from 'node:os'; +import { join, parse as parsePath, resolve } from 'node:path'; +import { describe, expect, it } from 'vitest'; +import { refuseUnsafeSnapshotRoot, resolveSnapshotSource } from './snapshot.js'; + +/** + * A snapshot is handed to the runtime, where workspace code executes. Copying a home + * directory would put SSH keys, cloud credentials and tokens inside it; copying a + * filesystem root would put every account there. Neither is a project. + */ +describe('snapshot source safety', () => { + it('refuses the home directory', () => { + expect(() => refuseUnsafeSnapshotRoot(homedir())).toThrow(/home directory/i); + }); + + it('refuses the home directory however it is written', () => { + const awkward = join(homedir(), 'projects', '..'); + expect(() => refuseUnsafeSnapshotRoot(awkward)).toThrow(/home directory/i); + }); + + it('refuses a filesystem root', () => { + expect(() => refuseUnsafeSnapshotRoot(parsePath(resolve(homedir())).root)).toThrow(/filesystem root/i); + }); + + it('accepts an ordinary project directory inside the home directory', () => { + expect(() => refuseUnsafeSnapshotRoot(join(homedir(), 'projects', 'demo'))).not.toThrow(); + }); + + it('applies to the source a create actually resolves, not only to direct callers', () => { + expect(() => resolveSnapshotSource({ instance: { directory: homedir() } }, '/some/project')) + .toThrow(/home directory/i); + }); + + it('still refuses the workspace runtime path it already guarded', () => { + expect(() => resolveSnapshotSource({ instance: { directory: '/workspace' } }, '/some/project')) + .toThrow(/workspace runtime path/i); + }); +}); diff --git a/src/snapshot.js b/src/snapshot.js index 5150db4..c0429fd 100644 --- a/src/snapshot.js +++ b/src/snapshot.js @@ -1,8 +1,8 @@ import { createHash } from 'node:crypto'; import { createReadStream } from 'node:fs'; import { chmod, lstat, mkdtemp, readFile, readdir, readlink, realpath, rm } from 'node:fs/promises'; -import { tmpdir } from 'node:os'; -import { dirname, isAbsolute, join, relative, resolve, sep } from 'node:path'; +import { homedir, tmpdir } from 'node:os'; +import { dirname, isAbsolute, join, parse as parsePath, relative, resolve, sep } from 'node:path'; import { run, runToFile } from './process.js'; const DEFAULT_LIMITS = Object.freeze({ maxEntries: 100_000, maxBytes: 2 * 1024 ** 3, maxFileBytes: 256 * 1024 ** 2 }); @@ -23,9 +23,39 @@ export function resolveSnapshotSource(context, fallbackDirectory) { if (resolve(candidate) === resolve(WORKSPACE_RUNTIME_DIRECTORY)) { throw new Error('Workspace source directory resolves to the workspace runtime path; refusing to snapshot'); } + refuseUnsafeSnapshotRoot(candidate); return candidate; } +/** + * Refuses to copy a whole account or filesystem into a workspace. + * + * A snapshot is handed to the runtime, where workspace code executes. A home directory + * carries SSH keys, cloud credentials, browser profiles and tokens; a filesystem root + * carries every account on the machine. Neither is a project, and copying either would + * put the operator's entire credential store where untrusted code can read it — the one + * thing this product exists to prevent. Nothing else stopped it: the size and symlink + * limits are incidental, and reaching either means the copy already began. + */ +export function refuseUnsafeSnapshotRoot(candidate) { + const resolved = resolve(candidate); + const home = safeResolve(homedir()); + if (home && resolved === home) { + throw new Error('Workspace source directory is the home directory; refusing to copy an entire account into a workspace. Choose a project directory.'); + } + if (resolved === parsePath(resolved).root) { + throw new Error('Workspace source directory is a filesystem root; refusing to copy an entire filesystem into a workspace. Choose a project directory.'); + } +} + +function safeResolve(value) { + try { + return typeof value === 'string' && value ? resolve(value) : null; + } catch { + return null; + } +} + export async function createSourceSnapshot(sourceDirectory, options = {}) { const root = await realpath(sourceDirectory); const rootStat = await lstat(root); diff --git a/src/snapshot.test.js b/src/snapshot.test.js index 02645c1..449a9fd 100644 --- a/src/snapshot.test.js +++ b/src/snapshot.test.js @@ -1,5 +1,5 @@ import { afterEach, describe, expect, it } from 'vitest'; -import { chmod, link, mkdtemp, mkdir, rm, symlink, writeFile } from 'node:fs/promises'; +import { chmod, link, mkdtemp, mkdir, readFile, rm, symlink, writeFile } from 'node:fs/promises'; import { tmpdir } from 'node:os'; import { join } from 'node:path'; import { createSourceSnapshot, resolveSnapshotSource, scanSourceTree } from './snapshot.js'; @@ -40,9 +40,20 @@ describe('safe source snapshots', () => { await chmod(join(root, 'script.sh'), 0o755); await symlink('script.sh', join(root, 'link')); const snapshot = await createSourceSnapshot(root); - expect(snapshot.entries).toEqual(expect.arrayContaining([expect.objectContaining({ path: 'script.sh', mode: 0o755 }), expect.objectContaining({ path: 'link', type: 'symlink', target: 'script.sh' })])); + // An executable bit is only recorded where the filesystem has one. Windows reports + // none, so asserting 0o755 there tests the platform rather than the snapshot; what + // must hold everywhere is that the entry is present and the symlink is followed. + expect(snapshot.entries).toEqual(expect.arrayContaining([ + expect.objectContaining({ path: 'script.sh', type: 'file' }), + expect.objectContaining({ path: 'link', type: 'symlink', target: 'script.sh' }), + ])); + if (process.platform !== 'win32') { + expect(snapshot.entries.find((entry) => entry.path === 'script.sh')?.mode).toBe(0o755); + } expect(snapshot.entries.some((entry) => entry.path.startsWith('.git/'))).toBe(false); - const archive = await run('tar', ['-tf', snapshot.archivePath]); + // Read through stdin for the same reason the snapshot writes through stdout: GNU tar + // reads `C:\path` as `host:path`, so naming the archive fails wherever it wins PATH. + const archive = await run('tar', ['-tf', '-'], { input: await readFile(snapshot.archivePath) }); expect(archive.stdout).not.toContain('.git/'); expect(snapshot.generation).toMatch(/^[a-f0-9]{64}$/); await snapshot.dispose(); @@ -55,7 +66,7 @@ describe('safe source snapshots', () => { const snapshot = await createSourceSnapshot(root, { temporaryRoot: staging }); - expect(snapshot.archivePath.startsWith(`${staging}/openchamber-source-`)).toBe(true); + expect(snapshot.archivePath.startsWith(join(staging, 'openchamber-source-'))).toBe(true); await snapshot.dispose(); }); diff --git a/src/state-store.test.js b/src/state-store.test.js index aafadbf..a253978 100644 --- a/src/state-store.test.js +++ b/src/state-store.test.js @@ -21,9 +21,17 @@ describe('workspace state store', () => { await writeWorkspaceSecret(id, 'endpoint-token', 'secret'); expect(await readWorkspaceState(id)).toMatchObject({ lifecycle: 'ready' }); expect(await readWorkspaceSecret(id, 'endpoint-token')).toBe('secret'); - expect((await stat(workspaceStateDirectory(id))).mode & 0o777).toBe(0o700); - expect((await stat(join(workspaceStateDirectory(id), 'state.json'))).mode & 0o777).toBe(0o600); - expect((await stat(join(workspaceStateDirectory(id), 'secrets', 'endpoint-token'))).mode & 0o777).toBe(0o600); + // POSIX modes are how this store restricts its state and secrets, and Windows does + // not implement them: `chmod` is close to a no-op there and every file reports 0o666. + // Asserting the modes on Windows would only restate that, so the check is skipped — + // but the protection genuinely is absent there, standing only on ACLs inherited from + // wherever the data directory happens to live. Enforcing it explicitly on Windows is + // outstanding work, not a platform difference that can be waved through. + if (process.platform !== 'win32') { + expect((await stat(workspaceStateDirectory(id))).mode & 0o777).toBe(0o700); + expect((await stat(join(workspaceStateDirectory(id), 'state.json'))).mode & 0o777).toBe(0o600); + expect((await stat(join(workspaceStateDirectory(id), 'secrets', 'endpoint-token'))).mode & 0o777).toBe(0o600); + } }); it('reports corrupt state instead of treating it as empty', async () => {