fix: security hardening batch 1 (SEC-002 through SEC-010)

crates/openshell-cli/src/run.rs

@@ -14,7 +14,7 @@ use hyper::{Request, StatusCode};
 use hyper_rustls::HttpsConnectorBuilder;
 use hyper_util::{client::legacy::Client, rt::TokioExecutor};
 use indicatif::{MultiProgress, ProgressBar, ProgressStyle};
-use miette::{IntoDiagnostic, Result, WrapErr};
+use miette::{IntoDiagnostic, Result, WrapErr, miette};
 use openshell_bootstrap::{
     DeployOptions, GatewayMetadata, RemoteOptions, clear_active_gateway, container_name,
     extract_host_from_ssh_destination, get_gateway_metadata, list_gateways, load_active_gateway,
@@ -1653,6 +1653,7 @@ pub fn doctor_exec(
     ssh_key: Option<&str>,
     command: &[String],
 ) -> Result<()> {
+    validate_gateway_name(name)?;
     let container = container_name(name);
     let is_tty = std::io::stdin().is_terminal();
 
@@ -1676,7 +1677,15 @@ pub fn doctor_exec(
     };
 
     let mut cmd = if let Some(ref host) = remote_host {
+        validate_ssh_host(host)?;
+
         // Remote: ssh <host> docker exec [-it] <container> sh -lc '<inner_cmd>'
+        //
+        // SSH concatenates all arguments after the hostname into a single
+        // string for the remote shell, so inner_cmd must be escaped twice:
+        // once for `sh -lc` (already done above) and once for the SSH
+        // remote shell (done here).
+        let ssh_escaped_cmd = shell_escape(&inner_cmd);
         let mut c = Command::new("ssh");
         if let Some(key) = ssh_key {
             c.args(["-i", key]);
@@ -1693,7 +1702,7 @@ pub fn doctor_exec(
         } else {
             c.arg("-i");
         }
-        c.args([&container, "sh", "-lc", &inner_cmd]);
+        c.args([&container, "sh", "-lc", &ssh_escaped_cmd]);
         c
     } else {
         // Local: docker exec [-it] <container> sh -lc '<inner_cmd>'
@@ -1790,6 +1799,42 @@ fn shell_escape(s: &str) -> String {
     format!("'{}'", s.replace('\'', "'\\''"))
 }
 
+/// Validate that a gateway name is safe for use in container/volume/network
+/// names and shell commands. Rejects names with characters outside the set
+/// `[a-zA-Z0-9._-]`.
+fn validate_gateway_name(name: &str) -> Result<()> {
+    if name.is_empty() {
+        return Err(miette!("gateway name is empty"));
+    }
+    if !name
+        .bytes()
+        .all(|b| b.is_ascii_alphanumeric() || matches!(b, b'.' | b'-' | b'_'))
+    {
+        return Err(miette!(
+            "gateway name contains invalid characters (allowed: alphanumeric, '.', '-', '_')"
+        ));
+    }
+    Ok(())
+}
+
+/// Validate that an SSH host string is a reasonable hostname or IP address.
+/// Rejects values with shell metacharacters, spaces, or control characters
+/// that could be used for injection via a poisoned metadata.json.
+fn validate_ssh_host(host: &str) -> Result<()> {
+    if host.is_empty() {
+        return Err(miette!("SSH host is empty"));
+    }
+    // Allow: alphanumeric, dots, hyphens, colons (IPv6), square brackets ([::1]),
+    // and @ (user@host).
+    if !host
+        .bytes()
+        .all(|b| b.is_ascii_alphanumeric() || matches!(b, b'.' | b'-' | b':' | b'[' | b']' | b'@'))
+    {
+        return Err(miette!("SSH host contains invalid characters: {host}"));
+    }
+    Ok(())
+}
+
 /// Create a sandbox when no gateway is configured.
 ///
 /// Bootstraps a new gateway first, then delegates to [`sandbox_create`].
@@ -4942,7 +4987,7 @@ mod tests {
         git_sync_files, http_health_check, image_requests_gpu, inferred_provider_type,
         parse_cli_setting_value, parse_credential_pairs, provisioning_timeout_message,
         ready_false_condition_message, resolve_gateway_control_target_from, sandbox_should_persist,
-        source_requests_gpu,
+        shell_escape, source_requests_gpu, validate_gateway_name, validate_ssh_host,
     };
     use crate::TEST_ENV_LOCK;
     use hyper::StatusCode;
@@ -5457,4 +5502,58 @@ mod tests {
         server.join().expect("server thread");
         assert_eq!(status, Some(StatusCode::OK));
     }
+
+    // ---- SEC-004: validate_gateway_name, validate_ssh_host, shell_escape ----
+
+    #[test]
+    fn validate_gateway_name_accepts_valid_names() {
+        assert!(validate_gateway_name("openshell").is_ok());
+        assert!(validate_gateway_name("my-gateway").is_ok());
+        assert!(validate_gateway_name("gateway_v2").is_ok());
+        assert!(validate_gateway_name("gw.prod").is_ok());
+    }
+
+    #[test]
+    fn validate_gateway_name_rejects_invalid_names() {
+        assert!(validate_gateway_name("").is_err());
+        assert!(validate_gateway_name("gw;rm -rf /").is_err());
+        assert!(validate_gateway_name("gw name").is_err());
+        assert!(validate_gateway_name("gw$(id)").is_err());
+        assert!(validate_gateway_name("gw\nmalicious").is_err());
+    }
+
+    #[test]
+    fn validate_ssh_host_accepts_valid_hosts() {
+        assert!(validate_ssh_host("192.168.1.1").is_ok());
+        assert!(validate_ssh_host("example.com").is_ok());
+        assert!(validate_ssh_host("user@host.com").is_ok());
+        assert!(validate_ssh_host("[::1]").is_ok());
+        assert!(validate_ssh_host("2001:db8::1").is_ok());
+    }
+
+    #[test]
+    fn validate_ssh_host_rejects_invalid_hosts() {
+        assert!(validate_ssh_host("").is_err());
+        assert!(validate_ssh_host("host;rm -rf /").is_err());
+        assert!(validate_ssh_host("host$(id)").is_err());
+        assert!(validate_ssh_host("host name").is_err());
+        assert!(validate_ssh_host("host\nmalicious").is_err());
+    }
+
+    #[test]
+    fn shell_escape_double_escape_for_ssh() {
+        // Simulate the double-escape path for SSH:
+        // First escape for sh -lc, then escape again for SSH remote shell.
+        let inner_cmd = "KUBECONFIG=/etc/rancher/k3s/k3s.yaml echo 'hello world'";
+        let ssh_escaped = shell_escape(inner_cmd);
+        // The result should be single-quoted (wrapping the entire inner_cmd)
+        assert!(
+            ssh_escaped.starts_with('\''),
+            "should be single-quoted: {ssh_escaped}"
+        );
+        assert!(
+            ssh_escaped.ends_with('\''),
+            "should end with single-quote: {ssh_escaped}"
+        );
+    }
 }

crates/openshell-sandbox/src/l7/inference.rs

@@ -171,43 +171,69 @@ pub fn try_parse_http_request(buf: &[u8]) -> ParseResult {
     )
 }
 
+/// Maximum decoded body size from chunked transfer encoding (10 MiB).
+/// Matches the caller's `MAX_INFERENCE_BUF` limit.
+const MAX_CHUNKED_BODY: usize = 10 * 1024 * 1024;
+
+/// Maximum number of chunks to process.  Normal HTTP clients send the body
+/// in a handful of large chunks; thousands of tiny chunks indicate abuse.
+const MAX_CHUNK_COUNT: usize = 4096;
+
 /// Parse an HTTP chunked body from `buf[start..]`.
 ///
 /// Returns `(decoded_body, total_consumed_bytes_from_buf_start)` when complete,
-/// or `None` if more bytes are needed.
+/// or `None` if more bytes are needed or resource limits are exceeded.
 fn parse_chunked_body(buf: &[u8], start: usize) -> Option<(Vec<u8>, usize)> {
     let mut pos = start;
     let mut body = Vec::new();
+    let mut chunk_count: usize = 0;
 
     loop {
+        chunk_count += 1;
+        if chunk_count > MAX_CHUNK_COUNT {
+            return None;
+        }
+
         let size_line_end = find_crlf(buf, pos)?;
         let size_line = std::str::from_utf8(&buf[pos..size_line_end]).ok()?;
         let size_token = size_line.split(';').next()?.trim();
         let chunk_size = usize::from_str_radix(size_token, 16).ok()?;
-        pos = size_line_end + 2;
+        pos = size_line_end.checked_add(2)?;
 
         if chunk_size == 0 {
             // Parse trailers (if any). Terminates on empty trailer line.
             loop {
                 let trailer_end = find_crlf(buf, pos)?;
                 let trailer_line = &buf[pos..trailer_end];
-                pos = trailer_end + 2;
+                pos = trailer_end.checked_add(2)?;
                 if trailer_line.is_empty() {
                     return Some((body, pos));
                 }
             }
         }
 
+        // Early reject: chunk cannot possibly fit in remaining buffer.
+        let remaining = buf.len().saturating_sub(pos);
+        if chunk_size > remaining {
+            return None;
+        }
+
+        // Reject if decoded body would exceed size limit.
+        if body.len().saturating_add(chunk_size) > MAX_CHUNKED_BODY {
+            return None;
+        }
+
         let chunk_end = pos.checked_add(chunk_size)?;
-        if buf.len() < chunk_end + 2 {
+        let chunk_crlf_end = chunk_end.checked_add(2)?;
+        if buf.len() < chunk_crlf_end {
             return None;
         }
-        if &buf[chunk_end..chunk_end + 2] != b"\r\n" {
+        if &buf[chunk_end..chunk_crlf_end] != b"\r\n" {
             return None;
         }
 
         body.extend_from_slice(&buf[pos..chunk_end]);
-        pos = chunk_end + 2;
+        pos = chunk_crlf_end;
     }
 }
 
@@ -484,4 +510,46 @@ mod tests {
         assert!(chunk.ends_with(b"\r\n"));
         assert_eq!(chunk.len(), 3 + 2 + 256 + 2); // "100" + \r\n + data + \r\n
     }
+
+    // ---- SEC-010: parse_chunked_body resource limits ----
+
+    #[test]
+    fn parse_chunked_multi_chunk_body() {
+        // Two chunks: 5 bytes + 6 bytes
+        let request = b"POST /v1/chat HTTP/1.1\r\nTransfer-Encoding: chunked\r\n\r\n5\r\nhello\r\n6\r\n world\r\n0\r\n\r\n";
+        let ParseResult::Complete(parsed, _) = try_parse_http_request(request) else {
+            panic!("expected Complete");
+        };
+        assert_eq!(parsed.body, b"hello world");
+    }
+
+    #[test]
+    fn parse_chunked_rejects_too_many_chunks() {
+        // Build a request with MAX_CHUNK_COUNT + 1 tiny chunks
+        let mut buf = Vec::new();
+        buf.extend_from_slice(b"POST /v1/chat HTTP/1.1\r\nTransfer-Encoding: chunked\r\n\r\n");
+        for _ in 0..=MAX_CHUNK_COUNT {
+            buf.extend_from_slice(b"1\r\nX\r\n");
+        }
+        buf.extend_from_slice(b"0\r\n\r\n");
+        assert!(matches!(
+            try_parse_http_request(&buf),
+            ParseResult::Incomplete
+        ));
+    }
+
+    #[test]
+    fn parse_chunked_within_chunk_count_limit() {
+        // MAX_CHUNK_COUNT chunks should succeed
+        let mut buf = Vec::new();
+        buf.extend_from_slice(b"POST /v1/chat HTTP/1.1\r\nTransfer-Encoding: chunked\r\n\r\n");
+        for _ in 0..100 {
+            buf.extend_from_slice(b"1\r\nX\r\n");
+        }
+        buf.extend_from_slice(b"0\r\n\r\n");
+        let ParseResult::Complete(parsed, _) = try_parse_http_request(&buf) else {
+            panic!("expected Complete for 100 chunks");
+        };
+        assert_eq!(parsed.body.len(), 100);
+    }
 }