Skip to content
Closed
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
5 changes: 5 additions & 0 deletions .changeset/realpath-symlink-guard.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,5 @@
---
'@electric-ax/agents-runtime': patch
---

The built-in `read`, `write`, and `edit` tools now reject paths whose realpath resolves outside the working directory. Previously the cwd guard was a string prefix check on the un-resolved path, which followed symlinks transparently — a symlink at `<cwd>/link.txt` pointing to `/etc/hostname` was readable, and a symlinked directory could redirect writes outside the workspace (CVE-2025-53109/53110 class). Intra-workspace symlinks (pnpm `.pnpm` trees, etc.) keep working. For non-existent write/edit targets the guard realpaths the deepest existing ancestor instead. Known gap: hardlinks across the cwd boundary still bypass; a proper fix requires a sandbox.
15 changes: 7 additions & 8 deletions packages/agents-runtime/src/tools/edit.ts
Original file line number Diff line number Diff line change
@@ -1,8 +1,9 @@
import { readFile, writeFile } from 'node:fs/promises'
import { relative, resolve } from 'node:path'
import { relative } from 'node:path'
import { createTwoFilesPatch } from 'diff'
import { Type } from '@sinclair/typebox'
import { runtimeLog } from '../log'
import { resolveInsideWorkdir } from './path-guard'
import type { AgentTool } from '@mariozechner/pi-agent-core'

