diff --git a/packages/code/src/storage.test.ts b/packages/code/src/storage.test.ts index 3a17705..b45d351 100644 --- a/packages/code/src/storage.test.ts +++ b/packages/code/src/storage.test.ts @@ -1,5 +1,15 @@ import assert from 'node:assert/strict'; -import { mkdtemp, open, rm, stat, writeFile } from 'node:fs/promises'; +import { + chmod, + mkdir, + mkdtemp, + open, + readdir, + rm, + stat, + symlink, + writeFile, +} from 'node:fs/promises'; import { tmpdir } from 'node:os'; import { join } from 'node:path'; import test from 'node:test'; @@ -170,7 +180,8 @@ test('workspace mutation quarantine persists until explicitly cleared', async (t await clearWorkspaceMutationQuarantine(path); assert.equal(syncCalls, process.platform === 'win32' ? 1 : 4); assert.equal(await loadWorkspaceMutationQuarantine(path), undefined); - await writeFile(path, '{bad json', 'utf8'); + /* Owner-only, so this exercises the parse failure and not the mode check. */ + await writeFile(path, '{bad json', { encoding: 'utf8', mode: 0o600 }); await assert.rejects( loadWorkspaceMutationQuarantine(path), /invalid workspace quarantine file/i, @@ -213,3 +224,152 @@ test('workspace mutation quarantine cannot be replaced or cleared by another own await rm(directory, { recursive: true, force: true }); } }); + +const SAMPLE_IDENTITY = { + protocolVersion: 1 as const, + workerId: 'vm-1', + codeApiUrl: 'https://code.example/v1', + credential: 'credential', + expiresAt: new Date(Date.now() + 60_000).toISOString(), + publicKey: 'public', + privateKey: 'private', +}; + +/** + * Locate a mount that ignores POSIX permissions (WSL2 DrvFs under `/mnt/`). + * Returns undefined on hosts where every writable filesystem honours chmod. + */ +async function findChmodIgnoringDirectory(): Promise { + const roots: string[] = []; + const configured = process.env.LIBRECHAT_CODE_TEST_NONPOSIX_DIR?.trim(); + if (configured) roots.push(configured); + try { + for (const entry of await readdir('/mnt')) roots.push(join('/mnt', entry)); + } catch { + /* No /mnt on this host. */ + } + for (const root of roots) { + let directory: string | undefined; + try { + directory = await mkdtemp(join(root, 'librechat-code-mode-')); + const probe = join(directory, 'probe'); + await writeFile(probe, '', { mode: 0o600 }); + await chmod(probe, 0o600); + if (((await stat(probe)).mode & 0o077) !== 0) return directory; + } catch { + /* Root is absent or not writable. */ + } + if (directory) await rm(directory, { recursive: true, force: true }); + } + return undefined; +} + +test('credential storage fails closed on filesystems that ignore chmod', async (t) => { + const directory = await findChmodIgnoringDirectory(); + if (!directory) { + t.skip('no chmod-ignoring filesystem available on this host'); + return; + } + try { + const identityPath = join(directory, 'worker.json'); + await assert.rejects( + saveBridgeIdentity(identityPath, SAMPLE_IDENTITY), + /owner-only access/, + ); + /* The private key must not be left behind on a world-readable path. */ + await assert.rejects(stat(identityPath), { code: 'ENOENT' }); + + await assert.rejects( + ensurePrivateWorkspaceDirectory(join(directory, 'workspace')), + /owner-only access/, + ); + + const quarantinePath = join(directory, 'quarantine.json'); + await assert.rejects( + saveWorkspaceMutationQuarantine(quarantinePath, { + version: 1, + workerId: 'vm-1', + workspaceId: 'primary', + quarantinedAt: new Date().toISOString(), + reason: 'test', + }), + /owner-only access/, + ); + /* An unreadable half-written marker would wedge every later load. */ + await assert.rejects(stat(quarantinePath), { code: 'ENOENT' }); + } finally { + await rm(directory, { recursive: true, force: true }); + } +}); + +test('an already-exposed identity is refused on load', async (t) => { + const directory = await findChmodIgnoringDirectory(); + if (!directory) { + t.skip('no chmod-ignoring filesystem available on this host'); + return; + } + try { + /* Written the way a release without the save-time check would have. */ + const path = join(directory, 'legacy-worker.json'); + await writeFile(path, JSON.stringify(SAMPLE_IDENTITY), { mode: 0o600 }); + await assert.rejects(loadBridgeIdentity(path), /accessible beyond its owner/); + } finally { + await rm(directory, { recursive: true, force: true }); + } +}); + +test('an already-exposed quarantine marker is refused on load', async (t) => { + const directory = await findChmodIgnoringDirectory(); + if (!directory) { + t.skip('no chmod-ignoring filesystem available on this host'); + return; + } + try { + const path = join(directory, 'quarantine.json'); + await writeFile( + path, + JSON.stringify({ + version: 1, + workerId: 'vm-1', + workspaceId: 'primary', + ownerId: 'incarnation-1', + quarantinedAt: new Date().toISOString(), + reason: 'test', + }), + { mode: 0o600 }, + ); + await assert.rejects( + loadWorkspaceMutationQuarantine(path), + /accessible beyond its owner/, + ); + } finally { + await rm(directory, { recursive: true, force: true }); + } +}); + +test('a symlinked owner-only identity is accepted', async () => { + const base = await mkdtemp(join(tmpdir(), 'librechat-code-storage-')); + try { + const target = join(base, 'real.json'); + await saveBridgeIdentity(target, SAMPLE_IDENTITY); + /* A link's own mode is always 0777; the credential's mode is the target's. */ + const link = join(base, 'link.json'); + await symlink(target, link); + assert.deepEqual(await loadBridgeIdentity(link), SAMPLE_IDENTITY); + } finally { + await rm(base, { recursive: true, force: true }); + } +}); + +test('default workspace directories are tightened when they already exist', async () => { + const base = await mkdtemp(join(tmpdir(), 'librechat-code-storage-')); + try { + const workspace = join(base, 'workspace'); + await mkdir(workspace, { mode: 0o777 }); + await chmod(workspace, 0o777); + await ensurePrivateWorkspaceDirectory(workspace); + assert.equal((await stat(workspace)).mode & 0o777, 0o700); + } finally { + await rm(base, { recursive: true, force: true }); + } +}); diff --git a/packages/code/src/storage.ts b/packages/code/src/storage.ts index 2a7eef5..8c0d1cb 100644 --- a/packages/code/src/storage.ts +++ b/packages/code/src/storage.ts @@ -1,5 +1,5 @@ import { createHash, randomBytes } from 'node:crypto'; -import { chmod, lstat, mkdir, open, readFile, rename, rm } from 'node:fs/promises'; +import { chmod, lstat, mkdir, open, readFile, rename, rm, stat } from 'node:fs/promises'; import { homedir } from 'node:os'; import { dirname, join, resolve } from 'node:path'; @@ -149,6 +149,67 @@ export function defaultWorkspaceQuarantinePath( ); } +/** + * Verify a path really is owner-only. `chmod` reports success without effect on + * mounts that do not implement POSIX permissions - notably WSL2 DrvFs + * (`/mnt/`), where the result stays world-accessible - so a credential + * that cannot be protected must fail closed rather than appear protected. + * + * Symlinks are resolved: a link's own mode is always `0777` and ignored by the + * kernel, so the file the bytes live in is what counts. + * + * This reads POSIX mode bits, which is not the whole access story everywhere. + * A Linux POSIX ACL surfaces its mask in the group bits and so is caught, but + * a macOS extended ACL inherited from the parent directory is invisible here + * and survives `chmod`, and Windows is exempt entirely. Establishing owner-only + * storage on those needs real ACL inspection; until then this verifies what the + * mode can express and nothing more. + */ +async function groupOrOtherAccessMode( + path: string, +): Promise { + if (process.platform === 'win32') return undefined; + const mode = (await stat(path)).mode & 0o777; + return (mode & 0o077) === 0 ? undefined : mode; +} + +async function assertOwnerOnlyPath( + path: string, + reportedPath: string = path, +): Promise { + const mode = await groupOrOtherAccessMode(path); + if (mode === undefined) return; + throw new BridgeProtocolError( + `Cannot restrict ${reportedPath} to owner-only access (mode ${mode.toString(8)}). ` + + 'Filesystems that ignore POSIX permissions, such as Windows drives mounted ' + + 'under /mnt, cannot protect worker credentials or workspaces. Use a path on a ' + + 'native Linux filesystem.', + ); +} + +/** + * Validate and read through one descriptor. Checking a path and then reading it + * resolves the name twice, so a symlink retargeted in between would let the file + * that was judged differ from the file that is read. + */ +async function readGuardedFile( + path: string, + exposed: (mode: string) => string, +): Promise { + const handle = await open(path, 'r'); + try { + if (process.platform !== 'win32') { + const mode = (await handle.stat()).mode & 0o777; + if ((mode & 0o077) !== 0) { + throw new BridgeProtocolError(exposed(mode.toString(8))); + } + } + return await handle.readFile('utf8'); + } finally { + await handle.close(); + } +} + export async function ensurePrivateWorkspaceDirectory( path: string, ): Promise { @@ -158,6 +219,7 @@ export async function ensurePrivateWorkspaceDirectory( throw new BridgeProtocolError('Default workspace path must be a directory'); } await chmod(path, 0o700); + await assertOwnerOnlyPath(path); } export async function saveBridgeIdentity( @@ -169,6 +231,10 @@ export async function saveBridgeIdentity( try { const file = await open(temporaryPath, 'wx', 0o600); try { + /* Tighten every way available before judging, and judge before the key + * is written, so no private key reaches a world-readable path. */ + await file.chmod(0o600); + await assertOwnerOnlyPath(temporaryPath, path); await file.writeFile(`${JSON.stringify(identity, null, 2)}\n`, 'utf8'); await file.sync(); } finally { @@ -205,10 +271,24 @@ export async function saveWorkspaceMutationQuarantine( await ensureDurableDirectory(dirname(path)); const file = await open(path, 'wx', 0o600); try { - await file.writeFile(`${JSON.stringify(record, null, 2)}\n`, 'utf8'); - await file.sync(); - } finally { - await file.close(); + try { + await file.chmod(0o600); + await assertOwnerOnlyPath(path); + await file.writeFile(`${JSON.stringify(record, null, 2)}\n`, 'utf8'); + await file.sync(); + } finally { + await file.close(); + } + } catch (error) { + /* A partial marker fails every later load, so undoing it has to reach the + * disk as durably as the write it is undoing. */ + await rm(path, { force: true }); + try { + await syncParentDirectory(path); + } catch { + /* Surface the original failure, not a cleanup-durability one. */ + } + throw error; } await syncParentDirectory(path); } @@ -218,13 +298,15 @@ export async function loadWorkspaceMutationQuarantine( ): Promise { let content: string; try { - content = await readFile(path, 'utf8'); + content = await readGuardedFile( + path, + (mode) => + `Workspace quarantine ${path} is accessible beyond its owner (mode ${mode}). ` + + 'Another local account could clear or forge it. Keep worker state on a ' + + 'native Linux filesystem.', + ); } catch (error) { - if ( - isRecord(error) && - 'code' in error && - error.code === 'ENOENT' - ) { + if (isMissingPathError(error)) { return undefined; } throw error; @@ -278,7 +360,16 @@ export async function assertWorkspaceMutationQuarantineOwner( export async function loadBridgeIdentity( path: string, ): Promise { - const identity = JSON.parse(await readFile(path, 'utf8')) as unknown; + /* An identity written before this check, or by an older release, is still a + * private key other local accounts can read. Refuse it rather than booting. */ + const content = await readGuardedFile( + path, + (mode) => + `Bridge identity ${path} is accessible beyond its owner (mode ${mode}). ` + + 'Treat its private key as compromised: revoke the worker and pair again with an ' + + 'identity path on a native Linux filesystem.', + ); + const identity = JSON.parse(content) as unknown; if (!isPairedIdentity(identity)) { throw new BridgeProtocolError(`Invalid bridge identity file: ${path}`); }