From f55cb031643644f5d91f131ee1628b0ea1adc0c1 Mon Sep 17 00:00:00 2001
From: Nathan Flurry <git@nathanflurry.com>
Date: Tue, 17 Mar 2026 16:47:39 -0700
Subject: [PATCH] feat: [US-038] - Fix path traversal vulnerability in browser
 context_id

Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
---
 .../sandbox-agent/src/browser_context.rs      | 70 ++++++++++++++++++-
 .../sandbox-agent/src/browser_runtime.rs      |  7 +-
 2 files changed, 71 insertions(+), 6 deletions(-)

diff --git a/server/packages/sandbox-agent/src/browser_context.rs b/server/packages/sandbox-agent/src/browser_context.rs
index 1bdd0fb..3352795 100644
--- a/server/packages/sandbox-agent/src/browser_context.rs
+++ b/server/packages/sandbox-agent/src/browser_context.rs
@@ -17,6 +17,38 @@ struct ContextMeta {
 
 const META_FILE: &str = "context.json";
 
+/// Validate that a context_id is a valid hex string (generated by us) and that
+/// the resulting path does not escape the base directory.  Returns the
+/// validated, canonical context directory path on success.
+pub fn validate_context_id(base: &Path, context_id: &str) -> Result<PathBuf, BrowserProblem> {
+    // Only hex characters are valid – rejects ../,  %2F, etc.
+    if !context_id.chars().all(|c| c.is_ascii_hexdigit()) || context_id.is_empty() {
+        return Err(BrowserProblem::not_found(format!(
+            "browser context '{context_id}' not found"
+        )));
+    }
+
+    let context_dir = base.join(context_id);
+
+    // Defence-in-depth: even after the regex check, canonicalize and verify
+    // the resolved path is still under the base directory.
+    if context_dir.exists() {
+        let canonical = context_dir.canonicalize().map_err(|e| {
+            BrowserProblem::internal_error(format!("failed to canonicalize context path: {e}"))
+        })?;
+        let canonical_base = base.canonicalize().map_err(|e| {
+            BrowserProblem::internal_error(format!("failed to canonicalize base path: {e}"))
+        })?;
+        if !canonical.starts_with(&canonical_base) {
+            return Err(BrowserProblem::not_found(format!(
+                "browser context '{context_id}' not found"
+            )));
+        }
+    }
+
+    Ok(context_dir)
+}
+
 /// Return the base directory that holds all browser context directories.
 pub fn contexts_base_dir(state_dir: &Path) -> PathBuf {
     state_dir.join("browser-contexts")
@@ -107,7 +139,7 @@ pub fn create_context(
 /// Delete a browser context directory.
 pub fn delete_context(state_dir: &Path, context_id: &str) -> Result<(), BrowserProblem> {
     let base = contexts_base_dir(state_dir);
-    let context_dir = base.join(context_id);
+    let context_dir = validate_context_id(&base, context_id)?;
 
     if !context_dir.exists() {
         return Err(BrowserProblem::not_found(format!(
@@ -221,4 +253,40 @@ mod tests {
         let result = delete_context(tmp.path(), "nonexistent");
         assert!(result.is_err());
     }
+
+    #[test]
+    fn test_delete_context_path_traversal_dotdot() {
+        let tmp = TempDir::new().unwrap();
+        let result = delete_context(tmp.path(), "../../../tmp");
+        assert!(result.is_err());
+    }
+
+    #[test]
+    fn test_delete_context_path_traversal_encoded() {
+        let tmp = TempDir::new().unwrap();
+        let result = delete_context(tmp.path(), "..%2F..%2Ftmp");
+        assert!(result.is_err());
+    }
+
+    #[test]
+    fn test_delete_context_valid_hex_not_found() {
+        let tmp = TempDir::new().unwrap();
+        let result = delete_context(tmp.path(), "abcdef0123456789");
+        assert!(result.is_err());
+    }
+
+    #[test]
+    fn test_validate_context_id_rejects_empty() {
+        let tmp = TempDir::new().unwrap();
+        let base = contexts_base_dir(tmp.path());
+        assert!(validate_context_id(&base, "").is_err());
+    }
+
+    #[test]
+    fn test_validate_context_id_rejects_slashes() {
+        let tmp = TempDir::new().unwrap();
+        let base = contexts_base_dir(tmp.path());
+        assert!(validate_context_id(&base, "abc/def").is_err());
+        assert!(validate_context_id(&base, "abc\\def").is_err());
+    }
 }
diff --git a/server/packages/sandbox-agent/src/browser_runtime.rs b/server/packages/sandbox-agent/src/browser_runtime.rs
index faeb5d4..3830874 100644
--- a/server/packages/sandbox-agent/src/browser_runtime.rs
+++ b/server/packages/sandbox-agent/src/browser_runtime.rs
@@ -899,11 +899,8 @@ impl BrowserRuntime {
 
         // Set user-data-dir for persistent contexts
         if let Some(ref context_id) = state.context_id {
-            let context_dir = self
-                .config
-                .state_dir
-                .join("browser-contexts")
-                .join(context_id);
+            let base = crate::browser_context::contexts_base_dir(&self.config.state_dir);
+            let context_dir = crate::browser_context::validate_context_id(&base, context_id)?;
             args.push(format!("--user-data-dir={}", context_dir.display()));
         }