fix(security): prevent path traversal bypass in WASM HTTP allowlist (#137)

* fix(security): prevent path traversal bypass in WASM HTTP allowlist

The allowlist validator checked url_path.starts_with(prefix) on the
raw, unnormalized path. A WASM tool could request a URL like:

  https://api.openai.com/v1/../admin

The starts_with("/v1/") check would pass, but the server would
resolve the ".." and serve /admin — effectively bypassing the
path prefix restriction.

This commit adds normalize_path() which resolves . and .. segments
before validation, closing the bypass. It also includes 6 new tests
covering traversal attacks and normalization correctness.

* deslop: remove redundant comments, consolidate tests

* chore(allowlist): trim nonessential traversal helper comment

* harden URL parsing for wasm allowlist and proxy paths

---------

Co-authored-by: Illia Polosukhin <[email protected]>

src/sandbox/proxy/allowlist.rs

@@ -144,41 +144,16 @@ impl Default for DomainAllowlist {
 
 /// Parse host from a URL string.
 pub fn extract_host(url: &str) -> Option<String> {
-    // Determine scheme and extract the rest
-    let rest = if let Some(stripped) = url.strip_prefix("https://") {
-        stripped
-    } else if let Some(stripped) = url.strip_prefix("http://") {
-        stripped
-    } else {
+    let parsed = url::Url::parse(url).ok()?;
+    if !matches!(parsed.scheme(), "http" | "https") {
         return None;
-    };
-
-    // Find the end of the host (start of path, query, or end of string)
-    let host_end = rest.find('/').unwrap_or(rest.len());
-    let host_and_port = &rest[..host_end];
-
-    // Remove port if present
-    let host = if let Some(bracket_idx) = host_and_port.find('[') {
-        // IPv6 address
-        let close_bracket = host_and_port.find(']')?;
-        &host_and_port[bracket_idx + 1..close_bracket]
-    } else if let Some(colon_idx) = host_and_port.rfind(':') {
-        // Check if this is a port (all digits after colon)
-        let after_colon = &host_and_port[colon_idx + 1..];
-        if after_colon.chars().all(|c| c.is_ascii_digit()) {
-            &host_and_port[..colon_idx]
-        } else {
-            host_and_port
-        }
-    } else {
-        host_and_port
-    };
-
-    if host.is_empty() {
-        None
-    } else {
-        Some(host.to_lowercase())
     }
+    parsed.host_str().map(|h| {
+        h.strip_prefix('[')
+            .and_then(|v| v.strip_suffix(']'))
+            .unwrap_or(h)
+            .to_lowercase()
+    })
 }
 
 #[cfg(test)]
@@ -246,6 +221,15 @@ mod tests {
             extract_host("https://EXAMPLE.COM"),
             Some("example.com".to_string())
         );
+        assert_eq!(
+            extract_host("https://user:[email protected]:443/path"),
+            Some("api.example.com".to_string())
+        );
+        assert_eq!(
+            extract_host("http://[::1]:8080/path"),
+            Some("::1".to_string())
+        );
         assert_eq!(extract_host("not-a-url"), None);
+        assert_eq!(extract_host("ftp://example.com/file"), None);
     }
 }

src/sandbox/proxy/policy.rs

@@ -24,8 +24,18 @@ pub struct NetworkRequest {
 impl NetworkRequest {
     /// Create from a URL string.
     pub fn from_url(method: &str, url: &str) -> Option<Self> {
-        let host = crate::sandbox::proxy::allowlist::extract_host(url)?;
-        let path = extract_path(url);
+        let parsed = url::Url::parse(url).ok()?;
+        if !matches!(parsed.scheme(), "http" | "https") {
+            return None;
+        }
+
+        let host = parsed.host_str()?;
+        let host = host
+            .strip_prefix('[')
+            .and_then(|v| v.strip_suffix(']'))
+            .unwrap_or(host)
+            .to_lowercase();
+        let path = parsed.path().to_string();
 
         Some(Self {
             method: method.to_uppercase(),
@@ -37,15 +47,15 @@ impl NetworkRequest {
 }
 
 /// Extract path from a URL.
+#[cfg(test)]
 fn extract_path(url: &str) -> String {
-    // Find the start of the path (after ://)
-    if let Some(idx) = url.find("://") {
-        let rest = &url[idx + 3..];
-        if let Some(path_start) = rest.find('/') {
-            return rest[path_start..].to_string();
-        }
+    let Ok(parsed) = url::Url::parse(url) else {
+        return "/".to_string();
+    };
+    if !matches!(parsed.scheme(), "http" | "https") {
+        return "/".to_string();
     }
-    "/".to_string()
+    parsed.path().to_string()
 }
 
 /// Decision for a network request.
@@ -203,6 +213,11 @@ mod tests {
         );
         assert_eq!(extract_path("https://example.com"), "/".to_string());
         assert_eq!(extract_path("https://example.com/"), "/".to_string());
+        assert_eq!(
+            extract_path("https://example.com/path?q=1#frag"),
+            "/path".to_string()
+        );
+        assert_eq!(extract_path("ftp://example.com/path"), "/".to_string());
     }
 
     #[tokio::test]

src/tools/wasm/allowlist.rs

@@ -170,74 +170,89 @@ struct ParsedUrl {
     path: String,
 }
 
-/// Simple URL parser (avoids pulling in a full URL crate).
+/// Parse and normalize URL components for allowlist matching.
 fn parse_url(url: &str) -> Result<ParsedUrl, String> {
-    // Find scheme
-    let (scheme, rest) = url
-        .split_once("://")
-        .ok_or_else(|| "Missing scheme (expected http:// or https://)".to_string())?;
-
-    let scheme = scheme.to_lowercase();
+    let parsed = url::Url::parse(url).map_err(|e| format!("URL parse failed: {e}"))?;
+    let scheme = parsed.scheme().to_lowercase();
     if scheme != "http" && scheme != "https" {
         return Err(format!("Unsupported scheme: {}", scheme));
     }
 
-    // Reject URLs with userinfo (user:pass@host) to prevent allowlist bypass.
-    // A URL like https://[email protected]/ would match the allowlist
-    // for api.openai.com but actually send traffic to evil.com.
-    let authority = match rest.find('/') {
-        Some(idx) => &rest[..idx],
-        None => rest,
-    };
-    if authority.contains('@') {
+    // Reject URLs with userinfo (user:pass@host) to prevent host-confusion bypasses.
+    if !parsed.username().is_empty() || parsed.password().is_some() {
         return Err("URL contains userinfo (@) which is not allowed".to_string());
     }
 
-    // Split host from path
-    let (host_and_port, path) = match rest.find('/') {
-        Some(idx) => (&rest[..idx], &rest[idx..]),
-        None => (rest, "/"),
-    };
-
-    // Remove port from host
-    let host = match host_and_port.rfind(':') {
-        Some(idx) => {
-            // Make sure this isn't an IPv6 address
-            if host_and_port.starts_with('[') {
-                // IPv6: [::1]:8080 or [::1]
-                if let Some(bracket_idx) = host_and_port.find(']') {
-                    // Extract the IPv6 address without brackets
-                    &host_and_port[1..bracket_idx]
-                } else {
-                    return Err("Invalid IPv6 address".to_string());
-                }
-            } else {
-                &host_and_port[..idx]
-            }
-        }
-        None => host_and_port,
-    };
-
-    // Reject URLs with userinfo (user:pass@host).
-    // A URL like https://[email protected]/ confuses the parser into
-    // seeing "api.openai.com" as the host, but reqwest actually sends to
-    // "evil.com". Block any '@' in the authority section to prevent this.
-    if host.contains('@') || host_and_port.contains('@') {
-        return Err("URL contains userinfo (@) which is not allowed".to_string());
-    }
-
-    // Validate host
-    if host.is_empty() {
-        return Err("Empty host".to_string());
-    }
+    let host = parsed.host_str().ok_or_else(|| "Empty host".to_string())?;
+    let host = host
+        .strip_prefix('[')
+        .and_then(|h| h.strip_suffix(']'))
+        .unwrap_or(host)
+        .to_lowercase();
+    let normalized_path = normalize_path(parsed.path())?;
 
     Ok(ParsedUrl {
         scheme,
-        host: host.to_lowercase(),
-        path: path.to_string(),
+        host,
+        path: normalized_path,
     })
 }
 
+fn normalize_path(path: &str) -> Result<String, String> {
+    let mut segments: Vec<String> = Vec::new();
+    for raw_segment in path.split('/') {
+        if !has_valid_percent_encoding(raw_segment) {
+            return Err(format!(
+                "Invalid percent-encoding in path segment: {raw_segment}"
+            ));
+        }
+
+        let segment = urlencoding::decode(raw_segment)
+            .map_err(|_| format!("Invalid percent-encoding in path segment: {raw_segment}"))?;
+        let segment = segment.as_ref();
+
+        // Encoded separators introduce ambiguous semantics across downstream handlers.
+        if segment.contains('/') || segment.contains('\\') {
+            return Err("Path segment contains encoded path separator".to_string());
+        }
+
+        match segment {
+            "" | "." => {}
+            ".." => {
+                segments.pop();
+            }
+            _ => segments.push(segment.to_string()),
+        }
+    }
+
+    let mut result = String::with_capacity(path.len().max(1));
+    result.push('/');
+    result.push_str(&segments.join("/"));
+    if path.len() > 1 && path.ends_with('/') && !result.ends_with('/') {
+        result.push('/');
+    }
+    Ok(result)
+}
+
+fn has_valid_percent_encoding(segment: &str) -> bool {
+    let bytes = segment.as_bytes();
+    let mut i = 0;
+    while i < bytes.len() {
+        if bytes[i] == b'%' {
+            if i + 2 >= bytes.len()
+                || !bytes[i + 1].is_ascii_hexdigit()
+                || !bytes[i + 2].is_ascii_hexdigit()
+            {
+                return false;
+            }
+            i += 3;
+        } else {
+            i += 1;
+        }
+    }
+    true
+}
+
 #[cfg(test)]
 mod tests {
     use crate::tools::wasm::allowlist::{AllowlistValidator, DenyReason};
@@ -380,6 +395,68 @@ mod tests {
         }
     }
 
+    #[test]
+    fn test_path_traversal_blocked() {
+        let validator = validator_with_patterns();
+        assert!(!validator.validate("https://api.openai.com/v1/../admin", "GET").is_allowed());
+        assert!(!validator.validate("https://api.openai.com/v1/../../etc/passwd", "GET").is_allowed());
+        assert!(!validator.validate("https://api.openai.com/v1/%2E%2E/admin", "GET").is_allowed());
+        assert!(!validator.validate("https://api.openai.com/v1/%2e%2e/%2e%2e/root", "GET").is_allowed());
+        assert!(validator.validate("https://api.openai.com/v1/chat/completions", "POST").is_allowed());
+    }
+
+    #[test]
+    fn test_normalize_path() {
+        use super::normalize_path;
+        assert_eq!(normalize_path("/v1/../admin").unwrap(), "/admin");
+        assert_eq!(
+            normalize_path("/v1/chat/completions").unwrap(),
+            "/v1/chat/completions"
+        );
+        assert_eq!(normalize_path("/v1/./chat").unwrap(), "/v1/chat");
+        assert_eq!(
+            normalize_path("/v1/../../../etc/passwd").unwrap(),
+            "/etc/passwd"
+        );
+        assert_eq!(normalize_path("/v1/%2e%2e/admin").unwrap(), "/admin");
+        assert_eq!(normalize_path("/").unwrap(), "/");
+        assert_eq!(normalize_path("/v1/").unwrap(), "/v1/");
+    }
+
+    #[test]
+    fn test_invalid_encoded_path_rejected() {
+        let validator = validator_with_patterns();
+        let result = validator.validate("https://api.openai.com/v1/%ZZ/chat", "GET");
+        assert!(!result.is_allowed());
+        if let super::AllowlistResult::Denied(reason) = result {
+            assert!(matches!(reason, DenyReason::InvalidUrl(_)));
+        } else {
+            panic!("Expected denied");
+        }
+    }
+
+    #[test]
+    fn test_encoded_separator_rejected() {
+        let validator = validator_with_patterns();
+        let result = validator.validate("https://api.openai.com/v1/%2Fadmin", "GET");
+        assert!(!result.is_allowed());
+        if let super::AllowlistResult::Denied(reason) = result {
+            assert!(matches!(reason, DenyReason::InvalidUrl(_)));
+        } else {
+            panic!("Expected denied");
+        }
+    }
+
+    #[test]
+    fn test_percent_encoding_validator() {
+        use super::has_valid_percent_encoding;
+        assert!(has_valid_percent_encoding("%2F"));
+        assert!(has_valid_percent_encoding("hello%20world"));
+        assert!(!has_valid_percent_encoding("%"));
+        assert!(!has_valid_percent_encoding("%2"));
+        assert!(!has_valid_percent_encoding("%ZZ"));
+    }
+
     #[test]
     fn test_url_with_port() {
         let validator =

src/tools/wasm/wrapper.rs

@@ -992,51 +992,37 @@ async fn resolve_host_credentials(
 /// Also handles IPv6 bracket notation like `http://[::1]:8080/path`.
 /// Returns None for malformed URLs.
 fn extract_host_from_url(url: &str) -> Option<String> {
-    let after_scheme = url
-        .strip_prefix("https://")
-        .or_else(|| url.strip_prefix("http://"))?;
-    let end = after_scheme
-        .find(['/', '?', '#'])
-        .unwrap_or(after_scheme.len());
-    let host_port = &after_scheme[..end];
-    // Strip userinfo (user:pass@host)
-    let after_userinfo = host_port
-        .rfind('@')
-        .map(|i| &host_port[i + 1..])
-        .unwrap_or(host_port);
-    // Handle IPv6 bracket notation: [::1]:port -> ::1
-    if after_userinfo.starts_with('[') {
-        let closing = after_userinfo.find(']')?;
-        return Some(after_userinfo[1..closing].to_string());
+    let parsed = url::Url::parse(url).ok()?;
+    if !matches!(parsed.scheme(), "http" | "https") {
+        return None;
     }
-    // Regular host:port -> host
-    let host = after_userinfo
-        .rfind(':')
-        .map(|i| &after_userinfo[..i])
-        .unwrap_or(after_userinfo);
-    Some(host.to_string())
+    parsed.host_str().map(|h| {
+        h.strip_prefix('[')
+            .and_then(|v| v.strip_suffix(']'))
+            .unwrap_or(h)
+            .to_lowercase()
+    })
 }
 
 /// Resolve the URL's hostname and reject connections to private/internal IP addresses.
 /// This prevents DNS rebinding attacks where an attacker's domain resolves to an
 /// internal IP after passing the allowlist check.
 fn reject_private_ip(url: &str) -> Result<(), String> {
-    let host = url
-        .split("://")
-        .nth(1)
-        .and_then(|rest| {
-            let host_and_port = rest.split('/').next().unwrap_or(rest);
-            // Strip port
-            if host_and_port.starts_with('[') {
-                // IPv6
-                host_and_port.find(']').map(|i| &host_and_port[1..i])
-            } else {
-                Some(
-                    host_and_port
-                        .rfind(':')
-                        .map_or(host_and_port, |i| &host_and_port[..i]),
-                )
-            }
+    let parsed = url::Url::parse(url)
+        .map_err(|e| format!("Failed to parse URL: {e}"))?;
+    if !matches!(parsed.scheme(), "http" | "https") {
+        return Err(format!("Unsupported URL scheme: {}", parsed.scheme()));
+    }
+    if !parsed.username().is_empty() || parsed.password().is_some() {
+        return Err("URL contains userinfo (@) which is not allowed".to_string());
+    }
+
+    let host = parsed
+        .host_str()
+        .map(|h| {
+            h.strip_prefix('[')
+                .and_then(|v| v.strip_suffix(']'))
+                .unwrap_or(h)
         })
         .ok_or_else(|| "Failed to parse host from URL".to_string())?;