From 90706fe0538dc8ca597d4f8c964f10b43fe9f763 Mon Sep 17 00:00:00 2001 From: archipelago Date: Tue, 4 Aug 2026 01:28:29 -0400 Subject: [PATCH] feat(13-05): expand curated tool registry to full D-06/D-09 allowlist, wire D-16 default-closed grants Task 1: registry() grows from the tracer's single system_disk_status tool to the full 13-tool D-06 curated allowlist (9 read tools, 4 destructive write tools), each hand-written with its own JSON Schema, PermissionCategory and destructive flag -- nothing derived from api::rpc's method table. Adds EXCLUDED_AUTHORITY_TERMS (D-09's excluded authority, scanned by Task 3's registry-wide test), SETTABLE_KEYS/READABLE_SETTINGS_KEYS (AIUI-02's hand-picked settings surface, claude_api_key permanently absent from SETTABLE_KEYS), tools::dispatch (per-tool RPC dispatch) and tools::validate_business_rules (allowlisted-key / installed-app-id validation that runs before the destructive/confirm gate so a plainly-wrong request is refused with the real reason instead of the generic "not yet implemented" placeholder). assistant_dispatch_tool gains a params argument and the RPC method table Task 1's tools need. Task 2: grants.rs adds Grants (D-16 default-closed permission-category store, persisted 0600 under data_dir/assistant/grants.json; a missing file is default_closed(), never permissive). CallerScope::granted_categories becomes async and reads the persisted store instead of a hardcoded default; CallerScope::Mesh gains an `authorized` field so a mesh peer's ceiling is never wider than the operator's own grants. ToolExecCtx gains the AI-SPEC S-13 consecutive-validation-failure counter (>2 failures for the same tool name aborts the turn with an apology, checked in run_loop). build_system_prompt appends only currently-granted-category tools' names/descriptions -- an ungranted tool never appears in the prompt string (defense in depth; the execute_tool grant re-check is the actual gate). assistant_chat.rs adds assistant.list-tools / assistant.grants-get / assistant.grants-set, all routed through the existing single assistant.* dispatcher arm (dispatcher.rs untouched, verified by git diff --exit-code). dispatcher.rs is not touched -- all new RPC surface goes through 13-01's assistant.* prefix arm. Co-Authored-By: Claude Opus 5 (1M context) --- .../archipelago/src/api/rpc/assistant_chat.rs | 116 ++- core/archipelago/src/assistant/grants.rs | 137 +++ core/archipelago/src/assistant/loop_.rs | 108 ++- core/archipelago/src/assistant/mod.rs | 318 ++++++- core/archipelago/src/assistant/tools.rs | 888 +++++++++++++++++- 5 files changed, 1451 insertions(+), 116 deletions(-) create mode 100644 core/archipelago/src/assistant/grants.rs diff --git a/core/archipelago/src/api/rpc/assistant_chat.rs b/core/archipelago/src/api/rpc/assistant_chat.rs index afc3e9ac..82432436 100644 --- a/core/archipelago/src/api/rpc/assistant_chat.rs +++ b/core/archipelago/src/api/rpc/assistant_chat.rs @@ -25,10 +25,75 @@ impl RpcHandler { ) -> Result { match method { "assistant.chat" => self.handle_assistant_chat(params, session_token).await, + "assistant.list-tools" => self.handle_assistant_list_tools().await, + "assistant.grants-get" => self.handle_assistant_grants_get().await, + "assistant.grants-set" => self.handle_assistant_grants_set(params).await, other => anyhow::bail!("no such assistant method: {other}"), } } + /// assistant.list-tools — the tools currently visible to the local + /// operator (i.e. whose category is currently granted), each with its + /// category and destructive flag, so neode-ui can render an honest + /// capability list rather than guessing from the grants alone. + async fn handle_assistant_list_tools(self: &Arc) -> Result { + let grants = crate::assistant::grants::Grants::load(self.data_dir()).await; + let registry = crate::assistant::tools::registry(); + let visible = registry.visible_to(grants.categories()); + let tools: Vec = visible + .iter() + .map(|t| { + serde_json::json!({ + "name": t.name, + "description": t.description, + "category": t.category, + "destructive": t.destructive, + }) + }) + .collect(); + Ok(serde_json::json!({ "tools": tools })) + } + + /// assistant.grants-get — the current open/closed state of all ten + /// D-16 permission categories, for the browser's AI-permissions UI to + /// render honestly (including the ones still closed). + async fn handle_assistant_grants_get(self: &Arc) -> Result { + let grants = crate::assistant::grants::Grants::load(self.data_dir()).await; + Ok(serde_json::json!({ "categories": grants_categories_json(&grants) })) + } + + /// assistant.grants-set — open or close one permission category. + /// Params: `{ "category": string, "granted": bool }`, where `category` + /// is one of the ten kebab-case ids also used by + /// `neode-ui/src/stores/aiPermissions.ts` (`"apps"`, `"ai-local"`, + /// etc). Persists immediately via `Grants::save` — the next + /// `assistant.chat` turn (not only the next session) sees the change, + /// per `grant_revocation_takes_effect_next_turn`. + async fn handle_assistant_grants_set( + self: &Arc, + params: Option, + ) -> Result { + let params = params.ok_or_else(|| anyhow::anyhow!("Missing params"))?; + let category_str = params + .get("category") + .and_then(|v| v.as_str()) + .ok_or_else(|| anyhow::anyhow!("Missing category"))?; + let granted = params + .get("granted") + .and_then(|v| v.as_bool()) + .ok_or_else(|| anyhow::anyhow!("Missing granted"))?; + + let category: crate::assistant::PermissionCategory = + serde_json::from_value(serde_json::Value::String(category_str.to_string())) + .map_err(|_| anyhow::anyhow!("Unknown permission category: {category_str}"))?; + + let mut grants = crate::assistant::grants::Grants::load(self.data_dir()).await; + grants.set(category, granted); + grants.save(self.data_dir()).await?; + + Ok(serde_json::json!({ "categories": grants_categories_json(&grants) })) + } + /// assistant.chat — a single chat turn from the authenticated local /// operator. Params: `{ "text": string }`. Returns `{ "text": string }`. async fn handle_assistant_chat( @@ -56,18 +121,37 @@ impl RpcHandler { /// Internal-only bridge: executes a curated assistant tool against the /// SAME `RpcHandler` method every authenticated RPC caller dispatches /// through (never an AI-only backdoor). NOT itself an RPC method — only - /// `assistant::loop_::execute_tool` calls this, and only for tool names - /// present in the curated D-06 registry. - /// - /// Rust module privacy is what requires this thin bridge: - /// `handle_system_disk_status` is `pub(in crate::api::rpc)`, so - /// `crate::assistant` (outside that module subtree) cannot call it - /// directly. This function lives inside `api::rpc` so it CAN call the - /// private handler, and re-exposes only the one curated method name a - /// tool call is allowed to reach — not the general RPC surface. - pub(crate) async fn assistant_dispatch_tool(&self, method: &str) -> Result { + /// `assistant::tools::dispatch` calls this, and only for the RPC method + /// names its own hand-written match arms name explicitly (D-06 — this + /// list is exactly as curated as the tool registry itself; it is not a + /// general-purpose relay onto `api::rpc`'s method table). + pub(crate) async fn assistant_dispatch_tool( + &self, + method: &str, + params: Option, + ) -> Result { match method { "system.disk-status" => self.handle_system_disk_status().await, + "system.stats" => self.handle_system_stats().await, + "container-list" => self.handle_container_list().await, + "container-logs" => self.handle_container_logs(params).await, + "container-start" => self.handle_container_start(params).await, + "container-stop" => self.handle_container_stop(params).await, + "container-restart" => self.handle_container_restart(params).await, + "bitcoin.getinfo" => self.handle_bitcoin_getinfo().await, + "network.get-visibility" => self.handle_network_get_visibility().await, + "network.diagnostics" => self.handle_network_diagnostics().await, + "network.set-visibility" => self.handle_network_set_visibility(params).await, + "network.set-wifi-radio" => self.handle_network_set_wifi_radio(params).await, + "mesh.status" => self.handle_mesh_status().await, + "content.list-mine" => self.handle_content_list_mine().await, + "system.settings.get" => self.handle_system_settings_get(params).await, + "system.settings.set" => self.handle_system_settings_set(params).await, + "system.kiosk-display.get" => self.handle_system_kiosk_display_get().await, + "system.kiosk-display.set" => self.handle_system_kiosk_display_set(params).await, + "bitcoin.relay-update-settings" => { + self.handle_bitcoin_relay_update_settings(params).await + } other => anyhow::bail!("assistant_dispatch_tool: no such handler for {other}"), } } @@ -79,3 +163,15 @@ impl RpcHandler { &self.config.data_dir } } + +/// Shared shape for `assistant.grants-get` and `assistant.grants-set`'s +/// response: all ten categories, each with its current `granted` state — +/// never only the open ones, so the browser can render an honest +/// still-closed list too. +fn grants_categories_json(grants: &crate::assistant::grants::Grants) -> serde_json::Value { + let categories: Vec = crate::assistant::PermissionCategory::ALL + .iter() + .map(|c| serde_json::json!({ "category": c, "granted": grants.allows(*c) })) + .collect(); + serde_json::json!(categories) +} diff --git a/core/archipelago/src/assistant/grants.rs b/core/archipelago/src/assistant/grants.rs new file mode 100644 index 00000000..961ed3d6 --- /dev/null +++ b/core/archipelago/src/assistant/grants.rs @@ -0,0 +1,137 @@ +//! D-16: default-closed permission-category grants, persisted under +//! `data_dir`. All ten `PermissionCategory` variants are closed on a fresh +//! node — nothing is shared with the model until the operator deliberately +//! opens a category. A missing or unreadable grants file is +//! `default_closed()`, never an error and never a permissive default: the +//! assistant looking unconfigured on a fresh node is an accepted cost +//! (13-CONTEXT.md D-16), not a bug to work around by defaulting open. + +use std::collections::BTreeSet; +use std::path::Path; + +use anyhow::{Context, Result}; +use serde::{Deserialize, Serialize}; + +use super::PermissionCategory; + +const GRANTS_FILE: &str = "assistant/grants.json"; + +/// The set of currently-open permission categories. Construct via +/// [`Grants::default_closed`] or [`Grants::load`] — never via a `Default` +/// impl that could silently be permissive. +#[derive(Debug, Clone, Serialize, Deserialize)] +pub struct Grants { + categories: BTreeSet, +} + +impl Grants { + /// D-16: a fresh node grants nothing. Every one of the ten categories is + /// closed until the operator explicitly opens it. + pub fn default_closed() -> Grants { + Grants { + categories: BTreeSet::new(), + } + } + + /// Whether `category` is currently open. + pub fn allows(&self, category: PermissionCategory) -> bool { + self.categories.contains(&category) + } + + /// The full set of currently-open categories. + pub fn categories(&self) -> &BTreeSet { + &self.categories + } + + /// Open or close a single category. Callers must still call + /// [`Grants::save`] to persist the change. + pub fn set(&mut self, category: PermissionCategory, granted: bool) { + if granted { + self.categories.insert(category); + } else { + self.categories.remove(&category); + } + } + + /// Load the persisted grants for this node. A missing file, or one that + /// fails to parse, is `default_closed()` — never an error, and never + /// anything other than empty. This is the one place D-16's "nothing is + /// shared with the model until deliberately granted" is enforced at the + /// data layer; `CallerScope::granted_categories` has no other source of + /// authority to fall back to. + pub async fn load(data_dir: &Path) -> Grants { + let path = data_dir.join(GRANTS_FILE); + let Ok(content) = tokio::fs::read_to_string(&path).await else { + return Grants::default_closed(); + }; + serde_json::from_str(&content).unwrap_or_else(|_| Grants::default_closed()) + } + + /// Persist the grants for this node, 0600 (following + /// `streaming/session.rs`'s `data_dir`-scoped persisted-state + /// convention, and this codebase's convention of keeping + /// non-world-readable anything that shapes what a model or a remote + /// peer can reach on this node). + pub async fn save(&self, data_dir: &Path) -> Result<()> { + let dir = data_dir.join("assistant"); + tokio::fs::create_dir_all(&dir) + .await + .context("Failed to create assistant dir")?; + let path = data_dir.join(GRANTS_FILE); + let content = serde_json::to_string_pretty(self).context("Failed to serialize grants")?; + tokio::fs::write(&path, &content) + .await + .context("Failed to write grants file")?; + #[cfg(unix)] + { + use std::os::unix::fs::PermissionsExt; + std::fs::set_permissions(&path, std::fs::Permissions::from_mode(0o600)).ok(); + } + Ok(()) + } +} + +#[cfg(test)] +mod tests { + use super::*; + + #[tokio::test] + async fn fresh_node_grants_are_empty() { + let tmp = tempfile::tempdir().expect("tempdir"); + let grants = Grants::load(tmp.path()).await; + assert!( + grants.categories().is_empty(), + "a fresh node with no grants file must grant nothing" + ); + for category in PermissionCategory::ALL { + assert!(!grants.allows(category), "{category:?} must be closed by default"); + } + } + + #[tokio::test] + async fn grant_persists_across_load() { + let tmp = tempfile::tempdir().expect("tempdir"); + let mut grants = Grants::load(tmp.path()).await; + grants.set(PermissionCategory::System, true); + grants.save(tmp.path()).await.expect("save"); + + let reloaded = Grants::load(tmp.path()).await; + assert!(reloaded.allows(PermissionCategory::System)); + assert!(!reloaded.allows(PermissionCategory::Wallet)); + } + + #[tokio::test] + async fn revoke_removes_the_category() { + let tmp = tempfile::tempdir().expect("tempdir"); + let mut grants = Grants::load(tmp.path()).await; + grants.set(PermissionCategory::Network, true); + grants.save(tmp.path()).await.expect("save"); + + let mut grants = Grants::load(tmp.path()).await; + grants.set(PermissionCategory::Network, false); + grants.save(tmp.path()).await.expect("save"); + + let reloaded = Grants::load(tmp.path()).await; + assert!(!reloaded.allows(PermissionCategory::Network)); + } +} diff --git a/core/archipelago/src/assistant/loop_.rs b/core/archipelago/src/assistant/loop_.rs index 9640dbe2..c4c1b1f9 100644 --- a/core/archipelago/src/assistant/loop_.rs +++ b/core/archipelago/src/assistant/loop_.rs @@ -41,6 +41,19 @@ pub async fn run_loop( for call in &calls { results.push(execute_tool(call, ctx).await); } + // AI-SPEC §4b.1 / D-05: a model that keeps emitting + // malformed args for the same tool name must not be + // allowed to spin for the full MAX_TURNS budget — abort + // with an apology as soon as any tool name crosses 2 + // consecutive validation failures, rather than continuing + // to ask the model to try again. + if ctx.should_abort() { + return Ok( + "I'm stopping here — the same tool call kept failing validation. \ + Could you rephrase what you'd like me to do?" + .to_string(), + ); + } history.push(ChatMessage { role: Role::Tool, text: None, @@ -58,11 +71,16 @@ pub async fn run_loop( /// unknown names are refused, never silently ignored), D-16 (default-closed /// category grants — re-checked here even though the system prompt already /// omits ungranted tools; never trust that as the only enforcement layer), -/// schema validation (never coerce, never guess), and D-07 (every -/// destructive tool suspends for confirmation — 13-08 fills that branch in; -/// there are no destructive tools registered yet, so it is unreachable -/// today). -async fn execute_tool(call: &ToolCall, ctx: &ToolExecCtx) -> ToolResult { +/// schema validation (never coerce, never guess — AI-SPEC §4b.1), and D-07 +/// (every destructive tool suspends for confirmation — 13-08 fills that +/// branch in; there are no reachable destructive tools yet, so it is +/// unreachable today even though the registry now has some). +/// +/// `pub(crate)` (not private) so `assistant::tools`'s own test module can +/// exercise this exact choke point directly for S-05/S-07 — the point of +/// those tests is that the gate holds even when called the same way the +/// real loop calls it, not a reimplementation of the gate in the test. +pub(crate) async fn execute_tool(call: &ToolCall, ctx: &ToolExecCtx) -> ToolResult { let Some(tool) = ctx.registry.get(&call.name) else { return ToolResult { call_id: call.id.clone(), @@ -71,7 +89,8 @@ async fn execute_tool(call: &ToolCall, ctx: &ToolExecCtx) -> ToolResult { }; }; - if !ctx.caller.granted_categories().contains(&tool.category) { + let granted = ctx.caller.granted_categories(ctx.handler.data_dir()).await; + if !granted.contains(&tool.category) { return ToolResult { call_id: call.id.clone(), is_error: true, @@ -79,11 +98,33 @@ async fn execute_tool(call: &ToolCall, ctx: &ToolExecCtx) -> ToolResult { }; } - if let Err(e) = tool.validate(&call.arguments) { + let args = match tool.validate(&call.arguments) { + Ok(args) => { + ctx.reset_validation_failures(&call.name); + args + } + Err(e) => { + ctx.note_validation_failure(&call.name); + return ToolResult { + call_id: call.id.clone(), + is_error: true, + content: format!("invalid arguments: {e}"), + }; + } + }; + + // Business-rule validation (an allowlisted settings key, an installed + // app id) runs BEFORE the destructive/confirm gate below — otherwise a + // plainly-wrong request (an unlisted key, `claude_api_key`, an unknown + // app id) would be swallowed by the destructive branch's generic + // "not yet implemented" placeholder instead of being refused with the + // real reason (13-05 Task 1's `` criterion). This performs no + // mutation itself — only a read-only id lookup for the app tools. + if let Err(msg) = super::tools::validate_business_rules(&call.name, &args, ctx.handler.as_ref()).await { return ToolResult { call_id: call.id.clone(), is_error: true, - content: format!("invalid arguments: {e}"), + content: msg, }; } @@ -95,30 +136,19 @@ async fn execute_tool(call: &ToolCall, ctx: &ToolExecCtx) -> ToolResult { }; } - match call.name.as_str() { - // Dispatches to the SAME RpcHandler method every other authenticated - // caller uses (no AI-only backdoor) — see `assistant_dispatch_tool` - // in `api/rpc/assistant_chat.rs` for why this bridge exists. - "system_disk_status" => match ctx - .handler - .assistant_dispatch_tool("system.disk-status") - .await - { - Ok(v) => ToolResult { - call_id: call.id.clone(), - is_error: false, - content: v.to_string(), - }, - Err(e) => ToolResult { - call_id: call.id.clone(), - is_error: true, - content: format!("tool execution failed: {e}"), - }, + // D-06: dispatch is a per-tool, hand-written decision recorded in + // `tools::dispatch` — never a generic pass-through of the model's tool + // name onto the RPC surface. + match super::tools::dispatch(&call.name, &args, ctx.handler.as_ref()).await { + Ok(v) => ToolResult { + call_id: call.id.clone(), + is_error: false, + content: v.to_string(), }, - other => ToolResult { + Err(msg) => ToolResult { call_id: call.id.clone(), is_error: true, - content: format!("no execution wired for tool: {other}"), + content: msg, }, } } @@ -159,24 +189,34 @@ mod tests { } fn local_operator_ctx(handler: Arc) -> ToolExecCtx { - ToolExecCtx { - registry: registry(), - caller: CallerScope::LocalOperator { + ToolExecCtx::new( + registry(), + CallerScope::LocalOperator { session_id: "test-session".to_string(), }, handler, - } + ) + } + + /// D-16 defaults to closed, so tests that exercise a real tool call + /// must explicitly open the category first — this is the test-side + /// analog of an operator toggling a category on in neode-ui. + async fn grant(handler: &Arc, category: PermissionCategory) { + let mut g = crate::assistant::grants::Grants::load(handler.data_dir()).await; + g.set(category, true); + g.save(handler.data_dir()).await.expect("save grants"); } #[tokio::test] async fn disk_status_tool_executes() { let (handler, _tmp) = test_rpc_handler().await; + grant(&handler, PermissionCategory::System).await; // The real figures the tool path returns must match what the SAME // handler returns when dispatched directly — proving `execute_tool` // is not a parallel, AI-only code path. let direct = handler - .assistant_dispatch_tool("system.disk-status") + .assistant_dispatch_tool("system.disk-status", None) .await .expect("direct dispatch"); diff --git a/core/archipelago/src/assistant/mod.rs b/core/archipelago/src/assistant/mod.rs index 8ca3cb53..77ff9ab4 100644 --- a/core/archipelago/src/assistant/mod.rs +++ b/core/archipelago/src/assistant/mod.rs @@ -11,19 +11,27 @@ //! authenticated caller uses. See `13-01-PLAN.md` for the full spine. pub mod backends; +pub mod grants; pub mod loop_; pub mod tools; -use std::collections::BTreeSet; -use std::sync::Arc; +use std::collections::{BTreeSet, HashMap}; +use std::path::Path; +use std::sync::{Arc, Mutex}; use anyhow::Result; +use serde::{Deserialize, Serialize}; use crate::api::rpc::RpcHandler; /// D-16's ten permission categories. All default-closed on a fresh node — -/// nothing is shared with the model until deliberately granted. -#[derive(Debug, Clone, Copy, PartialEq, Eq, PartialOrd, Ord)] +/// nothing is shared with the model until deliberately granted. Serialized +/// with `rename_all = "kebab-case"` so the wire form (`"ai-local"`, etc.) +/// matches `neode-ui/src/stores/aiPermissions.ts`'s category ids +/// one-for-one — 13-05's acceptance criterion diffs the two lists by hand, +/// but the serde rename is what keeps them from drifting silently again. +#[derive(Debug, Clone, Copy, PartialEq, Eq, PartialOrd, Ord, Serialize, Deserialize)] +#[serde(rename_all = "kebab-case")] pub enum PermissionCategory { Apps, System, @@ -37,6 +45,23 @@ pub enum PermissionCategory { Bitcoin, } +impl PermissionCategory { + /// All ten categories, for grants-get/list-tools enumeration and for + /// tests that need to assert over the whole set. + pub const ALL: [PermissionCategory; 10] = [ + PermissionCategory::Apps, + PermissionCategory::System, + PermissionCategory::Network, + PermissionCategory::Wallet, + PermissionCategory::Files, + PermissionCategory::Media, + PermissionCategory::Search, + PermissionCategory::AiLocal, + PermissionCategory::Notes, + PermissionCategory::Bitcoin, + ]; +} + /// D-02's promoted primary noun: a caller identity carrying the permission /// scope its tool calls resolve authority through. "A mesh peer" and "the /// local operator in AIUI" are two variants of it; Pine voice will be a @@ -47,8 +72,15 @@ pub enum CallerScope { /// A mesh/LoRa peer. Not exercised by this plan (mesh's existing /// `!ai` path is Q&A-only, per `mesh/listener/assist.rs`'s own doc /// comment) — the variant exists so the shape is right when a future - /// plan wires mesh callers into the shared loop. - Mesh { peer_id: String }, + /// plan wires mesh callers into the shared loop. `authorized` is where + /// that future plan threads the existing per-caller + /// `trusted_only`/`allowed_contacts`/`denied_askers` resolution + /// (`api/rpc/mesh/assistant.rs`) through: a mesh peer's authority is + /// never wider than the operator's own persisted grants, only ever a + /// subset of them (all-or-nothing today; a future plan may narrow this + /// to a per-peer category subset without changing this variant's + /// shape). + Mesh { peer_id: String, authorized: bool }, /// The authenticated operator using AIUI, identified by their neode-ui /// session. This is the only variant this tracer's `assistant.chat` /// RPC constructs. @@ -60,24 +92,26 @@ impl CallerScope { /// `execute_tool` branch may read a caller-specific field directly /// instead of going through this — that would reintroduce the /// mesh-only assumption D-02 exists to retire. - pub fn granted_categories(&self) -> BTreeSet { + /// + /// Both variants resolve through the SAME persisted `Grants` store + /// (D-16's default-closed categories, per `data_dir`) — never a + /// hardcoded default, and never two divergent sources of authority. + pub async fn granted_categories(&self, data_dir: &Path) -> BTreeSet { + let persisted = grants::Grants::load(data_dir).await; match self { - // 13-05 replaces this hardcoded default with the persisted - // D-16 default-closed grants store — a data-source change, not - // an architectural one (per the plan's assumption-delta note). - CallerScope::LocalOperator { .. } => { - let mut set = BTreeSet::new(); - set.insert(PermissionCategory::System); - set + CallerScope::LocalOperator { .. } => persisted.categories().clone(), + // A mesh peer can never exceed what the operator opened: its + // authority is the persisted grants intersected with whether + // this peer is itself authorized to use the assistant at all + // (today a single boolean; a future plan may narrow this to a + // per-peer category subset without changing this call site). + CallerScope::Mesh { authorized, .. } => { + if *authorized { + persisted.categories().clone() + } else { + BTreeSet::new() + } } - // Intentionally conservative for this tracer: mesh has no - // tool-calling caller path wired up yet (today's mesh `!ai` is - // Q&A-only), so there is no real trusted_only/allowed_contacts - // grant to resolve. A future plan that wires the Mesh variant - // into the shared loop threads those existing per-caller - // controls through here — this is explicitly NOT the place a - // mesh-only field gets read directly by `execute_tool`. - CallerScope::Mesh { .. } => BTreeSet::new(), } } } @@ -86,10 +120,94 @@ impl CallerScope { /// the tool call: the curated registry, the caller's resolved authority, /// and a handle back to the SAME `RpcHandler` every other authenticated /// caller dispatches through — never an AI-only backdoor. +/// +/// Also carries the AI-SPEC §4b.1 "≤ 2 consecutive validation failures per +/// tool name" counter. Construct via [`ToolExecCtx::new`] — the counter +/// field is private so every call site shares the same reset/note logic +/// rather than reimplementing it. pub struct ToolExecCtx { pub registry: tools::ToolRegistry, pub caller: CallerScope, pub handler: Arc, + validation_failures: Mutex>, +} + +impl ToolExecCtx { + pub fn new(registry: tools::ToolRegistry, caller: CallerScope, handler: Arc) -> Self { + Self { + registry, + caller, + handler, + validation_failures: Mutex::new(HashMap::new()), + } + } + + /// A tool name's argument validation just succeeded — its consecutive + /// failure streak resets. + pub(crate) fn reset_validation_failures(&self, tool_name: &str) { + self.validation_failures + .lock() + .expect("validation_failures mutex poisoned") + .remove(tool_name); + } + + /// A tool name's argument validation just failed. Returns `true` if + /// this failure is the third (or later) consecutive one for this tool + /// name — the signal `run_loop` uses to abort the turn with an apology + /// rather than spinning (D-05, AI-SPEC §4b.1). + pub(crate) fn note_validation_failure(&self, tool_name: &str) -> bool { + let mut map = self + .validation_failures + .lock() + .expect("validation_failures mutex poisoned"); + let count = map.entry(tool_name.to_string()).or_insert(0); + *count += 1; + *count > 2 + } + + /// Whether any tool name has crossed the consecutive-failure threshold + /// this turn. Checked by `run_loop` after each batch of tool calls. + pub(crate) fn should_abort(&self) -> bool { + self.validation_failures + .lock() + .expect("validation_failures mutex poisoned") + .values() + .any(|&c| c > 2) + } +} + +/// D-16/AI-SPEC §4b.3: one static, phase-authored persona and confirm-gate +/// statement, never assembled from prior model output and never editable +/// by AIUI. Appends **only** the currently-granted-category tools' names +/// and descriptions — an ungranted tool's name never appears in this +/// string, which is the prompt-side half of the two-layer defense (the +/// `execute_tool` grant re-check in `loop_.rs` is the other, and the gate +/// that actually matters — see `ungranted_tool_absent_from_system_prompt` +/// and `settings_tool_respects_category_grant`). +const SYSTEM_PROMPT_PREAMBLE: &str = "You are the Archipelago node's operator-control assistant. \ +Only use the tools explicitly listed below for this turn — never invent a tool name or call one \ +that is not listed here, even if it sounds like something this node could plausibly do. Every \ +write requires a human confirmation you cannot bypass, skip, or pre-approve on the user's \ +behalf. If the user asks for something outside the tools listed below (including anything \ +touching keys, seeds, wallet spends, federation trust, or a factory reset), refuse plainly and, \ +if there is a real path in neode-ui's Settings screen for it, name that path instead of \ +fabricating a tool call."; + +pub fn build_system_prompt(visible_tools: &[tools::ToolDef]) -> String { + let mut prompt = String::from(SYSTEM_PROMPT_PREAMBLE); + if visible_tools.is_empty() { + prompt.push_str( + "\n\nNo permission categories are granted on this node right now, so no tools are \ + available this turn. Say so plainly if asked to do something — do not guess, and do \ + not pretend a category is open.", + ); + } else { + prompt.push_str("\n\nTools available this turn:\n"); + for tool in visible_tools { + prompt.push_str(&format!("- {}: {}\n", tool.name, tool.description)); + } + } + prompt } /// Entry point: run one chat turn for `caller` through the shared loop. @@ -98,15 +216,12 @@ pub struct ToolExecCtx { /// backend (Claude only, in this tracer), and runs it to a final answer. pub async fn chat(handler: Arc, caller: CallerScope, user_text: String) -> Result { let registry = tools::registry(); - let grants = caller.granted_categories(); + let grants = caller.granted_categories(handler.data_dir()).await; let visible_tools = registry.visible_to(&grants); let backend = backends::select_backend(handler.data_dir()); - let system_prompt = "You are the Archipelago node's operator-control assistant. \ - Only use the tools explicitly listed for this turn — never invent a tool name or call \ - one that isn't listed. Every write requires human confirmation you cannot bypass or \ - pre-approve on the user's behalf."; + let system_prompt = build_system_prompt(&visible_tools); let history = vec![tools::ChatMessage { role: tools::Role::User, @@ -115,11 +230,148 @@ pub async fn chat(handler: Arc, caller: CallerScope, user_text: Stri tool_results: vec![], }]; - let ctx = ToolExecCtx { - registry, - caller, - handler, - }; + let ctx = ToolExecCtx::new(registry, caller, handler); - loop_::run_loop(backend.as_ref(), system_prompt, &visible_tools, history, &ctx).await + loop_::run_loop(backend.as_ref(), &system_prompt, &visible_tools, history, &ctx).await +} + +#[cfg(test)] +mod tests { + use super::*; + use crate::api::rpc::RpcHandler; + + /// A minimal but real `RpcHandler` for tests, matching + /// `loop_::tests::test_rpc_handler` — a fresh temp `data_dir`, no + /// orchestrator. + async fn test_rpc_handler() -> (Arc, tempfile::TempDir) { + let tmp = tempfile::tempdir().expect("tempdir"); + let mut config = crate::config::Config::default(); + config.data_dir = tmp.path().to_path_buf(); + let state_manager = Arc::new(crate::state::StateManager::new()); + let metrics_store = Arc::new(crate::monitoring::MetricsStore::new()); + let session_store = + crate::session::SessionStore::new_for_tests(tmp.path().join("sessions.json")); + let handler = RpcHandler::new( + config, + state_manager, + metrics_store, + session_store, + None, + None, + ) + .await + .expect("RpcHandler::new"); + (Arc::new(handler), tmp) + } + + /// S-06 / D-16: a fresh node's `LocalOperator` resolves to no granted + /// categories at all. + #[tokio::test] + async fn fresh_node_grants_are_empty() { + let (handler, _tmp) = test_rpc_handler().await; + let caller = CallerScope::LocalOperator { + session_id: "s".to_string(), + }; + let granted = caller.granted_categories(handler.data_dir()).await; + assert!(granted.is_empty(), "fresh node must grant nothing: {granted:?}"); + } + + /// The system prompt built for a caller must never mention a tool + /// whose category is not currently granted — the prompt filter is + /// defense in depth, never the gate (S-05's gate is + /// `settings_tool_respects_category_grant` in tools.rs), but it must + /// still hold. + #[test] + fn ungranted_tool_absent_from_system_prompt() { + let reg = tools::registry(); + let mut grants = BTreeSet::new(); + grants.insert(PermissionCategory::System); + let visible = reg.visible_to(&grants); + let prompt = build_system_prompt(&visible); + + for tool in reg.all() { + if tool.category == PermissionCategory::System { + continue; + } + assert!( + !prompt.contains(tool.name), + "ungranted tool {} leaked into the system prompt", + tool.name + ); + } + // Sanity: at least one granted tool IS present, so this isn't + // trivially passing because the prompt is empty. + assert!( + reg.visible_to(&grants).iter().any(|t| prompt.contains(t.name)), + "expected at least one granted-category tool name in the prompt" + ); + } + + /// D-16: revoking a category takes effect on the very next + /// `granted_categories` resolution — not only on the next session / + /// process restart. + #[tokio::test] + async fn grant_revocation_takes_effect_next_turn() { + let (handler, _tmp) = test_rpc_handler().await; + let caller = CallerScope::LocalOperator { + session_id: "s".to_string(), + }; + + let mut g = grants::Grants::load(handler.data_dir()).await; + g.set(PermissionCategory::System, true); + g.save(handler.data_dir()).await.expect("save"); + + let granted = caller.granted_categories(handler.data_dir()).await; + assert!(granted.contains(&PermissionCategory::System)); + + let mut g = grants::Grants::load(handler.data_dir()).await; + g.set(PermissionCategory::System, false); + g.save(handler.data_dir()).await.expect("save"); + + // Same caller, same process, no restart — the very next resolution + // must reflect the revocation. + let granted = caller.granted_categories(handler.data_dir()).await; + assert!( + !granted.contains(&PermissionCategory::System), + "revocation must take effect on the next resolution, not only on restart" + ); + } + + /// D-02's promoted-primary contract: both `CallerScope` variants + /// resolve authority through the SAME method, and a mesh peer's + /// authority is never wider than the operator's own persisted grants. + #[tokio::test] + async fn every_caller_variant_resolves_authority_through_caller_scope() { + let (handler, _tmp) = test_rpc_handler().await; + + let mut g = grants::Grants::load(handler.data_dir()).await; + g.set(PermissionCategory::Bitcoin, true); + g.save(handler.data_dir()).await.expect("save"); + + let operator = CallerScope::LocalOperator { + session_id: "s".to_string(), + }; + let operator_grants = operator.granted_categories(handler.data_dir()).await; + assert!(operator_grants.contains(&PermissionCategory::Bitcoin)); + + let authorized_peer = CallerScope::Mesh { + peer_id: "peer-1".to_string(), + authorized: true, + }; + let peer_grants = authorized_peer.granted_categories(handler.data_dir()).await; + assert_eq!( + peer_grants, operator_grants, + "an authorized mesh peer's ceiling is exactly the operator's persisted grants" + ); + + let unauthorized_peer = CallerScope::Mesh { + peer_id: "peer-2".to_string(), + authorized: false, + }; + let peer_grants = unauthorized_peer.granted_categories(handler.data_dir()).await; + assert!( + peer_grants.is_empty(), + "an unauthorized mesh peer must resolve to no authority regardless of what the operator granted" + ); + } } diff --git a/core/archipelago/src/assistant/tools.rs b/core/archipelago/src/assistant/tools.rs index 92cf01ce..3b55b79f 100644 --- a/core/archipelago/src/assistant/tools.rs +++ b/core/archipelago/src/assistant/tools.rs @@ -1,9 +1,18 @@ -//! D-06: a curated, hand-written tool registry. Never derived from -//! `api::rpc::dispatcher`'s method table — every capability the chat has is -//! a deliberate decision recorded here, and the model never sees the full +//! D-06: a curated, hand-written tool registry. Never derived from the RPC +//! method table in `api::rpc` — every capability the chat has is a +//! deliberate decision recorded here, and the model never sees the full //! RPC surface. No `schemars` — that crate is absent from `Cargo.toml` and //! from 13-RESEARCH.md's Package Legitimacy Audit, so `parameters` below is //! a hand-written JSON Schema object literal instead. +//! +//! D-09's authority ceiling — reads within granted categories, app +//! lifecycle (start/stop/restart), and settings writes; keys, seeds, +//! wallet spends, federation trust and factory reset permanently excluded +//! — is enforced by **absence**: there is no `ToolDef` anywhere below for +//! any of those, and `EXCLUDED_AUTHORITY_TERMS` gives +//! `registry_never_exposes_excluded_authority` (13-05 Task 3) something +//! concrete to assert over so a future out-of-bounds addition fails a +//! test, not a review. use std::collections::{BTreeSet, HashMap}; @@ -12,6 +21,7 @@ use serde::Deserialize; use serde_json::{json, Value}; use super::PermissionCategory; +use crate::api::rpc::RpcHandler; /// The backend-agnostic in/out of a tool invocation — the same shape /// regardless of which adapter (Ollama/Claude/Routstr) produced it. @@ -49,41 +59,151 @@ pub struct ChatMessage { } /// D-06: one curated, hand-written tool. Never generated from the RPC -/// dispatcher — the curated set IS the D-09 authority boundary. +/// method table — the curated set IS the D-09 authority boundary. #[derive(Clone)] pub struct ToolDef { pub name: &'static str, pub description: &'static str, /// JSON Schema `{"type":"object","properties":{...},"required":[...]}`, /// hand-written and pinned adjacent to the args struct it must never - /// drift from — see `disk_status_schema_round_trips_required_keys`. + /// drift from — see + /// `every_tool_schema_round_trips_required_keys_into_its_args_struct`. pub parameters: Value, pub category: PermissionCategory, - /// D-07: true => confirm gate, no exceptions. There are no destructive - /// tools in this tracer's registry; `execute_tool` refuses this branch - /// with a not-yet-implemented error until 13-08 fills it in. + /// D-07: true => confirm gate, no exceptions. 13-08 fills in the real + /// confirm flow; `execute_tool` refuses this branch with a + /// not-yet-implemented error until then. pub destructive: bool, } -/// Args for `system_disk_status` — takes no parameters. +/// D-09's excluded authority, in one place. `registry_never_exposes_excluded_authority` +/// (Task 3) scans every `ToolDef`'s name and description for these phrases +/// at runtime, over the WHOLE registry, so a tool added in a later phase +/// that crosses the ceiling fails that test. §1b's regulatory rationale: +/// this exclusion is what keeps the software inside the MiCA/GENIUS +/// non-custodial carve-out — relaxing it is a compliance decision, not a +/// code-review nit. +/// +/// Deliberately does NOT itself spell out the specific forbidden tool-name +/// identifiers this plan's acceptance criteria grep for directly against +/// the source (their absence from every `ToolDef` literal below is what +/// that grep verifies) — this const is the separate, phrase-based set this +/// file's own test module scans tool text against. +pub const EXCLUDED_AUTHORITY_TERMS: &[&str] = &[ + "seed", + "mnemonic", + "private key", + "macaroon", + "spend", + "send sats", + "pay invoice", + "federation trust", + "factory reset", + "wipe", +]; + +/// AIUI-02's settable-key allowlist. Hand-picked from the surfaces that +/// actually exist (13-05-PLAN.md's Task 1 action) — `claude_api_key` is +/// deliberately **not** here: it is key material, D-09 puts keys +/// permanently outside chat reach, and being the only key +/// `system.settings.set` accepts today is not a reason to include it. +pub const SETTABLE_KEYS: &[&str] = &[ + "network_visibility", + "kiosk_display_preset", + "wifi_radio", + "bitcoin_relay_settings", +]; + +/// `settings_get`'s read-side allowlist — deliberately separate from +/// `SETTABLE_KEYS` (a key can be safely readable, like whether the Claude +/// key is *set*, without being safely writable or without exposing the +/// key material itself). +pub const READABLE_SETTINGS_KEYS: &[&str] = + &["network_visibility", "kiosk_display_preset", "claude_api_key_set"]; + +/// Args for tools that take no parameters at all +/// (`system_disk_status`, `system_stats`, `apps_list`, `bitcoin_status`, +/// `network_status`, `mesh_status`, `content_list`). #[derive(Debug, Deserialize)] pub struct SystemDiskStatusArgs {} +/// Args for `app_start` / `app_stop` / `app_restart` — an exact installed +/// app id, never fuzzy-matched. +#[derive(Debug, Deserialize)] +pub struct AppIdArgs { + pub app_id: String, +} + +/// Args for `app_logs` — an exact app id, plus an optional line count +/// capped at 200 regardless of what the model asks for. +#[derive(Debug, Deserialize)] +pub struct AppLogsArgs { + pub app_id: String, + #[serde(default)] + pub lines: Option, +} + +/// Args for `settings_get` — a key from `READABLE_SETTINGS_KEYS`. +#[derive(Debug, Deserialize)] +pub struct SettingsGetArgs { + pub key: String, +} + +/// Args for `settings_set` — a key from `SETTABLE_KEYS`, plus a value +/// whose JSON type depends on the key (string, bool, or object — see each +/// key's schema description). Kept as a raw `Value` rather than a fixed +/// Rust type because the settable keys deliberately span different value +/// shapes; `dispatch` below validates the shape per key before use. +#[derive(Debug, Deserialize)] +pub struct SettingsSetArgs { + pub key: String, + pub value: Value, +} + +/// The deserialized, schema-validated form of a tool call's arguments — +/// `ToolDef::validate`'s return type. One variant per distinct args shape +/// in the registry (several read-only, no-arg tools share +/// `Empty(SystemDiskStatusArgs)`). +pub enum ToolArgs { + Empty(SystemDiskStatusArgs), + AppId(AppIdArgs), + AppLogs(AppLogsArgs), + SettingsGet(SettingsGetArgs), + SettingsSet(SettingsSetArgs), +} + impl ToolDef { /// Deserialize + validate model-produced arguments before ANY /// execution. Never coerce, never guess, never panic on a mismatch — /// refuse and let the caller turn the error into a tool result the - /// model can recover from. - /// - /// This tracer's registry has exactly one tool, so this is a direct - /// deserialize; a future plan adding a second tool dispatches by - /// `self.name` here before deserializing into that tool's own args type. - pub fn validate(&self, raw: &Value) -> Result { - serde_json::from_value(raw.clone()) - .context("tool arguments did not match the declared schema") + /// model can recover from (AI-SPEC §4b.1). + pub fn validate(&self, raw: &Value) -> Result { + match self.name { + "system_disk_status" | "system_stats" | "apps_list" | "bitcoin_status" + | "network_status" | "mesh_status" | "content_list" => serde_json::from_value(raw.clone()) + .map(ToolArgs::Empty) + .context("tool arguments did not match the declared schema"), + "app_logs" => serde_json::from_value(raw.clone()) + .map(ToolArgs::AppLogs) + .context("tool arguments did not match the declared schema"), + "app_start" | "app_stop" | "app_restart" => serde_json::from_value(raw.clone()) + .map(ToolArgs::AppId) + .context("tool arguments did not match the declared schema"), + "settings_get" => serde_json::from_value(raw.clone()) + .map(ToolArgs::SettingsGet) + .context("tool arguments did not match the declared schema"), + "settings_set" => serde_json::from_value(raw.clone()) + .map(ToolArgs::SettingsSet) + .context("tool arguments did not match the declared schema"), + other => anyhow::bail!("no validator registered for tool: {other}"), + } } } +// --------------------------------------------------------------------- +// Read tools (destructive: false) +// --------------------------------------------------------------------- + /// `system_disk_status` — category `System`, read-only. Reports free and /// total disk space on this node via the same `system.disk-status` handler /// every other authenticated caller uses. @@ -101,7 +221,225 @@ pub fn system_disk_status_tool() -> ToolDef { } } -/// D-06's curated allowlist, name-indexed. +/// `system_stats` — category `System`, read-only. CPU, RAM, disk, uptime, +/// load average. +pub fn system_stats_tool() -> ToolDef { + ToolDef { + name: "system_stats", + description: "Report CPU usage, RAM used/total, disk used/total, uptime and load average on this node.", + parameters: json!({ + "type": "object", + "properties": {}, + "required": [], + }), + category: PermissionCategory::System, + destructive: false, + } +} + +/// `apps_list` — category `Apps`, read-only. Installed app ids and state. +pub fn apps_list_tool() -> ToolDef { + ToolDef { + name: "apps_list", + description: "List installed apps on this node with their id and current state (running/stopped/exited/etc).", + parameters: json!({ + "type": "object", + "properties": {}, + "required": [], + }), + category: PermissionCategory::Apps, + destructive: false, + } +} + +/// `app_logs` — category `Apps`, read-only. Recent log lines for one app. +pub fn app_logs_tool() -> ToolDef { + ToolDef { + name: "app_logs", + description: "Fetch recent log lines for one installed app, by its exact app id (never guess or fuzzy-match an id — call apps_list first if unsure). \ + Example: {\"app_id\": \"bitcoin-core\", \"lines\": 50}.", + parameters: json!({ + "type": "object", + "properties": { + "app_id": {"type": "string", "description": "Exact installed app id, e.g. \"bitcoin-core\"."}, + "lines": {"type": "integer", "description": "Number of trailing log lines to return; capped at 200 regardless of what is requested."}, + }, + "required": ["app_id"], + }), + category: PermissionCategory::Apps, + destructive: false, + } +} + +/// `bitcoin_status` — category `Bitcoin`, read-only. Block height, sync +/// progress, mempool stats — no wallet keys. +pub fn bitcoin_status_tool() -> ToolDef { + ToolDef { + name: "bitcoin_status", + description: "Report this node's Bitcoin sync status: block height, sync progress and mempool stats. Never returns wallet balances, addresses or keys.", + parameters: json!({ + "type": "object", + "properties": {}, + "required": [], + }), + category: PermissionCategory::Bitcoin, + destructive: false, + } +} + +/// `network_status` — category `Network`, read-only. Visibility + +/// diagnostics — no IP addresses beyond what diagnostics already surfaces +/// to the operator elsewhere in the UI. +pub fn network_status_tool() -> ToolDef { + ToolDef { + name: "network_status", + description: "Report this node's network visibility setting and connectivity diagnostics (NAT type, UPnP, Tor connectivity, DNS).", + parameters: json!({ + "type": "object", + "properties": {}, + "required": [], + }), + category: PermissionCategory::Network, + destructive: false, + } +} + +/// `mesh_status` — category `Network`, read-only. Mesh radio status, +/// device info, peer count. +pub fn mesh_status_tool() -> ToolDef { + ToolDef { + name: "mesh_status", + description: "Report this node's mesh (LoRa/Meshtastic) radio status: whether mesh is enabled, device detection, and peer count.", + parameters: json!({ + "type": "object", + "properties": {}, + "required": [], + }), + category: PermissionCategory::Network, + destructive: false, + } +} + +/// `content_list` — category `Media`, read-only. Content this node is +/// sharing. +pub fn content_list_tool() -> ToolDef { + ToolDef { + name: "content_list", + description: "List content this node is currently sharing (filename, mime type, size, access level). Never returns file contents.", + parameters: json!({ + "type": "object", + "properties": {}, + "required": [], + }), + category: PermissionCategory::Media, + destructive: false, + } +} + +/// `settings_get` — category `System`, read-only, behind +/// `READABLE_SETTINGS_KEYS`. +pub fn settings_get_tool() -> ToolDef { + ToolDef { + name: "settings_get", + description: "Read one node setting by key. Only a hand-picked set of keys is readable this way: network_visibility, kiosk_display_preset, claude_api_key_set (whether a Claude API key is configured — never the key itself). \ + Example: {\"key\": \"network_visibility\"}.", + parameters: json!({ + "type": "object", + "properties": { + "key": {"type": "string", "description": "One of: network_visibility, kiosk_display_preset, claude_api_key_set."}, + }, + "required": ["key"], + }), + category: PermissionCategory::System, + destructive: false, + } +} + +// --------------------------------------------------------------------- +// Write tools (every entry below is marked destructive) +// --------------------------------------------------------------------- + +/// `app_start` — category `Apps`, destructive (changes node state; still +/// gated by the D-07 confirm flow once 13-08 lands). +pub fn app_start_tool() -> ToolDef { + ToolDef { + name: "app_start", + description: "Start an installed app by its EXACT app id — never fuzzy-matched or guessed; ask the user which app if unsure, or call apps_list first. \ + Example: {\"app_id\": \"bitcoin-core\"}.", + parameters: json!({ + "type": "object", + "properties": { + "app_id": {"type": "string", "description": "Exact installed app id, e.g. \"bitcoin-core\"."}, + }, + "required": ["app_id"], + }), + category: PermissionCategory::Apps, + destructive: true, + } +} + +/// `app_stop` — category `Apps`, destructive. +pub fn app_stop_tool() -> ToolDef { + ToolDef { + name: "app_stop", + description: "Stop an installed app by its EXACT app id — never fuzzy-matched or guessed; ask the user which app if unsure, or call apps_list first. \ + Example: {\"app_id\": \"bitcoin-core\"}.", + parameters: json!({ + "type": "object", + "properties": { + "app_id": {"type": "string", "description": "Exact installed app id, e.g. \"bitcoin-core\"."}, + }, + "required": ["app_id"], + }), + category: PermissionCategory::Apps, + destructive: true, + } +} + +/// `app_restart` — category `Apps`, destructive. EV-08's "restart the +/// node" case: this tool restarts ONE app, and refuses an id that does not +/// exactly match an installed app rather than guessing which one was +/// meant. +pub fn app_restart_tool() -> ToolDef { + ToolDef { + name: "app_restart", + description: "Restart an installed app by its EXACT app id — never fuzzy-matched or guessed. If the user says something like \"restart the node\" without naming an app, ask which app (there is no single \"restart everything\" tool) rather than guessing. \ + Example: {\"app_id\": \"bitcoin-core\"}.", + parameters: json!({ + "type": "object", + "properties": { + "app_id": {"type": "string", "description": "Exact installed app id, e.g. \"bitcoin-core\"."}, + }, + "required": ["app_id"], + }), + category: PermissionCategory::Apps, + destructive: true, + } +} + +/// `settings_set` — category `System`, destructive. AIUI-02's surface, +/// bounded by `SETTABLE_KEYS`. `claude_api_key` is permanently excluded — +/// see the module doc comment and D-09. +pub fn settings_set_tool() -> ToolDef { + ToolDef { + name: "settings_set", + description: "Change one node setting by key. Only a hand-picked set of keys is settable this way: network_visibility (string: hidden|discoverable|public), kiosk_display_preset (string), wifi_radio (boolean), bitcoin_relay_settings (object). \ + claude_api_key is NEVER settable here — refuse and point at neode-ui's Settings screen if asked. \ + Example: {\"key\": \"wifi_radio\", \"value\": false}.", + parameters: json!({ + "type": "object", + "properties": { + "key": {"type": "string", "description": "One of: network_visibility, kiosk_display_preset, wifi_radio, bitcoin_relay_settings."}, + "value": {"description": "Type depends on key: string for network_visibility/kiosk_display_preset, boolean for wifi_radio, object for bitcoin_relay_settings."}, + }, + "required": ["key", "value"], + }), + category: PermissionCategory::System, + destructive: true, + } +} + +/// D-06's curated registry, name-indexed. pub struct ToolRegistry { tools: HashMap<&'static str, ToolDef>, } @@ -121,42 +459,420 @@ impl ToolRegistry { .cloned() .collect() } + + /// Every registered tool, regardless of grants — for registry-wide + /// structural assertions (Task 3) and for `assistant.list-tools`' + /// underlying data before it is filtered to what is granted. + pub fn all(&self) -> Vec { + self.tools.values().cloned().collect() + } } -/// The curated D-06 registry. This tracer registers exactly one tool. +/// D-06's curated allowlist. Every entry below is a hand-written decision +/// — nothing here is derived from `api::rpc`'s method table. pub fn registry() -> ToolRegistry { let mut tools = HashMap::new(); - let tool = system_disk_status_tool(); - tools.insert(tool.name, tool); + for tool in [ + system_disk_status_tool(), + system_stats_tool(), + apps_list_tool(), + app_logs_tool(), + bitcoin_status_tool(), + network_status_tool(), + mesh_status_tool(), + content_list_tool(), + settings_get_tool(), + app_start_tool(), + app_stop_tool(), + app_restart_tool(), + settings_set_tool(), + ] { + tools.insert(tool.name, tool); + } ToolRegistry { tools } } +/// Resolve `app_id` against the SAME `container-list` handler every other +/// authenticated caller uses. Refuses an id that is not an EXACT match — +/// no fuzzy match, no nearest-neighbour (EV-08 / T-13-27) — and the +/// refusal lists the installed ids instead of guessing. +async fn resolve_installed_app_id(app_id: &str, handler: &RpcHandler) -> Result { + let list = handler + .assistant_dispatch_tool("container-list", None) + .await + .map_err(|e| format!("could not list installed apps: {e}"))?; + let ids: Vec = list + .as_array() + .map(|arr| { + arr.iter() + .filter_map(|v| v.get("id").and_then(|i| i.as_str()).map(String::from)) + .collect() + }) + .unwrap_or_default(); + if ids.iter().any(|id| id == app_id) { + Ok(app_id.to_string()) + } else if ids.is_empty() { + Err(format!( + "no such app id: \"{app_id}\" — this node has no installed apps to restart/start/stop right now" + )) + } else { + Err(format!( + "no such app id: \"{app_id}\". Installed apps: {}", + ids.join(", ") + )) + } +} + +/// Business-rule validation for a tool call: whether THIS specific +/// request is well-formed enough to act on at all — an allowlisted +/// settings key, an app id that actually exists — independent of whether +/// the D-07 confirm gate (13-08) has cleared it yet. Never mutates +/// anything (a read-only `container-list` lookup is the only RPC call it +/// makes, for `app_start`/`app_stop`/`app_restart`'s id resolution). +/// +/// `execute_tool` (`loop_.rs`) calls this BEFORE the destructive/confirm +/// gate, specifically so a plainly-wrong request (an unlisted settings +/// key, `claude_api_key`, an unknown app id) is refused with the real +/// reason instead of being swallowed by the destructive branch's generic +/// "not yet implemented" placeholder — Task 1's `` criterion ("a +/// settings key outside the allowlist and an app id that does not exist +/// are both refused with a message that names the real path"). `dispatch` +/// below also calls this first, so it stays correct and self-contained +/// once 13-08 wires the real confirm-then-execute flow in and this stops +/// being called from two places. +pub async fn validate_business_rules(name: &str, args: &ToolArgs, handler: &RpcHandler) -> Result<(), String> { + match name { + "settings_set" => { + let ToolArgs::SettingsSet(a) = args else { + return Err("internal error: args/tool mismatch for settings_set".to_string()); + }; + if a.key == "claude_api_key" { + return Err( + "claude_api_key is key material and is permanently excluded from chat \ + reach (D-09). Change it in neode-ui's Settings screen instead." + .to_string(), + ); + } + if !SETTABLE_KEYS.contains(&a.key.as_str()) { + return Err(format!( + "\"{}\" is not settable via chat. Settable keys: {}. Use neode-ui's Settings \ + screen for anything else.", + a.key, + SETTABLE_KEYS.join(", ") + )); + } + match a.key.as_str() { + "network_visibility" | "kiosk_display_preset" => { + if a.value.as_str().is_none() { + return Err(format!("{}'s value must be a string", a.key)); + } + } + "wifi_radio" => { + if a.value.as_bool().is_none() { + return Err("wifi_radio's value must be a boolean".to_string()); + } + } + "bitcoin_relay_settings" => { + if !a.value.is_object() { + return Err("bitcoin_relay_settings's value must be an object".to_string()); + } + } + _ => {} + } + Ok(()) + } + "app_start" | "app_stop" | "app_restart" => { + let ToolArgs::AppId(a) = args else { + return Err(format!("internal error: args/tool mismatch for {name}")); + }; + resolve_installed_app_id(&a.app_id, handler).await.map(|_| ()) + } + _ => Ok(()), + } +} + +/// D-06's per-tool dispatch: the hand-written decision of which RPC method +/// (if any) a given tool name reaches, and what params to build for it. +/// Every arm below calls into `RpcHandler::assistant_dispatch_tool`, which +/// bridges into the SAME method every other authenticated caller uses — +/// never a parallel AI-only path (see `api/rpc/assistant_chat.rs`). +/// +/// Returns `Err(String)` (not `anyhow::Error`) because every error path +/// here is meant to become the tool result's `content` verbatim — a +/// message the model (and, through it, the user) can read and act on, not +/// an internal diagnostic. +pub async fn dispatch(name: &str, args: &ToolArgs, handler: &RpcHandler) -> Result { + validate_business_rules(name, args, handler).await?; + match name { + "system_disk_status" => handler + .assistant_dispatch_tool("system.disk-status", None) + .await + .map_err(|e| format!("tool execution failed: {e}")), + "system_stats" => handler + .assistant_dispatch_tool("system.stats", None) + .await + .map_err(|e| format!("tool execution failed: {e}")), + "apps_list" => handler + .assistant_dispatch_tool("container-list", None) + .await + .map_err(|e| format!("tool execution failed: {e}")), + "app_logs" => { + let ToolArgs::AppLogs(a) = args else { + return Err("internal error: args/tool mismatch for app_logs".to_string()); + }; + let lines = a.lines.unwrap_or(100).min(200); + let params = json!({ "app_id": a.app_id, "lines": lines }); + handler + .assistant_dispatch_tool("container-logs", Some(params)) + .await + .map_err(|e| format!("tool execution failed: {e}")) + } + "bitcoin_status" => handler + .assistant_dispatch_tool("bitcoin.getinfo", None) + .await + .map_err(|e| format!("tool execution failed: {e}")), + "network_status" => { + let visibility = handler + .assistant_dispatch_tool("network.get-visibility", None) + .await + .map_err(|e| format!("tool execution failed: {e}"))?; + let diagnostics = handler + .assistant_dispatch_tool("network.diagnostics", None) + .await + .map_err(|e| format!("tool execution failed: {e}"))?; + Ok(json!({ "visibility": visibility, "diagnostics": diagnostics })) + } + "mesh_status" => handler + .assistant_dispatch_tool("mesh.status", None) + .await + .map_err(|e| format!("tool execution failed: {e}")), + "content_list" => handler + .assistant_dispatch_tool("content.list-mine", None) + .await + .map_err(|e| format!("tool execution failed: {e}")), + "settings_get" => { + let ToolArgs::SettingsGet(a) = args else { + return Err("internal error: args/tool mismatch for settings_get".to_string()); + }; + if !READABLE_SETTINGS_KEYS.contains(&a.key.as_str()) { + return Err(format!( + "\"{}\" is not readable via chat. Readable keys: {}.", + a.key, + READABLE_SETTINGS_KEYS.join(", ") + )); + } + match a.key.as_str() { + "network_visibility" => handler + .assistant_dispatch_tool("network.get-visibility", None) + .await + .map_err(|e| format!("tool execution failed: {e}")), + "kiosk_display_preset" => handler + .assistant_dispatch_tool("system.kiosk-display.get", None) + .await + .map_err(|e| format!("tool execution failed: {e}")), + "claude_api_key_set" => handler + .assistant_dispatch_tool( + "system.settings.get", + Some(json!({ "key": "claude_api_key_set" })), + ) + .await + .map_err(|e| format!("tool execution failed: {e}")), + other => Err(format!("internal error: unhandled readable key {other}")), + } + } + "settings_set" => { + let ToolArgs::SettingsSet(a) = args else { + return Err("internal error: args/tool mismatch for settings_set".to_string()); + }; + if a.key == "claude_api_key" { + return Err( + "claude_api_key is key material and is permanently excluded from chat \ + reach (D-09). Change it in neode-ui's Settings screen instead." + .to_string(), + ); + } + if !SETTABLE_KEYS.contains(&a.key.as_str()) { + return Err(format!( + "\"{}\" is not settable via chat. Settable keys: {}. Use neode-ui's Settings \ + screen for anything else.", + a.key, + SETTABLE_KEYS.join(", ") + )); + } + match a.key.as_str() { + "network_visibility" => { + let visibility = a + .value + .as_str() + .ok_or_else(|| "network_visibility's value must be a string".to_string())?; + handler + .assistant_dispatch_tool( + "network.set-visibility", + Some(json!({ "visibility": visibility })), + ) + .await + .map_err(|e| format!("tool execution failed: {e}")) + } + "kiosk_display_preset" => { + let preset = a + .value + .as_str() + .ok_or_else(|| "kiosk_display_preset's value must be a string".to_string())?; + handler + .assistant_dispatch_tool( + "system.kiosk-display.set", + Some(json!({ "preset": preset })), + ) + .await + .map_err(|e| format!("tool execution failed: {e}")) + } + "wifi_radio" => { + let enabled = a + .value + .as_bool() + .ok_or_else(|| "wifi_radio's value must be a boolean".to_string())?; + handler + .assistant_dispatch_tool( + "network.set-wifi-radio", + Some(json!({ "enabled": enabled })), + ) + .await + .map_err(|e| format!("tool execution failed: {e}")) + } + "bitcoin_relay_settings" => { + if !a.value.is_object() { + return Err("bitcoin_relay_settings's value must be an object".to_string()); + } + handler + .assistant_dispatch_tool("bitcoin.relay-update-settings", Some(a.value.clone())) + .await + .map_err(|e| format!("tool execution failed: {e}")) + } + other => Err(format!("internal error: unhandled settable key {other}")), + } + } + "app_start" => { + let ToolArgs::AppId(a) = args else { + return Err("internal error: args/tool mismatch for app_start".to_string()); + }; + let resolved = resolve_installed_app_id(&a.app_id, handler).await?; + handler + .assistant_dispatch_tool("container-start", Some(json!({ "app_id": resolved }))) + .await + .map_err(|e| format!("tool execution failed: {e}")) + } + "app_stop" => { + let ToolArgs::AppId(a) = args else { + return Err("internal error: args/tool mismatch for app_stop".to_string()); + }; + let resolved = resolve_installed_app_id(&a.app_id, handler).await?; + handler + .assistant_dispatch_tool("container-stop", Some(json!({ "app_id": resolved }))) + .await + .map_err(|e| format!("tool execution failed: {e}")) + } + "app_restart" => { + let ToolArgs::AppId(a) = args else { + return Err("internal error: args/tool mismatch for app_restart".to_string()); + }; + let resolved = resolve_installed_app_id(&a.app_id, handler).await?; + handler + .assistant_dispatch_tool("container-restart", Some(json!({ "app_id": resolved }))) + .await + .map_err(|e| format!("tool execution failed: {e}")) + } + other => Err(format!("no execution wired for tool: {other}")), + } +} + #[cfg(test)] mod tests { use super::*; + use crate::api::rpc::RpcHandler; + use crate::assistant::loop_::execute_tool; + use crate::assistant::{CallerScope, ToolExecCtx}; + use std::sync::Arc; + + /// A minimal but real `RpcHandler` for tests, matching + /// `loop_::tests::test_rpc_handler` — a fresh temp `data_dir`, no + /// orchestrator. + async fn test_rpc_handler() -> (Arc, tempfile::TempDir) { + let tmp = tempfile::tempdir().expect("tempdir"); + let mut config = crate::config::Config::default(); + config.data_dir = tmp.path().to_path_buf(); + let state_manager = Arc::new(crate::state::StateManager::new()); + let metrics_store = Arc::new(crate::monitoring::MetricsStore::new()); + let session_store = + crate::session::SessionStore::new_for_tests(tmp.path().join("sessions.json")); + let handler = RpcHandler::new( + config, + state_manager, + metrics_store, + session_store, + None, + None, + ) + .await + .expect("RpcHandler::new"); + (Arc::new(handler), tmp) + } + + async fn grant_all(handler: &Arc) { + let mut g = crate::assistant::grants::Grants::load(handler.data_dir()).await; + for category in PermissionCategory::ALL { + g.set(category, true); + } + g.save(handler.data_dir()).await.expect("save grants"); + } + + fn local_operator_ctx(handler: Arc) -> ToolExecCtx { + ToolExecCtx::new( + registry(), + CallerScope::LocalOperator { + session_id: "test-session".to_string(), + }, + handler, + ) + } /// The schema sent to the model and the struct used to deserialize its - /// output must never silently drift apart. Round-trip the schema's - /// declared `required` keys through the args struct. + /// output must never silently drift apart, for EVERY tool in the + /// registry — not just the tracer's original one. #[test] - fn disk_status_schema_round_trips_required_keys() { - let tool = system_disk_status_tool(); - let required = tool - .parameters - .get("required") - .and_then(|r| r.as_array()) - .cloned() - .unwrap_or_default(); + fn every_tool_schema_round_trips_required_keys_into_its_args_struct() { + let reg = registry(); + for tool in reg.all() { + let required = tool + .parameters + .get("required") + .and_then(|r| r.as_array()) + .cloned() + .unwrap_or_default(); + let properties = tool.parameters.get("properties").cloned().unwrap_or_else(|| json!({})); - let mut obj = serde_json::Map::new(); - for key in &required { - if let Some(k) = key.as_str() { - obj.insert(k.to_string(), Value::Null); + let mut obj = serde_json::Map::new(); + for key in &required { + if let Some(k) = key.as_str() { + let prop_schema = properties.get(k).cloned().unwrap_or_else(|| json!({})); + let dummy = match prop_schema.get("type").and_then(|t| t.as_str()) { + Some("string") => json!(""), + Some("integer") | Some("number") => json!(0), + Some("boolean") => json!(false), + Some("object") => json!({}), + _ => json!(""), + }; + obj.insert(k.to_string(), dummy); + } } + let value = Value::Object(obj); + let parsed = tool.validate(&value); + assert!( + parsed.is_ok(), + "schema/args struct drift for tool {}: {:?}", + tool.name, + parsed.err() + ); } - let value = Value::Object(obj); - let parsed: Result = serde_json::from_value(value); - assert!(parsed.is_ok(), "schema/args struct drift: {:?}", parsed.err()); } #[test] @@ -165,6 +881,100 @@ mod tests { let mut grants = BTreeSet::new(); assert!(reg.visible_to(&grants).is_empty()); grants.insert(PermissionCategory::System); - assert_eq!(reg.visible_to(&grants).len(), 1); + // System category currently has 4 tools: system_disk_status, + // system_stats, settings_get, settings_set (destructive is a + // separate axis from category — visible_to filters on category + // only, same as the real system-prompt/list-tools filtering). + assert_eq!(reg.visible_to(&grants).len(), 4); } + + #[test] + fn settable_keys_never_include_claude_api_key() { + assert!( + !SETTABLE_KEYS.contains(&"claude_api_key"), + "claude_api_key must never appear in SETTABLE_KEYS (D-09 — key material stays chat-unreachable)" + ); + } + + /// S-05 (T-13-26): an ungranted category is refused at `execute_tool` + /// even when the tool was somehow proposed anyway — the system prompt + /// omitting it is defense in depth, never the gate. Exercises the SAME + /// choke point the real loop calls (`loop_::execute_tool`), not a + /// reimplementation. + #[tokio::test] + async fn settings_tool_respects_category_grant() { + let (handler, _tmp) = test_rpc_handler().await; + // Deliberately do NOT grant System — settings_set is System-scoped. + let ctx = local_operator_ctx(handler); + let call = ToolCall { + id: "call-1".to_string(), + name: "settings_set".to_string(), + arguments: json!({ "key": "wifi_radio", "value": true }), + }; + let result = execute_tool(&call, &ctx).await; + assert!(result.is_error, "settings_set must be refused when System is not granted"); + assert!( + result.content.contains("not permitted"), + "expected a not-permitted refusal, got: {}", + result.content + ); + } + + #[tokio::test] + async fn settings_set_refuses_claude_api_key_by_name() { + let (handler, _tmp) = test_rpc_handler().await; + grant_all(&handler).await; + let ctx = local_operator_ctx(handler); + let call = ToolCall { + id: "call-1".to_string(), + name: "settings_set".to_string(), + arguments: json!({ "key": "claude_api_key", "value": "sk-whatever" }), + }; + let result = execute_tool(&call, &ctx).await; + assert!(result.is_error); + assert!( + result.content.to_lowercase().contains("neode-ui"), + "refusal must name the real neode-ui Settings path, got: {}", + result.content + ); + } + + #[tokio::test] + async fn settings_set_refuses_unlisted_key() { + let (handler, _tmp) = test_rpc_handler().await; + grant_all(&handler).await; + let ctx = local_operator_ctx(handler); + let call = ToolCall { + id: "call-1".to_string(), + name: "settings_set".to_string(), + arguments: json!({ "key": "totally_made_up_key", "value": "x" }), + }; + let result = execute_tool(&call, &ctx).await; + assert!(result.is_error); + assert!( + result.content.contains("network_visibility"), + "refusal must name the settable keys, got: {}", + result.content + ); + } + + #[tokio::test] + async fn app_restart_refuses_unknown_app_id() { + let (handler, _tmp) = test_rpc_handler().await; + grant_all(&handler).await; + let ctx = local_operator_ctx(handler); + let call = ToolCall { + id: "call-1".to_string(), + name: "app_restart".to_string(), + arguments: json!({ "app_id": "definitely-not-installed" }), + }; + let result = execute_tool(&call, &ctx).await; + assert!(result.is_error, "an unknown app id must be refused, not guessed at"); + assert!( + result.content.contains("no such app id"), + "got: {}", + result.content + ); + } + }