From b3df2c87712310af7f9493529cff6a5a6b8fc691 Mon Sep 17 00:00:00 2001 From: Danny Avila Date: Wed, 30 Sep 2026 07:31:56 -0400 Subject: [PATCH 1/2] =?UTF-8?q?=F0=9F=9B=A1=EF=B8=8F=20fix:=20Deny=20a=20N?= =?UTF-8?q?on-Lane=20Root's=20Own=20Git=20Metadata=20in=20the=20Native=20S?= =?UTF-8?q?RT=20Sandbox?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit A trusted-vm command in a native (non-lane) SRT workspace could write the registered root's own `.git/hooks/*` and `.git/config`, planting a hook or a `core.fsmonitor` / `filter.*` entry that then runs unsandboxed the next time the user — or trusted worker code — invokes Git in that checkout. The non-lane config relied on SRT's mandatory `.git/hooks` and `.git/config` denies, which on Linux are computed from the worker *process's* current directory (see `sandbox/linux-sandbox-utils.js`). The worker's cwd is its home (the systemd user unit sets no WorkingDirectory), never the registered root, so those denies landed elsewhere and the root's `.git` was writable. macOS was unaffected because it applies global `**/.git/hooks/**` and `**/.git/config` Seatbelt patterns. `NativeSrtWorkspaceCommandSandbox` now adds explicit `denyWrite` entries for the registered root's own Git metadata when `/.git` is a directory (lanes are untouched — they already keep the whole common Git directory read-only): - `/.git/hooks` and `/.git/config` unconditionally; - `/.git/config.worktree` and `/.git/commondir` where they exist; - the same files for every submodule Git directory under `.git/modules/**` (including nested submodules) and every `.git/worktrees/*`. `commondir` and `config.worktree` are denied only where present: Git reads them strictly and SRT would otherwise mask an absent target with an empty `/dev/null` bind Git cannot parse, breaking ordinary commands. Denied files that exist are re-bound read-only, so `.git/config` stays readable (remotes keep working) while writes fail; a benign `git commit` in the workspace is unaffected. Tests: a live SRT test gated on `LIBRECHAT_CODE_LIVE_SRT_TESTS=1` runs the sandbox with `process.cwd()` outside the root and asserts hooks, config, an existing per-worktree config, a submodule config, and a linked-worktree commondir cannot be tampered while a normal commit still succeeds; wired into the Linux native-sandbox CI job. A fast unit test asserts the computed `denyWrite` set. `packages/code/README.md` documents the guarantee and the residual writable-`.git` vectors (whole-`.git` replacement, a fresh top-level commondir, a nested repo) that the personal-machine SRT trust model accepts. --- .github/workflows/ci.yml | 7 +- packages/code/README.md | 23 +++ packages/code/src/native-sandbox.test.ts | 51 +++++++ packages/code/src/native-sandbox.ts | 108 ++++++++++++++- .../code/src/root-git-denies-live.test.ts | 131 ++++++++++++++++++ 5 files changed, 317 insertions(+), 3 deletions(-) create mode 100644 packages/code/src/root-git-denies-live.test.ts diff --git a/.github/workflows/ci.yml b/.github/workflows/ci.yml index f7393284..e6a23b61 100644 --- a/.github/workflows/ci.yml +++ b/.github/workflows/ci.yml @@ -254,10 +254,13 @@ jobs: sudo sysctl -w kernel.apparmor_restrict_unprivileged_userns=0 - run: npm ci - run: npm run build - - name: Linked worktree lane containment + - name: Linked worktree lane and root Git metadata containment env: LIBRECHAT_CODE_LIVE_SRT_TESTS: '1' - run: node --test dist/linked-worktrees-live.test.js + run: >- + node --test + dist/linked-worktrees-live.test.js + dist/root-git-denies-live.test.js lambda-microvm-provisioning: name: Lambda MicroVM Provisioning diff --git a/packages/code/README.md b/packages/code/README.md index ba7ec4d7..06580187 100644 --- a/packages/code/README.md +++ b/packages/code/README.md @@ -213,12 +213,35 @@ SRT with: worker's private scratch directory; - read access denied to the worker's home directory except for that workspace; - paired identity and mutation-quarantine files explicitly denied; +- the registered workspace's own Git metadata denied writes — `.git/hooks` + and `.git/config` always, plus `.git/config.worktree`, `.git/commondir`, + and the equivalent files under `.git/modules/*` and `.git/worktrees/*` + where they already exist — so a sandboxed command cannot plant a hook or a + `filter`/`fsmonitor`/`diff` config entry that would run unsandboxed the + next time Git runs in the checkout; - `LIBRECHAT_CODE_*` and nonessential inherited environment variables removed; - network egress denied by default, local binding denied, and Unix sockets denied; and - bounded time and aggregate output, with best-effort process-group termination on cancellation, timeout, and completion. +The Git-metadata denies are applied to the registered root directly rather +than relying on SRT's Linux mandatory denies, which are derived from the +worker process's own current directory — the worker home, not the workspace — +and so never covered the registered root; macOS already enforced the +equivalent through global Seatbelt patterns, and this keeps the guarantee +identical on both platforms regardless of the worker's cwd. `.git/commondir` +and `.git/config.worktree` are denied only where they already exist, because +Git reads them strictly and SRT would otherwise mask an absent one with an +empty bind Git cannot parse. Because the whole workspace stays writable, a +sandboxed command can still stage Git configuration Git will honor later by +other means — for example replacing the entire `.git` directory, writing a +new top-level `.git/commondir` that redirects the common directory, or +initializing a fresh nested repository. Denying those safely would require +making the workspace's Git storage structurally read-only, which the +personal-machine SRT trust model does not; use the Docker/NsJail backend or a +dedicated VM boundary when a workspace command must be treated as adversarial. + SRT restrictions remain inherited by descendants. Windows additionally uses a kill-on-close Job Object. Native macOS does not provide an equivalent hard process-lifetime boundary: a deliberately daemonized descendant can outlive diff --git a/packages/code/src/native-sandbox.test.ts b/packages/code/src/native-sandbox.test.ts index 5b79aa1a..c64b59e5 100644 --- a/packages/code/src/native-sandbox.test.ts +++ b/packages/code/src/native-sandbox.test.ts @@ -1751,3 +1751,54 @@ test('linked worktree Git guard refuses Windows rather than admitting an unguard ); await sandbox.close(); }); + +test('non-lane roots deny their own executable Git metadata regardless of the worker cwd', async t => { + const root = await realpath(await mkdtemp(join(tmpdir(), 'librechat-code-gitmeta-'))); + t.after(() => rm(root, { recursive: true, force: true })); + const gitDir = join(root, '.git'); + await mkdir(join(gitDir, 'hooks'), { recursive: true }); + await writeFile(join(gitDir, 'config'), '[core]\n'); + await writeFile(join(gitDir, 'config.worktree'), '[core]\n'); + // A submodule Git directory (identified by its HEAD file) and a nested one. + // This submodule has no config.worktree, so it is not denied one. + await mkdir(join(gitDir, 'modules', 'sub', 'modules', 'inner'), { recursive: true }); + await writeFile(join(gitDir, 'modules', 'sub', 'HEAD'), 'ref: refs/heads/main\n'); + await writeFile(join(gitDir, 'modules', 'sub', 'config'), '[core]\n'); + await writeFile( + join(gitDir, 'modules', 'sub', 'modules', 'inner', 'HEAD'), + 'ref: refs/heads/main\n', + ); + // A linked worktree's per-worktree metadata: an existing commondir and + // config.worktree are both denied. + await mkdir(join(gitDir, 'worktrees', 'wt'), { recursive: true }); + await writeFile(join(gitDir, 'worktrees', 'wt', 'commondir'), '../..\n'); + await writeFile(join(gitDir, 'worktrees', 'wt', 'config.worktree'), '[core]\n'); + + const fake = fakeManager(); + const sandbox = new NativeSrtWorkspaceCommandSandbox({ + workspaceRoot: root, + environment: { PATH: '/usr/bin', LANG: 'C.UTF-8' }, + manager: fake.manager, + }); + t.after(() => sandbox.close()); + await sandbox.prepare(); + + const denyWrite = fake.config?.filesystem.denyWrite ?? []; + for (const relativePath of [ + '.git/hooks', + '.git/config', + '.git/config.worktree', + '.git/modules/sub/hooks', + '.git/modules/sub/config', + '.git/modules/sub/modules/inner/hooks', + '.git/modules/sub/modules/inner/config', + '.git/worktrees/wt/config.worktree', + '.git/worktrees/wt/commondir', + ]) { + assert.ok(denyWrite.includes(join(root, relativePath)), relativePath); + } + // Deny-if-exists paths are skipped when absent: masking them with an empty + // bind would make Git read a broken redirect or config from every command. + assert.ok(!denyWrite.includes(join(gitDir, 'commondir'))); + assert.ok(!denyWrite.includes(join(gitDir, 'modules', 'sub', 'config.worktree'))); +}); diff --git a/packages/code/src/native-sandbox.ts b/packages/code/src/native-sandbox.ts index 49fe6ecf..6c8fd9f2 100644 --- a/packages/code/src/native-sandbox.ts +++ b/packages/code/src/native-sandbox.ts @@ -11,7 +11,7 @@ import { sep, } from 'node:path'; import { constants as fsConstants } from 'node:fs'; -import { access, mkdtemp, open, realpath, rm, stat } from 'node:fs/promises'; +import { access, mkdtemp, open, readdir, realpath, rm, stat } from 'node:fs/promises'; import type { FileHandle } from 'node:fs/promises'; import { matchesWorkspaceRoot } from './root-identity.js'; import type { WorkspaceRootIdentity } from './root-identity.js'; @@ -234,6 +234,98 @@ async function canonicalPath(path: string): Promise { } } +/** + * Paths inside a repository's own Git directory that Git treats as executable + * configuration: hooks run on the next Git invocation, and `config` / + * an existing `config.worktree` can define filter, diff, or fsmonitor commands + * that later run unsandboxed. An existing `commondir` is denied too, since it + * redirects the whole common Git directory to attacker-controlled config. + * + * A sandboxed command may write anywhere in the workspace root, so these are + * denied explicitly for the registered root's `.git`. SRT's Linux mandatory + * denies derive equivalents from the worker process's current directory, which + * is not the registered root, so they never cover it; macOS instead applies + * global `.git/hooks` and `.git/config` Seatbelt patterns. Denying the root's + * own metadata directly keeps the guarantee identical on both platforms and + * independent of the worker's cwd. Lane workspaces keep the whole common Git + * directory read-only already, so this augments only the non-lane root. + * + * Only submodule and linked-worktree Git directories that already exist are + * enumerated: masking a non-existent `.git/modules` would block a later + * `git submodule add` from creating it during the same command. `.git/info` + * is intentionally omitted — its attributes reference filter/diff drivers by + * name, but the commands those names resolve to live in the denied config, so + * `info` alone cannot introduce a new executable. + */ +async function collectGitMetadataDenies(gitDir: string): Promise { + const denies = [join(gitDir, 'hooks'), join(gitDir, 'config')]; + await pushExistingDeny(denies, join(gitDir, 'config.worktree')); + await pushExistingDeny(denies, join(gitDir, 'commondir')); + for (const submoduleGitDir of await collectSubmoduleGitDirs( + join(gitDir, 'modules'), + )) { + denies.push(join(submoduleGitDir, 'hooks'), join(submoduleGitDir, 'config')); + await pushExistingDeny(denies, join(submoduleGitDir, 'config.worktree')); + await pushExistingDeny(denies, join(submoduleGitDir, 'commondir')); + } + const worktreesDir = join(gitDir, 'worktrees'); + const worktreeEntries = await readdir(worktreesDir, { + withFileTypes: true, + }).catch(() => []); + for (const entry of worktreeEntries) { + if (!entry.isDirectory()) continue; + const worktreeGitDir = join(worktreesDir, entry.name); + await pushExistingDeny(denies, join(worktreeGitDir, 'config.worktree')); + await pushExistingDeny(denies, join(worktreeGitDir, 'commondir')); + } + return denies; +} + +/** + * `commondir` and `config.worktree` are read strictly by Git when present — + * `commondir` at every startup, `config.worktree` whenever `worktreeConfig` + * is enabled — and SRT masks a non-existent deny target with an empty + * `/dev/null` bind that Git then fails to read. Denying them only where they + * already exist keeps the mask a read-only bind of the real file. `config` + * and `hooks` stay unconditional: they always exist in a real repository, + * and blocking their creation stops an attacker planting them. An attacker + * cannot make Git honor a freshly planted `config.worktree` without first + * enabling the extension in the denied `.git/config`. + */ +async function pushExistingDeny(paths: string[], candidate: string): Promise { + if (await access(candidate).then(() => true, () => false)) { + paths.push(candidate); + } +} + +/** + * Enumerate every submodule Git directory beneath `.git/modules`, following the + * `/modules/` nesting Git uses for recursive submodules. A + * directory holding a `HEAD` file is a Git directory; any other directory is an + * intermediate segment of a submodule path that itself contains slashes. + */ +async function collectSubmoduleGitDirs(modulesDir: string): Promise { + const found: string[] = []; + const stack = [modulesDir]; + while (stack.length > 0) { + const dir = stack.pop()!; + const entries = await readdir(dir, { withFileTypes: true }).catch(() => []); + if (entries.some(entry => entry.isFile() && entry.name === 'HEAD')) { + found.push(dir); + if ( + entries.some(entry => entry.isDirectory() && entry.name === 'modules') + ) { + stack.push(join(dir, 'modules')); + } + continue; + } + for (const entry of entries) { + if (entry.isDirectory()) stack.push(join(dir, entry.name)); + } + } + return found; +} + function boundedUtf8(buffer: Buffer, budget: number): string { let end = Math.min(buffer.byteLength, budget); while (end > 0) { @@ -517,6 +609,18 @@ export class NativeSrtWorkspaceCommandSandbox implements WorkspaceCommandSandbox const gitGuardDirectory = lane ? await this.createLinkedWorktreeGitGuard(canonicalScratchDirectory!) : undefined; + // A lane already keeps the shared common Git directory read-only; only + // the non-lane root exposes a writable `.git` whose executable metadata + // SRT's cwd-derived mandatory denies do not reach. + const rootGitDir = join(root, '.git'); + const rootGitMetadataDenies = + lane || + !(await stat(rootGitDir).then( + entry => entry.isDirectory(), + () => false, + )) + ? [] + : await collectGitMetadataDenies(rootGitDir); const commandPolicy = normalizeNativeSrtCommandPolicy( this.options.commandPolicy, ); @@ -558,6 +662,7 @@ export class NativeSrtWorkspaceCommandSandbox implements WorkspaceCommandSandbox denyWrite: [ ...protectedPaths, ...deniedInheritedWritablePaths, + ...rootGitMetadataDenies, ...(gitGuardDirectory ? [gitGuardDirectory] : []), ], allowGitConfig: false, @@ -628,6 +733,7 @@ export class NativeSrtWorkspaceCommandSandbox implements WorkspaceCommandSandbox this.denyWritePaths = [ ...protectedPaths, ...deniedInheritedWritablePaths, + ...rootGitMetadataDenies, ...(gitGuardDirectory ? [gitGuardDirectory] : []), ]; } diff --git a/packages/code/src/root-git-denies-live.test.ts b/packages/code/src/root-git-denies-live.test.ts new file mode 100644 index 00000000..dfa64c34 --- /dev/null +++ b/packages/code/src/root-git-denies-live.test.ts @@ -0,0 +1,131 @@ +import assert from 'node:assert/strict'; +import { execFile } from 'node:child_process'; +import { mkdir, mkdtemp, readFile, realpath, rm, stat, writeFile } from 'node:fs/promises'; +import { tmpdir } from 'node:os'; +import { join } from 'node:path'; +import { promisify } from 'node:util'; +import test from 'node:test'; + +import { NativeSrtWorkspaceCommandSandbox } from './native-sandbox.js'; +import { resolveNativeSrtCommandPolicy } from './native-policy.js'; + +const execFileAsync = promisify(execFile); +const IDENTITY = ['-c', 'user.name=t', '-c', 'user.email=t@example.com', '-c', 'commit.gpgsign=false']; + +async function git(cwd: string, ...args: string[]): Promise { + return (await execFileAsync('git', [...IDENTITY, ...args], { cwd })).stdout.trim(); +} + +async function snapshot(paths: string[]): Promise> { + const entries = await Promise.all( + paths.map(async (path) => [path, await readFile(path, 'utf8').catch(() => null)] as const), + ); + return Object.fromEntries(entries); +} + +async function exists(path: string): Promise { + return stat(path).then(() => true, () => false); +} + +/** + * SRT's Linux mandatory denies for `.git/hooks` and `.git/config` are computed + * from the worker process's own current directory, which is never the registered + * workspace root. This exercises the sandbox from a cwd outside the root and + * asserts the root's executable Git metadata is unwritable anyway — hooks, + * config, an existing per-worktree config, and a submodule's config cannot be + * tampered, so nothing an attacker writes runs the next time git is invoked on + * the host. + */ +test('real SRT denies a non-lane root its own Git hooks and config even when the worker cwd is elsewhere', { + skip: process.env.LIBRECHAT_CODE_LIVE_SRT_TESTS !== '1', + timeout: 90_000, +}, async (t) => { + const outside = await realpath(await mkdtemp(join(tmpdir(), 'git-denies-cwd-'))); + const base = await realpath(await mkdtemp(join(tmpdir(), 'git-denies-'))); + t.after(() => rm(base, { recursive: true, force: true })); + t.after(() => rm(outside, { recursive: true, force: true })); + + // A standalone repo used only as a local submodule source. + const upstream = join(base, 'sub-src'); + await mkdir(upstream); + await git(upstream, 'init', '-q', '-b', 'main'); + await writeFile(join(upstream, 's.txt'), 'sub\n'); + await git(upstream, 'add', 's.txt'); + await git(upstream, 'commit', '-qm', 'sub'); + + const root = join(base, 'repo'); + await mkdir(root); + await git(root, 'init', '-q', '-b', 'main'); + await writeFile(join(root, 'tracked.txt'), 'checkout\n'); + await git(root, 'add', 'tracked.txt'); + await git(root, 'commit', '-qm', 'init'); + await git(root, '-c', 'protocol.file.allow=always', 'submodule', 'add', '-q', upstream, 'sub'); + await git(root, 'commit', '-qm', 'add submodule'); + await mkdir(join(root, '.worktrees')); + await git(root, 'worktree', 'add', '-q', '.worktrees/wt', '-b', 'wt'); + // An existing per-worktree config for the linked worktree; --worktree needs + // the repository extension enabled first. + await git(root, 'config', 'extensions.worktreeConfig', 'true'); + await git(join(root, '.worktrees', 'wt'), 'config', '--worktree', 'core.testflag', 'true'); + + const gitDir = join(root, '.git'); + const submoduleGitDir = join(gitDir, 'modules', 'sub'); + const worktreeGitDir = join(gitDir, 'worktrees', 'wt'); + const guarded = [ + join(gitDir, 'hooks', 'post-checkout'), + join(gitDir, 'config'), + join(submoduleGitDir, 'config'), + join(submoduleGitDir, 'hooks', 'post-checkout'), + join(worktreeGitDir, 'config.worktree'), + join(worktreeGitDir, 'commondir'), + ]; + for (const path of guarded) { + assert.ok(await exists(path) || path.endsWith('post-checkout'), `precondition: ${path}`); + } + const before = await snapshot(guarded); + + const sandbox = new NativeSrtWorkspaceCommandSandbox({ + workspaceRoot: root, + commandPolicy: resolveNativeSrtCommandPolicy('trusted-vm'), + environment: { PATH: process.env.PATH, LANG: 'C.UTF-8' }, + }); + t.after(() => sandbox.close()); + const run = (command: string) => + sandbox.execute({ + protocolVersion: 1, + operation: 'execute_command', + workspaceId: 'root', + command, + timeoutMs: 30_000, + maxOutputBytes: 16_384, + }); + + // The worker process runs from outside the registered root, the way the + // systemd user unit does (its cwd is the worker home, not the workspace). + const previousCwd = process.cwd(); + process.chdir(outside); + t.after(() => process.chdir(previousCwd)); + + for (const path of guarded) { + const attempt = await run(`printf tampered > '${path}'`); + assert.notEqual(attempt.exitCode, 0, `expected a write to ${path} to be denied`); + } + // Planting a fresh hook file (not just overwriting one) must also be blocked. + const plantHook = await run( + `printf '#!/bin/sh\\ntouch ${join(base, 'PWNED')}\\n' > .git/hooks/pre-commit && chmod +x .git/hooks/pre-commit`, + ); + assert.notEqual(plantHook.exitCode, 0); + + // A benign commit in the workspace must still succeed: the guard makes Git + // metadata read-only, not the checkout or the writable object store. + const committed = await run( + `printf work > work.txt && git ${IDENTITY.join(' ')} add work.txt && git ${IDENTITY.join(' ')} commit -qm work`, + ); + assert.equal(committed.exitCode, 0, committed.stderr); + + await sandbox.close(); + + assert.equal(await exists(join(base, 'PWNED')), false); + assert.deepEqual(await snapshot(guarded), before); + assert.equal(await git(root, 'log', '-1', '--format=%s'), 'work'); +}); From cdfcce085c086e393b78fdbe78860e4f5b6fe7ed Mon Sep 17 00:00:00 2001 From: Danny Avila Date: Wed, 30 Sep 2026 07:47:18 -0400 Subject: [PATCH 2/2] =?UTF-8?q?=F0=9F=94=81=20fix:=20Refresh=20Root=20Git?= =?UTF-8?q?=20Metadata=20Denies=20per=20Command=20and=20Fail=20Closed?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The root's Git metadata denies were a snapshot taken at sandbox initialization, but the worker is long-lived: a repository, submodule, or linked worktree created on the host afterwards stayed writable until restart. Ordinary root commands now recompute the set and pass it as a per-command `denyWrite` override (every other filesystem field is the session policy), the way SRT recomputes its own mandatory denies on every wrap. Native Windows keeps the initialization snapshot because srt-win rejects per-command allowRead/allowWrite. Inspecting the metadata now treats only ENOENT/ENOTDIR as absence; any other failure propagates, so an unreadable `.git` fails the command closed (COMMAND_UNAVAILABLE) instead of silently dropping a deny. Corrects the `config.worktree` comment: in a repository that already enables `worktreeConfig`, a missing per-worktree config stays creatable, because denying an absent file makes every Git command fail. The README lists it with the other writable-workspace residuals. --- packages/code/README.md | 21 ++- packages/code/src/native-sandbox.test.ts | 73 ++++++++- packages/code/src/native-sandbox.ts | 140 +++++++++++++----- .../code/src/root-git-denies-live.test.ts | 9 ++ 4 files changed, 198 insertions(+), 45 deletions(-) diff --git a/packages/code/README.md b/packages/code/README.md index 06580187..d421e444 100644 --- a/packages/code/README.md +++ b/packages/code/README.md @@ -230,14 +230,19 @@ than relying on SRT's Linux mandatory denies, which are derived from the worker process's own current directory — the worker home, not the workspace — and so never covered the registered root; macOS already enforced the equivalent through global Seatbelt patterns, and this keeps the guarantee -identical on both platforms regardless of the worker's cwd. `.git/commondir` -and `.git/config.worktree` are denied only where they already exist, because -Git reads them strictly and SRT would otherwise mask an absent one with an -empty bind Git cannot parse. Because the whole workspace stays writable, a -sandboxed command can still stage Git configuration Git will honor later by -other means — for example replacing the entire `.git` directory, writing a -new top-level `.git/commondir` that redirects the common directory, or -initializing a fresh nested repository. Denying those safely would require +identical on both platforms regardless of the worker's cwd. The set is +recomputed before every command, so a repository, submodule, or linked +worktree created on the host after the worker starts is covered from the next +command, and metadata the worker cannot inspect fails the command closed. +`.git/commondir` and `.git/config.worktree` are denied only where they already +exist, because Git reads them strictly and SRT would otherwise mask an absent +one with a stub Git cannot open, failing every Git command. Because the whole +workspace stays writable, a sandboxed command can still stage Git +configuration Git will honor later by other means — for example replacing +the entire `.git` directory, writing a new `commondir` that redirects the +common directory, creating a missing `config.worktree` in a repository that +already enables the `worktreeConfig` extension, or initializing a fresh +nested repository. Denying those safely would require making the workspace's Git storage structurally read-only, which the personal-machine SRT trust model does not; use the Docker/NsJail backend or a dedicated VM boundary when a workspace command must be treated as adversarial. diff --git a/packages/code/src/native-sandbox.test.ts b/packages/code/src/native-sandbox.test.ts index c64b59e5..9001102f 100644 --- a/packages/code/src/native-sandbox.test.ts +++ b/packages/code/src/native-sandbox.test.ts @@ -1752,7 +1752,7 @@ test('linked worktree Git guard refuses Windows rather than admitting an unguard await sandbox.close(); }); -test('non-lane roots deny their own executable Git metadata regardless of the worker cwd', async t => { +test('non-lane roots deny their own executable Git metadata at initialization', async t => { const root = await realpath(await mkdtemp(join(tmpdir(), 'librechat-code-gitmeta-'))); t.after(() => rm(root, { recursive: true, force: true })); const gitDir = join(root, '.git'); @@ -1802,3 +1802,74 @@ test('non-lane roots deny their own executable Git metadata regardless of the wo assert.ok(!denyWrite.includes(join(gitDir, 'commondir'))); assert.ok(!denyWrite.includes(join(gitDir, 'modules', 'sub', 'config.worktree'))); }); + +test('ordinary root commands deny Git metadata created after initialization', async t => { + const root = await realpath(await mkdtemp(join(tmpdir(), 'librechat-code-gitmeta-late-'))); + t.after(() => rm(root, { recursive: true, force: true })); + const fake = fakeManager(); + const sandbox = new NativeSrtWorkspaceCommandSandbox({ + workspaceRoot: root, + environment: { PATH: '/usr/bin', LANG: 'C.UTF-8' }, + manager: fake.manager, + }); + t.after(() => sandbox.close()); + await sandbox.prepare(); + const session = fake.config!.filesystem; + const gitDir = join(root, '.git'); + assert.ok(!session.denyWrite.some(path => path.startsWith(gitDir))); + + // The host initializes the repository and adds a submodule and a linked + // worktree while the worker keeps running. + await mkdir(join(gitDir, 'hooks'), { recursive: true }); + await writeFile(join(gitDir, 'config'), '[core]\n'); + await mkdir(join(gitDir, 'modules', 'late'), { recursive: true }); + await writeFile(join(gitDir, 'modules', 'late', 'HEAD'), 'ref: refs/heads/main\n'); + await mkdir(join(gitDir, 'worktrees', 'late'), { recursive: true }); + await writeFile(join(gitDir, 'worktrees', 'late', 'commondir'), '../..\n'); + + const result = await sandbox.execute(request); + assert.equal(result.exitCode, 0); + const filesystem = fake.customConfigSeenDuringWrap?.filesystem; + for (const relativePath of [ + '.git/hooks', + '.git/config', + '.git/modules/late/hooks', + '.git/modules/late/config', + '.git/worktrees/late/commondir', + ]) { + assert.ok(filesystem?.denyWrite.includes(join(root, relativePath)), relativePath); + } + // Only denyWrite differs from the session filesystem policy. + assert.deepEqual({ ...filesystem, denyWrite: session.denyWrite }, session); +}); + +test('a root command fails closed when its Git metadata cannot be inspected', async t => { + if (process.getuid?.() === 0) { + t.skip('directory permissions do not restrict root'); + return; + } + const root = await realpath(await mkdtemp(join(tmpdir(), 'librechat-code-gitmeta-denied-'))); + t.after(() => rm(root, { recursive: true, force: true })); + const modules = join(root, '.git', 'modules'); + await mkdir(modules, { recursive: true }); + await writeFile(join(root, '.git', 'config'), '[core]\n'); + const fake = fakeManager(); + const sandbox = new NativeSrtWorkspaceCommandSandbox({ + workspaceRoot: root, + environment: { PATH: '/usr/bin', LANG: 'C.UTF-8' }, + manager: fake.manager, + }); + t.after(() => sandbox.close()); + await sandbox.prepare(); + + await chmod(modules, 0o000); + try { + await assert.rejects( + sandbox.execute(request), + (error: unknown) => + error instanceof WorkspaceToolError && error.code === 'COMMAND_UNAVAILABLE', + ); + } finally { + await chmod(modules, 0o700); + } +}); diff --git a/packages/code/src/native-sandbox.ts b/packages/code/src/native-sandbox.ts index 6c8fd9f2..5d39d405 100644 --- a/packages/code/src/native-sandbox.ts +++ b/packages/code/src/native-sandbox.ts @@ -11,6 +11,7 @@ import { sep, } from 'node:path'; import { constants as fsConstants } from 'node:fs'; +import type { Dirent, Stats } from 'node:fs'; import { access, mkdtemp, open, readdir, realpath, rm, stat } from 'node:fs/promises'; import type { FileHandle } from 'node:fs/promises'; import { matchesWorkspaceRoot } from './root-identity.js'; @@ -250,33 +251,32 @@ async function canonicalPath(path: string): Promise { * independent of the worker's cwd. Lane workspaces keep the whole common Git * directory read-only already, so this augments only the non-lane root. * - * Only submodule and linked-worktree Git directories that already exist are - * enumerated: masking a non-existent `.git/modules` would block a later - * `git submodule add` from creating it during the same command. `.git/info` + * The set is recomputed before every ordinary root command, so a repository, + * submodule, or linked worktree created after initialization is covered from + * the next command on. Only submodule and linked-worktree Git directories + * that exist at that moment are enumerated: masking a non-existent + * `.git/modules` would block a later `git submodule add` from creating it + * during the same command. `.git/info` * is intentionally omitted — its attributes reference filter/diff drivers by * name, but the commands those names resolve to live in the denied config, so * `info` alone cannot introduce a new executable. */ -async function collectGitMetadataDenies(gitDir: string): Promise { +async function collectGitMetadataDenies(root: string): Promise { + const gitDir = join(root, '.git'); + if (!(await statIfPresent(gitDir))?.isDirectory()) return []; const denies = [join(gitDir, 'hooks'), join(gitDir, 'config')]; - await pushExistingDeny(denies, join(gitDir, 'config.worktree')); - await pushExistingDeny(denies, join(gitDir, 'commondir')); + await pushExistingDenies(denies, gitDir); for (const submoduleGitDir of await collectSubmoduleGitDirs( join(gitDir, 'modules'), )) { denies.push(join(submoduleGitDir, 'hooks'), join(submoduleGitDir, 'config')); - await pushExistingDeny(denies, join(submoduleGitDir, 'config.worktree')); - await pushExistingDeny(denies, join(submoduleGitDir, 'commondir')); + await pushExistingDenies(denies, submoduleGitDir); } const worktreesDir = join(gitDir, 'worktrees'); - const worktreeEntries = await readdir(worktreesDir, { - withFileTypes: true, - }).catch(() => []); - for (const entry of worktreeEntries) { - if (!entry.isDirectory()) continue; - const worktreeGitDir = join(worktreesDir, entry.name); - await pushExistingDeny(denies, join(worktreeGitDir, 'config.worktree')); - await pushExistingDeny(denies, join(worktreeGitDir, 'commondir')); + for (const entry of await readdirIfPresent(worktreesDir)) { + if (entry.isDirectory()) { + await pushExistingDenies(denies, join(worktreesDir, entry.name)); + } } return denies; } @@ -285,16 +285,18 @@ async function collectGitMetadataDenies(gitDir: string): Promise { * `commondir` and `config.worktree` are read strictly by Git when present — * `commondir` at every startup, `config.worktree` whenever `worktreeConfig` * is enabled — and SRT masks a non-existent deny target with an empty - * `/dev/null` bind that Git then fails to read. Denying them only where they - * already exist keeps the mask a read-only bind of the real file. `config` - * and `hooks` stay unconditional: they always exist in a real repository, - * and blocking their creation stops an attacker planting them. An attacker - * cannot make Git honor a freshly planted `config.worktree` without first - * enabling the extension in the denied `.git/config`. + * `/dev/null` bind that Git cannot open, failing every Git command. Denying + * them only where they already exist keeps the mask a read-only bind of the + * real file. `config` and `hooks` stay unconditional: they always exist in a + * real repository. In a repository that already enables `worktreeConfig`, a + * missing `config.worktree` therefore stays creatable, like a new + * `commondir`; the package README lists both with the other residuals of a + * writable workspace. */ -async function pushExistingDeny(paths: string[], candidate: string): Promise { - if (await access(candidate).then(() => true, () => false)) { - paths.push(candidate); +async function pushExistingDenies(paths: string[], gitDir: string): Promise { + for (const name of ['config.worktree', 'commondir']) { + const candidate = join(gitDir, name); + if (await statIfPresent(candidate)) paths.push(candidate); } } @@ -309,7 +311,7 @@ async function collectSubmoduleGitDirs(modulesDir: string): Promise { const stack = [modulesDir]; while (stack.length > 0) { const dir = stack.pop()!; - const entries = await readdir(dir, { withFileTypes: true }).catch(() => []); + const entries = await readdirIfPresent(dir); if (entries.some(entry => entry.isFile() && entry.name === 'HEAD')) { found.push(dir); if ( @@ -326,6 +328,37 @@ async function collectSubmoduleGitDirs(modulesDir: string): Promise { return found; } +/** + * Absence is an expected answer while inspecting Git metadata. Any other + * failure (for example `EACCES`) propagates, so metadata that cannot be + * inspected fails the command closed instead of silently dropping a deny. + */ +function isMissing(error: unknown): boolean { + return ( + error instanceof Error && + 'code' in error && + (error.code === 'ENOENT' || error.code === 'ENOTDIR') + ); +} + +async function statIfPresent(path: string): Promise { + try { + return await stat(path); + } catch (error) { + if (isMissing(error)) return undefined; + throw error; + } +} + +async function readdirIfPresent(path: string): Promise { + try { + return await readdir(path, { withFileTypes: true }); + } catch (error) { + if (isMissing(error)) return []; + throw error; + } +} + function boundedUtf8(buffer: Buffer, budget: number): string { let end = Math.min(buffer.byteLength, budget); while (end > 0) { @@ -407,6 +440,8 @@ export class NativeSrtWorkspaceCommandSandbox implements WorkspaceCommandSandbox private runtimeConfig?: SandboxRuntimeConfig; private denyReadPaths: string[] = []; private denyWritePaths: string[] = []; + /** Worker-owned write denies that precede each command's live Git metadata denies. */ + private baseDenyWritePaths: string[] = []; private get gitEnvironment(): | typeof TRUSTED_GIT_ENVIRONMENT | typeof LINKED_WORKTREE_GIT_ENVIRONMENT { @@ -612,15 +647,9 @@ export class NativeSrtWorkspaceCommandSandbox implements WorkspaceCommandSandbox // A lane already keeps the shared common Git directory read-only; only // the non-lane root exposes a writable `.git` whose executable metadata // SRT's cwd-derived mandatory denies do not reach. - const rootGitDir = join(root, '.git'); - const rootGitMetadataDenies = - lane || - !(await stat(rootGitDir).then( - entry => entry.isDirectory(), - () => false, - )) - ? [] - : await collectGitMetadataDenies(rootGitDir); + const rootGitMetadataDenies = lane + ? [] + : await collectGitMetadataDenies(root); const commandPolicy = normalizeNativeSrtCommandPolicy( this.options.commandPolicy, ); @@ -730,6 +759,10 @@ export class NativeSrtWorkspaceCommandSandbox implements WorkspaceCommandSandbox deniedInheritedWritablePaths.includes(path), ), ]; + this.baseDenyWritePaths = [ + ...protectedPaths, + ...deniedInheritedWritablePaths, + ]; this.denyWritePaths = [ ...protectedPaths, ...deniedInheritedWritablePaths, @@ -1043,6 +1076,40 @@ export class NativeSrtWorkspaceCommandSandbox implements WorkspaceCommandSandbox } } + /** + * The session policy snapshots the root's Git metadata at initialization, but + * the worker outlives that snapshot: a repository, submodule, or linked + * worktree created on the host afterwards must be protected without a + * restart, the way SRT recomputes its own mandatory denies on every wrap. + * Only `denyWrite` differs from the session filesystem policy. Lanes keep + * the whole common Git directory read-only already. Native Windows keeps the + * initialization snapshot, because `srt-win` rejects the per-command + * `allowRead`/`allowWrite` a full filesystem override carries. + */ + private async rootCommandConfig(): Promise< + Partial | undefined + > { + const config = this.runtimeConfig; + const root = this.canonicalRoot; + if ( + !config || + !root || + this.options.linkedWorktree || + this.platform === 'win32' + ) { + return undefined; + } + return { + filesystem: { + ...config.filesystem, + denyWrite: [ + ...this.baseDenyWritePaths, + ...(await collectGitMetadataDenies(root)), + ], + }, + }; + } + private async executeBound( request: WorkspaceExecuteCommandRequest, signal?: AbortSignal, @@ -1087,6 +1154,7 @@ export class NativeSrtWorkspaceCommandSandbox implements WorkspaceCommandSandbox ReturnType >; try { + const commandConfig = customConfig ?? (await this.rootCommandConfig()); const credentialEnvironment = await this.options.maskedEnvironment?.resolve(signal, cwd); const sandboxedCommand = this.options.maskedEnvironment?.wrapCommand @@ -1108,7 +1176,7 @@ export class NativeSrtWorkspaceCommandSandbox implements WorkspaceCommandSandbox this.platform === 'win32' ? undefined : (this.options.shellPath ?? '/bin/bash'), - customConfig, + commandConfig, signal, cwd, { commandId, commandText: request.command }, diff --git a/packages/code/src/root-git-denies-live.test.ts b/packages/code/src/root-git-denies-live.test.ts index dfa64c34..12dc820c 100644 --- a/packages/code/src/root-git-denies-live.test.ts +++ b/packages/code/src/root-git-denies-live.test.ts @@ -116,6 +116,15 @@ test('real SRT denies a non-lane root its own Git hooks and config even when the ); assert.notEqual(plantHook.exitCode, 0); + // Metadata the host creates while the worker runs is covered from the next + // command, not only what existed when the sandbox initialized. + await git(root, 'worktree', 'add', '-q', '.worktrees/late', '-b', 'late'); + const lateCommondir = join(gitDir, 'worktrees', 'late', 'commondir'); + const lateBefore = await readFile(lateCommondir, 'utf8'); + const lateAttempt = await run(`printf tampered > '${lateCommondir}'`); + assert.notEqual(lateAttempt.exitCode, 0, 'expected the new worktree metadata to be denied'); + assert.equal(await readFile(lateCommondir, 'utf8'), lateBefore); + // A benign commit in the workspace must still succeed: the guard makes Git // metadata read-only, not the checkout or the writable object store. const committed = await run(