langchain-ai / langchain-ai/deepagentsjs

Filesystem permission layer matches globs against a non-canonical path: an intermediate symlinked directory bypasses `allow`/`deny` rules (directory traversal)

Open
#633 0 comments 0 reactions 0 assignees View on GitHub

Nobody has claimed this yet.

Dominant language
TypeScript
Stars
1.6k
Forks
272
Avg merge
1d 13h
Merged PRs (30d)
38

Description

`createFilesystemMiddleware`'s permission check matches allow/deny globs against a **lexically
validated** path string, not a filesystem-canonical one. It rejects literal `..`/`~` and guards a
**final-component** symlink at I/O time (`O_NOFOLLOW`), but it never resolves **intermediate**
symlinked directories before the glob decision. So a symlinked directory inside the allowed root
whose target is outside it lets a tool read or write a file outside the sandbox, while the path
still matches an `allow /**` rule.

Notably, the library already ships realpath-based containment (`isSafePath`, used by the Agent
Skills loader, whose own docstring says it "prevents directory traversal attacks via symlinks") —
it is simply not wired into the fs-tool permission path. So two filesystem surfaces in the same
package disagree on what "contained" means.

## Affected code (1.10.5)

`libs/deepagents/src/permissions/enforce.ts`:

```ts
// Docstring claims to "Canonicalize and validate", but it is purely lexical:
export function validatePath(raw: string): string {
if (typeof raw !== "string" || raw.length === 0) throw new Error("path must be a non-empty string");
if (!raw.startsWith("/")) throw new Error(`path must be absolute: ${JSON.stringify(raw)}`);
const segments = raw.split("/").filter((s) => s.length > 0);
if (segments.includes("..")) throw new Error(`path must not contain "..": ${JSON.stringify(raw)}`);
if (segments.includes("~")) throw new Error(`path must not contain "~": ${JSON.stringify(raw)}`);
return `/${segments.join("/")}`; // no fs.realpath / lstat
}

export function decidePathAccess(rules, operation, path): PermissionMode {
for (const rule of rules) {
if (!rule.operations.includes(operation)) continue;
if (rule.paths.some((pattern) => globMatch(path, pattern))) return rule.mode ?? "allow";
}
return "allow"; // first-match-wins, permissive default
}

export function enforcePermission(rules, operation, path): void {
if (rules.length === 0) return;
const canonical = validatePath(path); // "canonical" is lexical only
if (decidePathAccess(rules, operation, canonical) === "deny") throw new Error(...);
}
```

`middleware/fs.ts` passes the raw model-supplied path straight in, e.g. `read_file`:
`enforcePermission(permissions, "read", input.file_path)`.

The realpath helper that exists but is not used here, `libs/deepagents/src/skills/...`:

```ts
/* This prevents directory traversal attacks via symlinks or path manipulation.
...resolves both paths to their canonical form (following symlinks)... */
function isSafePath(targetPath: string, baseDir: string): boolean {
const resolvedPath = fs.realpathSync(targetPath);
const resolvedBase = fs.realpathSync(baseDir);
return resolvedPath.startsWith(resolvedBase + path.sep) || resolvedPath === resolvedBase;
}
```

## Reproduction

Minimal, adapted from a working integration test. Non-virtual backend, rules = `allow /**`,
`deny /**`, an intermediate symlinked directory planted inside cwd.

