fix(profile): classify every tool; boundary tests fail on unclassified ones

Sources/InstanceMCPCore/ToolCatalog.swift

@@ -0,0 +1,11 @@
+import Foundation
+
+/// Every local tool this daemon registers, in one place. `main.swift` serves this
+/// list and `ProfileBoundaryTests` checks it, so a tool cannot be served without
+/// also being classified (`ToolProfile.localToolClass`).
+public enum ToolCatalog {
+    public static func local(agentVersion: String) -> [any Tool] {
+        [SysInfoTool(agentVersion: agentVersion), ExecTool(), ExecStartTool(), ExecPollTool(),
+         ExecListTool(), ExecCancelTool(), ScreenshotTool(), MouseTool(), KeyTool(), OsascriptTool()]
+    }
+}

Sources/InstanceMCPCore/ToolProfile.swift

@@ -44,9 +44,39 @@ public enum ToolProfile: String, Codable, Sendable, CaseIterable {
     /// upstream — is denied under `observe` until it is listed here.
     public static let observeTools: Set<String> = ["sys_info", "screenshot"]
 
-    /// Local tools that reach a shell as the desktop user, directly or by driving
-    /// the GUI. Any profile with `isShellEquivalent == false` must allow none.
-    public static let shellCapableTools: Set<String> = ["osascript", "key", "mouse"]
+    /// What a tool can do, as far as a boundary is concerned.
+    public enum ToolClass: String, Sendable, CaseIterable {
+        /// Reads only; changes nothing.
+        case observe
+        /// Changes state, but cannot run code as the desktop user.
+        case act
+        /// Reaches the desktop user's shell, directly or by driving the GUI.
+        case shell
+    }
+
+    /// Every local tool, classified. **Exhaustive by test**: a tool in
+    /// `ToolCatalog.local` without an entry here fails `ProfileBoundaryTests`,
+    /// so a new tool cannot slip past the boundary check by simply not being
+    /// listed as dangerous. Classify conservatively: if it can open, type into,
+    /// or script anything that runs code, it is `.shell`.
+    public static let localToolClass: [String: ToolClass] = [
+        "sys_info": .observe,
+        "screenshot": .observe,
+        "exec_poll": .observe,     // reads a job's state and output; starts nothing
+        "exec_list": .observe,
+        "exec_cancel": .act,       // signals a job's process group
+        "exec": .shell,
+        "exec_start": .shell,
+        "osascript": .shell,       // `do shell script`, JXA, NSTask
+        "key": .shell,             // can type into a terminal
+        "mouse": .shell,           // can open one
+    ]
+
+    /// Local tools that reach a shell as the desktop user. Any profile with
+    /// `isShellEquivalent == false` must allow none.
+    public static var shellCapableTools: Set<String> {
+        Set(localToolClass.filter { $0.value == .shell }.keys)
+    }
 
     /// Match is on the tool name.
     public func allows(_ toolName: String) -> Bool {
@@ -57,7 +87,9 @@ public enum ToolProfile: String, Codable, Sendable, CaseIterable {
             if toolName.hasPrefix("browser_") { return Self.desktopBrowserTools.contains(toolName) }
             return true
         case .observe:
-            return Self.observeTools.contains(toolName)
+            // Both: on the allowlist *and* classified as observation, so a tool added
+            // to the allowlist by mistake is still refused unless it changes nothing.
+            return Self.observeTools.contains(toolName) && Self.localToolClass[toolName] == .observe
         }
     }
 

Sources/oab-instance-mcp/main.swift

@@ -137,7 +137,7 @@ let server = MCPServer(
         no screenshots. Fall back to `screenshot` only for things outside the browser.
         """)
         """,
-    tools: [SysInfoTool(agentVersion: version), ExecTool(), ExecStartTool(), ExecPollTool(), ExecListTool(), ExecCancelTool(), ScreenshotTool(), MouseTool(), KeyTool(), OsascriptTool()],
+    tools: ToolCatalog.local(agentVersion: version),
     upstreams: opts.upstreams.map { UpstreamMCP(name: $0.0, url: $0.1, log: log) }
 )
 let attachManager: AttachManager? = opts.attach

Tests/InstanceMCPCoreTests/ProfileBoundaryTests.swift

@@ -8,40 +8,53 @@ import XCTest
 /// that hides `exec*` but keeps those is not narrower in privilege, only in
 /// convenience. These tests make that impossible to claim by accident.
 final class ProfileBoundaryTests: XCTestCase {
-    /// Every local tool this daemon ships, by name, straight from the source so a
-    /// new tool is covered without anyone remembering to list it here.
-    private func shippedLocalToolNames() throws -> Set<String> {
+    /// The tools the daemon actually serves (the same list `main.swift` uses).
+    private var served: [String] { ToolCatalog.local(agentVersion: "test").map(\.name) }
+
+    /// Tool names declared in the Tools sources, as a cross-check that nothing is
+    /// served from outside the catalog.
+    private func declaredToolNames() throws -> Set<String> {
         let dir = URL(fileURLWithPath: #filePath)
             .deletingLastPathComponent().deletingLastPathComponent().deletingLastPathComponent()
             .appendingPathComponent("Sources/InstanceMCPCore/Tools")
         var names = Set<String>()
+        let regex = try NSRegularExpression(pattern: #"\bname\s*=\s*"([a-z_]+)""#)
         for file in try FileManager.default.contentsOfDirectory(atPath: dir.path) where file.hasSuffix(".swift") {
             let text = try String(contentsOf: dir.appendingPathComponent(file), encoding: .utf8)
-            let regex = try NSRegularExpression(pattern: #"let name = "([a-z_]+)""#)
             for m in regex.matches(in: text, range: NSRange(text.startIndex..., in: text)) {
                 if let r = Range(m.range(at: 1), in: text) { names.insert(String(text[r])) }
             }
         }
         return names
     }
 
-    func testTheToolScanSeesTheShellCapableTools() throws {
-        let shipped = try shippedLocalToolNames()
-        for tool in ["exec", "osascript", "key", "mouse", "screenshot", "sys_info"] {
-            XCTAssertTrue(shipped.contains(tool), "tool scan missed \(tool); shipped: \(shipped.sorted())")
-        }
+    /// The fix for "the dangerous list is hand-maintained": classification is
+    /// exhaustive. A new tool that nobody classified fails here instead of being
+    /// silently treated as safe.
+    func testEveryServedToolIsClassified() {
+        let unclassified = served.filter { ToolProfile.localToolClass[$0] == nil }
+        XCTAssertTrue(unclassified.isEmpty,
+                      "classify in ToolProfile.localToolClass (observe / act / shell): \(unclassified.sorted())")
+        let stale = Set(ToolProfile.localToolClass.keys).subtracting(served)
+        XCTAssertTrue(stale.isEmpty, "classified but not served: \(stale.sorted())")
+        XCTAssertEqual(served.count, Set(served).count, "duplicate tool names")
+    }
+
+    func testEveryDeclaredToolIsInTheCatalog() throws {
+        let declared = try declaredToolNames()
+        XCTAssertTrue(declared.isSuperset(of: ["exec", "osascript", "key", "mouse", "screenshot", "sys_info"]),
+                      "source scan broke: \(declared.sorted())")
+        let outside = declared.subtracting(served)
+        XCTAssertTrue(outside.isEmpty, "declared but not in ToolCatalog.local: \(outside.sorted())")
     }
 
     /// The adversary check: a profile that does not declare itself shell-equivalent
-    /// must allow no tool that reaches a shell. Today no such profile exists and
-    /// `desktop` must say so; a future `observe` / `browser` profile is held to it.
-    func testNoProfileClaimsToBeNarrowerThanItIs() throws {
-        let shellReaching = try shippedLocalToolNames().filter {
-            $0.hasPrefix("exec") || ToolProfile.shellCapableTools.contains($0)
-        }
-        XCTAssertFalse(shellReaching.isEmpty)
+    /// must allow no tool classified `.shell`; one that does must actually allow one.
+    func testNoProfileClaimsToBeNarrowerThanItIs() {
+        let shell = served.filter { ToolProfile.localToolClass[$0] == .shell }
+        XCTAssertFalse(shell.isEmpty)
         for profile in ToolProfile.allCases {
-            let reachable = shellReaching.filter(profile.allows)
+            let reachable = shell.filter(profile.allows)
             if !profile.isShellEquivalent {
                 XCTAssertTrue(reachable.isEmpty,
                               "\(profile.rawValue) claims no shell but allows \(reachable.sorted())")
@@ -51,6 +64,12 @@ final class ProfileBoundaryTests: XCTestCase {
         }
     }
 
+    /// `observe` may only hold tools that change nothing.
+    func testObserveAllowsOnlyObserveClassTools() {
+        let wrong = served.filter { ToolProfile.observe.allows($0) && ToolProfile.localToolClass[$0] != .observe }
+        XCTAssertTrue(wrong.isEmpty, "observe allows tools that act: \(wrong.sorted())")
+    }
+
     func testDesktopIsDeclaredShellEquivalent() {
         XCTAssertTrue(ToolProfile.desktop.isShellEquivalent)
         for tool in ["osascript", "key", "mouse"] {
@@ -60,10 +79,9 @@ final class ProfileBoundaryTests: XCTestCase {
     }
 
     /// `observe` is the one real boundary today: look, never act.
-    func testObserveCanOnlyLook() throws {
+    func testObserveCanOnlyLook() {
         XCTAssertFalse(ToolProfile.observe.isShellEquivalent)
-        let shipped = try shippedLocalToolNames()
-        XCTAssertEqual(Set(shipped.filter(ToolProfile.observe.allows)), ["sys_info", "screenshot"])
+        XCTAssertEqual(Set(served.filter(ToolProfile.observe.allows)), ["sys_info", "screenshot"])
         for tool in ["browser_navigate", "browser_snapshot", "browser_take_screenshot", "browser_something_new", "exec", "osascript"] {
             XCTAssertFalse(ToolProfile.observe.allows(tool), "\(tool) is an action, or unknown")
         }

poc/reverse-attach-linux/src/mcp.rs

@@ -185,11 +185,54 @@ pub(crate) fn upstream_tool_allowed(name: &str, profile: &str) -> bool {
 /// Local tools `observe` may call: look, never act (instance-mcp#45).
 pub(crate) const OBSERVE_TOOLS: &[&str] = &["sys_info", "screenshot"];
 
+/// What a tool can do, as far as a boundary is concerned (same classes as the
+/// Swift `ToolProfile.ToolClass`).
+#[derive(Clone, Copy, Debug, PartialEq, Eq)]
+pub(crate) enum ToolClass {
+    /// Reads only; changes nothing.
+    Observe,
+    /// Changes state, but cannot run code as the desktop user.
+    #[allow(dead_code)]
+    Act,
+    /// Reaches the desktop user's shell, directly or by driving the GUI.
+    #[cfg_attr(not(test), allow(dead_code))]
+    Shell,
+}
+
+/// Every local tool, classified. Exhaustive by test: a tool in `LOCAL_TOOL_NAMES`
+/// (or served by `tool_list`) without an entry here fails `profile_tests`, so a new
+/// tool cannot pass the boundary check just by not being listed as dangerous.
+pub(crate) const LOCAL_TOOL_CLASS: &[(&str, ToolClass)] = &[
+    ("sys_info", ToolClass::Observe),
+    ("screenshot", ToolClass::Observe),
+    ("bash", ToolClass::Shell),
+    ("mouse", ToolClass::Shell), // can open a terminal
+    ("key", ToolClass::Shell),   // can type into one
+];
+
+pub(crate) fn local_tool_class(name: &str) -> Option<ToolClass> {
+    LOCAL_TOOL_CLASS
+        .iter()
+        .find(|(n, _)| *n == name)
+        .map(|(_, c)| *c)
+}
+
+/// Whether a profile honestly grants the node user's shell (instance-mcp#45).
+#[cfg(test)]
+pub(crate) fn profile_is_shell_equivalent(profile: &str) -> bool {
+    matches!(normalize_profile(profile), Some("owner") | Some("desktop"))
+}
+
 /// Whether a local tool is visible/callable under `profile`.
 pub(crate) fn local_tool_allowed(name: &str, profile: &str) -> bool {
     match normalize_profile(profile) {
         Some("owner") | Some("desktop") => true,
-        Some("observe") => OBSERVE_TOOLS.contains(&name),
+        // Both: on the allowlist *and* classified as observation. A tool added to the
+        // allowlist by mistake is still refused unless it was also classified as
+        // changing nothing.
+        Some("observe") => {
+            OBSERVE_TOOLS.contains(&name) && local_tool_class(name) == Some(ToolClass::Observe)
+        }
         _ => false,
     }
 }
@@ -433,6 +476,72 @@ mod profile_tests {
         assert!(upstream_tool_allowed("browser_navigate", "desktop"));
     }
 
+    /// Local tool names as actually served under `profile` (no upstream in tests).
+    fn served(profile: &str) -> Vec<String> {
+        tool_list(profile)
+            .as_array()
+            .unwrap()
+            .iter()
+            .map(|t| t["name"].as_str().unwrap().to_owned())
+            .filter(|n| !n.starts_with("browser_"))
+            .collect()
+    }
+
+    #[test]
+    fn every_served_tool_is_classified_and_the_lists_agree() {
+        let served = served("owner");
+        let mut sorted_served = served.clone();
+        sorted_served.sort();
+        let mut names: Vec<String> = LOCAL_TOOL_NAMES.iter().map(|s| s.to_string()).collect();
+        names.sort();
+        assert_eq!(
+            sorted_served, names,
+            "tool_list and LOCAL_TOOL_NAMES disagree"
+        );
+        let unclassified: Vec<_> = served
+            .iter()
+            .filter(|n| local_tool_class(n).is_none())
+            .collect();
+        assert!(
+            unclassified.is_empty(),
+            "classify in LOCAL_TOOL_CLASS: {unclassified:?}"
+        );
+        let stale: Vec<_> = LOCAL_TOOL_CLASS
+            .iter()
+            .filter(|(n, _)| !served.iter().any(|s| s == n))
+            .collect();
+        assert!(stale.is_empty(), "classified but not served: {stale:?}");
+    }
+
+    #[test]
+    fn no_profile_claims_to_be_narrower_than_it_is() {
+        for profile in ["owner", "desktop", "observe"] {
+            let shell: Vec<String> = served(profile)
+                .into_iter()
+                .filter(|n| local_tool_class(n) == Some(ToolClass::Shell))
+                .collect();
+            if profile_is_shell_equivalent(profile) {
+                assert!(
+                    !shell.is_empty(),
+                    "{profile} is marked shell-equivalent; keep it honest"
+                );
+            } else {
+                assert!(
+                    shell.is_empty(),
+                    "{profile} claims no shell but serves {shell:?}"
+                );
+                let acting: Vec<String> = served(profile)
+                    .into_iter()
+                    .filter(|n| local_tool_class(n) != Some(ToolClass::Observe))
+                    .collect();
+                assert!(
+                    acting.is_empty(),
+                    "{profile} serves tools that act: {acting:?}"
+                );
+            }
+        }
+    }
+
     #[test]
     fn observe_can_only_look() {
         for tool in ["sys_info", "screenshot"] {