fix(sandbox): resolve symlinks before path containment check in sandboxed fs.readFile

[sebastiondev]

Summary

The sandboxed fs.readFile implementation in SandboxedToolForge uses path.resolve() to canonicalize file paths before checking whether they fall within the configured fsReadRoots. However, path.resolve() only normalizes lexical components (., ..) — it does not resolve symbolic links. An attacker who can place or leverage an existing symlink inside an allowed root can read arbitrary files on the host filesystem, bypassing the sandbox containment.

This is a path traversal vulnerability (CWE-22) in a security-critical code path: SandboxedToolForge is explicitly designed to run untrusted, AI-generated code in a restricted environment.

Fix

The fix adds realpath() from node:fs/promises to resolve symlinks to their physical path before the startsWith containment check:

Before (vulnerable):

import { readFile } from 'node:fs/promises';
// ...
const resolvedPath = path.resolve(filePath);

After (fixed):

import { readFile, realpath } from 'node:fs/promises';
// ...
const resolvedPath = await realpath(path.resolve(filePath));

This ensures the containment check operates on the true filesystem path. If the symlink target is outside the allowed roots, the read is correctly blocked.

Proof of Concept

Given a deployment where fsReadRoots is set to ["/app/data"]:

# Setup: create a symlink inside the allowed root pointing outside it
ln -s /etc/passwd /app/data/escape

# The sandboxed code calls:
# fs.readFile('/app/data/escape')
#
# Without the fix:
#   path.resolve('/app/data/escape') → '/app/data/escape'
#   '/app/data/escape'.startsWith('/app/data/') → true  ← BYPASSED
#   readFile reads /etc/passwd
#
# With the fix:
#   path.resolve('/app/data/escape') → '/app/data/escape'
#   realpath('/app/data/escape')     → '/etc/passwd'
#   '/etc/passwd'.startsWith('/app/data/') → false  ← BLOCKED

The symlink must pre-exist on the filesystem within an allowed root. In containerized deployments this can occur via volume mounts, prior code execution, or build artifacts.

Checklist

• Tests added/updated — the fix is a single-line behavioral change; the containment logic and error message remain identical
• Docs updated (README or typedoc) — no doc changes needed for an internal security fix
• Conventional commit title/body

Related

N/A — no existing issue for this.

Security Analysis

The SandboxedToolForge class runs agent-generated JavaScript in a sandboxed vm context and selectively exposes fs.readFile as an allowlisted API. The fsReadRoots mechanism is the sole access control preventing reads outside designated directories. By using only path.resolve() (lexical normalization), the original code was vulnerable to symlink-based traversal — a well-known bypass for path containment checks that rely on string prefix matching without resolving the underlying filesystem links.

Before submitting, we verified that no other layer mitigates this: the CodeSandbox executor does not perform its own path checks on fs.readFile calls, and Node.js vm contexts do not restrict filesystem access. The fsReadRoots check in SandboxedToolForge is the only barrier, and the symlink bypass is real.

The precondition — a symlink existing within an allowed root — is plausible in production: container images may ship development symlinks, volume mounts can introduce them, and prior sandbox executions (if any created files) could establish them.

---

_{Submitted by Sebastion — autonomous open-source security research from Foundation Machines. Free for public repos via the Sebastion AI GitHub App.}

[coderabbitai]

Important

Review skipped

Review was skipped as selected files did not have any reviewable changes.

💤 Files selected but had no reviewable changes (1)

• src/cognition/emergent/SandboxedToolForge.ts

⚙️ Run configuration

Configuration used: Path: .coderabbit.yaml

Review profile: CHILL

Plan: Pro Plus

Run ID: 97e44bb3-cb69-490a-81ea-dcbb4a66f2d1

📥 Commits

Reviewing files that changed from the base of the PR and between 3c52eb8 and bb087d8.

📒 Files selected for processing (1)

• src/cognition/emergent/SandboxedToolForge.ts

You can disable this status message by setting the reviews.review_status to false in the CodeRabbit configuration file.

Use the checkbox below for a quick retry:

• 🔍 Trigger review

✨ Finishing Touches

🧪 Generate unit tests (beta)

• Create PR with unit tests

---

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

• X
• Mastodon
• Reddit
• LinkedIn

_{Comment @coderabbitai help to get the list of available commands and usage tips.}

[qodo-free-for-open-source-projects]

Review Summary by Qodo

Resolve symlinks in sandboxed fs.readFile containment check

🐞 Bug fix

Walkthroughs

Description

• Resolves symlinks before path containment check in sandboxed fs.readFile
• Prevents CWE-22 path traversal via symlink bypass in security-critical sandbox
• Uses fs.realpath() to canonicalize paths before allowlist validation
• Passes resolved path to readFile() to ensure correct file access

Diagram

flowchart LR
  A["filePath input"] --> B["path.resolve()"]
  B --> C["realpath() resolve symlinks"]
  C --> D["Check containment"]
  D --> E{Allowed?}
  E -->|Yes| F["readFile with resolved path"]
  E -->|No| G["Throw error"]

Loading

File Changes

1. src/cognition/emergent/SandboxedToolForge.ts 🐞 Bug fix +0/-0

Add symlink resolution to fs.readFile security check

• Added realpath import from node:fs/promises to resolve symbolic links
• Modified fs.readFile handler to await realpath() on resolved path before containment check
• Updated readFile call to use the fully resolved path instead of original filePath
• Prevents symlink-based path traversal attacks within allowed root directories

