fix(core): resolve dangling symlinks in the filesystem sandbox check (#626)

* fix(core): resolve dangling symlinks in the filesystem sandbox check

The sandbox resolver treated a dangling symlink as a path that does not exist yet, so the containment check passed on the link's own location and file_write then created the link's target, which could be outside the agent's working directory. The resolver now follows a dangling symlink to the target it names, rejects targets outside the root, keeps in-root targets writable, and bounds resolution at 40 hops so a loop fails the check.

Addresses GHSA-h42w-g3ch-rjjm (CVE-2026-108600).

* fix(core): fail the sandbox check when realpath reports ELOOP

A symlink chain longer than the kernel's link limit made realpath fail with ELOOP, and the resolver then followed the first link by hand and resolved the rest, accepting a path the kernel itself refuses. ELOOP from realpath now fails the check, and the resolver's own hop limit stays as the bound for links it follows by hand.

* fix(core): resolve symlink targets one component at a time

Following a dangling symlink, the resolver normalized the target with path.resolve, which collapses a/.. lexically. When a is itself a symlink the kernel steps out of the directory a points to, so a target such as pivot/../created.txt resolved to a different file than a write through the link would create. The resolver now walks the target component by component, following symlinks before applying .., and fails when .. would step out of a missing directory, as the kernel does.

The previous test for a self-referencing target built the link with path.join, which folded ghost/../self to self, so it only exercised a plain loop. Link targets in these tests are now joined without normalization.

* fix(core): keep the directory requirement of a trailing separator in symlink targets

A symlink target ending in a separator or "." names a directory, so the kernel refuses to open or create a file through it. The resolver dropped those components, and native realpath does the same on macOS, so a link to victim/ resolved to the file victim and file_write overwrote it. The resolver now requires an existing directory at the end of such a target, and confirms each realpath result with stat, which takes the same lookup as open, before trusting it.
This commit is contained in:
Jack Chen authored and GitHub committed 2026-10-16 10:42:32 +08:00
1 parent 6ec5498643
commit d4084ef803
4 files changed
+308 -10

No files matched your search

+9
View File
@@ -2,6 +2,15 @@
## Unreleased
### Security
- `file_write` no longer writes outside the agent's working directory through a
dangling symlink (GHSA-h42w-g3ch-rjjm, CVE-2026-108600). The sandbox check
now resolves a dangling symlink to the target it names, so a link pointing
outside the root is rejected while one pointing inside it keeps working. The
check is shared by every built-in filesystem tool, and a symlink loop now
fails the check instead of reaching the filesystem call.
## 1.21.2 - 2026-10-09
### Changed
+4 -1
View File
@@ -39,7 +39,10 @@ Built-in filesystem tools are sandboxed to a per-agent working directory. Paths
must be absolute and resolve inside that directory; symlinks are resolved
before the check so they cannot escape the configured root. The resolved,
symlink-free path is what reaches the `fs` API, which closes the window where a
symlink inside the candidate path could be swapped after the check.
symlink inside the candidate path could be swapped after the check. A dangling
symlink is resolved to the target it names, since writing through it would
create that target, so a link pointing outside the root is rejected like any
other outside path.
A relative path is rejected outright, and a path that resolves outside the root
returns an error `ToolResult` naming the root and the configuration fields that
+94 -8
View File
@@ -1,5 +1,5 @@
import { mkdir, realpath } from 'fs/promises'
import { basename, dirname, isAbsolute, join, relative, resolve } from 'path'
import { mkdir, readlink, realpath, stat } from 'fs/promises'
import { basename, dirname, isAbsolute, join, parse, relative, resolve, sep } from 'path'
import type { ToolUseContext } from '../../types.js'
/**
@@ -84,7 +84,13 @@ export async function resolvePathWithinCwd(
// Resolve symlinks all the way through the candidate. For paths that do
// not yet exist (e.g. `file_write` creating a new file), resolve the
// longest existing prefix and re-attach the non-existent suffix.
const realCandidate = await realpathTolerant(candidate)
let realCandidate: string
try {
realCandidate = await realpathTolerant(candidate, { hops: 0 })
} catch (err) {
const message = err instanceof Error ? err.message : 'Unknown error'
return { ok: false, error: `Could not resolve path "${candidate}": ${message}` }
}
if (!isWithin(realCandidate, realRoot)) {
return outsideRoot(candidate, realRoot)
@@ -101,14 +107,94 @@ function isWithin(candidate: string, root: string): boolean {
return rel === '' || (!rel.startsWith('..') && !isAbsolute(rel))
}
async function realpathTolerant(path: string): Promise<string> {
/**
* Bound on symlinks followed by hand, matching Linux MAXSYMLINKS. Chains that
* realpath walks itself are bounded by the kernel, which reports ELOOP.
*/
const MAX_SYMLINK_HOPS = 40
const TOO_MANY_SYMLINKS = 'Too many levels of symbolic links'
async function realpathTolerant(path: string, state: { hops: number }): Promise<string> {
try {
return await realpath(path)
} catch {
const real = await realpath(path)
// Native realpath accepts a symlink whose target ends in a separator even
// when that target is a file, which the kernel refuses to open. stat takes
// the same lookup as open, so a failure here sends the path through the
// manual resolution below.
await stat(path)
return real
} catch (err) {
// The kernel already refused this chain as too long or cyclic. Following
// it by hand would accept a path that no fs call could open as given.
if ((err as NodeJS.ErrnoException).code === 'ELOOP') {
throw new Error(TOO_MANY_SYMLINKS)
}
const parent = dirname(path)
if (parent === path) return path
const realParent = await realpathTolerant(parent)
return join(realParent, basename(path))
const realParent = await realpathTolerant(parent, state)
const joined = join(realParent, basename(path))
// realpath also fails on a dangling symlink, which exists even though its
// target does not. Writing to it creates the target, so follow the link
// and resolve the target instead of treating the link as a new file.
let target: string
try {
target = await readlink(joined)
} catch {
return joined
}
if (++state.hops > MAX_SYMLINK_HOPS) {
throw new Error(TOO_MANY_SYMLINKS)
}
return resolveLinkTarget(realParent, target, state)
}
}
const TARGET_SEPARATOR = sep === '\\' ? /[\\/]/ : /\//
/**
* Resolve a symlink target the way the kernel does: one component at a time,
* starting from the directory holding the link when the target is relative.
* `path.resolve` would collapse `a/..` lexically, but when `a` is itself a
* symlink the kernel steps out of the directory `a` points to instead.
*/
async function resolveLinkTarget(
linkDir: string,
target: string,
state: { hops: number },
): Promise<string> {
const targetRoot = parse(target).root
const parts = target.slice(targetRoot.length).split(TARGET_SEPARATOR)
let current = targetRoot === '' ? linkDir : targetRoot
for (const part of parts) {
if (part === '' || part === '.') continue
if (part === '..') {
// `current` is symlink-free, so its lexical parent is its real parent,
// provided it exists. The kernel cannot step out of a missing directory.
await requireDirectory(current, target)
current = dirname(current)
continue
}
current = await realpathTolerant(join(current, part), state)
}
// A trailing separator or `.` names a directory, so the kernel refuses to
// open a file, or create one, through the link.
const last = parts[parts.length - 1]
if (last === '' || last === '.') {
await requireDirectory(current, target)
}
return current
}
async function requireDirectory(path: string, target: string): Promise<void> {
let isDirectory = false
try {
isDirectory = (await stat(path)).isDirectory()
} catch {
// Missing: rejected below.
}
if (!isDirectory) {
throw new Error(`Symlink target "${target}" requires "${path}" to be an existing directory`)
}
}
+201 -1
View File
@@ -1,6 +1,6 @@
import { describe, it, expect, beforeEach, afterEach, vi } from 'vitest'
import { access, mkdir, mkdtemp, rm, symlink, writeFile, readFile } from 'fs/promises'
import { basename, join } from 'path'
import { basename, join, sep } from 'path'
import { tmpdir } from 'os'
import { detectSymlinkSupport } from './helpers/symlink-support.js'
import { fileReadTool } from '../src/tool/built-in/file-read.js'
@@ -562,6 +562,206 @@ describe('filesystem sandbox', () => {
})
})
describe.skipIf(!symlinksSupported)('filesystem sandbox symlinks', () => {
let root: string
let outside: string
let ctx: ToolUseContext
beforeEach(async () => {
root = join(tmpDir, 'workspace')
outside = join(tmpDir, 'outside')
await mkdir(root)
await mkdir(outside)
ctx = { ...defaultContext, cwd: root }
})
async function write(path: string, content = 'escaped') {
return fileWriteTool.execute({ path, content }, ctx)
}
it('rejects file_write through a dangling symlink that points outside the root', async () => {
const target = join(outside, 'planted.txt')
await symlink(target, join(root, 'dangling.txt'))
const result = await write(join(root, 'dangling.txt'))
expect(result.isError).toBe(true)
expect(result.data).toContain("outside the agent's working directory")
expect(await fileExists(target)).toBe(false)
})
it('rejects file_write through a relative dangling symlink that points outside the root', async () => {
await symlink(join('..', 'outside', 'planted.txt'), join(root, 'dangling.txt'))
const result = await write(join(root, 'dangling.txt'))
expect(result.isError).toBe(true)
expect(result.data).toContain("outside the agent's working directory")
expect(await fileExists(join(outside, 'planted.txt'))).toBe(false)
})
it('rejects file_write through a chain of dangling symlinks that ends outside the root', async () => {
await symlink(join(root, 'second'), join(root, 'first'))
await symlink(join(outside, 'planted.txt'), join(root, 'second'))
const result = await write(join(root, 'first'))
expect(result.isError).toBe(true)
expect(result.data).toContain("outside the agent's working directory")
expect(await fileExists(join(outside, 'planted.txt'))).toBe(false)
})
it('rejects file_write into a symlinked directory outside the root', async () => {
await symlink(outside, join(root, 'linked-dir'))
const result = await write(join(root, 'linked-dir', 'new.txt'))
expect(result.isError).toBe(true)
expect(result.data).toContain("outside the agent's working directory")
expect(await fileExists(join(outside, 'new.txt'))).toBe(false)
})
it('rejects file_write when a middle path segment is a symlink outside the root', async () => {
await mkdir(join(root, 'a'))
await symlink(outside, join(root, 'a', 'b'))
const result = await write(join(root, 'a', 'b', 'c', 'new.txt'))
expect(result.isError).toBe(true)
expect(result.data).toContain("outside the agent's working directory")
expect(await fileExists(join(outside, 'c'))).toBe(false)
})
it('rejects file_write when a middle path segment is a dangling symlink outside the root', async () => {
await mkdir(join(root, 'a'))
await symlink(join(outside, 'missing-dir'), join(root, 'a', 'b'))
const result = await write(join(root, 'a', 'b', 'c', 'new.txt'))
expect(result.isError).toBe(true)
expect(result.data).toContain("outside the agent's working directory")
expect(await fileExists(join(outside, 'missing-dir'))).toBe(false)
})
it('rejects file_write through a symlink loop', async () => {
await symlink(join(root, 'loop-b'), join(root, 'loop-a'))
await symlink(join(root, 'loop-a'), join(root, 'loop-b'))
const result = await write(join(root, 'loop-a'))
expect(result.isError).toBe(true)
expect(result.data).toContain('symbolic links')
})
it('rejects file_write through a symlink chain longer than the kernel allows', async () => {
// 41 links exceeds MAXSYMLINKS on Linux (40) and macOS (32), so the
// kernel refuses the chain even though every link resolves inside the root.
await writeFile(join(root, 'end.txt'), 'keep')
let next = join(root, 'end.txt')
for (let i = 0; i < 41; i++) {
const link = join(root, `chain-${i}`)
await symlink(next, link)
next = link
}
const result = await write(next)
expect(result.isError).toBe(true)
expect(result.data).toContain('symbolic links')
expect(await readFile(join(root, 'end.txt'), 'utf8')).toBe('keep')
})
it('rejects a dangling symlink whose target steps out of a missing directory', async () => {
// The kernel cannot resolve `ghost/..` because `ghost` dangles, so the
// write must fail rather than collapse the target to `self` lexically.
await symlink(join(root, 'missing-dir'), join(root, 'ghost'))
await symlink(['ghost', '..', 'self'].join(sep), join(root, 'self'))
const result = await write(join(root, 'self'))
expect(result.isError).toBe(true)
expect(result.data).toContain('to be an existing directory')
expect(await fileExists(join(root, 'missing-dir'))).toBe(false)
})
it.each([
['a trailing separator', sep],
['a trailing .', `${sep}.`],
])('rejects a symlink target with %s that names a file', async (_label, suffix) => {
// The suffix makes the kernel require a directory, so it refuses to open
// `victim.txt` through the link.
await writeFile(join(root, 'victim.txt'), 'keep')
await symlink(`victim.txt${suffix}`, join(root, 'link'))
const result = await write(join(root, 'link'))
expect(result.isError).toBe(true)
expect(result.data).toContain('to be an existing directory')
expect(await readFile(join(root, 'victim.txt'), 'utf8')).toBe('keep')
})
it('follows a symlink before applying .. in a dangling symlink target', async () => {
// `pivot/..` is `nested`, not the root, because `pivot` points into it.
await mkdir(join(root, 'nested', 'deep'), { recursive: true })
await symlink(join('nested', 'deep'), join(root, 'pivot'))
await symlink(['pivot', '..', 'created.txt'].join(sep), join(root, 'link'))
await writeFile(join(root, 'created.txt'), 'unrelated')
const result = await write(join(root, 'link'), 'through link')
expect(result.isError).toBe(false)
expect(await readFile(join(root, 'nested', 'created.txt'), 'utf8')).toBe('through link')
expect(await readFile(join(root, 'created.txt'), 'utf8')).toBe('unrelated')
})
it('writes through a symlink to a file inside the root', async () => {
await writeFile(join(root, 'real.txt'), 'old')
await symlink(join(root, 'real.txt'), join(root, 'alias.txt'))
const result = await write(join(root, 'alias.txt'), 'new')
expect(result.isError).toBe(false)
expect(result.data).toContain('Updated')
expect(await readFile(join(root, 'real.txt'), 'utf8')).toBe('new')
})
it('writes through a symlinked directory inside the root', async () => {
await mkdir(join(root, 'real-dir'))
await symlink(join(root, 'real-dir'), join(root, 'dir-alias'))
const result = await write(join(root, 'dir-alias', 'nested', 'new.txt'), 'inside')
expect(result.isError).toBe(false)
expect(await readFile(join(root, 'real-dir', 'nested', 'new.txt'), 'utf8')).toBe('inside')
})
it('writes through a dangling symlink whose target is inside the root', async () => {
await mkdir(join(root, 'sub'))
// Relative targets resolve against the directory holding the link.
await symlink(join('..', 'later.txt'), join(root, 'sub', 'pending.txt'))
const result = await write(join(root, 'sub', 'pending.txt'), 'created')
expect(result.isError).toBe(false)
expect(result.data).toContain('Created')
expect(await readFile(join(root, 'later.txt'), 'utf8')).toBe('created')
})
it('rejects file_edit through a symlink to a file outside the root', async () => {
await writeFile(join(outside, 'secret.txt'), 'keep')
await symlink(join(outside, 'secret.txt'), join(root, 'linked.txt'))
const result = await fileEditTool.execute(
{ path: join(root, 'linked.txt'), old_string: 'keep', new_string: 'changed' },
ctx,
)
expect(result.isError).toBe(true)
expect(result.data).toContain("outside the agent's working directory")
expect(await readFile(join(outside, 'secret.txt'), 'utf8')).toBe('keep')
})
})
// ===========================================================================
// glob
// ===========================================================================