fix(network): fail closed when credential placeholders cannot be rewritten

crates/openshell-core/src/secrets.rs

@@ -929,6 +929,20 @@ pub fn rewrite_http_header_block(
     resolver: Option<&SecretResolver>,
 ) -> Result<RewriteResult, UnresolvedPlaceholderError> {
     let Some(resolver) = resolver else {
+        // Fail-closed: with no resolver there is nothing that can rewrite a
+        // credential placeholder, so forwarding the block verbatim would leak
+        // the reserved token to the upstream. Scan the header region (mirroring
+        // the resolved-path scan below) and reject if any reserved marker is
+        // present. Marker-free traffic still passes through unchanged, which is
+        // the intended behavior when the endpoint injects no credentials.
+        let scan_end = raw
+            .windows(4)
+            .position(|w| w == b"\r\n\r\n")
+            .map_or(raw.len(), |p| raw.len().min(p + 4 + 256));
+        let header_region = String::from_utf8_lossy(&raw[..scan_end]);
+        if contains_reserved_credential_marker(&header_region) {
+            return Err(UnresolvedPlaceholderError { location: "header" });
+        }
         return Ok(RewriteResult {
             rewritten: raw.to_vec(),
             redacted_target: None,
@@ -1889,11 +1903,33 @@ mod tests {
     }
 
     #[test]
-    fn no_resolver_passes_through_without_scanning() {
-        // Even if placeholders are present, None resolver means no scanning
+    fn no_resolver_with_placeholder_in_request_line_fails_closed() {
+        // Fail-closed: a placeholder in the request line with no resolver must
+        // never be forwarded verbatim — it would leak the reserved token.
         let raw = b"GET /api/openshell:resolve:env:KEY HTTP/1.1\r\nHost: x\r\n\r\n";
-        let result = rewrite_http_header_block(raw, None).expect("should succeed");
-        assert_eq!(raw.as_slice(), result.rewritten.as_slice());
+        let err = rewrite_http_header_block(raw, None)
+            .expect_err("placeholder without resolver must fail closed");
+        assert_eq!(err.location, "header");
+    }
+
+    #[test]
+    fn no_resolver_with_placeholder_in_header_value_fails_closed() {
+        let raw =
+            b"GET / HTTP/1.1\r\nAuthorization: Bearer openshell:resolve:env:KEY\r\nHost: x\r\n\r\n";
+        let err = rewrite_http_header_block(raw, None)
+            .expect_err("placeholder without resolver must fail closed");
+        assert_eq!(err.location, "header");
+    }
+
+    #[test]
+    fn no_resolver_with_provider_alias_marker_fails_closed() {
+        // The provider-shaped alias marker is also a reserved credential token
+        // and must fail closed when no resolver can rewrite it.
+        let raw =
+            b"GET / HTTP/1.1\r\nX-Api-Key: OPENSHELL-RESOLVE-ENV-CUSTOM_TOKEN\r\nHost: x\r\n\r\n";
+        let err = rewrite_http_header_block(raw, None)
+            .expect_err("alias marker without resolver must fail closed");
+        assert_eq!(err.location, "header");
     }
 
     #[test]

crates/openshell-supervisor-network/src/l7/rest.rs

@@ -4857,12 +4857,15 @@ mod tests {
         assert!(forwarded.contains("Host: integrate.api.nvidia.com\r\n"));
     }
 
-    /// Verifies that without a `SecretResolver` (i.e. the L4-only raw tunnel
-    /// path, or no TLS termination), credential placeholders pass through
-    /// unmodified. This documents the behavior that causes 401 errors when
-    /// `tls: terminate` is missing from the endpoint config.
+    /// Verifies that when a request carries a credential placeholder but no
+    /// `SecretResolver` is available to rewrite it, the relay fails closed
+    /// instead of forwarding the reserved token verbatim. A `None` resolver
+    /// here stands in for a degraded internal state (e.g. secrets could not be
+    /// wired up); forwarding the placeholder would leak it to the upstream and
+    /// cause 401s. The fail-closed scan in `rewrite_http_header_block` rejects
+    /// the request before any byte reaches upstream.
     #[tokio::test]
-    async fn relay_request_without_resolver_leaks_placeholders() {
+    async fn relay_request_without_resolver_fails_closed_on_placeholder() {
         let (child_env, _resolver) = SecretResolver::from_provider_env(
             [("NVIDIA_API_KEY".to_string(), "nvapi-secret".to_string())]
                 .into_iter()
@@ -4887,55 +4890,42 @@ mod tests {
             body_length: BodyLength::ContentLength(2),
         };
 
-        let upstream_task = tokio::spawn(async move {
-            let mut buf = vec![0u8; 4096];
-            let mut total = 0;
-            loop {
-                let n = upstream_side.read(&mut buf[total..]).await.unwrap();
-                if n == 0 {
-                    break;
-                }
-                total += n;
-                if let Some(hdr_end) = buf[..total].windows(4).position(|w| w == b"\r\n\r\n") {
-                    if total >= hdr_end + 4 + 2 {
-                        break;
-                    }
-                }
-            }
-            upstream_side
-                .write_all(b"HTTP/1.1 200 OK\r\nContent-Length: 2\r\n\r\nok")
-                .await
-                .unwrap();
-            upstream_side.flush().await.unwrap();
-            String::from_utf8_lossy(&buf[..total]).to_string()
-        });
-
-        // Pass `None` for the resolver — simulates the L4 path where no
-        // rewriting occurs.
+        // Pass `None` for the resolver — simulates a degraded state where no
+        // rewriting can occur. The relay must reject the request.
         let relay = tokio::time::timeout(
             std::time::Duration::from_secs(5),
             relay_http_request_with_resolver(
                 &req,
                 &mut proxy_to_client,
                 &mut proxy_to_upstream,
-                None, // <-- No resolver, as in the L4 raw tunnel path
+                None,
             ),
         )
         .await
         .expect("relay must not deadlock");
-        relay.expect("relay should succeed");
 
-        let forwarded = upstream_task.await.expect("upstream task should complete");
+        assert!(
+            relay.is_err(),
+            "relay must fail closed when a placeholder cannot be resolved"
+        );
 
-        // Without a resolver, the placeholder LEAKS to upstream — this is the
-        // documented behavior that causes 401s when `tls: terminate` is missing.
+        // Nothing must have reached upstream. Drop the proxy write half so the
+        // read observes EOF instead of blocking, then confirm neither the
+        // placeholder nor the secret leaked.
+        drop(proxy_to_upstream);
+        let mut forwarded = Vec::new();
+        upstream_side
+            .read_to_end(&mut forwarded)
+            .await
+            .expect("upstream read should complete");
+        let forwarded = String::from_utf8_lossy(&forwarded);
         assert!(
-            forwarded.contains("openshell:resolve:env:NVIDIA_API_KEY"),
-            "Expected placeholder to leak without resolver, got: {forwarded}"
+            !forwarded.contains("openshell:resolve:env:"),
+            "placeholder must not leak to upstream: {forwarded}"
         );
         assert!(
             !forwarded.contains("nvapi-secret"),
-            "Real secret should NOT appear without resolver, got: {forwarded}"
+            "secret must not leak to upstream: {forwarded}"
         );
     }
 

crates/openshell-supervisor-network/src/proxy.rs

@@ -1207,21 +1207,54 @@ async fn handle_tcp_connection(
                 }
             }
         } else {
-            {
-                let event = NetworkActivityBuilder::new(openshell_ocsf::ctx::ctx())
-                    .activity(ActivityId::Fail)
-                    .severity(SeverityId::Low)
-                    .status(StatusId::Failure)
-                    .dst_endpoint(Endpoint::from_domain(&host_lc, port))
-                    .message(format!(
-                        "TLS detected but TLS state not configured for {host_lc}:{port}, falling back to raw tunnel"
-                    ))
-                    .build();
-                ocsf_emit!(event);
-            }
-            let _ = tokio::io::copy_bidirectional(&mut client, &mut upstream)
-                .await
-                .into_diagnostic()?;
+            // Fail-closed: TLS was detected but no TLS termination state is
+            // available. Inside the proxy handler this can only mean the
+            // ephemeral CA failed to generate or write at startup (run.rs) —
+            // the `mode != Proxy` case never starts this handler, and
+            // `tls: skip` is handled earlier. Raw-tunneling here would forward
+            // the client's TLS stream straight to the upstream, bypassing
+            // credential rewrite and leaking any `openshell:resolve:env:*`
+            // placeholder verbatim. Refuse the connection instead.
+            const DETAIL: &str = "TLS termination unavailable (CA initialization failed); \
+                 refusing to tunnel — credential rewrite would be bypassed";
+            let event = NetworkActivityBuilder::new(openshell_ocsf::ctx::ctx())
+                .activity(ActivityId::Open)
+                .action(ActionId::Denied)
+                .disposition(DispositionId::Blocked)
+                .severity(SeverityId::High)
+                .status(StatusId::Failure)
+                .dst_endpoint(Endpoint::from_domain(&host_lc, port))
+                .src_endpoint_addr(peer_addr.ip(), peer_addr.port())
+                .actor_process(
+                    Process::from_bypass(&binary_str, &pid_str, &ancestors_str)
+                        .with_cmd_line(&cmdline_str),
+                )
+                .firewall_rule(policy_str, "tls")
+                .message(format!("CONNECT refused for {host_lc}:{port}: {DETAIL}"))
+                .status_detail(DETAIL)
+                .build();
+            ocsf_emit!(event);
+            emit_activity_simple(activity_tx.as_ref(), true, "tls_termination_unavailable");
+            emit_denial(
+                &denial_tx,
+                &host_lc,
+                port,
+                &binary_str,
+                &decision,
+                DETAIL,
+                "connect-tls-termination-unavailable",
+            );
+            respond(
+                &mut client,
+                &build_json_error_response(
+                    503,
+                    "Service Unavailable",
+                    "tls_termination_unavailable",
+                    DETAIL,
+                ),
+            )
+            .await?;
+            return Ok(());
         }
     } else if tunnel_protocol == TunnelProtocol::Http1 {
         // Plaintext HTTP detected.
@@ -7764,6 +7797,32 @@ network_policies:
         assert_eq!(body["detail"], "connection to api.example.com:443 failed");
     }
 
+    /// Locks the fail-closed response the CONNECT handler sends when TLS is
+    /// detected but no termination state exists (ephemeral CA setup failed).
+    /// The proxy must refuse the connection with a 503 instead of raw-tunneling
+    /// the TLS stream, which would bypass credential rewrite and leak
+    /// placeholders verbatim.
+    #[test]
+    fn test_json_error_response_503_tls_termination_unavailable() {
+        let detail = "TLS termination unavailable (CA initialization failed); \
+             refusing to tunnel — credential rewrite would be bypassed";
+        let resp = build_json_error_response(
+            503,
+            "Service Unavailable",
+            "tls_termination_unavailable",
+            detail,
+        );
+        let resp_str = String::from_utf8(resp).unwrap();
+
+        assert!(resp_str.starts_with("HTTP/1.1 503 Service Unavailable\r\n"));
+        assert!(resp_str.contains("Connection: close\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"], "tls_termination_unavailable");
+        assert_eq!(body["detail"], detail);
+    }
+
     #[test]
     fn test_json_error_response_content_length_matches() {
         let resp = build_json_error_response(403, "Forbidden", "test", "detail");

crates/openshell-supervisor-network/src/run.rs

@@ -223,9 +223,13 @@ pub async fn run_networking(
                         (Some(state), Some(paths))
                     }
                     Err(e) => {
+                        // High severity: with TLS termination disabled the proxy
+                        // cannot rewrite credentials, so it fails closed on
+                        // TLS-bearing connections (see proxy.rs) rather than
+                        // leaking placeholders through a raw tunnel.
                         ocsf_emit!(
                             ConfigStateChangeBuilder::new(ocsf_ctx())
-                                .severity(SeverityId::Medium)
+                                .severity(SeverityId::High)
                                 .status(StatusId::Failure)
                                 .state(StateId::Disabled, "disabled")
                                 .message(format!(
@@ -238,9 +242,13 @@ pub async fn run_networking(
                 }
             }
             Err(e) => {
+                // High severity: with TLS termination disabled the proxy cannot
+                // rewrite credentials, so it fails closed on TLS-bearing
+                // connections (see proxy.rs) rather than leaking placeholders
+                // through a raw tunnel.
                 ocsf_emit!(
                     ConfigStateChangeBuilder::new(ocsf_ctx())
-                        .severity(SeverityId::Medium)
+                        .severity(SeverityId::High)
                         .status(StatusId::Failure)
                         .state(StateId::Disabled, "disabled")
                         .message(format!(