fix(sandbox): add JSON bodies to proxy 403/502 responses and include port in HTTP log URLs (#809)

L4 CONNECT and forward-proxy denial responses (403) and upstream
failure responses (502) were returned with empty bodies, making them
indistinguishable from each other when using curl -s. Add a
build_json_error_response() helper that produces consistent JSON
bodies with error and detail fields, matching the format already
used by the L7 deny path.

Also fix Url::to_display_string() in openshell-ocsf to include
non-default ports in the shorthand log output. Requests to e.g.
http://host:9876/path were logged as http://host/path, dropping the
port number.

Closes #807, closes #808

Author: johntmyers

crates/openshell-ocsf/src/format/shorthand.rs

@@ -616,6 +616,35 @@ mod tests {
         );
     }
 
+    #[test]
+    fn test_http_activity_shorthand_non_default_port() {
+        let event = OcsfEvent::HttpActivity(HttpActivityEvent {
+            base: base(4002, "HTTP Activity", 4, "Network Activity", 3, "Get"),
+            http_request: Some(HttpRequest::new(
+                "GET",
+                Url::new("http", "172.20.0.1", "/test", 9876),
+            )),
+            http_response: None,
+            src_endpoint: None,
+            dst_endpoint: None,
+            proxy_endpoint: None,
+            actor: Some(Actor {
+                process: Process::new("curl", 68),
+            }),
+            firewall_rule: Some(FirewallRule::new("allow_host_9876", "mechanistic")),
+            action: Some(ActionId::Allowed),
+            disposition: None,
+            observation_point_id: None,
+            is_src_dst_assignment_known: None,
+        });
+
+        let shorthand = event.format_shorthand();
+        assert_eq!(
+            shorthand,
+            "HTTP:GET [INFO] ALLOWED curl(68) -> GET http://172.20.0.1:9876/test [policy:allow_host_9876]"
+        );
+    }
+
     #[test]
     fn test_ssh_activity_shorthand() {
         let event = OcsfEvent::SshActivity(SshActivityEvent {

crates/openshell-ocsf/src/objects/http.rs

@@ -58,12 +58,21 @@ impl Url {
     }
 
     /// Format as a display string.
+    ///
+    /// Includes the port when it is present and differs from the scheme default
+    /// (443 for `https`, 80 for `http`).
     #[must_use]
     pub fn to_display_string(&self) -> String {
         let scheme = self.scheme.as_deref().unwrap_or("https");
         let hostname = self.hostname.as_deref().unwrap_or("unknown");
         let path = self.path.as_deref().unwrap_or("/");
-        format!("{scheme}://{hostname}{path}")
+        let port_suffix = match self.port {
+            Some(443) if scheme == "https" => String::new(),
+            Some(80) if scheme == "http" => String::new(),
+            Some(p) => format!(":{p}"),
+            None => String::new(),
+        };
+        format!("{scheme}://{hostname}{port_suffix}{path}")
     }
 }
 
@@ -94,9 +103,39 @@ mod tests {
     }
 
     #[test]
-    fn test_url_display_string() {
+    fn test_url_display_string_default_port() {
         let url = Url::new("https", "api.example.com", "/v1/data", 443);
         assert_eq!(url.to_display_string(), "https://api.example.com/v1/data");
+
+        let url = Url::new("http", "example.com", "/index", 80);
+        assert_eq!(url.to_display_string(), "http://example.com/index");
+    }
+
+    #[test]
+    fn test_url_display_string_non_default_port() {
+        let url = Url::new("http", "172.20.0.1", "/test", 9876);
+        assert_eq!(url.to_display_string(), "http://172.20.0.1:9876/test");
+
+        let url = Url::new("https", "api.example.com", "/v1/data", 8443);
+        assert_eq!(
+            url.to_display_string(),
+            "https://api.example.com:8443/v1/data"
+        );
+
+        // HTTP on 443 is non-default — should show port
+        let url = Url::new("http", "example.com", "/path", 443);
+        assert_eq!(url.to_display_string(), "http://example.com:443/path");
+    }
+
+    #[test]
+    fn test_url_display_string_no_port() {
+        let url = Url {
+            scheme: Some("https".to_string()),
+            hostname: Some("example.com".to_string()),
+            path: Some("/path".to_string()),
+            port: None,
+        };
+        assert_eq!(url.to_display_string(), "https://example.com/path");
     }
 
     #[test]

crates/openshell-sandbox/src/proxy.rs

@@ -464,7 +464,16 @@ async fn handle_tcp_connection(
             &deny_reason,
             "connect",
         );
-        respond(&mut client, b"HTTP/1.1 403 Forbidden\r\n\r\n").await?;
+        respond(
+            &mut client,
+            &build_json_error_response(
+                403,
+                "Forbidden",
+                "policy_denied",
+                &format!("CONNECT {host_lc}:{port} not permitted by policy"),
+            ),
+        )
+        .await?;
         return Ok(());
     }
 
@@ -519,7 +528,16 @@ async fn handle_tcp_connection(
                         &reason,
                         "ssrf",
                     );
-                    respond(&mut client, b"HTTP/1.1 403 Forbidden\r\n\r\n").await?;
+                    respond(
+                        &mut client,
+                        &build_json_error_response(
+                            403,
+                            "Forbidden",
+                            "ssrf_denied",
+                            &format!("CONNECT {host_lc}:{port} blocked: allowed_ips check failed"),
+                        ),
+                    )
+                    .await?;
                     return Ok(());
                 }
             },
