From e333f14edf9e928e4ff1c5d408dcf1f9bf82c8dc Mon Sep 17 00:00:00 2001 From: Adel Zaalouk Date: Mon, 20 Jul 2026 11:26:27 +0200 Subject: [PATCH 1/2] fix(proxy): include OPA deny reason in CONNECT 403 response When a CONNECT request was denied by OPA policy, the 403 response used a generic "not permitted by policy" message for both "endpoint not in policy" and "endpoint matched but binary didn't match." Users had no way to distinguish the two without reading supervisor logs. The OPA policy already computes a detailed deny_reason (e.g., "binary '/usr/bin/node' not allowed in policy 'X'") but the proxy was not including it in the HTTP response. Now the CONNECT deny response includes a "reason" field with the OPA deny reason when available. When the reason is empty, the field is omitted for backward compatibility. Fixes #2355 Signed-off-by: Adel Zaalouk --- .../openshell-supervisor-network/src/proxy.rs | 73 ++++++++++++++++++- 1 file changed, 72 insertions(+), 1 deletion(-) diff --git a/crates/openshell-supervisor-network/src/proxy.rs b/crates/openshell-supervisor-network/src/proxy.rs index ab2313b89..f6372099d 100644 --- a/crates/openshell-supervisor-network/src/proxy.rs +++ b/crates/openshell-supervisor-network/src/proxy.rs @@ -825,11 +825,12 @@ async fn handle_tcp_connection( emit_activity(&activity_tx, true, "connect_policy"); respond( &mut client, - &build_json_error_response( + &build_json_error_response_with_reason( 403, "Forbidden", "policy_denied", &format!("CONNECT {host_lc}:{port} not permitted by policy"), + &deny_reason, ), ) .await?; @@ -4878,6 +4879,34 @@ fn build_json_error_response(status: u16, status_text: &str, error: &str, detail .into_bytes() } +fn build_json_error_response_with_reason( + status: u16, + status_text: &str, + error: &str, + detail: &str, + reason: &str, +) -> Vec { + let mut body = serde_json::json!({ + "error": error, + "detail": detail, + }); + if !reason.is_empty() { + body["reason"] = serde_json::json!(reason); + } + 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() +} + fn build_middleware_deny_response( policy_name: &str, denial: &openshell_supervisor_middleware::MiddlewareDenial, @@ -5127,6 +5156,48 @@ network_policies: {} assert!(body.get("agent_guidance").is_none()); } + #[test] + fn policy_deny_response_includes_reason() { + let response = build_json_error_response_with_reason( + 403, + "Forbidden", + "policy_denied", + "CONNECT api.example.com:443 not permitted by policy", + "binary '/usr/bin/node' not allowed in policy 'allow_api' (ancestors: [/usr/local/bin/claude])", + ); + let response = String::from_utf8(response).expect("UTF-8 error response"); + assert!(response.starts_with("HTTP/1.1 403 Forbidden")); + let (_, body) = response.split_once("\r\n\r\n").expect("HTTP response"); + let body: serde_json::Value = serde_json::from_str(body).expect("JSON response"); + + assert_eq!(body["error"], "policy_denied"); + assert_eq!( + body["detail"], + "CONNECT api.example.com:443 not permitted by policy" + ); + assert_eq!( + body["reason"], + "binary '/usr/bin/node' not allowed in policy 'allow_api' (ancestors: [/usr/local/bin/claude])" + ); + } + + #[test] + fn policy_deny_response_omits_empty_reason() { + let response = build_json_error_response_with_reason( + 403, + "Forbidden", + "policy_denied", + "CONNECT api.example.com:443 not permitted by policy", + "", + ); + let response = String::from_utf8(response).expect("UTF-8 error response"); + let (_, body) = response.split_once("\r\n\r\n").expect("HTTP response"); + let body: serde_json::Value = serde_json::from_str(body).expect("JSON response"); + + assert_eq!(body["error"], "policy_denied"); + assert!(body.get("reason").is_none()); + } + #[test] fn endpoint_only_opa_allows_declared_endpoint_without_process_identity() { let policy = include_str!("../data/sandbox-policy.rego"); From 785d231546ad98fee9ef0e92424c7263191bc84a Mon Sep 17 00:00:00 2001 From: Adel Zaalouk Date: Tue, 21 Jul 2026 12:38:08 +0200 Subject: [PATCH 2/2] docs(observability): document optional reason field in CONNECT 403 response The proxy now includes a reason field in the JSON body of denied CONNECT responses when the policy engine provides a specific denial cause. Update the Proxy Error Responses section to show the field and describe when it is present vs omitted. Signed-off-by: Adel Zaalouk --- docs/observability/logging.mdx | 7 +++++-- 1 file changed, 5 insertions(+), 2 deletions(-) diff --git a/docs/observability/logging.mdx b/docs/observability/logging.mdx index 7a13ffea0..1ab0b5bfe 100644 --- a/docs/observability/logging.mdx +++ b/docs/observability/logging.mdx @@ -192,10 +192,13 @@ A denied CONNECT returns `403 Forbidden`: ```json { "error": "policy_denied", - "detail": "CONNECT api.example.com:443 not permitted by policy" + "detail": "CONNECT api.example.com:443 not permitted by policy", + "reason": "binary '/usr/bin/node' not allowed in policy 'allow_api' (ancestors: [/usr/local/bin/claude])" } ``` +The `reason` field is included when the policy engine provides a specific denial reason (for example, which binary or rule caused the rejection). It is omitted when no additional detail is available. + An upstream that the proxy cannot reach returns `502 Bad Gateway`: ```json @@ -205,7 +208,7 @@ An upstream that the proxy cannot reach returns `502 Bad Gateway`: } ``` -The `error` field is a short machine-readable code (`policy_denied`, `middleware_denied`, `middleware_failed`, `ssrf_denied`, `upstream_unreachable`). The `detail` field is a human-readable explanation suitable for display in an agent transcript. +The `error` field is a short machine-readable code (`policy_denied`, `middleware_denied`, `middleware_failed`, `ssrf_denied`, `upstream_unreachable`). The `detail` field is a human-readable explanation suitable for display in an agent transcript. The optional `reason` field, when present, provides the specific denial cause from the policy engine (for example, which binary was not allowed or which rule was missing). For L7 REST policy denials, the body also includes structured policy fields such as `method`, `path`, `rule_missing`, and `next_steps`. When policy advisor is enabled, it also includes `agent_guidance`, a short plain-language instruction telling the agent to read `/etc/openshell/skills/policy_advisor.md`, propose the narrowest rule through `http://policy.local/v1/proposals`, wait for `policy_reloaded: true`, and retry. A middleware denial instead identifies the policy-local config in `middleware` and can include a validated `reason_code`. A fail-closed runtime failure uses `middleware_failed` with platform-owned text. Both middleware responses omit `rule_missing`, `next_steps`, and `agent_guidance` because no policy rule is missing.