Fix path sanitization through symlinked roots

[yujunz]

What this PR does / why we need it:

SanitizePath resolves the sandbox root before comparing full absolute user paths against it. When the root is accessed through a symlink alias, such as macOS /var mapping to /private/var, that comparison misses and duplicates the absolute path beneath the root.

This change preserves the cleaned lexical root for prefix recognition while continuing to use the resolved root for final path confinement. It also adds a portable regression test using a symlinked sandbox root.

Which issue(s) this PR is related to:

None.

Release Note

Fix path sanitization when sandbox roots are accessed through symlink aliases.

Summary by CodeRabbit

• Bug Fixes
  • Improved handling of absolute paths when the sandbox root is accessed through a symlink.
  • Paths that remain within the aliased sandbox root are now resolved correctly without being rejected as escapes.

[netlify]

✅ Deploy Preview for agent-sandbox canceled.

Name	Link
🔨 Latest commit	092b059
🔍 Latest deploy log	https://app.netlify.com/projects/agent-sandbox/deploys/6aa0c5f5dca6c000089fa7e9

[coderabbitai]

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info

⚙️ Run configuration

Configuration used: Path: .coderabbit.yaml

Review profile: CHILL

Plan: Advanced

Run ID: c00fdc31-3c41-4e8e-a155-e53ab99da1c1

📥 Commits

Reviewing files that changed from the base of the PR and between e87bc38 and 092b059.

📒 Files selected for processing (2)

• packages/sandboxd/pkg/pathutil/sandbox.go
• packages/sandboxd/pkg/pathutil/sandbox_test.go

Included review availability: Your plan provides up to 8 included reviews per hour; 7 remain after this review.

---

📝 Walkthrough

Walkthrough

SanitizePath now handles absolute paths passed through symlinked sandbox roots. A regression test verifies that the path resolves to the real root without escaping the sandbox.

Changes

Sandbox path sanitization

Layer / File(s)	Summary
Root alias matching and regression coverage packages/sandboxd/pkg/pathutil/sandbox.go, packages/sandboxd/pkg/pathutil/sandbox_test.go	SanitizePath retains the lexical root and accepts absolute paths under either the lexical or resolved root. The test verifies confinement through a root symlink.

Priority: ⬇️ Low

Estimated code review effort: 2 (Simple) | ~10 minutes

Merge Risk: ⚪ Minimal · up to 092b0

Sandbox paths accessed through symlink aliases now normalize to the resolved sandbox root without duplicating path segments, while confinement remains enforced. The focused regression coverage indicates no remaining merge-blocking risk.

🚥 Pre-merge checks | ✅ 4 | ❌ 1

❌ Failed checks (1 warning)

Check name	Status	Explanation	Resolution
Docstring Coverage	⚠️ Warning	Docstring coverage is 25.00% which is insufficient. The required threshold is 80.00%. Docstring coverage is scoped to functions touched by this diff. Analyzed 4 functions across 2 files.	Write docstrings for the functions missing them to satisfy the coverage threshold.

✅ Passed checks (4 passed)

Check name	Status	Explanation
Title check	✅ Passed	The title clearly and concisely describes the main change: fixing path sanitization for symlinked sandbox roots.
Description check	✅ Passed	The description includes the required sections, explains the problem and solution, identifies related issues as none, and provides a release note.
Linked Issues check	✅ Passed	Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check	✅ Passed	Check skipped because no linked issues were found for this pull request.

• Fix all pre-merge checks with AI

✨ Finishing Touches

🧪 Generate unit tests (beta)

• Create PR with unit tests

---

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

[Copilot]

🟢 Approval recommended

The change is narrowly scoped, preserves the security invariant (resolved path must remain under the resolved root), and adds a targeted regression test for the reported symlink-alias case.

Pull request overview

This PR fixes an edge case in packages/sandboxd/pkg/pathutil where SanitizePath could mishandle absolute user paths when the sandbox root is accessed via a symlinked/aliased path (e.g., lexical vs resolved root paths). It preserves the lexical root for absolute-prefix stripping while continuing to enforce confinement against the symlink-resolved root.

Changes:

• Track both the lexical cleaned root and the symlink-resolved root, and strip either prefix from absolute user paths before joining.
• Add a regression test that uses a symlinked sandbox root to ensure absolute paths under the alias are confined correctly.

File summaries

File	Description
packages/sandboxd/pkg/pathutil/sandbox.go	Preserve lexical root for prefix recognition while still confining using the resolved root.
packages/sandboxd/pkg/pathutil/sandbox_test.go	Add regression coverage for absolute paths under a symlinked root alias.

Review details

• Files reviewed: 2/2 changed files
• Comments generated: 0
• Review effort level: Lite

---

💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.

[aditya-shantanu]

Verified the confinement invariant is untouched: the lexical-root trim only changes prefix recognition, and the final EvalSymlinks + resolved-root boundary check still gates every result. Regression test is portable and targeted.

/lgtm

[janetkuo]

Can be address in a follow up

[janetkuo]

Consider adding a case where userPath is rootAlias itself, as well as passing the resolved root while rootDir is rootAlias.

[kubernetes-prow]

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: janetkuo, yujunz

The full list of commands accepted by this bot can be found here.

The pull request process is described here

Details

Needs approval from an approver in each of these files:

• packages/sandboxd/OWNERS [janetkuo]

Approvers can indicate their approval by writing /approve in a comment
Approvers can cancel approval by writing /approve cancel in a comment