← Back to PRs

#8124: fix(browser): add path validation for file upload and download

by yubrew open 2026-02-03 16:27 View on GitHub →
stale
## Summary Add path traversal protection for browser file upload and download operations (CWE-22). ## The Problem The browser file upload endpoint (`/hooks/file-chooser`) and download endpoints (`/download`, `/wait/download`) accepted arbitrary file paths without validation. An attacker who can trigger browser automation could: - Upload sensitive server files (e.g., `/etc/passwd`, `~/.openclaw/credentials/auth-profiles.json`) to web forms - Save downloaded files to arbitrary server locations ## Changes - `src/browser/routes/utils.ts`: Added `validateBrowserFilePath()` and `validateBrowserFilePaths()` functions that ensure paths are within the user's home directory or current working directory - `src/browser/routes/agent.act.ts`: Added path validation to all three affected endpoints: - `/hooks/file-chooser` - validates upload paths - `/wait/download` - validates download destination (when provided) - `/download` - validates download destination ## Test Plan - [x] `pnpm build && pnpm test` passes - [x] Added `src/browser/routes/utils.path-validation.test.ts` with tests for: - Paths within home directory are allowed - Paths within current working directory are allowed - Paths outside home/cwd are rejected - Path traversal patterns (`../`) are rejected - Empty paths are rejected ## Related - [CWE-22: Improper Limitation of a Pathname to a Restricted Directory](https://cwe.mitre.org/data/definitions/22.html) --- Internal reference: VULN-015 This PR was generated with the following prompt: > Add path traversal protection for browser file upload and download operations (CWE-22) 🤖 Discovered by [bitsec.ai](https://bitsec.ai) <!-- greptile_comment --> <h2>Greptile Overview</h2> <h3>Greptile Summary</h3> Adds server-side path validation for browser automation file upload (`/hooks/file-chooser`) and download (`/download`, `/wait/download`) endpoints to mitigate path traversal (CWE-22). The new helpers in `src/browser/routes/utils.ts` resolve paths and enforce that destinations/sources stay under either the user’s home directory or the process current working directory, and the new Vitest file covers allowed/rejected cases. Main concern: the traversal check uses `path.normalize(filePath).includes("..")`, which will reject legitimate paths containing `..` in segment names, potentially breaking valid uploads/downloads. Consider tightening this to only reject actual parent-directory segments (`..`) rather than substring matches. There’s also some policy ambiguity around relative paths being resolved against server `cwd`, which can be surprising if clients assume a different base directory. <h3>Confidence Score: 4/5</h3> - This PR is generally safe to merge and meaningfully improves security, with a small risk of false positives blocking legitimate paths. - Core change is additive and narrowly scoped to the affected endpoints, with tests added. The main risk is functional: the `includes("..")` traversal detection is too broad and may reject valid paths containing `..` in directory/file names, potentially causing regressions in upload/download flows. - src/browser/routes/utils.ts <!-- greptile_other_comments_section --> <!-- /greptile_comment -->

Most Similar PRs