XuebinMa · XuebinMa · Jun 3, 2026 · Jun 3, 2026
@@ -2,7 +2,12 @@ use serde::{Deserialize, Serialize};
 
 // ── GuardDecision ─────────────────────────────────────────────────────────────
 
-#[derive(Debug, Clone, PartialEq, Eq, Serialize, Deserialize)]
+// `GuardDecision` drives enforcement, so it is `Serialize`-only by design: it
+// must never be reconstructed from external input. Deriving `Deserialize` would
+// let untrusted JSON (`{"decision":"allow"}`) synthesize an `Allow` without
+// passing through the policy engine. Audit/receipt consumers read the
+// serialized form; they never deserialize it back to drive a decision.
+#[derive(Debug, Clone, PartialEq, Eq, Serialize)]
 #[serde(tag = "decision", rename_all = "snake_case")]
 pub enum GuardDecision {
     Allow,
@@ -137,7 +142,11 @@ impl RuntimeDecision {
 
 // ── DecisionReason ────────────────────────────────────────────────────────────
 
+// `#[non_exhaustive]` forces cross-crate callers through `DecisionReason::new`
+// (and its builders), which always supply a non-empty `message`, instead of a
+// struct literal that could set `message: ""` and bypass that guarantee.
 #[derive(Debug, Clone, PartialEq, Eq, Serialize, Deserialize)]
+#[non_exhaustive]
 pub struct DecisionReason {
     pub code: DecisionCode,
     pub message: String,

@@ -59,6 +59,10 @@ impl AnomalyDetector {
             Ok(guard) => guard,
             Err(_) => {
                 // If the mutex is poisoned, we fail-closed for security.
+                tracing::error!(
+                    actor = actor,
+                    "AnomalyDetector state mutex poisoned in check; failing closed to Locked"
+                );
                 return AnomalyStatus::Locked;
             }
         };
@@ -123,7 +127,16 @@ impl AnomalyDetector {
         let now = Instant::now();
         let mut states = match self.states.lock() {
             Ok(guard) => guard,
-            Err(_) => return, // Silent return on poison for non-critical update
+            // Fail-closed: recover the poisoned guard so the denial is still
+            // recorded and the Deny Fuse can still trip. Dropping it here would
+            // let an actor exhaust the threshold without ever locking.
+            Err(poisoned) => {
+                tracing::error!(
+                    actor = actor,
+                    "AnomalyDetector state mutex poisoned in report_denial; recovering to record denial"
+                );
+                poisoned.into_inner()
+            }
         };
         compact_states(&mut states, config, now);
         let state = states.entry(actor.to_string()).or_default();

@@ -64,7 +64,20 @@ fn scannable_text(tool: &Tool, payload: &str) -> Option<String> {
         Tool::HttpRequest => "body",
         _ => return None,
     };
-    let value: serde_json::Value = serde_json::from_str(payload).ok()?;
+    let value: serde_json::Value = match serde_json::from_str(payload) {
+        Ok(value) => value,
+        Err(e) => {
+            // A malformed payload means the scan/mask path is skipped. Warn so
+            // "no findings" is distinguishable from "scan skipped due to a
+            // payload the guard could not parse".
+            tracing::warn!(
+                tool = ?tool,
+                error = %e,
+                "content scan skipped: tool payload is not valid JSON"
+            );
+            return None;
+        }
+    };
     value
         .get(field)
         .and_then(serde_json::Value::as_str)

@@ -13,7 +13,7 @@ use agent_guard_sandbox::{Sandbox, SandboxContext, SandboxError, SandboxOutput};
 use serde::Serialize;
 use uuid::Uuid;
 
-use crate::approval::{ApprovalConfig, ApprovalRecord, ApprovalStatus};
+use crate::approval::{ApprovalConfig, ApprovalError, ApprovalRecord, ApprovalStatus};
 use crate::executors::{
     execute_http_request, execute_write_file, extract_bash_command_for_execution,
 };
@@ -526,12 +526,24 @@ impl Guard {
                 }
                 ApprovalStatus::Expired | ApprovalStatus::Pending => {
                     if Instant::now() >= deadline {
-                        // Best-effort terminal mark; ignore if already decided.
-                        let _ = config.ledger.decide(
+                        // Terminal mark. `AlreadyDecided` is expected when the
+                        // request was resolved concurrently; any other error is
+                        // a real ledger write failure that would leave the
+                        // ledger `Pending` while we return a denial, so surface
+                        // it rather than discarding it silently.
+                        if let Err(e) = config.ledger.decide(
                             &request_id,
                             ApprovalStatus::Expired,
                             Some("timeout".to_string()),
-                        );
+                        ) {
+                            if !matches!(e, ApprovalError::AlreadyDecided { .. }) {
+                                tracing::warn!(
+                                    request_id = %request_id,
+                                    error = %e,
+                                    "failed to mark approval request Expired in ledger; state/audit may be inconsistent"
+                                );
+                            }
+                        }
                         return Ok(RuntimeOutcome::Denied {
                             request_id,
                             reason: DecisionReason::new(
@@ -668,9 +680,14 @@ impl Guard {
 
         // `stderr` is captured in the HandoffResult type for future surface
         // expansion (e.g. SIEM details), but is intentionally not part of the
-        // core ExecutionEvent schema today; flagging presence keeps this
-        // signal from being silently dropped.
-        let _ = stderr_present;
+        // core ExecutionEvent schema today. Emit a debug signal when it is
+        // present so the omission is observable rather than silently dropped.
+        if stderr_present {
+            tracing::debug!(
+                request_id = request_id,
+                "handoff result carried stderr; not included in core ExecutionEvent schema"
+            );
+        }
     }
 }
 

@@ -172,6 +172,7 @@ pub(crate) fn is_always_blocked_ip(ip: &IpAddr) -> bool {
                 || v4.is_broadcast()
                 || v4.is_multicast()
                 || v4.is_private()
+                || is_v4_shared_or_benchmark(v4)
         }
         IpAddr::V6(v6) => {
             // IPv4-mapped (`::ffff:a.b.c.d`), IPv4-compatible (`::a.b.c.d`),
@@ -189,6 +190,22 @@ pub(crate) fn is_always_blocked_ip(ip: &IpAddr) -> bool {
     }
 }
 
+/// IPv4 ranges that `std`'s helpers do not cover but that must never be
+/// reachable from the outbound HTTP executor:
+///   * `100.64.0.0/10` — RFC 6598 shared/CGNAT space, routed to internal
+///     services and proxies in some cloud/ISP environments.
+///   * `198.18.0.0/15` — RFC 2544 benchmarking range.
+///   * `0.0.0.0/8` — RFC 1122 "this network"; `is_unspecified()` only catches
+///     the single `0.0.0.0` address, not the whole block.
+///
+/// Closes 2026-06-01 MEDIUM (SSRF deny-list gap).
+fn is_v4_shared_or_benchmark(v4: &Ipv4Addr) -> bool {
+    let o = v4.octets();
+    (o[0] == 100 && (o[1] & 0xc0) == 64) // 100.64.0.0/10
+        || (o[0] == 198 && (o[1] & 0xfe) == 18) // 198.18.0.0/15
+        || o[0] == 0 // 0.0.0.0/8
+}
+
 pub(crate) fn is_ipv6_link_local(ip: &Ipv6Addr) -> bool {
     (ip.segments()[0] & 0xffc0) == 0xfe80
 }
@@ -224,6 +241,17 @@ pub(crate) fn ipv6_extract_embedded_ipv4(v6: &Ipv6Addr) -> Option<Ipv4Addr> {
             (segments[7] & 0xff) as u8,
         ));
     }
+    // 6to4 (`2002::/16`, RFC 3056): the embedded IPv4 is the two segments after
+    // the `2002` prefix, so `2002:AABB:CCDD::/48` carries `AA.BB.CC.DD`.
+    // Closes 2026-06-01 MEDIUM (embedded-private 6to4 SSRF gap).
+    if segments[0] == 0x2002 {
+        return Some(Ipv4Addr::new(
+            (segments[1] >> 8) as u8,
+            (segments[1] & 0xff) as u8,
+            (segments[2] >> 8) as u8,
+            (segments[2] & 0xff) as u8,
+        ));
+    }
     None
 }
 
@@ -473,6 +501,56 @@ mod tests {
         assert!(!is_always_blocked_ip(&ip("64:ff9b::8.8.8.8")));
     }
 
+    // ── 2026-06-01 MEDIUM: SSRF deny-list gaps (CGNAT / benchmark /
+    // this-network / 6to4) ─────────────────────────────────────────────────
+
+    #[test]
+    fn blocks_rfc6598_cgnat_shared_space() {
+        // 100.64.0.0/10 spans the 100.64.x.x .. 100.127.x.x second octet.
+        assert!(is_always_blocked_ip(&ip("100.64.0.1")));
+        assert!(is_always_blocked_ip(&ip("100.100.0.1")));
+        assert!(is_always_blocked_ip(&ip("100.127.255.254")));
+    }
+
+    #[test]
+    fn allows_just_outside_cgnat_range() {
+        // 100.63.x.x and 100.128.x.x are public — must remain reachable.
+        assert!(!is_always_blocked_ip(&ip("100.63.255.255")));
+        assert!(!is_always_blocked_ip(&ip("100.128.0.1")));
+    }
+
+    #[test]
+    fn blocks_rfc2544_benchmark_range() {
+        // 198.18.0.0/15 covers 198.18.x.x and 198.19.x.x.
+        assert!(is_always_blocked_ip(&ip("198.18.0.1")));
+        assert!(is_always_blocked_ip(&ip("198.19.255.254")));
+        // 198.17.x.x and 198.20.x.x are outside the block.
+        assert!(!is_always_blocked_ip(&ip("198.17.0.1")));
+        assert!(!is_always_blocked_ip(&ip("198.20.0.1")));
+    }
+
+    #[test]
+    fn blocks_this_network_zero_slash_eight() {
+        // The whole 0.0.0.0/8 block, not just the unspecified 0.0.0.0.
+        assert!(is_always_blocked_ip(&ip("0.1.2.3")));
+        assert!(is_always_blocked_ip(&ip("0.255.255.255")));
+    }
+
+    #[test]
+    fn blocks_6to4_embedded_private_ipv4() {
+        // 2002:AABB:CCDD::/48 routes to embedded AA.BB.CC.DD on 6to4 stacks.
+        // 2002:0a00:0001:: → 10.0.0.1 (RFC1918); 2002:a9fe:a9fe:: → metadata.
+        assert!(is_always_blocked_ip(&ip("2002:0a00:0001::")));
+        assert!(is_always_blocked_ip(&ip("2002:7f00:0001::"))); // 127.0.0.1
+        assert!(is_always_blocked_ip(&ip("2002:a9fe:a9fe::"))); // 169.254.169.254
+    }
+
+    #[test]
+    fn allows_6to4_to_public_ipv4() {
+        // 2002:0808:0808:: → 8.8.8.8 (public) must stay reachable.
+        assert!(!is_always_blocked_ip(&ip("2002:0808:0808::")));
+    }
+
     // ── 2026-05-25-2 HIGH: payload_declares_mutation_http silent-failure ───
     //
     // Until this fix the function returned `false` on JSON parse failure

@@ -726,12 +726,11 @@ fn audit_event_jsonl_is_valid_json() {
     use agent_guard_core::AuditEvent;
     let tool = Tool::ReadFile;
     let decision = GuardDecision::Deny {
-        reason: agent_guard_sdk::DecisionReason {
-            code: agent_guard_sdk::DecisionCode::DeniedByRule,
-            message: "test".to_string(),
-            details: None,
-            matched_rule: Some("tools.read_file.deny[0]".to_string()),
-        },
+        reason: agent_guard_sdk::DecisionReason::new(
+            agent_guard_sdk::DecisionCode::DeniedByRule,
+            "test",
+        )
+        .matched_rule("tools.read_file.deny[0]"),
     };
     let event = AuditEvent::from_decision(
         "req-3".to_string(),