fix(proxy): include OPA deny reason in CONNECT 403 response (#2363)

* 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 <azaalouk@redhat.com>

* 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 <azaalouk@redhat.com>

---------

Signed-off-by: Adel Zaalouk <azaalouk@redhat.com>

Author: zanetworker

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?;
@@ -4884,6 +4885,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<u8> {
+    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,
@@ -5133,6 +5162,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");

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.