← Back to PRs

#16929: fix(security): block access to sensitive directories from within sandbox

by CornBrother0x open 2026-02-15 07:53 View on GitHub →
docker agents size: M
## Summary - **Problem:** When `sandboxRoot` is the user's home directory (default), `~/.openclaw/` containing API keys and credentials is inside the sandbox and readable by the agent - **Why it matters:** A prompt-injected LLM can read `~/.openclaw/openclaw.json` and exfiltrate API keys, bot tokens, and auth credentials - **What changed:** Added `isSensitivePath()` to `resolveSandboxPath()` + post-symlink realpath check in `assertSandboxPath()` to block access to known sensitive directories - **What did NOT change:** Normal sandbox path resolution, escape detection, and symlink traversal checks are untouched Closes #8588 Related: #12958 (competing PR with two Greptile-identified bypass vectors — this PR addresses both) ## Why this approach vs PR #12958 PR #12958 implemented the check at the read tool level, but had two bypass vectors: 1. **Parameter aliasing:** `file_path` vs `path` normalization let the check see `""` while the read proceeded 2. **Unicode normalization mismatch:** Separate `expandTilde()` vs `expandPath()` normalizers created a reliable bypass This PR checks at the `sandbox-paths` level using the **same `expandPath()` normalizer** already used by the sandbox resolver. The async `assertSandboxPath()` also re-checks after `realpath()` to catch symlink bypass (e.g., `~/workspace/link -> ~/.openclaw/`). ## Changes **`src/agents/sandbox-paths.ts`**: - Added `isSensitivePath()` — checks resolved path against known sensitive dirs (`~/.openclaw/`, `~/.ssh/`, `~/.gnupg/`, `~/.aws/`, `~/.config/gcloud/`, legacy state dirs) - Integrated sync check in `resolveSandboxPath()` + async post-symlink check in `assertSandboxPath()` - `skipSensitiveCheck` escape hatch for elevated/internal operations **`src/agents/sandbox-paths.sensitive.test.ts`** (new): - 17 tests: direct paths, absolute paths, tilde expansion, symlink bypass, safe paths allowed, escape hatch ## Security Impact - Data access scope changed? Yes — restricts agent access to sensitive directories within sandbox - Only adds restrictions. State dir resolved via `resolveStateDir()` so `$OPENCLAW_STATE_DIR` overrides are respected. ## Testing All 17 tests pass locally. Symlink bypass test creates real symlinks to verify the post-realpath check works. ## Compatibility Backward compatible — only adds restrictions. `skipSensitiveCheck: true` available for operations that legitimately need access. ## Failure Recovery Revert commit or use `skipSensitiveCheck: true` for specific operations. Changes isolated to `sandbox-paths.ts`. ## AI Disclosure AI-assisted (Claude via OpenClaw). All code reviewed, understood, and tested. <!-- greptile_comment --> <h3>Greptile Summary</h3> This PR adds sensitive directory protection to the sandbox path resolver, preventing agent access to directories containing credentials and secrets (`~/.openclaw/`, `~/.ssh/`, `~/.gnupg/`, `~/.aws/`, `~/.config/gcloud/`, legacy state dirs) even when those directories fall inside the sandbox root. - Adds `isSensitivePath()` with both a sync check in `resolveSandboxPath()` and an async post-symlink realpath check in `assertSandboxPath()`, correctly closing both direct-access and symlink-bypass vectors - Uses the same `expandPath()` normalizer as the sandbox resolver, avoiding the normalization mismatch bypass identified in the competing PR #12958 - `skipSensitiveCheck` escape hatch is defined but currently unused by any callers — may want to remove it until needed to reduce API surface - The symlink bypass test (`"blocks symlinks that resolve to sensitive directories"`) silently skips when `~/.openclaw` doesn't exist (typical in CI), meaning the most security-critical test likely doesn't run in automated testing - Minor: `resolveSensitivePaths()` re-resolves (including `fs.existsSync`) on every path check; could be memoized for hot-path performance <h3>Confidence Score: 3/5</h3> - The security logic is sound and additive-only, but the most critical test (symlink bypass) likely doesn't run in CI. - The core implementation is well-designed — the dual-layer check (sync string check + async post-realpath check) correctly addresses both direct and symlink bypass vectors. However, the symlink bypass test silently skips in CI environments where ~/.openclaw doesn't exist, leaving the most security-critical behavior unverified in automated testing. The code is additive-only and backward compatible, which limits blast radius. - Pay close attention to `src/agents/sandbox-paths.sensitive.test.ts` — the symlink bypass test needs to be made CI-deterministic to ensure the security guarantee is actually verified. <sub>Last reviewed commit: dd0aee6</sub> <!-- greptile_other_comments_section --> <!-- /greptile_comment -->

Most Similar PRs