@@ -554,7 +572,16 @@ async fn handle_tcp_connection(
                     &reason,
                     "ssrf",
                 );
-                respond(&mut client, b"HTTP/1.1 403 Forbidden\r\n\r\n").await?;
+                respond(
+                    &mut client,
+                    &build_json_error_response(
+                        403,
+                        "Forbidden",
+                        "ssrf_denied",
+                        &format!("CONNECT {host_lc}:{port} blocked: invalid allowed_ips in policy"),
+                    ),
+                )
+                .await?;
                 return Ok(());
             }
         }
@@ -595,7 +622,16 @@ async fn handle_tcp_connection(
                     &reason,
                     "ssrf",
                 );
-                respond(&mut client, b"HTTP/1.1 403 Forbidden\r\n\r\n").await?;
+                respond(
+                    &mut client,
+                    &build_json_error_response(
+                        403,
+                        "Forbidden",
+                        "ssrf_denied",
+                        &format!("CONNECT {host_lc}:{port} blocked: internal address"),
+                    ),
+                )
+                .await?;
                 return Ok(());
             }
         }
@@ -2040,7 +2076,16 @@ async fn handle_forward_proxy(
                 reason,
                 "forward",
             );
-            respond(client, b"HTTP/1.1 403 Forbidden\r\n\r\n").await?;
+            respond(
+                client,
+                &build_json_error_response(
+                    403,
+                    "Forbidden",
+                    "policy_denied",
+                    &format!("{method} {host_lc}:{port}{path} not permitted by policy"),
+                ),
+            )
+            .await?;
             return Ok(());
         }
     };
@@ -2168,7 +2213,16 @@ async fn handle_forward_proxy(
                 &reason,
                 "forward-l7-deny",
             );
-            respond(client, b"HTTP/1.1 403 Forbidden\r\n\r\n").await?;
+            respond(
+                client,
+                &build_json_error_response(
+                    403,
+                    "Forbidden",
+                    "policy_denied",
+                    &format!("{method} {host_lc}:{port}{path} denied by L7 policy"),
+                ),
+            )
+            .await?;
             return Ok(());
         }
     }
@@ -2224,7 +2278,16 @@ async fn handle_forward_proxy(
                         &reason,
                         "ssrf",
                     );
-                    respond(client, b"HTTP/1.1 403 Forbidden\r\n\r\n").await?;
+                    respond(
+                        client,
+                        &build_json_error_response(
+                            403,
+                            "Forbidden",
+                            "ssrf_denied",
+                            &format!("{method} {host_lc}:{port} blocked: allowed_ips check failed"),
+                        ),
+                    )
+                    .await?;
                     return Ok(());
                 }
             },
@@ -2263,7 +2326,18 @@ async fn handle_forward_proxy(
                     &reason,
                     "ssrf",
                 );
-                respond(client, b"HTTP/1.1 403 Forbidden\r\n\r\n").await?;
+                respond(
+                    client,
+                    &build_json_error_response(
+                        403,
+                        "Forbidden",
+                        "ssrf_denied",
+                        &format!(
+                            "{method} {host_lc}:{port} blocked: invalid allowed_ips in policy"
+                        ),
+                    ),
+                )
+                .await?;
                 return Ok(());
             }
         }
@@ -2306,7 +2380,16 @@ async fn handle_forward_proxy(
                     &reason,
                     "ssrf",
                 );
-                respond(client, b"HTTP/1.1 403 Forbidden\r\n\r\n").await?;
+                respond(
+                    client,
+                    &build_json_error_response(
+                        403,
+                        "Forbidden",
+                        "ssrf_denied",
+                        &format!("{method} {host_lc}:{port} blocked: internal address"),
+                    ),
+                )
+                .await?;
                 return Ok(());
             }
         }
@@ -2335,7 +2418,16 @@ async fn handle_forward_proxy(
                 ))
                 .build();
             ocsf_emit!(event);
