← Back to PRs

#8517: Browser: sandbox download/trace paths

by coygeek open 2026-02-04 03:25 View on GitHub →
stale
## Fix Summary The browser control API accepts an arbitrary filesystem path for `/download`, `/wait/download`, and `/trace/stop` without any path validation, allowing an attacker who can reach the control server (e.g., via DNS rebinding) to write files anywhere on the host. Fixes #8516 ## Issue Linkage Fixes #8516 ## Security Snapshot - CVSS v3.1: 9.6 (Critical) - CVSS v4.0: 8.5 (High) ## Implementation Details ### Files Changed - `src/browser/pw-tools-core.downloads.ts` (+33/-9) - `src/browser/pw-tools-core.waits-next-download-saves-it.test.ts` (+65/-4) - `src/browser/routes/agent.debug.ts` (+18/-5) - `src/browser/server-context.ts` (+3/-0) ### Technical Analysis The browser control API accepts an arbitrary filesystem path for `/download`, `/wait/download`, and `/trace/stop` without any path validation, allowing an attacker who can reach the control server (e.g., via DNS rebinding) to write files anywhere on the host. ## Validation Evidence - Command: `/download` - Status: passed ## Risk and Compatibility non-breaking; compatibility impact was not explicitly documented in the original PR body. ## AI-Assisted Disclosure AI-assisted: Codex CLI This fix was generated with AI assistance (Codex CLI). <details> <summary>Prompt and Log Snippets (truncated)</summary> _No prompt captured._ _No generation logs captured._ </details> <!-- greptile_comment --> <h2>Greptile Overview</h2> <h3>Greptile Summary</h3> Closes a critical sandbox escape in the browser control API by validating user-provided output paths for downloads (`/download`, `/wait/download`) and trace exports (`/trace/stop`). Implementation-wise, the PR introduces fixed roots under `/tmp/openclaw` and uses `assertSandboxPath(...)` to ensure resolved paths stay within those roots (including rejecting symlink traversal). Tests were updated/added to assert that out-of-root paths are rejected, and route responses now return the validated (already-resolved) paths. <h3>Confidence Score: 4/5</h3> - This PR is generally safe to merge and meaningfully improves security, with a couple small correctness/consistency issues to address. - Core mitigation (rooted path validation via `assertSandboxPath`) is sound and tests cover out-of-root rejection. Remaining issues are localized: a filename sanitization regex that unintentionally collapses most filenames, and minor inconsistency in error normalization for trace-path errors. - src/browser/pw-tools-core.downloads.ts, src/browser/server-context.ts <!-- greptile_other_comments_section --> **Context used:** - Context from `dashboard` - CLAUDE.md ([source](https://app.greptile.com/review/custom-context?memory=fd949e91-5c3a-4ab5-90a1-cbe184fd6ce8)) - Context from `dashboard` - AGENTS.md ([source](https://app.greptile.com/review/custom-context?memory=0d0c8278-ef8e-4d6c-ab21-f5527e322f13)) <!-- /greptile_comment -->

Most Similar PRs