fix(sandbox): allow intra-sandbox symlinks in safeTarExtract audit (#2268)

[cjagwani]

Summary

Closes #2268 — P1 regression in v0.0.22.

`safeTarExtract`'s post-extraction symlink audit (added in #2163 to block tar path-traversal attacks) flagged every symlink whose target resolved outside the extraction temp directory. The sandbox base image legitimately ships intra-sandbox symlinks like `/sandbox/.openclaw → /sandbox/.openclaw-data` — those resolve outside the host temp dir but inside the canonical `/sandbox` root, where they'll be correctly resolved when the backup is restored into a fresh sandbox.

Result: the audit nuked every backup, breaking `nemoclaw rebuild` and `nemoclaw snapshot create` on v0.0.22. The only workaround was `destroy + onboard` (loses all state).

Fix

• `auditExtractedSymlinks(dirPath, allowedRoots: string[])` — now takes an array of allowed roots instead of a single one
• `safeTarExtract` passes `[targetDir, "/sandbox"]` so intra-sandbox absolute symlinks are honored while genuine escape attempts (e.g. symlink to `/etc/passwd` or `../../.ssh/authorized_keys`) still abort the extraction

The security guardrail is intact: `/sandbox` is a subtree root, so crafted symlinks with absolute targets outside `/sandbox` (e.g. `/etc/passwd`) are still rejected.

Test plan

• New test: `allows symlinks whose target resolves within /sandbox (intra-sandbox layout)` — locks the regression fix
• New test: `blocks symlinks that escape /sandbox even with an absolute target` — confirms `/etc/passwd` symlinks still rejected
• Existing `blocks symlink escaping target directory` (relative `../../`) still passes
• All 21 sandbox-tar security tests pass
• Full CLI suite: 1783 passed

🤖 Generated with Claude Code

Summary by CodeRabbit

• Security & Bug Fixes
  • Improved symlink validation during tar archive extraction to correctly allow absolute symlink targets that resolve within the sandbox environment.
  • Enhanced security checks now reject symlinks with targets outside permitted directories.

[coderabbitai]

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info

⚙️ Run configuration

Configuration used: Path: .coderabbit.yaml

Review profile: CHILL

Plan: Pro Plus

Run ID: 02ce4a80-c2c2-4e56-9ac3-b6bf74549f0c

📥 Commits

Reviewing files that changed from the base of the PR and between 752bfb3 and e54d96c.

📒 Files selected for processing (2)

• src/lib/sandbox-state.ts
• test/security-sandbox-tar-traversal.test.ts

---

📝 Walkthrough

Walkthrough

The symlink validation logic in sandbox extraction is enhanced to accept multiple allowed root paths instead of a single root. The safeTarExtract function now permits symlink targets that resolve to either the target extraction directory or the /sandbox root, rather than exclusively the target directory.

Changes

Cohort / File(s)	Summary
Symlink Validation Enhancement src/lib/sandbox-state.ts	Modified auditExtractedSymlinks to accept allowedRoots array; validation now passes when resolved symlink targets exist within any of the allowed roots. Updated Phase 3 post-extraction audit in safeTarExtract to permit targets in both targetDir and /sandbox root.
Symlink Validation Tests test/security-sandbox-tar-traversal.test.ts	Added two test cases: one verifying absolute symlinks resolving within /sandbox are accepted, another confirming absolute symlinks resolving outside /sandbox (e.g., /etc/passwd) are rejected with symlink-related error message.

Estimated code review effort

🎯 2 (Simple) | ⏱️ ~15 minutes

Poem

🐰 Symlinks dance through /sandbox paths,
No longer bound by single roots,
Multiple homes now welcome them,
While dangerous escapes are barred,
Security and flexibility bloom! 🌿

🚥 Pre-merge checks | ✅ 5

✅ Passed checks (5 passed)

Check name	Status	Explanation
Description Check	✅ Passed	Check skipped - CodeRabbit’s high-level summary is enabled.
Title check	✅ Passed	The pull request title accurately summarizes the main change: allowing intra-sandbox symlinks in the safeTarExtract audit by modifying symlink validation logic.
Docstring Coverage	✅ Passed	Docstring coverage is 100.00% which is sufficient. The required threshold is 80.00%.
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.

_{✏️ Tip: You can configure your own custom pre-merge checks in the settings.}

✨ Finishing Touches

📝 Generate docstrings

• Create stacked PR
• Commit on current branch

🧪 Generate unit tests (beta)

• Create PR with unit tests
• Commit unit tests in branch fix/2268-safeTarExtract-sandbox-symlinks

---

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

[ericksoa]

LGTM. The allowedRoots array approach is the right fix — it preserves the security guardrail for genuine escape attempts while correctly allowing intra-sandbox absolute symlinks that are legitimate in the restored sandbox filesystem. The two new tests properly cover both the regression (intra-sandbox symlinks pass) and the security invariant (escape to /etc/passwd still blocked). All 20 CI checks green.