fix(security): stop exporting AWS secrets into the environment; frame bridged coding-agent tool output

crates/biorouter-server/src/routes/tool_bridge.rs

@@ -135,7 +135,12 @@ async fn call_tool(
     };
 
     // The child is answered with the model's view of the result: the blocks a
-    // model is sent, unannotated (`bridge::child_view`, QA-E F4).
+    // model is sent, unannotated (`bridge::child_view`, QA-E F4), and framed as
+    // untrusted data + scanned for injection and PII first (A2). The child is a
+    // whole agent reading third-party bytes, so the guardrail applies to it for
+    // the same reason it applies to the parent model; `call_for_child` is the
+    // bridge's half of that funnel, and the copy it keeps for the transcript is
+    // framed too, so a coding agent's transcript matches every other provider's.
     match grant.call_for_child(call, child_call_id).await {
         Ok(result) => rpc_ok(
             id,

crates/biorouter-server/tests/tool_bridge_routes.rs

@@ -152,6 +152,13 @@ fn marker_grant(marker: &str) -> bridge::BridgeGrant {
 /// The shape is `developer__shell`'s. Handing the child both blocks made it read
 /// every result twice, and the user block's `priority: 0.0` made codex-cli fail
 /// the call outright with "Unexpected response type".
+///
+/// **And A2 at the wire**: that view is now the *framed* result, and the copy
+/// kept for the transcript is the framed one too — the same bytes
+/// `Agent::integrate_tool_result` would have stored for any other provider.
+/// Before the fix this route answered raw text and stored raw text, so the same
+/// `date` call read framed under `versa_azure` and unframed under both coding
+/// agents.
 #[tokio::test]
 #[serial_test::serial]
 async fn the_child_is_answered_with_the_models_view_and_the_full_result_is_kept() {
@@ -166,6 +173,22 @@ async fn the_child_is_answered_with_the_models_view_and_the_full_result_is_kept(
             .with_audience(vec![rmcp::model::Role::User])
             .with_priority(0.0),
     ]);
+
+    // A2, at the HTTP layer: what a NON-bridged provider stores for this call —
+    // `Agent::integrate_tool_result`'s own expression, run with the same mode the
+    // grant sampled. Deriving the expectation rather than writing it out is what
+    // makes this row say the thing that matters ("the two providers agree")
+    // instead of pinning one spelling of the frame, and keeps it correct on a
+    // machine whose `BIOROUTER_TOOL_OUTPUT_GUARDRAIL` differs from CI's.
+    let mode = biorouter::guardrails::tool_output::ToolOutputGuardrailMode::from_config();
+    let (guarded, _) = biorouter::guardrails::tool_output::guard_tool_result(
+        Ok(shell.clone()),
+        Some("spokeagent__query_knowledge_graph"),
+        mode,
+    );
+    let guarded = guarded.expect("the guardrail passes an Ok through as Ok");
+    let expected_child = bridge::child_view(&guarded);
+
     let lease = bridge::issue(fixed_result_grant(shell.clone())).expect("issued");
     let nonce = lease.url().rsplit('/').next().expect("a nonce").to_string();
 
@@ -205,17 +228,30 @@ async fn the_child_is_answered_with_the_models_view_and_the_full_result_is_kept(
 
         assert_eq!(
             body["result"]["content"],
-            json!([{ "type": "text", "text": "Thu Sep 11" }]),
-            "the child gets the model's block, unannotated: {body}"
+            serde_json::to_value(&expected_child.content).expect("serialisable content"),
+            "the child gets the model's block, unannotated and framed exactly as \
+             a non-bridged provider's would be: {body}"
         );
         assert!(
             !body.to_string().contains("priority"),
             "codex-cli cannot parse a `priority` annotation: {body}"
         );
+        // Not vacuous: unless the operator turned the guardrail off, the frame
+        // really is on the wire. Before A2 this route answered raw text.
+        if mode != biorouter::guardrails::tool_output::ToolOutputGuardrailMode::Off {
+            assert!(
+                body["result"]["content"][0]["text"]
+                    .as_str()
+                    .unwrap_or_default()
+                    .starts_with(biorouter::guardrails::tool_output::TOOL_OUTPUT_FRAME_OPEN),
+                "the child agent read unframed tool output over the bridge: {body}"
+            );
+        }
         assert_eq!(
             bridge::take_recorded_result(lease.url(), child_call_id),
-            Some(shell.clone()),
-            "the full result is kept for the transcript under {child_call_id}"
+            Some(guarded.clone()),
+            "the full result is kept for the transcript under {child_call_id}, \
+             framed the way every other provider's transcript entry is"
         );
     }
 }

crates/biorouter/src/guardrails/tool_output.rs

@@ -1305,35 +1305,72 @@ mod tests {
         assert!(out.is_err());
     }
 
-    /// The frame is only unconditional if the choke point is unconditional.
+    /// The frame is only unconditional if every choke point is.
     ///
-    /// [`guard_tool_result`] is called from exactly one place,
-    /// `Agent::integrate_tool_result`, which every completed tool call passes
-    /// through on its way into the conversation. A second tool-result path that
-    /// forgot to call it would be a silent hole, so the count is asserted here
-    /// rather than left to reviewers. If this fails because you added a call
-    /// site, the right fix is usually to route through the existing funnel, not
-    /// to bump the number.
+    /// There are **two**, because a tool result reaches a model context by two
+    /// structurally different routes, and each has exactly one funnel:
+    ///
+    /// | Route | Funnel |
+    /// | --- | --- |
+    /// | the parent model's own calls | `Agent::integrate_tool_result` |
+    /// | a coding agent's child, over the MCP bridge | `BridgeGrant::call_for_child` |
+    ///
+    /// A bridged call never enters the agent's turn loop — the vendor CLI calls
+    /// `POST /tool_bridge/{nonce}` and the provider lifts the kept result
+    /// straight into the transcript — so `integrate_tool_result` alone left the
+    /// child agent reading raw, unscanned third-party text and stored raw text
+    /// in the transcript where every other provider stored a frame (A2).
+    ///
+    /// A third tool-result path that forgot to call this would be the same
+    /// silent hole again, so the counts are asserted here rather than left to
+    /// reviewers. If this fails because you added a call site, the right fix is
+    /// usually to route through one of the two existing funnels, not to bump a
+    /// number.
     #[test]
-    fn the_guardrail_has_exactly_one_call_site() {
-        let agent_rs = include_str!("../agents/agent.rs");
-        let calls = agent_rs
-            .matches("guardrails::tool_output::guard_tool_result(")
-            .count();
-        assert_eq!(
-            calls, 1,
-            "expected exactly one guard_tool_result call site in agent.rs, found {calls}"
-        );
-        // And it must be inside the result-integration funnel, not somewhere a
-        // path could branch around.
-        let funnel = agent_rs
-            .split("async fn integrate_tool_result(")
-            .nth(1)
-            .expect("integrate_tool_result must exist");
-        assert!(
-            funnel.contains("guardrails::tool_output::guard_tool_result("),
-            "the call site moved out of integrate_tool_result"
-        );
+    fn the_guardrail_has_one_call_site_in_each_of_its_two_funnels() {
+        for (file, source, funnel, signature) in [
+            (
+                "agents/agent.rs",
+                include_str!("../agents/agent.rs"),
+                "integrate_tool_result",
+                "async fn integrate_tool_result(",
+            ),
+            (
+                "providers/coding_agent/bridge.rs",
+                include_str!("../providers/coding_agent/bridge.rs"),
+                "call_for_child",
+                "pub async fn call_for_child(",
+            ),
+        ] {
+            // ⚠ The count is over the file's PRODUCTION half only. `bridge.rs`'s
+            // own suite calls the guardrail directly, to build the frame a
+            // non-bridged provider stores and compare the two — a test proving
+            // the funnel works must not read as a second funnel. Each file has
+            // exactly one `mod tests {` at column 0.
+            let production = source
+                .split("\nmod tests {")
+                .next()
+                .expect("split always yields a first part");
+            // `bridge.rs` imports the function by name and `agent.rs` spells
+            // the whole path, so the needle is the bare name — and it carries
+            // its opening paren, which is what keeps an import or a doc link
+            // from being counted as a call.
+            let calls = production.matches("guard_tool_result(").count();
+            assert_eq!(
+                calls, 1,
+                "expected exactly one guard_tool_result call site in {file}, found {calls}"
+            );
+            // And it must be inside the funnel, not somewhere a path could
+            // branch around.
+            let body = production
+                .split(signature)
+                .nth(1)
+                .unwrap_or_else(|| panic!("{funnel} must exist in {file}"));
+            assert!(
+                body.contains("guard_tool_result("),
+                "the call site moved out of {funnel} in {file}"
+            );
+        }
     }
 
     // ── the frame rewrites `text`, and nothing else ──