-            respond(client, b"HTTP/1.1 502 Bad Gateway\r\n\r\n").await?;
+            respond(
+                client,
+                &build_json_error_response(
+                    502,
+                    "Bad Gateway",
+                    "upstream_unreachable",
+                    &format!("connection to {host_lc}:{port} failed"),
+                ),
+            )
+            .await?;
             return Ok(());
         }
     };
@@ -2374,7 +2466,16 @@ async fn handle_forward_proxy(
                 error = %e,
                 "credential injection failed in forward proxy"
             );
-            respond(client, b"HTTP/1.1 500 Internal Server Error\r\n\r\n").await?;
+            respond(
+                client,
+                &build_json_error_response(
+                    500,
+                    "Internal Server Error",
+                    "credential_injection_failed",
+                    "unresolved credential placeholder in request",
+                ),
+            )
+            .await?;
             return Ok(());
         }
     };
@@ -2403,6 +2504,30 @@ async fn respond(client: &mut TcpStream, bytes: &[u8]) -> Result<()> {
     Ok(())
 }
 
+/// Build an HTTP error response with a JSON body.
+///
+/// Returns bytes ready to write to the client socket.  The body is a JSON
+/// object with `error` and `detail` fields, matching the format used by the
+/// L7 deny path in `l7/rest.rs`.
+fn build_json_error_response(status: u16, status_text: &str, error: &str, detail: &str) -> Vec<u8> {
+    let body = serde_json::json!({
+        "error": error,
+        "detail": detail,
+    });
+    let body_str = body.to_string();
+    format!(
+        "HTTP/1.1 {status} {status_text}\r\n\
+         Content-Type: application/json\r\n\
+         Content-Length: {}\r\n\
+         Connection: close\r\n\
+         \r\n\
+         {}",
+        body_str.len(),
+        body_str,
+    )
+    .into_bytes()
+}
+
 /// Check if a miette error represents a benign connection close.
 ///
 /// TLS handshake EOF, missing `close_notify`, connection resets, and broken
@@ -3355,4 +3480,65 @@ mod tests {
         let result = implicit_allowed_ips_for_ip_host("*.example.com");
         assert!(result.is_empty());
     }
+
+    // -- build_json_error_response --
+
+    #[test]
+    fn test_json_error_response_403() {
+        let resp = build_json_error_response(
+            403,
+            "Forbidden",
+            "policy_denied",
+            "CONNECT api.example.com:443 not permitted by policy",
+        );
+        let resp_str = String::from_utf8(resp).unwrap();
+
+        assert!(resp_str.starts_with("HTTP/1.1 403 Forbidden\r\n"));
+        assert!(resp_str.contains("Content-Type: application/json\r\n"));
+        assert!(resp_str.contains("Connection: close\r\n"));
+
+        // Extract body after \r\n\r\n
+        let body_start = resp_str.find("\r\n\r\n").unwrap() + 4;
+        let body: serde_json::Value = serde_json::from_str(&resp_str[body_start..]).unwrap();
+        assert_eq!(body["error"], "policy_denied");
+        assert_eq!(
+            body["detail"],
+            "CONNECT api.example.com:443 not permitted by policy"
+        );
+    }
+
+    #[test]
+    fn test_json_error_response_502() {
+        let resp = build_json_error_response(
+            502,
+            "Bad Gateway",
+            "upstream_unreachable",
+            "connection to api.example.com:443 failed",
+        );
+        let resp_str = String::from_utf8(resp).unwrap();
+
+        assert!(resp_str.starts_with("HTTP/1.1 502 Bad Gateway\r\n"));
+
+        let body_start = resp_str.find("\r\n\r\n").unwrap() + 4;
+        let body: serde_json::Value = serde_json::from_str(&resp_str[body_start..]).unwrap();
+        assert_eq!(body["error"], "upstream_unreachable");
+        assert_eq!(body["detail"], "connection to api.example.com:443 failed");
+    }
+
+    #[test]
+    fn test_json_error_response_content_length_matches() {
+        let resp = build_json_error_response(403, "Forbidden", "test", "detail");
+        let resp_str = String::from_utf8(resp).unwrap();
+
+        // Extract Content-Length value
+        let cl_line = resp_str
+            .lines()
+            .find(|l| l.starts_with("Content-Length:"))
+            .unwrap();
+        let cl: usize = cl_line.split(": ").nth(1).unwrap().trim().parse().unwrap();
+
+        // Verify body length matches
+        let body_start = resp_str.find("\r\n\r\n").unwrap() + 4;
+        assert_eq!(resp_str[body_start..].len(), cl);
+    }
 }