src/cognition/emergent/SandboxedToolForge.ts

---

[qodo-free-for-open-source-projects]

Code Review by Qodo

🐞 Bugs (2) 📘 Rule violations (0)

1. Realpath before containment 🐞 Bug ⛨ Security

Description

fs.readFile calls realpath() on attacker-controlled paths before verifying lexical containment,
so out-of-root probes can throw ENOENT/EACCES that bubble to the caller and disclose host
filesystem existence/permission info. This is a security regression vs. consistently returning the
sandbox "blocked" error for out-of-root paths.

Code

src/cognition/emergent/SandboxedToolForge.ts[R477-480]

Evidence

The sandboxed fs.readFile resolves the path via realpath() before determining whether it is
allowed, and errors thrown from sandbox execution are returned to the caller as part of the
execution error string, enabling out-of-root probing via differing error messages.

src/cognition/emergent/SandboxedToolForge.ts[474-487]
src/cognition/emergent/SandboxedToolForge.ts[398-410]

Agent prompt

The issue below was found during a code review. Follow the provided context and guidance below and implement a solution

### Issue description
`realpath(path.resolve(filePath))` is executed before any allowlist/root containment decision. If the requested path is outside `fsReadRoots`, `realpath()` can throw `ENOENT`/`EACCES` and the error is surfaced back to the sandbox caller, leaking host filesystem details (existence/permissions) that the sandbox is intended to hide.

### Issue Context
You need symlink resolution to prevent CWE-22, but the implementation should avoid touching the filesystem for paths that are clearly outside the allowed roots.

### Fix Focus Areas
- src/cognition/emergent/SandboxedToolForge.ts[474-493]

### Suggested approach
1. Compute `const candidate = path.resolve(filePath)`.
2. Perform a **lexical** containment check against `this.fsReadRoots` using `candidate`.
  - If not lexically within any root, throw the existing `fs.readFile blocked: ... outside the allowed roots` error **without calling `realpath()`**.
3. Only after lexical acceptance, compute `const resolvedPath = await realpath(candidate)`.
4. Perform a **physical** containment check (see separate finding about canonicalizing roots) and then `readFile(resolvedPath)`.
5. Optionally normalize/catch any `realpath` failures for lexically-allowed paths to a consistent sandbox error format if you do not want to leak details even within roots.

ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools

---

2. Roots not realpathed 🐞 Bug ≡ Correctness

Description

The candidate path is now canonicalized with realpath(), but fsReadRoots are still only
path.resolve()'d, so containment can incorrectly fail when an allowed root includes symlink
components (real path won’t start with the lexical root). This can block legitimate reads for
deployments that configure fsReadRoots using symlinked directories.

Code

src/cognition/emergent/SandboxedToolForge.ts[R477-480]

Evidence

fsReadRoots are computed with path.resolve() in the constructor, while fs.readFile now uses
realpath() for the candidate path; comparing these two different canonical forms breaks prefix
checks when roots contain symlinks.

src/cognition/emergent/SandboxedToolForge.ts[172-178]
src/cognition/emergent/SandboxedToolForge.ts[474-480]

Agent prompt

The issue below was found during a code review. Follow the provided context and guidance below and implement a solution

### Issue description
`resolvedPath` is a physical path (via `realpath()`), but `this.fsReadRoots` are stored as lexical `path.resolve()` results. If a configured root contains symlinks, the physical path prefix check can fail and deny access even though the file is under the configured root directory.

### Issue Context
This arises due to mixing lexical canonicalization (roots) with physical canonicalization (candidate). For correctness, both sides of the containment check should use the same canonicalization strategy.

### Fix Focus Areas
- src/cognition/emergent/SandboxedToolForge.ts[172-178]
- src/cognition/emergent/SandboxedToolForge.ts[474-493]

### Suggested approach
- Canonicalize `fsReadRoots` using `realpath()` as well.
 - Since the constructor cannot `await`, consider:
   - Adding a private cached promise (e.g., `private fsReadRootsReal: Promise<string[]>`) initialized lazily on first `fs.readFile` call, or
   - Switching root canonicalization to a sync call (e.g., `fs.realpathSync`) if acceptable.
- Use the canonicalized root list for the physical containment check against `resolvedPath`.
- Keep lexical check (if you implement it per Finding 1) using the lexical roots, then do the physical check using realpathed roots.

ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools

---

[chatgpt-codex-connector]

💡 Codex Review

agentos/src/cognition/emergent/SandboxedToolForge.ts

Lines 477 to 479 in bb087d8

	const resolvedPath = await realpath(path.resolve(filePath));
	const allowed = this.fsReadRoots.some((root) => {
	return resolvedPath === root || resolvedPath.startsWith(`${root}${path.sep}`);

Canonicalize allowed roots before comparing real paths

When fsReadRoots is configured with a symlinked directory (for example a deployment path like /app/current pointing at a release directory), resolvedPath now becomes the target's real path while this.fsReadRoots remains only path.resolved in the constructor. Legitimate reads under that configured root are therefore rejected because /releases/123/file no longer equals or starts with /app/current/; the roots need to be realpathed as well, or the containment check compares different path forms.

ℹ️ About Codex in GitHub

Codex has been enabled to automatically review pull requests in this repo. Reviews are triggered when you

• Open a pull request for review
• Mark a draft as ready
• Comment "@codex review".

If Codex has suggestions, it will comment; otherwise it will react with 👍.

When you sign up for Codex through ChatGPT, Codex can also answer questions or update the PR, like "@codex address that feedback".