From 59f6e774b8b769f1c3723b5fdd688091be6f82a3 Mon Sep 17 00:00:00 2001 From: AstroHan Date: Tue, 4 Aug 2026 04:32:47 +0800 Subject: [PATCH 1/6] fix(runtime): decide workspace path containment in one realpath space MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The local workspace executor compared a realpath'd session cwd against a merely resolved candidate path. Any session cwd under a symlink — macOS tmpdirs (`/var` → `/private/var`), or a workspace root organised with symlinks — therefore rejected every legitimate absolute path inside the workspace, so a model writing by absolute path could not touch its own workspace. Canonicalise the candidate through its deepest existing ancestor before the containment check, so both sides live in the realpath space and the target may still be missing. Following the symlinks does not weaken containment: a link inside the cwd pointing out of it now resolves to its outside target and is rejected, including for Write. The write-lock key gets the same canonicalisation, so relative and symlinked-absolute spellings of one file no longer take two different locks. `realpathAllowMissing` moves to path-containment.ts, the owner of the containment invariant, and replaces the two copies in sandbox-boundary-path.ts and filesystem-worker/operations.ts. --- .../src/__tests__/builtin-tools.test.ts | 99 +++++++++++++++++++ .../src/filesystem-worker/operations.ts | 13 +-- packages/runtime/src/path-containment.ts | 44 ++++++++- packages/runtime/src/sandbox-boundary-path.ts | 20 +--- packages/runtime/src/workspace-executor.ts | 55 +++++------ 5 files changed, 172 insertions(+), 59 deletions(-) diff --git a/packages/runtime/src/__tests__/builtin-tools.test.ts b/packages/runtime/src/__tests__/builtin-tools.test.ts index d12705ead2..8eaa213046 100644 --- a/packages/runtime/src/__tests__/builtin-tools.test.ts +++ b/packages/runtime/src/__tests__/builtin-tools.test.ts @@ -2100,6 +2100,74 @@ describe('builtin write tools path containment', () => { expect(await readFile(join(root, 'inside.txt'), 'utf8')).toBe('hello Maka'); }); + test('file tools stay usable when the session cwd is reached through a symlink', async () => { + // The session cwd itself sits under a symlink (macOS hands out `/var/...` + // tmpdirs whose realpath is `/private/var/...`, and workspace roots are often + // organised with symlinks). Containment must be decided in one path space, so + // absolute paths spelled through the link are inside — and escapes still are not. + const base = await realpath(await mkdtemp(join(tmpdir(), 'maka-symlink-cwd-'))); + const workspace = join(base, 'workspace'); + const outside = join(base, 'outside'); + await mkdir(join(workspace, 'src'), { recursive: true }); + await mkdir(outside, { recursive: true }); + await writeFile(join(workspace, 'src', 'inside.txt'), 'inside token\n', 'utf8'); + await writeFile(join(outside, 'secret.txt'), 'secret\n', 'utf8'); + await symlink(join(outside, 'secret.txt'), join(workspace, 'escape.txt')); + await symlink(outside, join(workspace, 'outside-link')); + const cwd = join(base, 'link-to-workspace'); + await symlink(workspace, cwd); + + const read = tool('Read'); + const write = tool('Write'); + const edit = tool('Edit'); + const glob = tool('Glob'); + const grep = tool('Grep'); + + expect(await runTool(read, { path: join(cwd, 'src', 'inside.txt') }, cwd)).toMatchObject({ + content: 'inside token\n', + }); + await runTool(write, { path: join(cwd, 'src', 'written.txt'), content: 'written\n' }, cwd); + expect(await readFile(join(workspace, 'src', 'written.txt'), 'utf8')).toBe('written\n'); + await runTool( + edit, + { path: join(cwd, 'src', 'inside.txt'), old_string: 'token', new_string: 'edited' }, + cwd, + ); + expect(await readFile(join(workspace, 'src', 'inside.txt'), 'utf8')).toBe('inside edited\n'); + expect(await runTool(glob, { pattern: '*.txt', cwd: join(cwd, 'src') }, cwd)).toMatchObject({ + files: ['inside.txt', 'written.txt'], + }); + const grepResult = await runTool(grep, { pattern: 'edited', path: join(cwd, 'src') }, cwd); + expect(JSON.stringify(grepResult).includes('inside.txt')).toBe(true); + + // Resolving into the canonical space must not turn "follow a symlink out of + // the workspace" into a legal path. + await expectRejects( + runTool(read, { path: join(outside, 'secret.txt') }, cwd), + /Read path must stay inside/, + ); + await expectRejects( + runTool(read, { path: join(cwd, 'escape.txt') }, cwd), + /Read path must stay inside/, + ); + await expectRejects( + runTool(write, { path: join(cwd, 'escape.txt'), content: 'x' }, cwd), + /Write path must stay inside/, + ); + await expectRejects( + runTool(write, { path: join(cwd, 'outside-link', 'new.txt'), content: 'x' }, cwd), + /Write path must stay inside/, + ); + await expectRejects( + runTool(grep, { pattern: 'secret', path: join(cwd, 'outside-link') }, cwd), + /Grep path must stay inside/, + ); + await expectRejects( + runTool(glob, { pattern: '*.txt', cwd: join(cwd, 'outside-link') }, cwd), + /Glob cwd path must stay inside/, + ); + }); + test('concurrent Edits to the same file serialize — no lost update', async () => { const root = await mkdtemp(join(tmpdir(), 'maka-edit-lock-')); const n = 20; @@ -2156,6 +2224,37 @@ describe('builtin write tools path containment', () => { expect(await readFile(join(root, 'data.txt'), 'utf8')).toBe(expected); }); + test('concurrent Edits through a symlinked cwd serialize on one key', async () => { + const base = await realpath(await mkdtemp(join(tmpdir(), 'maka-edit-symlink-'))); + const workspace = join(base, 'workspace'); + await mkdir(workspace, { recursive: true }); + const cwd = join(base, 'link-to-workspace'); + await symlink(workspace, cwd); + const n = 20; + const markers = Array.from({ length: n }, (_, i) => `marker-${String(i).padStart(2, '0')}`); + await writeFile(join(workspace, 'data.txt'), `${markers.join('\n')}\n`, 'utf8'); + const edit = tool('Edit'); + // The relative spelling resolves against the canonical cwd while the absolute + // one is spelled through the link. Unless the lock key canonicalises both, the + // two groups take different locks and clobber each other. + const results = await Promise.all( + markers.map((m, i) => + runTool( + edit, + { + path: i % 2 === 0 ? 'data.txt' : join(cwd, 'data.txt'), + old_string: m, + new_string: `done-${String(i).padStart(2, '0')}`, + }, + cwd, + ), + ), + ); + expect(results.every((r) => (r as { ok: boolean }).ok === true)).toBe(true); + const expected = `${Array.from({ length: n }, (_, i) => `done-${String(i).padStart(2, '0')}`).join('\n')}\n`; + expect(await readFile(join(workspace, 'data.txt'), 'utf8')).toBe(expected); + }); + test('Write then Edit on one file resolves inside the lock — the fresh file is found', async () => { const root = await mkdtemp(join(tmpdir(), 'maka-write-edit-')); const write = tool('Write'); diff --git a/packages/runtime/src/filesystem-worker/operations.ts b/packages/runtime/src/filesystem-worker/operations.ts index 4081e78da3..ef2d58a4fb 100644 --- a/packages/runtime/src/filesystem-worker/operations.ts +++ b/packages/runtime/src/filesystem-worker/operations.ts @@ -2,7 +2,7 @@ import { spawn } from 'node:child_process'; import { promises as fs } from 'node:fs'; import { glob as nodeGlob } from 'node:fs/promises'; import { dirname, isAbsolute, parse, resolve } from 'node:path'; -import { isPathInside } from '../path-containment.js'; +import { isPathInside, realpathAllowMissing } from '../path-containment.js'; import { sandboxBoundaryExpansionAllowsPath } from '@maka/core'; import { computeEditedSource } from '../edit-replace.js'; @@ -392,17 +392,6 @@ function assertContainedGlobPattern(pattern: string): void { } } -async function realpathAllowMissing(path: string): Promise { - try { - return await fs.realpath(path); - } catch (error) { - if (nodeErrorCode(error) !== 'ENOENT') throw error; - const parent = dirname(path); - if (parent === path) throw error; - return resolve(await realpathAllowMissing(parent), path.slice(parent.length + 1)); - } -} - async function targetTypeOf(path: string): Promise { try { const metadata = await fs.stat(path); diff --git a/packages/runtime/src/path-containment.ts b/packages/runtime/src/path-containment.ts index c278ad437a..b81110ffd1 100644 --- a/packages/runtime/src/path-containment.ts +++ b/packages/runtime/src/path-containment.ts @@ -1,6 +1,6 @@ import { createHash } from 'node:crypto'; import { lstat, mkdir, readFile, realpath, rename, unlink, writeFile } from 'node:fs/promises'; -import { isAbsolute, join, relative, sep } from 'node:path'; +import { basename, dirname, isAbsolute, join, relative, resolve, sep } from 'node:path'; /** * Shared filesystem-containment and identifier guards. This is the single @@ -58,6 +58,48 @@ export function toRelative(root: string, target: string): string { return rel === '' ? '.' : rel.split(sep).join('/'); } +/** + * Canonicalise `target` even when its leaf (or a run of trailing segments) does + * not exist yet: the deepest existing ancestor is realpath'd and the missing + * segments are appended. Containment can then be decided against a realpath'd + * root in a single path space, which is what {@link isPathInside} assumes — + * comparing a realpath'd root against a merely `resolve`d candidate rejects + * every legitimate absolute path whenever the root sits under a symlink (macOS + * `/var` → `/private/var`, symlinked workspace roots). + * + * Because symlinks are followed all the way to the leaf, this never legalises + * an escape: a link inside the root that points out of it resolves to its + * outside target and fails containment. + * + * Throws the underlying error when a path component is unreadable rather than + * missing, so permission problems are not silently treated as containment. + */ +export async function realpathAllowMissing(target: string): Promise { + let cursor = resolve(target); + const missing: string[] = []; + while (true) { + try { + const realExisting = await realpath(cursor); + return resolve(realExisting, ...missing.reverse()); + } catch (error) { + if (!isMissingPathError(error)) throw error; + const parent = dirname(cursor); + if (parent === cursor) throw error; + missing.push(basename(cursor)); + cursor = parent; + } + } +} + +function isMissingPathError(error: unknown): boolean { + return ( + typeof error === 'object' && + error !== null && + 'code' in error && + (error.code === 'ENOENT' || error.code === 'ENOTDIR') + ); +} + // ── Contained file I/O ──────────────────────────────────────────────────── /** diff --git a/packages/runtime/src/sandbox-boundary-path.ts b/packages/runtime/src/sandbox-boundary-path.ts index 9a1274df8d..ca32fa4720 100644 --- a/packages/runtime/src/sandbox-boundary-path.ts +++ b/packages/runtime/src/sandbox-boundary-path.ts @@ -1,5 +1,6 @@ import { promises as fs } from 'node:fs'; -import { dirname, resolve } from 'node:path'; +import { resolve } from 'node:path'; +import { realpathAllowMissing } from './path-containment.js'; import { MAX_SANDBOX_BOUNDARY_PATH_CHARS, validateSandboxBoundaryExpansion, @@ -73,23 +74,6 @@ export async function normalizeSandboxBoundaryExpansion( return normalized.expansion; } -async function realpathAllowMissing(target: string): Promise { - let cursor = target; - const missing: string[] = []; - while (true) { - try { - const realParent = await fs.realpath(cursor); - return resolve(realParent, ...missing.reverse()); - } catch (error) { - if (!isMissingPathError(error)) throw error; - const parent = dirname(cursor); - if (parent === cursor) throw error; - missing.push(cursor.slice(parent.length + (parent === '/' ? 0 : 1))); - cursor = parent; - } - } -} - async function targetTypeFor(path: string): Promise { try { const stat = await fs.stat(path); diff --git a/packages/runtime/src/workspace-executor.ts b/packages/runtime/src/workspace-executor.ts index 354f16e945..66e99548f2 100644 --- a/packages/runtime/src/workspace-executor.ts +++ b/packages/runtime/src/workspace-executor.ts @@ -1,8 +1,8 @@ import { promises as fs } from 'node:fs'; import { exec } from 'node:child_process'; import { glob as nodeGlob } from 'node:fs/promises'; -import { dirname, isAbsolute, resolve } from 'node:path'; -import { isPathInside } from './path-containment.js'; +import { isAbsolute, resolve } from 'node:path'; +import { isPathInside, realpathAllowMissing } from './path-containment.js'; import { promisify } from 'node:util'; import type { ToolExecutionFacts } from '@maka/core/permission'; import { runProcessWithBoundedTail, runShellWithBoundedTail } from './shell-exec.js'; @@ -249,11 +249,16 @@ export class LocalWorkspaceExecutor implements WorkspaceExecutor { } async resolveWritablePath(input: WorkspaceResolvePathInput): Promise { - return { path: await resolveWritableInsideCwd(input.cwd, input.path, input.label) }; + return { path: await canonicalPathInsideCwd(input.cwd, input.path, input.label) }; } async writeLockKey(input: WorkspaceWriteLockKeyInput): Promise { - return { key: resolve(await fs.realpath(input.cwd), input.path) }; + // Same canonicalisation as the resolvers, so every spelling of one file — + // relative, absolute, or through a symlink — takes the same lock. Escapes + // are rejected by the resolvers inside the lock, not here. + const root = await fs.realpath(input.cwd); + const requested = isAbsolute(input.path) ? resolve(input.path) : resolve(root, input.path); + return { key: await realpathAllowMissing(requested) }; } async globFiles(input: WorkspaceGlobInput): Promise { @@ -294,26 +299,32 @@ function shellEscape(arg: string): string { return `'${arg.replaceAll("'", "'\\''")}'`; } -async function resolveWritableInsideCwd( +/** + * Canonical path of `inputPath` relative to the session cwd, or a containment + * error. Both sides of the check live in the realpath space: the root and the + * candidate (through its deepest existing ancestor, since the target may not + * exist yet). Comparing a realpath'd root against a merely resolved candidate + * rejected every legitimate absolute path whenever the session cwd sat under a + * symlink — macOS tmpdirs (`/var` → `/private/var`) and symlinked workspace + * roots. Following the symlinks does not weaken containment: a link inside the + * cwd that points out of it resolves to its outside target and is rejected. + */ +async function canonicalPathInsideCwd( cwd: string, inputPath: string, label: string, ): Promise { const root = await fs.realpath(cwd); - const candidate = isAbsolute(inputPath) ? resolve(inputPath) : resolve(root, inputPath); + const requested = isAbsolute(inputPath) ? resolve(inputPath) : resolve(root, inputPath); + const candidate = await realpathAllowMissing(requested); if (!isPathInside(root, candidate)) { throw new Error( `${label} path must stay inside session cwd ${JSON.stringify(root)}; ` + - `received ${JSON.stringify(inputPath)}.`, + `received ${JSON.stringify(inputPath)}, which resolves to ${JSON.stringify(candidate)}.`, ); } - const parent = await fs.realpath(dirname(candidate)); - if (!isPathInside(root, parent)) { - throw new Error(`${label} path must stay inside session cwd`); - } - return candidate; } @@ -322,20 +333,8 @@ async function resolveExistingInsideCwd( inputPath: string, label: string, ): Promise { - const root = await fs.realpath(cwd); - const candidate = isAbsolute(inputPath) ? resolve(inputPath) : resolve(root, inputPath); - - if (!isPathInside(root, candidate)) { - throw new Error( - `${label} path must stay inside session cwd ${JSON.stringify(root)}; ` + - `received ${JSON.stringify(inputPath)}.`, - ); - } - - const target = await fs.realpath(candidate); - if (!isPathInside(root, target)) { - throw new Error(`${label} path must stay inside session cwd`); - } - - return target; + const candidate = await canonicalPathInsideCwd(cwd, inputPath, label); + // The read/search callers depend on the target existing; surface that here + // rather than as a downstream open/spawn failure. + return await fs.realpath(candidate); } From 6968f2f167bb5f009ad04f36220460fd27d7c2b6 Mon Sep 17 00:00:00 2001 From: AstroHan Date: Tue, 4 Aug 2026 09:40:04 +0800 Subject: [PATCH 2/6] fix(runtime): re-check containment after resolving an existing path MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Collapsing the two resolvers dropped the check the old existing-path variant ran on its final `fs.realpath` result. The candidate is already canonical, so the two can only diverge when a segment that was missing during canonicalisation becomes a symlink before the realpath — a narrow race, but one the old code caught and the new code returned unchecked to Read, Edit, FormatJson, Glob, and Grep. Restore it by making the containment assertion a named helper both call sites share. --- packages/runtime/src/workspace-executor.ts | 47 +++++++++++++--------- 1 file changed, 28 insertions(+), 19 deletions(-) diff --git a/packages/runtime/src/workspace-executor.ts b/packages/runtime/src/workspace-executor.ts index 66e99548f2..6489ddd6c8 100644 --- a/packages/runtime/src/workspace-executor.ts +++ b/packages/runtime/src/workspace-executor.ts @@ -249,7 +249,7 @@ export class LocalWorkspaceExecutor implements WorkspaceExecutor { } async resolveWritablePath(input: WorkspaceResolvePathInput): Promise { - return { path: await canonicalPathInsideCwd(input.cwd, input.path, input.label) }; + return { path: (await canonicalPathInsideCwd(input.cwd, input.path, input.label)).path }; } async writeLockKey(input: WorkspaceWriteLockKeyInput): Promise { @@ -300,10 +300,10 @@ function shellEscape(arg: string): string { } /** - * Canonical path of `inputPath` relative to the session cwd, or a containment - * error. Both sides of the check live in the realpath space: the root and the - * candidate (through its deepest existing ancestor, since the target may not - * exist yet). Comparing a realpath'd root against a merely resolved candidate + * Canonical session cwd and the canonical path of `inputPath` under it, or a + * containment error. Both sides of the check live in the realpath space: the + * root, and the candidate through its deepest existing ancestor (the target may + * not exist yet). Comparing a realpath'd root against a merely resolved candidate * rejected every legitimate absolute path whenever the session cwd sat under a * symlink — macOS tmpdirs (`/var` → `/private/var`) and symlinked workspace * roots. Following the symlinks does not weaken containment: a link inside the @@ -313,19 +313,11 @@ async function canonicalPathInsideCwd( cwd: string, inputPath: string, label: string, -): Promise { +): Promise<{ root: string; path: string }> { const root = await fs.realpath(cwd); const requested = isAbsolute(inputPath) ? resolve(inputPath) : resolve(root, inputPath); - const candidate = await realpathAllowMissing(requested); - - if (!isPathInside(root, candidate)) { - throw new Error( - `${label} path must stay inside session cwd ${JSON.stringify(root)}; ` + - `received ${JSON.stringify(inputPath)}, which resolves to ${JSON.stringify(candidate)}.`, - ); - } - - return candidate; + const candidate = assertInsideCwd(root, await realpathAllowMissing(requested), inputPath, label); + return { root, path: candidate }; } async function resolveExistingInsideCwd( @@ -333,8 +325,25 @@ async function resolveExistingInsideCwd( inputPath: string, label: string, ): Promise { - const candidate = await canonicalPathInsideCwd(cwd, inputPath, label); + const { root, path: candidate } = await canonicalPathInsideCwd(cwd, inputPath, label); // The read/search callers depend on the target existing; surface that here - // rather than as a downstream open/spawn failure. - return await fs.realpath(candidate); + // rather than as a downstream open/spawn failure. Re-check the result: a + // segment that was missing a moment ago can have become a symlink out of the + // cwd since it was canonicalised. + return assertInsideCwd(root, await fs.realpath(candidate), inputPath, label); +} + +function assertInsideCwd( + root: string, + candidate: string, + inputPath: string, + label: string, +): string { + if (!isPathInside(root, candidate)) { + throw new Error( + `${label} path must stay inside session cwd ${JSON.stringify(root)}; ` + + `received ${JSON.stringify(inputPath)}, which resolves to ${JSON.stringify(candidate)}.`, + ); + } + return candidate; } From 9fd7cc261bb31b8f71bfa33a3b65aba432d5e77f Mon Sep 17 00:00:00 2001 From: AstroHan Date: Tue, 4 Aug 2026 09:40:04 +0800 Subject: [PATCH 3/6] fix(runtime): follow dangling symlinks when canonicalising a path MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit `realpath` fails ENOENT on a symlink whose target does not exist, so `realpathAllowMissing` treated such a link as a plain missing leaf and returned the link's own path. A link inside the workspace pointing at a not-yet-created path outside it therefore read as contained, and a Write through it created the file outside the workspace — verified against the local executor before the fix. Read the link and follow it by hand instead. `realpath` reports a cycle as ELOOP, so each hop consumes one existing link and the walk terminates; the hop cap is a backstop, not the cycle guard. The two other consumers of the helper gain the same accuracy: a dangling link now normalises to the path a write would actually reach, which is the path the sandbox boundary should approve. Covers the helper with direct unit tests — it is now the canonicalisation authority for three subsystems and had none of its own. --- .../src/__tests__/builtin-tools.test.ts | 9 +++ .../src/__tests__/path-containment.test.ts | 80 ++++++++++++++++++- packages/runtime/src/path-containment.ts | 32 +++++++- 3 files changed, 115 insertions(+), 6 deletions(-) diff --git a/packages/runtime/src/__tests__/builtin-tools.test.ts b/packages/runtime/src/__tests__/builtin-tools.test.ts index 8eaa213046..166e5ba7f3 100644 --- a/packages/runtime/src/__tests__/builtin-tools.test.ts +++ b/packages/runtime/src/__tests__/builtin-tools.test.ts @@ -2158,6 +2158,15 @@ describe('builtin write tools path containment', () => { runTool(write, { path: join(cwd, 'outside-link', 'new.txt'), content: 'x' }, cwd), /Write path must stay inside/, ); + // A link whose target does not exist yet cannot be realpath'd, but a write + // through it still lands outside the workspace, so it must be followed by + // hand rather than treated as a plain missing leaf. + await symlink(join(outside, 'not-yet.txt'), join(workspace, 'dangling-escape.txt')); + await expectRejects( + runTool(write, { path: join(cwd, 'dangling-escape.txt'), content: 'x' }, cwd), + /Write path must stay inside/, + ); + await expectRejects(access(join(outside, 'not-yet.txt')), /ENOENT|no such file/); await expectRejects( runTool(grep, { pattern: 'secret', path: join(cwd, 'outside-link') }, cwd), /Grep path must stay inside/, diff --git a/packages/runtime/src/__tests__/path-containment.test.ts b/packages/runtime/src/__tests__/path-containment.test.ts index 167b3f024a..1207d84150 100644 --- a/packages/runtime/src/__tests__/path-containment.test.ts +++ b/packages/runtime/src/__tests__/path-containment.test.ts @@ -1,7 +1,15 @@ import { strict as assert } from 'node:assert'; -import { win32, posix } from 'node:path'; -import { describe, test } from 'node:test'; -import { isPathInside } from '../path-containment.js'; +import { mkdir, mkdtemp, realpath, rm, symlink, writeFile } from 'node:fs/promises'; +import { tmpdir } from 'node:os'; +import { join, win32, posix } from 'node:path'; +import { afterEach, describe, test } from 'node:test'; +import { isPathInside, realpathAllowMissing } from '../path-containment.js'; + +const cleanup: string[] = []; + +afterEach(async () => { + await Promise.all(cleanup.splice(0).map((path) => rm(path, { recursive: true, force: true }))); +}); describe('isPathInside', () => { test('rejects cross-drive Windows targets (different drive is not inside root)', () => { @@ -34,3 +42,69 @@ describe('isPathInside', () => { assert.equal(isPathInside('C:\\repo', 'C:\\repo\\..rules\\AGENTS.md', win32), true); }); }); + +describe('realpathAllowMissing', () => { + test('canonicalises an existing path through a symlinked ancestor', async () => { + const { workspace, link } = await symlinkedWorkspace('maka-realpath-existing-'); + await writeFile(join(workspace, 'file.txt'), 'x', 'utf8'); + + assert.equal(await realpathAllowMissing(join(link, 'file.txt')), join(workspace, 'file.txt')); + }); + + test('appends missing trailing segments to the deepest existing ancestor', async () => { + const { workspace, link } = await symlinkedWorkspace('maka-realpath-missing-'); + + assert.equal( + await realpathAllowMissing(join(link, 'absent', 'nested', 'file.txt')), + join(workspace, 'absent', 'nested', 'file.txt'), + ); + }); + + test('follows a dangling symlink to its target, which is where a write would land', async () => { + // realpath fails ENOENT on a link whose target does not exist. Treating it + // as a plain missing leaf returns the link's own path, which reads as + // contained while a write through it escapes. + const { base, workspace, link } = await symlinkedWorkspace('maka-realpath-dangling-'); + const outside = join(base, 'outside'); + await mkdir(outside); + await symlink(join(outside, 'not-yet.txt'), join(workspace, 'dangling.txt')); + + assert.equal( + await realpathAllowMissing(join(link, 'dangling.txt')), + join(outside, 'not-yet.txt'), + ); + }); + + test('follows a chain of dangling links and keeps the segments below them', async () => { + const { base, workspace, link } = await symlinkedWorkspace('maka-realpath-chain-'); + const outside = join(base, 'outside'); + await mkdir(outside); + await symlink(join(outside, 'absent-dir'), join(workspace, 'hop-b')); + await symlink(join(workspace, 'hop-b'), join(workspace, 'hop-a')); + + assert.equal( + await realpathAllowMissing(join(link, 'hop-a', 'file.txt')), + join(outside, 'absent-dir', 'file.txt'), + ); + }); + + test('propagates a symlink cycle instead of walking forever', async () => { + const { workspace } = await symlinkedWorkspace('maka-realpath-cycle-'); + await symlink(join(workspace, 'b'), join(workspace, 'a')); + await symlink(join(workspace, 'a'), join(workspace, 'b')); + + await assert.rejects(realpathAllowMissing(join(workspace, 'a')), { code: 'ELOOP' }); + }); +}); + +async function symlinkedWorkspace( + prefix: string, +): Promise<{ base: string; workspace: string; link: string }> { + const base = await realpath(await mkdtemp(join(tmpdir(), prefix))); + cleanup.push(base); + const workspace = join(base, 'workspace'); + const link = join(base, 'link-to-workspace'); + await mkdir(workspace); + await symlink(workspace, link); + return { base, workspace, link }; +} diff --git a/packages/runtime/src/path-containment.ts b/packages/runtime/src/path-containment.ts index b81110ffd1..9b6b866e9a 100644 --- a/packages/runtime/src/path-containment.ts +++ b/packages/runtime/src/path-containment.ts @@ -1,5 +1,14 @@ import { createHash } from 'node:crypto'; -import { lstat, mkdir, readFile, realpath, rename, unlink, writeFile } from 'node:fs/promises'; +import { + lstat, + mkdir, + readFile, + readlink, + realpath, + rename, + unlink, + writeFile, +} from 'node:fs/promises'; import { basename, dirname, isAbsolute, join, relative, resolve, sep } from 'node:path'; /** @@ -69,20 +78,34 @@ export function toRelative(root: string, target: string): string { * * Because symlinks are followed all the way to the leaf, this never legalises * an escape: a link inside the root that points out of it resolves to its - * outside target and fails containment. + * outside target and fails containment. That includes a dangling link, whose + * `realpath` fails ENOENT — the link is read and followed by hand, because a + * write through it lands on its target, not on the link. * * Throws the underlying error when a path component is unreadable rather than - * missing, so permission problems are not silently treated as containment. + * missing, so permission problems are not silently treated as containment, and + * when a symlink cycle keeps the walk from terminating. */ export async function realpathAllowMissing(target: string): Promise { let cursor = resolve(target); const missing: string[] = []; + // `realpath` reports a cycle as ELOOP, so each hop consumes one existing link + // and the walk terminates; the cap is a backstop against a pathological + // filesystem, not the cycle guard. + let hops = 0; while (true) { try { const realExisting = await realpath(cursor); return resolve(realExisting, ...missing.reverse()); } catch (error) { if (!isMissingPathError(error)) throw error; + const link = await readlink(cursor).catch(() => null); + if (link !== null) { + if (++hops > MAX_DANGLING_SYMLINK_HOPS) + throw new Error(`Path ${JSON.stringify(target)} traverses too many dangling symlinks.`); + cursor = resolve(dirname(cursor), link); + continue; + } const parent = dirname(cursor); if (parent === cursor) throw error; missing.push(basename(cursor)); @@ -91,6 +114,9 @@ export async function realpathAllowMissing(target: string): Promise { } } +/** Dangling links followed by {@link realpathAllowMissing} before it gives up. */ +const MAX_DANGLING_SYMLINK_HOPS = 32; + function isMissingPathError(error: unknown): boolean { return ( typeof error === 'object' && From 32c4b7fe655f19055059ca96a61cb1bd99d39031 Mon Sep 17 00:00:00 2001 From: AstroHan Date: Tue, 4 Aug 2026 10:17:24 +0800 Subject: [PATCH 4/6] fix(runtime): authorise the followed path when the worker writes MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The filesystem worker resolves a missing write target by realpathing its parent and then authorising the unresolved candidate. A dangling symlink inside the root therefore passed its boundary check while the write landed on the link's target outside the root — reproduced by handing the worker a request whose boundary names only the workspace-internal link. Reaching it takes a request whose boundary and expectedTarget disagree, which the in-repo client never sends, but the worker is a separate process that re-checks everything precisely so it does not have to trust its caller. Authorise the followed path, the same one `assertTargetUnchanged` already pins the request to. --- .../src/__tests__/filesystem-worker.test.ts | 24 +++++++++++++++++++ .../src/filesystem-worker/operations.ts | 14 +++++++---- 2 files changed, 34 insertions(+), 4 deletions(-) diff --git a/packages/runtime/src/__tests__/filesystem-worker.test.ts b/packages/runtime/src/__tests__/filesystem-worker.test.ts index 59ab46d65a..229992fe62 100644 --- a/packages/runtime/src/__tests__/filesystem-worker.test.ts +++ b/packages/runtime/src/__tests__/filesystem-worker.test.ts @@ -195,6 +195,30 @@ describe('filesystem worker operations', () => { await assert.rejects(readFile(outsidePath, 'utf8'), { code: 'ENOENT' }); }); + test('denies a write through a dangling symlink the boundary does not cover', async () => { + // The worker enforces its own boundary rather than trusting the caller to + // have canonicalised the path: a link inside the root whose target does not + // exist yet cannot be realpath'd, and a write through it lands on the + // target, outside the root, while the boundary names only the link. + const root = await temporaryDirectory('maka-worker-dangling-root-'); + const outside = await temporaryDirectory('maka-worker-dangling-outside-'); + const link = join(root, 'dangling.txt'); + const target = join(outside, 'not-yet.txt'); + await symlink(target, link); + + const response = await executeFilesystemWorkerRequest( + requestFor( + { kind: 'write', cwd: root, path: link, content: 'blocked' }, + { enforcementPath: target, access: 'write', scope: 'exact', targetType: 'missing' }, + link, + ), + ); + + assert.equal(response.ok, false); + if (!response.ok) assert.equal(response.error.code, 'path_denied'); + await assert.rejects(readFile(target, 'utf8'), { code: 'ENOENT' }); + }); + test('fails when an approved target changes type before execution', async () => { const root = await temporaryDirectory('maka-worker-type-'); const target = join(root, 'target'); diff --git a/packages/runtime/src/filesystem-worker/operations.ts b/packages/runtime/src/filesystem-worker/operations.ts index ef2d58a4fb..800f35336f 100644 --- a/packages/runtime/src/filesystem-worker/operations.ts +++ b/packages/runtime/src/filesystem-worker/operations.ts @@ -316,15 +316,21 @@ async function resolveWritableAllowed( } catch (error) { if (nodeErrorCode(error) !== 'ENOENT') throw error; } - const parent = await fs.realpath(dirname(candidate)); - assertAllowed(root, candidate, label, 'write', permission); - if (!isPathInside(root, parent) && !exactWriteCoversParent(permission, candidate, parent)) { + // The target does not exist, but it can still be a dangling symlink, and a + // write lands on what the link names rather than on the link. Authorise the + // followed path — the same one `assertTargetUnchanged` pins the request to — + // so the worker enforces its own boundary instead of trusting the caller to + // have canonicalised the path for it. + const followed = await realpathAllowMissing(candidate); + const parent = await fs.realpath(dirname(followed)); + assertAllowed(root, followed, label, 'write', permission); + if (!isPathInside(root, parent) && !exactWriteCoversParent(permission, followed, parent)) { throw operationError( 'path_denied', `${label} parent was not covered by the operation boundary.`, ); } - return candidate; + return followed; } async function resolveExistingAllowed( From 3794bb36b274b16ffe0cb2149f9bb049be623f69 Mon Sep 17 00:00:00 2001 From: AstroHan Date: Tue, 4 Aug 2026 10:17:24 +0800 Subject: [PATCH 5/6] test(runtime): pin realpathAllowMissing on ENOTDIR and pathological chains Neither branch had a test. The chain case documents where the bound actually comes from: the kernel caps its own symlink traversal well below the helper's hop cap, so it answers ELOOP first and the hop cap is a backstop for a filesystem that does not. The contract worth pinning is that the walk terminates by rejecting, not which layer rejects. Also corrects the module docstring: the leaf has not imported only `node:path` for some time. --- .../src/__tests__/path-containment.test.ts | 25 +++++++++++++++++++ packages/runtime/src/path-containment.ts | 3 ++- 2 files changed, 27 insertions(+), 1 deletion(-) diff --git a/packages/runtime/src/__tests__/path-containment.test.ts b/packages/runtime/src/__tests__/path-containment.test.ts index 1207d84150..dd8cb96628 100644 --- a/packages/runtime/src/__tests__/path-containment.test.ts +++ b/packages/runtime/src/__tests__/path-containment.test.ts @@ -88,6 +88,31 @@ describe('realpathAllowMissing', () => { ); }); + test('treats a non-directory ancestor as missing rather than throwing ENOTDIR', async () => { + const { workspace, link } = await symlinkedWorkspace('maka-realpath-notdir-'); + await writeFile(join(workspace, 'file.txt'), 'x', 'utf8'); + + assert.equal( + await realpathAllowMissing(join(link, 'file.txt', 'child')), + join(workspace, 'file.txt', 'child'), + ); + }); + + test('rejects a pathological dangling chain instead of walking it forever', async () => { + const { workspace } = await symlinkedWorkspace('maka-realpath-hops-'); + // Every link resolves to the next and only the last one dangles. The kernel + // caps its own symlink traversal well below this length, so it answers ELOOP + // before the helper's hop cap can fire; either way the walk must terminate + // by rejecting rather than by hopping forever. + const chain = 64; + await symlink(join(workspace, 'absent'), join(workspace, `hop-${chain}`)); + for (let index = chain; index > 0; index -= 1) { + await symlink(join(workspace, `hop-${index}`), join(workspace, `hop-${index - 1}`)); + } + + await assert.rejects(realpathAllowMissing(join(workspace, 'hop-0')), /ELOOP|too many/); + }); + test('propagates a symlink cycle instead of walking forever', async () => { const { workspace } = await symlinkedWorkspace('maka-realpath-cycle-'); await symlink(join(workspace, 'b'), join(workspace, 'a')); diff --git a/packages/runtime/src/path-containment.ts b/packages/runtime/src/path-containment.ts index 9b6b866e9a..616e159689 100644 --- a/packages/runtime/src/path-containment.ts +++ b/packages/runtime/src/path-containment.ts @@ -16,7 +16,8 @@ import { basename, dirname, isAbsolute, join, relative, resolve, sep } from 'nod * authority for path-containment checks across the runtime, the desktop main * process, and headless: both the pure-Node runtime and the desktop main (which * already depends on `@maka/runtime`) reach it here without reverse - * dependencies. The leaf imports only `node:path`. + * dependencies. {@link isPathInside} itself is a pure `node:path` predicate; the + * canonicalisation and contained-I/O helpers around it touch the filesystem. * * {@link isPathInside} is separator-aware: it rejects only a real * parent-reference segment (`..` exactly, or `..${sep}`-prefixed), so a child From c2b4468bc900e735d118b16cc9a690c32f774938 Mon Sep 17 00:00:00 2001 From: AstroHan Date: Tue, 4 Aug 2026 10:17:25 +0800 Subject: [PATCH 6/6] refactor(runtime): give the lock key and the resolvers one canonicaliser `writeLockKey` repeated the resolver's root/resolve/canonicalise sequence verbatim. Identical today, so the keys match the resolved paths, but the two spaces drifting apart is exactly what breaks write serialization silently. Split the canonicalisation from the containment assertion so the lock key uses the former and the resolvers add the latter. Also records why the post-realpath assertion in the existing-path resolver must not be deleted: it guards a race no deterministic test can drive, so removing it breaks nothing visible. --- packages/runtime/src/workspace-executor.ts | 57 ++++++++++++++-------- 1 file changed, 36 insertions(+), 21 deletions(-) diff --git a/packages/runtime/src/workspace-executor.ts b/packages/runtime/src/workspace-executor.ts index 6489ddd6c8..2ca6da6f12 100644 --- a/packages/runtime/src/workspace-executor.ts +++ b/packages/runtime/src/workspace-executor.ts @@ -253,12 +253,12 @@ export class LocalWorkspaceExecutor implements WorkspaceExecutor { } async writeLockKey(input: WorkspaceWriteLockKeyInput): Promise { - // Same canonicalisation as the resolvers, so every spelling of one file — - // relative, absolute, or through a symlink — takes the same lock. Escapes - // are rejected by the resolvers inside the lock, not here. - const root = await fs.realpath(input.cwd); - const requested = isAbsolute(input.path) ? resolve(input.path) : resolve(root, input.path); - return { key: await realpathAllowMissing(requested) }; + // The resolvers' canonicalisation without their containment check, so every + // spelling of one file — relative, absolute, or through a symlink — takes + // the same lock. Escapes are rejected by the resolvers inside the lock, not + // here. Sharing the canonicalisation is what keeps the lock-key space and + // the resolved-path space from drifting apart. + return { key: (await canonicalPathUnderCwd(input.cwd, input.path)).path }; } async globFiles(input: WorkspaceGlobInput): Promise { @@ -300,24 +300,35 @@ function shellEscape(arg: string): string { } /** - * Canonical session cwd and the canonical path of `inputPath` under it, or a - * containment error. Both sides of the check live in the realpath space: the - * root, and the candidate through its deepest existing ancestor (the target may - * not exist yet). Comparing a realpath'd root against a merely resolved candidate - * rejected every legitimate absolute path whenever the session cwd sat under a - * symlink — macOS tmpdirs (`/var` → `/private/var`) and symlinked workspace - * roots. Following the symlinks does not weaken containment: a link inside the - * cwd that points out of it resolves to its outside target and is rejected. + * Canonical session cwd and the canonical path `inputPath` names under it, with + * no containment check — the single place that decides which path space every + * caller works in. Both the root and the candidate are realpath'd, the candidate + * through its deepest existing ancestor since the target may not exist yet. + * Comparing a realpath'd root against a merely resolved candidate rejected every + * legitimate absolute path whenever the session cwd sat under a symlink — macOS + * tmpdirs (`/var` → `/private/var`) and symlinked workspace roots. */ -async function canonicalPathInsideCwd( +async function canonicalPathUnderCwd( cwd: string, inputPath: string, - label: string, ): Promise<{ root: string; path: string }> { const root = await fs.realpath(cwd); const requested = isAbsolute(inputPath) ? resolve(inputPath) : resolve(root, inputPath); - const candidate = assertInsideCwd(root, await realpathAllowMissing(requested), inputPath, label); - return { root, path: candidate }; + return { root, path: await realpathAllowMissing(requested) }; +} + +/** + * The same canonical path, rejected unless it stays inside the session cwd. + * Following the symlinks does not weaken containment: a link inside the cwd that + * points out of it resolves to its outside target and is rejected. + */ +async function canonicalPathInsideCwd( + cwd: string, + inputPath: string, + label: string, +): Promise<{ root: string; path: string }> { + const { root, path } = await canonicalPathUnderCwd(cwd, inputPath); + return { root, path: assertInsideCwd(root, path, inputPath, label) }; } async function resolveExistingInsideCwd( @@ -327,9 +338,13 @@ async function resolveExistingInsideCwd( ): Promise { const { root, path: candidate } = await canonicalPathInsideCwd(cwd, inputPath, label); // The read/search callers depend on the target existing; surface that here - // rather than as a downstream open/spawn failure. Re-check the result: a - // segment that was missing a moment ago can have become a symlink out of the - // cwd since it was canonicalised. + // rather than as a downstream open/spawn failure. + // + // Do not drop the second assertion: it closes the window between the two + // awaits, where a segment that was missing during canonicalisation can become + // a symlink out of the cwd before the realpath runs. It is deliberately + // defence-in-depth and no deterministic test can drive that race, so nothing + // will fail if it is removed. return assertInsideCwd(root, await fs.realpath(candidate), inputPath, label); }