From 410132506321cd0afd2669219eb8fff22ca9f598 Mon Sep 17 00:00:00 2001
From: John Myers <9696606+johntmyers@users.noreply.github.com>
Date: Mon, 16 Mar 2026 11:10:32 -0700
Subject: [PATCH] refactor(proxy): distinguish CONNECT_L7 from CONNECT in
 policy logs

Closes #362

Split the CONNECT log emission in the proxy so denied connections log
immediately as CONNECT while allowed connections defer logging until
after the L7 config check. When L7 inspection follows, emit CONNECT_L7
instead of CONNECT so log consumers can distinguish standalone L4
policy decisions from tunnel lifecycle events.
---
 crates/openshell-sandbox/src/proxy.rs       | 72 ++++++++++++++-------
 crates/openshell-tui/src/ui/sandbox_logs.rs | 20 ++++++
 2 files changed, 70 insertions(+), 22 deletions(-)

diff --git a/crates/openshell-sandbox/src/proxy.rs b/crates/openshell-sandbox/src/proxy.rs
index 44234f66..d662399b 100644
--- a/crates/openshell-sandbox/src/proxy.rs
+++ b/crates/openshell-sandbox/src/proxy.rs
@@ -347,12 +347,12 @@ async fn handle_tcp_connection(
     );
 
     // Extract action string and matched policy for logging
-    let (action_str, matched_policy, deny_reason) = match &decision.action {
-        NetworkAction::Allow { matched_policy } => ("allow", matched_policy.clone(), String::new()),
-        NetworkAction::Deny { reason } => ("deny", None, reason.clone()),
+    let (matched_policy, deny_reason) = match &decision.action {
+        NetworkAction::Allow { matched_policy } => (matched_policy.clone(), String::new()),
+        NetworkAction::Deny { reason } => (None, reason.clone()),
     };
 
-    // Unified log line: one info! per CONNECT with full context
+    // Build log context fields (shared by deny log below and deferred allow log after L7 check)
     let binary_str = decision
         .binary
         .as_ref()
@@ -382,24 +382,26 @@ async fn handle_tcp_connection(
     };
     let policy_str = matched_policy.as_deref().unwrap_or("-");
 
-    info!(
-        src_addr = %peer_addr.ip(),
-        src_port = peer_addr.port(),
-        proxy_addr = %local_addr,
-        dst_host = %host_lc,
-        dst_port = port,
-        binary = %binary_str,
-        binary_pid = %pid_str,
-        ancestors = %ancestors_str,
-        cmdline = %cmdline_str,
-        action = %action_str,
-        engine = "opa",
-        policy = %policy_str,
-        reason = %deny_reason,
-        "CONNECT",
-    );
-
+    // Log denied connections immediately — they never reach L7.
+    // Allowed connections are logged after the L7 config check (below)
+    // so we can distinguish CONNECT (L4-only) from CONNECT_L7 (L7 follows).
     if matches!(decision.action, NetworkAction::Deny { .. }) {
+        info!(
+            src_addr = %peer_addr.ip(),
+            src_port = peer_addr.port(),
+            proxy_addr = %local_addr,
+            dst_host = %host_lc,
+            dst_port = port,
+            binary = %binary_str,
+            binary_pid = %pid_str,
+            ancestors = %ancestors_str,
+            cmdline = %cmdline_str,
+            action = "deny",
+            engine = "opa",
+            policy = "-",
+            reason = %deny_reason,
+            "CONNECT",
+        );
         emit_denial(
             &denial_tx,
             &host_lc,
@@ -498,7 +500,33 @@ async fn handle_tcp_connection(
     respond(&mut client, b"HTTP/1.1 200 Connection Established\r\n\r\n").await?;
 
     // Check if endpoint has L7 config for protocol-aware inspection
-    if let Some(l7_config) = query_l7_config(&opa_engine, &decision, &host_lc, port) {
+    let l7_config = query_l7_config(&opa_engine, &decision, &host_lc, port);
+
+    // Log the allowed CONNECT — use CONNECT_L7 when L7 inspection follows,
+    // so log consumers can distinguish L4-only decisions from tunnel lifecycle events.
+    let connect_msg = if l7_config.is_some() {
+        "CONNECT_L7"
+    } else {
+        "CONNECT"
+    };
+    info!(
+        src_addr = %peer_addr.ip(),
+        src_port = peer_addr.port(),
+        proxy_addr = %local_addr,
+        dst_host = %host_lc,
+        dst_port = port,
+        binary = %binary_str,
+        binary_pid = %pid_str,
+        ancestors = %ancestors_str,
+        cmdline = %cmdline_str,
+        action = "allow",
+        engine = "opa",
+        policy = %policy_str,
+        reason = "",
+        connect_msg,
+    );
+
+    if let Some(l7_config) = l7_config {
         // Clone engine for per-tunnel L7 evaluation (cheap: shares compiled policy via Arc)
         let tunnel_engine = opa_engine.clone_engine_for_tunnel().unwrap_or_else(|e| {
             warn!(error = %e, "Failed to clone OPA engine for L7, falling back to L4-only");
diff --git a/crates/openshell-tui/src/ui/sandbox_logs.rs b/crates/openshell-tui/src/ui/sandbox_logs.rs
index 3171be11..2aa793d2 100644
--- a/crates/openshell-tui/src/ui/sandbox_logs.rs
+++ b/crates/openshell-tui/src/ui/sandbox_logs.rs
@@ -349,6 +349,7 @@ const L7_FIELD_ORDER: &[&str] = &[
 
 /// Return fields in a smart order based on the log message type.
 pub(crate) fn ordered_fields<'a>(log: &'a LogLine) -> Vec<(&'a str, &'a str)> {
+    // Matches both "CONNECT" (L4-only decision) and "CONNECT_L7" (tunnel lifecycle for L7 endpoints)
     let order: Option<&[&str]> = if log.message.starts_with("CONNECT") {
         Some(CONNECT_FIELD_ORDER)
     } else if log.message.starts_with("L7_REQUEST") {
@@ -517,6 +518,25 @@ mod tests {
         assert!(dst_pos < binary_pos);
     }
 
+    #[test]
+    fn plain_format_connect_l7_field_order() {
+        let log = make_log(
+            "CONNECT_L7",
+            vec![
+                ("binary", "/usr/bin/curl"),
+                ("action", "allow"),
+                ("dst_host", "api.example.com"),
+            ],
+        );
+        let result = format_log_line_plain(&log);
+        // CONNECT_L7 should use the same field ordering as CONNECT
+        let action_pos = result.find("action=").unwrap();
+        let dst_pos = result.find("dst_host=").unwrap();
+        let binary_pos = result.find("binary=").unwrap();
+        assert!(action_pos < dst_pos);
+        assert!(dst_pos < binary_pos);
+    }
+
     #[test]
     fn plain_format_l7_field_order() {
         let log = make_log(