fix(buzz-agent): reject structurally invalid permission response frames

wire::classify normalized malformed JSON-RPC responses into well-formed
allow results before the fail-closed broker could see them. A present
non-string `method` collapsed to "no method" via `as_str`, and a frame
with both `result` and `error` forwarded `result` unconditionally — so a
`selected`/`allow_once` payload in either shape was laundered into an
approval upstream of every authorization check.

classify now forwards `result` only for a structurally valid response:
no `method` member and exactly one of `result`/`error`. Any other shape
normalizes to Null, which the broker denies. The adversarial-response
tests attacked the result payload; these attack the frame structure.

Co-authored-by: Will Pfleger <pfleger.will@gmail.com>
Signed-off-by: Will Pfleger <pfleger.will@gmail.com>

crates/buzz-agent/src/permission.rs

@@ -519,6 +519,74 @@ mod tests {
         assert_eq!(broker.available_permits(), 4);
     }
 
+    // ── Malformed response frames deny (Carl's review) ────────────────────────
+
+    /// Route Carl's frame through the real `classify` → `deliver` path against a
+    /// live waiter and assert the tool is denied. Delivers on the exact id the
+    /// broker minted, so the only reason the waiter denies is that `classify`
+    /// refused to forward the ambiguous/malformed `result`. `provider_id`
+    /// carries which shape is under test so a failure names the mutant.
+    async fn assert_malformed_frame_denies(provider_id: &str, frame: Value) {
+        let broker = Arc::new(PermissionBroker::new(4, LONG));
+        let (tx, mut rx) = mpsc::channel(8);
+        let (_cancel_tx, mut cancel_rx) = watch::channel(false);
+
+        let b = Arc::clone(&broker);
+        let call = tool_call();
+        let task = tokio::spawn(async move {
+            b.request_permission(&tx, 2, "ses_a", &call, &mut cancel_rx)
+                .await
+        });
+
+        let id = next_request_id(&mut rx).await;
+        // Stamp the broker's minted id onto Carl's frame, then classify it
+        // exactly as the dispatch loop would before handing `result` to deliver.
+        let mut frame = frame;
+        frame["id"] = id.clone();
+        match crate::wire::classify(&frame) {
+            crate::wire::Inbound::Response { id, result } => broker.deliver(&id, result),
+            other => panic!("[{provider_id}] expected Response, got {other:?}"),
+        }
+
+        assert_eq!(
+            task.await.unwrap(),
+            PermissionDecision::Denied(PERMISSION_DENIED_MSG),
+            "[{provider_id}] malformed frame must deny, not authorize the tool",
+        );
+        assert_eq!(broker.pending_count(), 0);
+        assert_eq!(broker.available_permits(), 4);
+    }
+
+    /// Carl frame #1: `result` (well-formed `selected`/`allow_once`) AND `error`
+    /// both present. The tool must not run — the ambiguous frame denies.
+    #[tokio::test(flavor = "multi_thread", worker_threads = 2)]
+    async fn test_frame_with_result_and_error_denies_tool() {
+        assert_malformed_frame_denies(
+            "result+error",
+            json!({
+                "jsonrpc": "2.0",
+                "result": { "outcome": { "outcome": "selected", "optionId": ALLOW_OPTION_ID } },
+                "error": { "code": -32603, "message": "internal" },
+            }),
+        )
+        .await;
+    }
+
+    /// Carl frame #2: present non-string `method: 7` alongside a well-formed
+    /// `selected` `result`. It is not a valid response — the tool must not run.
+    #[tokio::test(flavor = "multi_thread", worker_threads = 2)]
+    async fn test_frame_with_non_string_method_denies_tool() {
+        assert_malformed_frame_denies(
+            "non-string-method",
+            json!({
+                "jsonrpc": "2.0",
+                "method": 7,
+                "result": { "outcome": { "outcome": "selected", "optionId": ALLOW_OPTION_ID } },
+            }),
+        )
+        .await;
+    }
+
     // ── Stale / unknown id ignored ────────────────────────────────────────────
 
     #[tokio::test(flavor = "multi_thread", worker_threads = 2)]

crates/buzz-agent/src/wire.rs

