From 9f579162d18aed837e5de39d105d176f218cc337 Mon Sep 17 00:00:00 2001 From: archipelago Date: Fri, 7 Aug 2026 06:14:15 -0400 Subject: [PATCH] fix(assistant): network_status strips WAN IP + Wi-Fi SSID from model context (S1) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The Network permission's Settings label promises 'no IP addresses', and the browser-side broker honours it — but the node-side tool forwarded network.diagnostics verbatim, so a granted Network category sent the node's WAN IP and SSID (both location-identifying) to cloud model backends. Strip both at the tool boundary; NAT/UPnP/Tor/DNS connectivity shape stays. Pure helper + unit test (123 assistant tests green). Co-Authored-By: Claude --- core/archipelago/src/assistant/tools.rs | 51 +++++++++++++++++++++++-- 1 file changed, 48 insertions(+), 3 deletions(-) diff --git a/core/archipelago/src/assistant/tools.rs b/core/archipelago/src/assistant/tools.rs index f9f5cfe8..e4b61bbf 100644 --- a/core/archipelago/src/assistant/tools.rs +++ b/core/archipelago/src/assistant/tools.rs @@ -352,8 +352,10 @@ pub fn bitcoin_status_tool() -> ToolDef { } /// `network_status` — category `Network`, read-only. Visibility + -/// diagnostics — no IP addresses beyond what diagnostics already surfaces -/// to the operator elsewhere in the UI. +/// diagnostics with the WAN IP and Wi-Fi SSID stripped at the tool +/// boundary (`strip_network_pii`): the permission's operator-facing label +/// promises "no IP addresses", and this result enters model context — +/// including cloud backends — so the promise must hold here too. pub fn network_status_tool() -> ToolDef { ToolDef { name: "network_status", @@ -368,6 +370,22 @@ pub fn network_status_tool() -> ToolDef { } } +/// Remove operator-identifying fields from a `network.diagnostics` result +/// before it enters model context. The Network permission's Settings label +/// promises "Connection status, peer count — no IP addresses or keys", and +/// the browser-side broker honours that (booleans only); a tool result that +/// carries the WAN IP or Wi-Fi SSID (both location-identifying) to a cloud +/// backend would make that consent label false. NAT type, UPnP, Tor and DNS +/// booleans stay — they are connectivity shape, not identity. +fn strip_network_pii(diagnostics: serde_json::Value) -> serde_json::Value { + let mut d = diagnostics; + if let Some(obj) = d.as_object_mut() { + obj.remove("wan_ip"); + obj.remove("wifi_ssid"); + } + d +} + /// `mesh_status` — category `Network`, read-only. Mesh radio status, /// device info, peer count. pub fn mesh_status_tool() -> ToolDef { @@ -716,7 +734,7 @@ pub async fn dispatch(name: &str, args: &ToolArgs, handler: &RpcHandler) -> Resu .assistant_dispatch_tool("network.diagnostics", None) .await .map_err(|e| format!("tool execution failed: {e}"))?; - Ok(json!({ "visibility": visibility, "diagnostics": diagnostics })) + Ok(json!({ "visibility": visibility, "diagnostics": strip_network_pii(diagnostics) })) } "mesh_status" => handler .assistant_dispatch_tool("mesh.status", None) @@ -1316,4 +1334,31 @@ mod tests { "expected exactly 4 destructive tools: app_start, app_stop, app_restart, settings_set" ); } + + /// The `network_status` tool result enters model context — including + /// cloud backends — under a permission whose operator-facing label + /// promises "no IP addresses". The strip must remove the WAN IP and + /// Wi-Fi SSID while keeping the connectivity shape the tool + /// description advertises (NAT type, UPnP, Tor, DNS). + #[test] + fn network_status_strips_wan_ip_and_wifi_ssid() { + let stripped = strip_network_pii(json!({ + "wan_ip": "203.0.113.7", + "nat_type": "full_cone", + "upnp_available": true, + "tor_connected": true, + "dns_working": true, + "recommendations": [], + "wifi_ssid": "Pretty Fly for a Wi-Fi", + })); + let obj = stripped.as_object().expect("diagnostics stays an object"); + assert!(!obj.contains_key("wan_ip"), "WAN IP must not enter model context"); + assert!(!obj.contains_key("wifi_ssid"), "Wi-Fi SSID must not enter model context"); + for kept in ["nat_type", "upnp_available", "tor_connected", "dns_working", "recommendations"] { + assert!(obj.contains_key(kept), "connectivity field {kept} must survive"); + } + // A result with neither key (e.g. offline diagnostics) passes through untouched. + let already_clean = json!({ "nat_type": null, "dns_working": false }); + assert_eq!(strip_network_pii(already_clean.clone()), already_clean); + } }