const READ_GUARD_MESSAGE = (rel: string): string =>
Expand Down Expand Up @@ -45,19 +46,17 @@ export function createEditTool(
replace_all?: boolean
}
try {
const resolved = resolve(workingDirectory, filePath)
const rel = relative(workingDirectory, resolved)
if (rel.startsWith(`..`)) {
const guard = await resolveInsideWorkdir(filePath, workingDirectory)
if (!guard.ok) {
return {
content: [
{
type: `text` as const,
text: `Error: Path "${filePath}" is outside the working directory`,
},
{ type: `text` as const, text: `Error: ${guard.reason}` },
],
details: { replacements: 0 },
}
}
const resolved = guard.resolved
const rel = relative(workingDirectory, resolved)

if (!readSet.has(resolved)) {
return {
Expand Down
49 changes: 49 additions & 0 deletions packages/agents-runtime/src/tools/path-guard.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,49 @@
import { realpath } from 'node:fs/promises'
import { dirname, relative, resolve } from 'node:path'

export type PathGuardResult =
| { ok: true; resolved: string }
| { ok: false; reason: string }

// Resolves `filePath` relative to `workingDirectory` and verifies it stays
// inside, both before and after symlink expansion. For non-existent
// targets (write/edit may create files) `realpath` is applied to the
// deepest existing ancestor. Hardlinks across the cwd boundary remain a
// known gap.
export async function resolveInsideWorkdir(
filePath: string,
workingDirectory: string
): Promise<PathGuardResult> {
const resolved = resolve(workingDirectory, filePath)
if (relative(workingDirectory, resolved).startsWith(`..`)) {
return {
ok: false,
reason: `Path "${filePath}" is outside the working directory`,
}
}
const realCwd = await realpath(workingDirectory)
let probe = resolved
while (true) {
try {
const realTarget = await realpath(probe)
if (relative(realCwd, realTarget).startsWith(`..`)) {
return {
ok: false,
reason: `Path "${filePath}" resolves outside the working directory via a symlink`,
}
}
return { ok: true, resolved }
} catch (err) {
const code = (err as NodeJS.ErrnoException).code
if (code !== `ENOENT` && code !== `ENOTDIR`) throw err
const parent = dirname(probe)
if (parent === probe) {
return {
ok: false,
reason: `Path "${filePath}" resolves outside the working directory via a symlink`,
}
}
probe = parent
}
}
}
13 changes: 5 additions & 8 deletions packages/agents-runtime/src/tools/read-file.ts
Original file line number Diff line number Diff line change
@@ -1,7 +1,7 @@
import { readFile, stat } from 'node:fs/promises'
import { relative, resolve } from 'node:path'
import { Type } from '@sinclair/typebox'
import { runtimeLog } from '../log'
import { resolveInsideWorkdir } from './path-guard'
import type { AgentTool } from '@mariozechner/pi-agent-core'

const MAX_FILE_SIZE = 512 * 1024 // 512 KB
Expand All @@ -22,19 +22,16 @@ export function createReadFileTool(
execute: async (_toolCallId, params) => {
const { path: filePath } = params as { path: string }
try {
const resolved = resolve(workingDirectory, filePath)
const rel = relative(workingDirectory, resolved)
if (rel.startsWith(`..`)) {
const guard = await resolveInsideWorkdir(filePath, workingDirectory)
if (!guard.ok) {
return {
content: [
{
type: `text` as const,
text: `Error: Path "${filePath}" is outside the working directory`,
},
{ type: `text` as const, text: `Error: ${guard.reason}` },
],
details: { charCount: 0 },
}
}
const resolved = guard.resolved

const fileStat = await stat(resolved)
if (fileStat.size > MAX_FILE_SIZE) {
Expand Down
15 changes: 7 additions & 8 deletions packages/agents-runtime/src/tools/write.ts
Original file line number Diff line number Diff line change
@@ -1,8 +1,9 @@
import { mkdir, readFile, writeFile } from 'node:fs/promises'
import { dirname, relative, resolve } from 'node:path'
import { dirname, relative } from 'node:path'
import { createTwoFilesPatch } from 'diff'
import { Type } from '@sinclair/typebox'
import { runtimeLog } from '../log'
import { resolveInsideWorkdir } from './path-guard'
import type { AgentTool } from '@mariozechner/pi-agent-core'

export function createWriteTool(
Expand All @@ -27,19 +28,17 @@ export function createWriteTool(
content: string
}
try {
const resolved = resolve(workingDirectory, filePath)
const rel = relative(workingDirectory, resolved)
if (rel.startsWith(`..`)) {
const guard = await resolveInsideWorkdir(filePath, workingDirectory)
if (!guard.ok) {
return {
content: [
{
type: `text` as const,
text: `Error: Path "${filePath}" is outside the working directory`,
},
{ type: `text` as const, text: `Error: ${guard.reason}` },
],
details: { bytesWritten: 0 },
}
}
const resolved = guard.resolved
const rel = relative(workingDirectory, resolved)

let original = ``
let existed = true
Expand Down
117 changes: 117 additions & 0 deletions packages/agents-runtime/test/path-guard.test.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,117 @@
import { mkdir, mkdtemp, rm, symlink, writeFile } from 'node:fs/promises'
import { realpathSync } from 'node:fs'
import { tmpdir } from 'node:os'
import { join } from 'node:path'
import { afterEach, beforeEach, describe, expect, it } from 'vitest'
import { resolveInsideWorkdir } from '../src/tools/path-guard'

describe(`resolveInsideWorkdir`, () => {
let cwd: string
let outside: string

beforeEach(async () => {
cwd = await mkdtemp(join(tmpdir(), `path-guard-`))
outside = await mkdtemp(join(tmpdir(), `path-guard-outside-`))
})

afterEach(async () => {
await rm(cwd, { recursive: true, force: true })
await rm(outside, { recursive: true, force: true })
})

it(`returns the resolved absolute path for an in-bounds relative path`, async () => {
await writeFile(join(cwd, `file.txt`), `hello`)
const result = await resolveInsideWorkdir(`file.txt`, cwd)
expect(result).toEqual({ ok: true, resolved: join(cwd, `file.txt`) })
})

it(`accepts an in-bounds path that does not yet exist`, async () => {
const result = await resolveInsideWorkdir(`fresh/dir/file.txt`, cwd)
expect(result).toEqual({
ok: true,
resolved: join(cwd, `fresh/dir/file.txt`),
})
})

it(`rejects ".." escape via the prefix check`, async () => {
const result = await resolveInsideWorkdir(`../escape.txt`, cwd)
expect(result).toEqual({
ok: false,
reason: `Path "../escape.txt" is outside the working directory`,
})
})

it(`rejects an absolute path outside the working directory`, async () => {
const result = await resolveInsideWorkdir(`/etc/hostname`, cwd)
expect(result).toEqual({
ok: false,
reason: `Path "/etc/hostname" is outside the working directory`,
})
})

it(`rejects a symlink that resolves outside the working directory`, async () => {
const secret = join(outside, `secret.txt`)
await writeFile(secret, `secret`)
await symlink(secret, join(cwd, `link.txt`))
const result = await resolveInsideWorkdir(`link.txt`, cwd)
expect(result).toEqual({
ok: false,
reason: `Path "link.txt" resolves outside the working directory via a symlink`,
})
})

it(`rejects a write target whose parent is a symlinked-out directory`, async () => {
await symlink(outside, join(cwd, `escape-dir`))
const result = await resolveInsideWorkdir(`escape-dir/new.txt`, cwd)
expect(result.ok).toBe(false)
if (!result.ok) {
expect(result.reason).toMatch(/resolves outside the working directory/)
}
})

it(`treats a regular-file ancestor like a missing ancestor (ENOTDIR walks up)`, async () => {
await writeFile(join(cwd, `not-a-dir`), `content`)
const result = await resolveInsideWorkdir(`not-a-dir/sub/new.txt`, cwd)
expect(result).toEqual({
ok: true,
resolved: join(cwd, `not-a-dir/sub/new.txt`),
})
})

it(`accepts a symlink that stays inside the working directory`, async () => {
const inner = join(cwd, `inner.txt`)
await writeFile(inner, `inner`)
await symlink(inner, join(cwd, `link.txt`))
const result = await resolveInsideWorkdir(`link.txt`, cwd)
expect(result).toEqual({ ok: true, resolved: join(cwd, `link.txt`) })
})

it(`accepts writing through a directory symlink that points inside cwd`, async () => {
await mkdir(join(cwd, `data`))
await symlink(join(cwd, `data`), join(cwd, `data-link`))
const result = await resolveInsideWorkdir(`data-link/new.txt`, cwd)
expect(result).toEqual({
ok: true,
resolved: join(cwd, `data-link/new.txt`),
})
})

it(`handles a working directory that is itself a symlink`, async () => {
const realDir = await mkdtemp(join(tmpdir(), `path-guard-real-`))
const linkDir = join(tmpdir(), `path-guard-link-${Date.now()}`)
await symlink(realDir, linkDir)
try {
await writeFile(join(realDir, `file.txt`), `data`)
const result = await resolveInsideWorkdir(`file.txt`, linkDir)
expect(result.ok).toBe(true)
if (result.ok) {
expect(realpathSync(result.resolved)).toBe(
realpathSync(join(realDir, `file.txt`))
)
}
} finally {
await rm(linkDir, { force: true })
await rm(realDir, { recursive: true, force: true })
}
})
})
68 changes: 16 additions & 52 deletions packages/agents-runtime/test/tool-path-symlink.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -6,12 +6,10 @@ import { createEditTool } from '../src/tools/edit'
import { createReadFileTool } from '../src/tools/read-file'
import { createWriteTool } from '../src/tools/write'

// Characterization: read/write/edit guard the working directory using a
// path-prefix check (resolve + relative + startsWith('..')) but do NOT call
// `realpath`, so a symlink inside the working directory that points outside
// is followed transparently — CVE-2025-53109/53110 class bypass. A follow-up
// PR will add realpath resolution; update these expectations when it lands.
describe(`tool path traversal — current symlink behavior`, () => {
// End-to-end coverage that read/write/edit reject escape paths and that
// the target on the other side of an escape symlink is left untouched.
// Path-resolution semantics themselves are exercised in path-guard.test.ts.
describe(`tool path traversal`, () => {
let cwd: string
let outside: string

Expand All @@ -25,74 +23,40 @@ describe(`tool path traversal — current symlink behavior`, () => {
await rm(outside, { recursive: true, force: true })
})

it(`read: ".." escape is rejected by the prefix check`, async () => {
const tool = createReadFileTool(cwd)
const result = await tool.execute(`r-dotdot`, { path: `../escape.txt` })
expect((result.content[0] as { text: string }).text).toMatch(
/outside the working directory/
)
})

it(`read: symlink inside cwd pointing outside currently succeeds`, async () => {
it(`read rejects an escape symlink with the guard's error message`, async () => {
const secret = join(outside, `secret.txt`)
await writeFile(secret, `secret data`, `utf-8`)
await symlink(secret, join(cwd, `link.txt`))
const tool = createReadFileTool(cwd)
const result = await tool.execute(`r-link`, { path: `link.txt` })
expect((result.content[0] as { text: string }).text).toBe(`secret data`)
})

it(`write: ".." escape is rejected by the prefix check`, async () => {
const tool = createWriteTool(cwd)
const result = await tool.execute(`w-dotdot`, {
path: `../escape.txt`,
content: `nope`,
})
const result = await tool.execute(`r`, { path: `link.txt` })
expect((result.content[0] as { text: string }).text).toMatch(
/outside the working directory/
/resolves outside the working directory via a symlink/
)
})

it(`write: symlink inside cwd pointing outside currently clobbers the target`, async () => {
it(`write rejects an escape symlink and leaves the outside target untouched`, async () => {
const target = join(outside, `target.txt`)
await writeFile(target, `original`, `utf-8`)
await symlink(target, join(cwd, `link.txt`))
const tool = createWriteTool(cwd)
const result = await tool.execute(`w-link`, {
path: `link.txt`,
content: `clobbered`,
})
expect(result.details).toMatchObject({ bytesWritten: 9 })
expect(await readFile(target, `utf-8`)).toBe(`clobbered`)
await tool.execute(`w`, { path: `link.txt`, content: `clobbered` })
expect(await readFile(target, `utf-8`)).toBe(`original`)
})

it(`edit: ".." escape is rejected by the prefix check`, async () => {
const tool = createEditTool(cwd, new Set())
const result = await tool.execute(`e-dotdot`, {
path: `../escape.txt`,
old_string: `a`,
new_string: `b`,
})
expect((result.content[0] as { text: string }).text).toMatch(
/outside the working directory/
)
})

it(`edit: symlink inside cwd pointing outside currently edits through the link`, async () => {
it(`edit rejects an escape symlink even when the link is in readSet`, async () => {
const target = join(outside, `t.txt`)
await writeFile(target, `hello world`, `utf-8`)
const linkPath = join(cwd, `link.txt`)
await symlink(target, linkPath)
// The edit tool requires the file to be in readSet; populate it with the
// resolved path the tool would compute. This mirrors what read would have
// done in the same session.
const tool = createEditTool(cwd, new Set([linkPath]))
const result = await tool.execute(`e-link`, {
const result = await tool.execute(`e`, {
path: `link.txt`,
old_string: `world`,
new_string: `there`,
})
expect(result.details).toMatchObject({ replacements: 1 })
expect(await readFile(target, `utf-8`)).toBe(`hello there`)
expect((result.content[0] as { text: string }).text).toMatch(
/resolves outside the working directory via a symlink/
)
expect(await readFile(target, `utf-8`)).toBe(`hello world`)
})
})
Loading