@@ -30,8 +30,12 @@ pub enum Inbound {
     /// A bare JSON-RPC response (id present, no method) — the client's answer
     /// to a request buzz-agent issued. Today the only such request is
     /// `session/request_permission`. `result` carries the JSON-RPC `result`
-    /// field, or `Null` for an `error`/malformed response; every non-`selected`
-    /// shape fails the broker's authorization predicate and denies.
+    /// field ONLY when the frame is a structurally valid response — no `method`
+    /// member and exactly one of `result`/`error`. Any malformed shape (present
+    /// non-string `method`, both `result` and `error`, or neither) is normalized
+    /// to `Null` so a possibly-`selected` payload is never laundered into an
+    /// approval; every non-`selected` shape fails the broker's authorization
+    /// predicate and denies.
     Response {
         id: Value,
         result: Value,
@@ -118,13 +122,26 @@ pub fn classify(msg: &Value) -> Inbound {
         },
         (Some(m), None) => Inbound::Notification { method: m, params },
         // Bare responses (id present, no method) answer a request buzz-agent
-        // issued — today only `session/request_permission`. Route the `result`
-        // (or `Null` on an `error`/absent result) to the permission broker,
-        // which matches it to a live correlation id or ignores it if unknown.
-        (None, Some(id)) => Inbound::Response {
-            id,
-            result: msg.get("result").cloned().unwrap_or(Value::Null),
-        },
+        // issued — today only `session/request_permission`. Route to the
+        // permission broker, which matches a live correlation id or ignores an
+        // unknown one. Forward the `result` ONLY when the frame is a
+        // structurally valid response — the exactly-one-of invariant: no
+        // `method` member at all, and `result` present with `error` absent. A
+        // present non-string `method` (which `as_str` above collapsed to
+        // `None`), both `result` and `error`, or neither is malformed; forward
+        // `Null` so the broker fails closed (deny) rather than laundering a
+        // possibly-`selected` payload into an approval.
+        (None, Some(id)) => {
+            let well_formed = msg.get("method").is_none()
+                && msg.get("result").is_some()
+                && msg.get("error").is_none();
+            let result = if well_formed {
+                msg.get("result").cloned().unwrap_or(Value::Null)
+            } else {
+                Value::Null
+            };
+            Inbound::Response { id, result }
+        }
         (None, None) => Inbound::Invalid {
             id: Value::Null,
             code: INVALID_REQUEST,
@@ -752,6 +769,58 @@ mod tests {
         }
     }
 
+    /// Carl's frame #1: a response carrying BOTH `result` and `error` is
+    /// structurally ambiguous and must NOT deliver the `result`, even when that
+    /// `result` is a well-formed `selected`/`allow_once` payload. The wire layer
+    /// normalizes it to `Null` so the broker denies instead of the frame
+    /// laundering an approval upstream of every fail-closed check.
+    #[test]
+    fn classify_response_with_both_result_and_error_denies() {
+        let msg = json!({
+            "jsonrpc": "2.0",
+            "id": "perm-3",
+            "result": { "outcome": { "outcome": "selected", "optionId": ALLOW_OPTION_ID } },
+            "error": { "code": -32603, "message": "internal" },
+        });
+        match classify(&msg) {
+            Inbound::Response { id, result } => {
+                assert_eq!(id, json!("perm-3"));
+                assert_eq!(
+                    result,
+                    Value::Null,
+                    "result+error is malformed → Null → deny, never forward the allow payload"
+                );
+            }
+            other => panic!("expected Response, got {other:?}"),
+        }
+    }
+
+    /// Carl's frame #2: a present but non-string `method` is NOT "method
+    /// absent". `as_str` collapses `method: 7` to `None`, which lands the frame
+    /// in the response arm, but it is not a valid response and must not forward
+    /// its `result` (a well-formed `selected` payload here). The structural
+    /// check sees the present `method` member and normalizes to `Null` → deny.
+    #[test]
+    fn classify_response_with_non_string_method_denies() {
+        let msg = json!({
+            "jsonrpc": "2.0",
+            "id": "perm-3",
+            "method": 7,
+            "result": { "outcome": { "outcome": "selected", "optionId": ALLOW_OPTION_ID } },
+        });
+        match classify(&msg) {
+            Inbound::Response { id, result } => {
+                assert_eq!(id, json!("perm-3"));
+                assert_eq!(
+                    result,
+                    Value::Null,
+                    "present non-string method → not a valid response → Null → deny"
+                );
+            }
+            other => panic!("expected Response, got {other:?}"),
+        }
+    }
+
     // ── send_checked: observable wire closure ────────────────────────────────
 
     /// `send_checked` reports `Ok` while the writer's receiver is alive and