test(security): add unit tests for build_isolated_env and detect_collisions

Author: bug-ops

crates/zeph-mcp/src/security.rs

@@ -327,4 +327,72 @@ mod tests {
         assert!(err.to_string().contains("LD_PRELOAD"));
         assert!(err.to_string().contains("blocked"));
     }
+
+    // --- build_isolated_env ---
+
+    #[test]
+    fn build_isolated_env_base_vars_present_when_set() {
+        // PATH and HOME are almost always set in CI and local environments.
+        // We test the ones most likely to be present.
+        let result = build_isolated_env(&HashMap::new());
+        // At minimum, PATH should appear (always set in any shell environment).
+        // We do a soft check: the result must be a strict subset of BASE_ENV_VARS + server_env.
+        for key in result.keys() {
+            assert!(
+                BASE_ENV_VARS.contains(&key.as_str()),
+                "unexpected key in isolated env: {key}"
+            );
+        }
+    }
+
+    #[test]
+    fn build_isolated_env_non_base_vars_absent() {
+        // build_isolated_env only propagates variables listed in BASE_ENV_VARS.
+        // Any key in the result must be in BASE_ENV_VARS (or in server_env, which is empty here).
+        let result = build_isolated_env(&HashMap::new());
+        for key in result.keys() {
+            assert!(
+                BASE_ENV_VARS.contains(&key.as_str()),
+                "unexpected key in isolated env (not in BASE_ENV_VARS and not in server_env): {key}"
+            );
+        }
+    }
+
+    #[test]
+    fn build_isolated_env_server_env_merged() {
+        let mut server_env = HashMap::new();
+        server_env.insert("MY_TOOL_TOKEN".into(), "tok_abc".into());
+        let result = build_isolated_env(&server_env);
+        assert_eq!(
+            result.get("MY_TOOL_TOKEN").map(String::as_str),
+            Some("tok_abc"),
+            "server-declared env must appear in isolated env"
+        );
+    }
+
+    #[test]
+    fn build_isolated_env_server_env_can_override_base_var() {
+        // Operator can pin a specific PATH — server_env merges after base vars.
+        let mut server_env = HashMap::new();
+        server_env.insert("PATH".into(), "/usr/local/bin:/custom/bin".into());
+        let result = build_isolated_env(&server_env);
+        assert_eq!(
+            result.get("PATH").map(String::as_str),
+            Some("/usr/local/bin:/custom/bin"),
+            "server-declared PATH must override the base PATH"
+        );
+    }
+
+    #[test]
+    fn build_isolated_env_xdg_vars_in_base() {
+        // XDG_RUNTIME_DIR and XDG_CONFIG_HOME must be in BASE_ENV_VARS.
+        assert!(
+            BASE_ENV_VARS.contains(&"XDG_RUNTIME_DIR"),
+            "XDG_RUNTIME_DIR must be in BASE_ENV_VARS"
+        );
+        assert!(
+            BASE_ENV_VARS.contains(&"XDG_CONFIG_HOME"),
+            "XDG_CONFIG_HOME must be in BASE_ENV_VARS"
+        );
+    }
 }

crates/zeph-mcp/src/tool.rs

@@ -551,4 +551,84 @@ mod tests {
         assert!(meta.capabilities.contains(&CapabilityClass::Network));
         assert_eq!(meta.data_sensitivity, DataSensitivity::Medium);
     }
+
+    // --- detect_collisions ---
+
+    fn trust_map(
+        entries: &[(&str, crate::manager::McpTrustLevel)],
+    ) -> std::collections::HashMap<String, crate::manager::McpTrustLevel> {
+        entries.iter().map(|(k, v)| ((*k).to_owned(), *v)).collect()
+    }
+
+    #[test]
+    fn detect_collisions_no_collision_happy_path() {
+        let tools = vec![
+            make_tool("server_a", "tool_one"),
+            make_tool("server_b", "tool_two"),
+        ];
+        let tm = trust_map(&[
+            ("server_a", crate::manager::McpTrustLevel::Trusted),
+            ("server_b", crate::manager::McpTrustLevel::Trusted),
+        ]);
+        let cols = detect_collisions(&tools, &tm);
+        assert!(cols.is_empty(), "different sanitized_ids must not collide");
+    }
+
+    #[test]
+    fn detect_collisions_different_trust_collision() {
+        // "a.b:c" and "a:b_c" both sanitize to "a_b_c" — collision across trust levels.
+        let tool_a = make_tool("a.b", "c");
+        let tool_b = make_tool("a", "b_c");
+        let tm = trust_map(&[
+            ("a.b", crate::manager::McpTrustLevel::Trusted),
+            ("a", crate::manager::McpTrustLevel::Untrusted),
+        ]);
+        let cols = detect_collisions(&[tool_a, tool_b], &tm);
+        assert_eq!(cols.len(), 1);
+        let col = &cols[0];
+        assert_eq!(col.sanitized_id, "a_b_c");
+        assert_eq!(col.server_a, "a.b");
+        assert_eq!(col.server_b, "a");
+        assert_eq!(col.trust_a, crate::manager::McpTrustLevel::Trusted);
+        assert_eq!(col.trust_b, crate::manager::McpTrustLevel::Untrusted);
+    }
+
+    #[test]
+    fn detect_collisions_same_trust_collision() {
+        // Both servers are Untrusted and share a sanitized_id.
+        let tool_a = make_tool("a.b", "c");
+        let tool_b = make_tool("a", "b_c");
+        let tm = trust_map(&[
+            ("a.b", crate::manager::McpTrustLevel::Untrusted),
+            ("a", crate::manager::McpTrustLevel::Untrusted),
+        ]);
+        let cols = detect_collisions(&[tool_a, tool_b], &tm);
+        assert_eq!(cols.len(), 1);
+        assert_eq!(cols[0].trust_a, crate::manager::McpTrustLevel::Untrusted);
+        assert_eq!(cols[0].trust_b, crate::manager::McpTrustLevel::Untrusted);
+    }
+
+    #[test]
+    fn detect_collisions_multiple_collisions_reported() {
+        // Three tools, all sharing the same sanitized_id "srv_tool".
+        let t1 = make_tool("srv", "tool");
+        let t2 = make_tool("srv.x", "tool"); // "srv_x_tool" — different, no collision with t1
+        let t3 = make_tool("srv", "tool"); // exact duplicate of t1 — collision
+        let tm = trust_map(&[("srv", crate::manager::McpTrustLevel::Untrusted)]);
+        let cols = detect_collisions(&[t1, t2, t3], &tm);
+        // t1 and t3 share "srv_tool"; t2 is "srv_x_tool" — one collision
+        assert_eq!(cols.len(), 1);
+        assert_eq!(cols[0].sanitized_id, "srv_tool");
+    }
+
+    #[test]
+    fn detect_collisions_unknown_server_defaults_to_untrusted() {
+        let tool_a = make_tool("a.b", "c");
+        let tool_b = make_tool("a", "b_c");
+        // No entries in trust_map — both should default to Untrusted.
+        let cols = detect_collisions(&[tool_a, tool_b], &std::collections::HashMap::new());
+        assert_eq!(cols.len(), 1);
+        assert_eq!(cols[0].trust_a, crate::manager::McpTrustLevel::Untrusted);
+        assert_eq!(cols[0].trust_b, crate::manager::McpTrustLevel::Untrusted);
+    }
 }