```ts
import { createFilesystemMiddleware, FilesystemBackend } from "deepagents";
import { mkdtempSync, mkdirSync, writeFileSync, symlinkSync } from "node:fs";
import { tmpdir } from "node:os";
import path from "node:path";

const root = mkdtempSync(path.join(tmpdir(), "da-symlink-"));
const cwd = path.join(root, "cwd");
const outside = path.join(root, "outside");
mkdirSync(cwd);
mkdirSync(outside);
writeFileSync(path.join(outside, "secret.txt"), "SECRET-OUTSIDE");

const backend = new FilesystemBackend({ rootDir: cwd, virtualMode: false });
const permissions = [
{ operations: ["read", "write"], paths: [`${cwd}/**`], mode: "allow" },
{ operations: ["read", "write"], paths: ["/**"], mode: "deny" },
];
const mw = createFilesystemMiddleware({ backend, permissions });
const tools = Object.fromEntries((mw as any).tools.map((t: any) => [t.name, t]));

// Plant an intermediate symlinked DIRECTORY inside cwd that targets outside cwd:
symlinkSync(outside, path.join(cwd, "linkdir")); // cwd/linkdir -> /outside

// The lexical path `cwd/linkdir/secret.txt` matches `allow cwd/**`; realpath is never resolved.
const out = await tools.read_file.invoke({ file_path: path.join(cwd, "linkdir", "secret.txt") });
console.log(JSON.stringify(out)); // contains "SECRET-OUTSIDE"
```

**Expected:** `read_file` is denied (the real target `/.../outside/secret.txt` is outside every
`allow` root and under `deny /**`).
**Actual:** the read succeeds and returns the outside file's contents. The same applies to
`write_file` (write outside the sandbox through a symlinked dir).

For contrast, two cases that *are* caught today, which is why this gap is easy to miss:
- a raw `..` segment (`cwd/../outside/secret.txt`) is rejected by `validatePath`;
- a **final-component** symlink (`cwd/linkfile -> /outside/secret.txt`) is refused at I/O by
`O_NOFOLLOW`. The intermediate-directory case slips between these two guards.

## Why it matters

Consumers configure `allow /**` + `deny /**` precisely to confine an agent to a working
directory, and present that confinement as a safety property. An intermediate symlink (planted by
a cloned repo, a dependency, or a prior tool action) plus a confused or adversarially steered model
is enough to read or write outside it. Severity is moderate (it needs a pre-existing in-cwd
symlink, not a remote drive-by), but it silently violates the stated boundary, and the fix is small
and already present elsewhere in the codebase.

## Also note

- The same lexical-only resolution exists in `virtualMode` (`resolvePath` is `path.resolve` + a
`..` check, no realpath), so this is not specific to non-virtual backends.
- `validatePath`'s docstring says it "Canonicalize[s]", which overstates what it does; the comment
and the `canonical` variable name in `enforcePermission` invite exactly this false sense of
safety.

## Proposed fix

Resolve the real path before the permission decision, reusing the existing `isSafePath`-style
logic so the two surfaces agree:

1. In `enforcePermission` (and the `filterByPermissions` post-filter), before `decidePathAccess`,
canonicalize the target with `fs.realpath` — for writes/creates that do not yet exist, realpath
the **parent directory** and re-append the final segment. Match globs against that resolved path.
2. Deny when the resolved path escapes every `allow` root (equivalently, reuse `isSafePath`
against the allowed roots).

Happy to send a PR if the direction is agreeable.

Contributor guide

Open the contributing guide

First steps

  1. Read the whole issue, then the project's contributing guide.
  2. Comment on the issue to say you are picking it up — it saves two people doing the same work.
  3. Fork the repository and make your change on a branch.
  4. Open a pull request that references the issue number.

Research direction

Start in libs/deepagents/src/permissions/enforce.ts and trace how middleware/fs.ts passes paths into enforcePermission and filterByPermissions. Compare that flow with the realpath-based isSafePath helper under libs/deepagents/src/skills, then reproduce the intermediate-directory symlink case. Done means reads and writes through symlinked directories are denied when their resolved targets escape the allowed root, including paths for new files.

Written by the indexing model from the issue text.

Assessment

Tech stack
typescript
Domain
security
Issue type
Bug
Difficulty
4/5
Estimated time
3-5 days
Activity status
Quiet
Clarity
Clearly specified
Newbie friendliness
72/100

Get new issues in your inbox

A short digest of beginner-friendly GitHub issues.