← Back to PRs

#17944: fix(security): fail-closed for local media paths without sandboxRoot

by Operative-001 open 2026-02-16 10:01 View on GitHub →
size: S trusted-contributor
## Summary Fixes #17936 - HIGH severity sandbox bypass allowing local file exfiltration. ## Changes When `sandboxRoot` is absent, `normalizeSandboxMediaParams` and `normalizeSandboxMediaList` now reject local filesystem paths (absolute, relative, file://) and only allow http(s) URLs. **Before (fail-open):** ```typescript if (!sandboxRoot) { continue; } // skips validation entirely ``` **After (fail-closed):** ```typescript if (!sandboxRoot && !/^https?:\/\//i.test(path)) { throw new Error("Local filesystem paths require sandboxRoot"); } ``` ## Testing Added 5 regression tests: - Rejects `/etc/passwd` without sandboxRoot - Rejects `file:///etc/passwd` without sandboxRoot - Rejects `./relative/path` without sandboxRoot - Allows `https://` URLs without sandboxRoot ✓ - Allows `http://` URLs without sandboxRoot ✓ ## Security Impact Previously, an attacker who could influence tool arguments (via prompt injection or API access) could exfiltrate arbitrary local files through messaging channels when sandboxRoot was unset. This patch closes that vector. <!-- greptile_comment --> <h3>Greptile Summary</h3> Closes a HIGH severity sandbox bypass (#17936) where local filesystem paths could be exfiltrated through messaging channels when `sandboxRoot` was not configured. - Both `normalizeSandboxMediaParams` and `normalizeSandboxMediaList` in `message-action-params.ts` now enforce a fail-closed policy: when `sandboxRoot` is absent, only `http(s)://` URLs are allowed; all local paths (absolute, relative, `file://`) are rejected with a descriptive error. - The fix covers all entry points: direct media params (`media`, `path`, `filePath`), and `MEDIA:` directives parsed from message text, since both flow through the updated validation functions before reaching downstream code. - 5 regression tests validate the new behavior across absolute paths, `file://` URLs, relative paths, and confirm that `http`/`https` URLs remain functional without `sandboxRoot`. - No issues found. The implementation is clean and consistent across both functions. <h3>Confidence Score: 5/5</h3> - This PR is safe to merge — it closes a real security vulnerability with a straightforward, well-tested fix. - The change is minimal and focused: two functions gain a fail-closed guard that rejects non-http(s) URLs when sandboxRoot is absent. The regex validation is correct (case-insensitive, applied after trimming). All code paths that handle media (direct params and MEDIA: directives) flow through these functions. The 5 regression tests cover the key bypass vectors. No regressions to existing behavior since http(s) URLs continue to work without sandboxRoot. - No files require special attention. <sub>Last reviewed commit: 9097b71</sub> <!-- greptile_other_comments_section --> <!-- /greptile_comment -->

Most Similar PRs