diff --git a/src/openhuman/about_app/catalog.rs b/src/openhuman/about_app/catalog.rs index 94e3fc70c..d5140280f 100644 --- a/src/openhuman/about_app/catalog.rs +++ b/src/openhuman/about_app/catalog.rs @@ -35,6 +35,18 @@ const MODEL_DOWNLOAD: Option = Some(CapabilityPrivacy { destinations: &["Hugging Face"], }); +// Self-update flows talk to GitHub Releases directly, not the OpenHuman +// backend. The outbound payload is metadata only (release list query for +// `update.check`, asset download URL request for `update.apply`) so +// `data_kind: Metadata` is the right label — but the destination must +// reflect that this is a third-party host, otherwise the capability +// catalog under-reports where the user's request actually goes. +const GITHUB_RELEASES_METADATA: Option = Some(CapabilityPrivacy { + leaves_device: true, + data_kind: PrivacyDataKind::Metadata, + destinations: &["GitHub Releases"], +}); + const CAPABILITIES: &[Capability] = &[ Capability { id: "conversation.create", @@ -926,20 +938,26 @@ const CAPABILITIES: &[Capability] = &[ name: "Check for Core Updates", domain: "update", category: CapabilityCategory::Settings, - description: "Query GitHub Releases to see if a newer core binary is available.", - how_to: "Settings > Developer Options > Check for Updates", + description: "Query GitHub Releases to see if a newer core binary is available. \ + Available to the orchestrator agent as the `update_check` tool so the \ + user can ask 'am I up to date?' in chat.", + how_to: "Settings > Developer Options > Check for Updates, or ask the orchestrator in chat.", status: CapabilityStatus::Beta, - privacy: DIAGNOSTICS_TO_BACKEND, + privacy: GITHUB_RELEASES_METADATA, }, Capability { id: "update.apply", name: "Apply Core Update", domain: "update", category: CapabilityCategory::Settings, - description: "Download and stage a newer core binary. Desktop builds can self-restart; headless deployments can hand restart off to a supervisor.", - how_to: "Settings > Developer Options > Apply Update", + description: "Download and stage a newer core binary. Desktop builds can self-restart; \ + headless deployments can hand restart off to a supervisor. Exposed to \ + the orchestrator agent as the `update_apply` tool, gated behind explicit \ + user consent (the agent must confirm via `ask_user_clarification` before \ + invoking) and the `config.update.rpc_mutations_enabled` policy switch.", + how_to: "Settings > Developer Options > Apply Update, or confirm an in-chat update prompt from the orchestrator.", status: CapabilityStatus::Beta, - privacy: None, + privacy: GITHUB_RELEASES_METADATA, }, ]; diff --git a/src/openhuman/agent/agents/orchestrator/agent.toml b/src/openhuman/agent/agents/orchestrator/agent.toml index cd3191a4a..13dafafca 100644 --- a/src/openhuman/agent/agents/orchestrator/agent.toml +++ b/src/openhuman/agent/agents/orchestrator/agent.toml @@ -106,4 +106,13 @@ named = [ # touch files itself. "todowrite", "plan_exit", + # Self-update — lets the orchestrator answer "am I up to date" / + # "update OpenHuman" without sending the user to Settings → + # Developer Options. `update_check` is read-only; `update_apply` + # is dangerous, requires explicit user consent through + # `ask_user_clarification`, and respects + # `config.update.rpc_mutations_enabled`. Implementation re-uses + # `src/openhuman/update/` end-to-end — no parallel release logic. + "update_check", + "update_apply", ] diff --git a/src/openhuman/security/mod.rs b/src/openhuman/security/mod.rs index ba3c10a3b..447e2af25 100644 --- a/src/openhuman/security/mod.rs +++ b/src/openhuman/security/mod.rs @@ -25,6 +25,7 @@ pub use pairing::PairingGuard; #[allow(unused_imports)] pub use policy::AutonomyLevel; pub use policy::SecurityPolicy; +pub use policy::ToolOperation; #[allow(unused_imports)] pub use secrets::SecretStore; #[allow(unused_imports)] diff --git a/src/openhuman/tools/impl/system/mod.rs b/src/openhuman/tools/impl/system/mod.rs index ffde5c104..5cb738218 100644 --- a/src/openhuman/tools/impl/system/mod.rs +++ b/src/openhuman/tools/impl/system/mod.rs @@ -8,6 +8,8 @@ mod pushover; mod schedule; mod shell; mod tool_stats; +mod update_apply; +mod update_check; mod workspace_state; pub use current_time::CurrentTimeTool; @@ -20,4 +22,6 @@ pub use pushover::PushoverTool; pub use schedule::ScheduleTool; pub use shell::ShellTool; pub use tool_stats::ToolStatsTool; +pub use update_apply::UpdateApplyTool; +pub use update_check::UpdateCheckTool; pub use workspace_state::WorkspaceStateTool; diff --git a/src/openhuman/tools/impl/system/update_apply.rs b/src/openhuman/tools/impl/system/update_apply.rs new file mode 100644 index 000000000..29948b5ca --- /dev/null +++ b/src/openhuman/tools/impl/system/update_apply.rs @@ -0,0 +1,156 @@ +//! Tool: `update_apply` — orchestrated check → download → stage → +//! restart of the OpenHuman core binary. +//! +//! Wraps [`crate::openhuman::update::rpc::update_run`] so the LLM can +//! finish an "update me to the latest version" intent in chat. The +//! underlying RPC already enforces: +//! +//! * `config.update.rpc_mutations_enabled` — fail-closed if disabled. +//! * GitHub host + asset-name validation before any download. +//! * The configured `restart_strategy` (`self_replace` vs +//! `supervisor`) — staging logic stays in the `update` domain. +//! +//! On top of that, this tool layers a tool-level autonomy check +//! (`SecurityPolicy::can_act`) so a read-only session can never trigger +//! a download, even if the policy gate is on. +//! +//! The tool description tells the orchestrator to confirm with the user +//! via `ask_user_clarification` before invoking — applying an update is +//! high impact (replaces a binary on disk, optionally restarts the core +//! process) and must not happen silently. + +use std::sync::Arc; + +use async_trait::async_trait; +use serde_json::{json, Value}; + +use crate::openhuman::security::{SecurityPolicy, ToolOperation}; +use crate::openhuman::tools::traits::{PermissionLevel, Tool, ToolResult}; +use crate::openhuman::update; + +pub struct UpdateApplyTool { + security: Arc, +} + +impl UpdateApplyTool { + pub fn new(security: Arc) -> Self { + Self { security } + } + + /// Delegate to the shared `SecurityPolicy::enforce_tool_operation` + /// path so this tool stays in lock-step with every other act-level + /// tool's autonomy + rate-limit handling. Hand-rolling + /// `can_act` + `record_action` here used to drift from the canonical + /// `[openhuman:policy]` log format and would silently miss any + /// future gate the enforcer grows (token budget, supervised + /// approval, etc.). + fn require_write_access(&self) -> Option { + self.security + .enforce_tool_operation(ToolOperation::Act, "update_apply") + .err() + .map(ToolResult::error) + } + + fn require_consent(args: &Value) -> Option { + let confirmed = args + .get("user_confirmed") + .and_then(Value::as_bool) + .unwrap_or(false); + if confirmed { + return None; + } + Some(ToolResult::error( + "update_apply blocked: needs explicit user consent. Confirm with the user via \ + `ask_user_clarification` (e.g. 'Should I download and stage the new core build now?'), \ + then call this tool again with `user_confirmed: true`.", + )) + } +} + +#[async_trait] +impl Tool for UpdateApplyTool { + fn name(&self) -> &str { + "update_apply" + } + + fn description(&self) -> &str { + "Download, verify, and stage the latest OpenHuman core binary, then \ + request a restart per the configured restart strategy. HIGH IMPACT — \ + replaces the running binary on disk and (under `self_replace`) \ + restarts the core process. ALWAYS confirm with the user via \ + `ask_user_clarification` first, then call this tool with \ + `user_confirmed: true`. Use `update_check` first to verify a newer \ + version actually exists. Disabled when \ + `config.update.rpc_mutations_enabled = false`." + } + + fn parameters_schema(&self) -> Value { + json!({ + "type": "object", + "properties": { + "user_confirmed": { + "type": "boolean", + "description": "Must be true. Set ONLY after the user has explicitly approved \ + applying the update via `ask_user_clarification`. The tool \ + refuses to run when this is missing or false." + } + }, + "required": ["user_confirmed"], + "additionalProperties": false + }) + } + + fn permission_level(&self) -> PermissionLevel { + PermissionLevel::Dangerous + } + + async fn execute(&self, args: Value) -> anyhow::Result { + tracing::debug!(args = %args, "[update_apply] execute start"); + + if let Some(blocked) = Self::require_consent(&args) { + tracing::warn!("[update_apply] blocked: missing user_confirmed"); + return Ok(blocked); + } + if let Some(blocked) = self.require_write_access() { + tracing::warn!("[update_apply] blocked: autonomy gate"); + return Ok(blocked); + } + + let outcome = update::rpc::update_run().await; + for log in &outcome.logs { + tracing::debug!(target: "update_apply", "{log}"); + } + let body = serde_json::to_string_pretty(&outcome.value)?; + // `RpcOutcome` does not carry an explicit status flag, so + // we have to read the shape: a `{"error": …}` body is the obvious + // failure case, but `update_run`'s soft-failure paths + // ("already current", "no platform asset for this target", + // "download/stage failed") return `applied: false` with a + // descriptive `message` and no `error` key. Surfacing those as + // `ToolResult::success` would mean the user sees a green tick in + // chat even though nothing was installed — treat any non-applied + // outcome as a tool error so the LLM has to acknowledge it. + let applied = outcome + .value + .get("applied") + .and_then(Value::as_bool) + .unwrap_or(false); + let has_error_key = outcome.value.get("error").is_some(); + let is_error = has_error_key || !applied; + tracing::debug!( + applied, + has_error_key, + is_error, + "[update_apply] execute done" + ); + Ok(if is_error { + ToolResult::error(body) + } else { + ToolResult::success(body) + }) + } +} + +#[cfg(test)] +#[path = "update_apply_tests.rs"] +mod tests; diff --git a/src/openhuman/tools/impl/system/update_apply_tests.rs b/src/openhuman/tools/impl/system/update_apply_tests.rs new file mode 100644 index 000000000..772736f83 --- /dev/null +++ b/src/openhuman/tools/impl/system/update_apply_tests.rs @@ -0,0 +1,98 @@ +use super::*; +use crate::openhuman::security::{AutonomyLevel, SecurityPolicy}; +use serde_json::json; +use std::sync::Arc; + +fn supervised() -> Arc { + Arc::new(SecurityPolicy { + autonomy: AutonomyLevel::Supervised, + workspace_dir: std::env::temp_dir(), + ..SecurityPolicy::default() + }) +} + +fn read_only() -> Arc { + Arc::new(SecurityPolicy { + autonomy: AutonomyLevel::ReadOnly, + workspace_dir: std::env::temp_dir(), + ..SecurityPolicy::default() + }) +} + +#[test] +fn name_and_permission() { + let tool = UpdateApplyTool::new(supervised()); + assert_eq!(tool.name(), "update_apply"); + assert_eq!(tool.permission_level(), PermissionLevel::Dangerous); +} + +#[test] +fn schema_requires_user_confirmed_boolean() { + let schema = UpdateApplyTool::new(supervised()).parameters_schema(); + assert_eq!(schema["type"], "object"); + assert_eq!(schema["additionalProperties"], false); + assert_eq!(schema["required"][0], "user_confirmed"); + assert_eq!(schema["properties"]["user_confirmed"]["type"], "boolean"); +} + +#[test] +fn description_calls_out_consent_and_companion_check_tool() { + let tool = UpdateApplyTool::new(supervised()); + let desc = tool.description(); + assert!(desc.contains("ask_user_clarification")); + assert!(desc.contains("update_check")); + assert!(desc.contains("user_confirmed")); + assert!(desc.contains("HIGH IMPACT")); +} + +#[tokio::test] +async fn rejects_when_user_confirmed_missing() { + let tool = UpdateApplyTool::new(supervised()); + let result = tool.execute(json!({})).await.unwrap(); + assert!(result.is_error); + assert!(result.output().contains("user consent")); + assert!(result.output().contains("ask_user_clarification")); +} + +#[tokio::test] +async fn rejects_when_user_confirmed_false() { + let tool = UpdateApplyTool::new(supervised()); + let result = tool + .execute(json!({ "user_confirmed": false })) + .await + .unwrap(); + assert!(result.is_error); + assert!(result.output().contains("user consent")); +} + +#[tokio::test] +async fn rejects_in_read_only_mode_even_with_consent() { + let tool = UpdateApplyTool::new(read_only()); + let result = tool + .execute(json!({ "user_confirmed": true })) + .await + .unwrap(); + assert!(result.is_error); + // The autonomy gate now delegates to `SecurityPolicy::enforce_tool_operation`, + // whose canonical read-only message is `"Security policy: read-only + // mode, cannot perform ''"` — assert against that phrasing + // and the operation name rather than the old hand-rolled string. + let body = result.output(); + assert!( + body.contains("read-only mode") && body.contains("update_apply"), + "expected shared enforcer's read-only message, got: {body}" + ); +} + +#[tokio::test] +async fn consent_check_runs_before_autonomy_check() { + // A read-only session that also forgot to confirm should see the + // consent error first — this keeps the LLM-facing failure mode + // consistent regardless of the host autonomy level (a downgraded + // session shouldn't suddenly mint different "fix this" hints). + let tool = UpdateApplyTool::new(read_only()); + let result = tool.execute(json!({})).await.unwrap(); + assert!(result.is_error); + assert!(result.output().contains("user consent")); + assert!(!result.output().contains("read-only mode")); +} diff --git a/src/openhuman/tools/impl/system/update_check.rs b/src/openhuman/tools/impl/system/update_check.rs new file mode 100644 index 000000000..26691a284 --- /dev/null +++ b/src/openhuman/tools/impl/system/update_check.rs @@ -0,0 +1,89 @@ +//! Tool: `update_check` — surface the orchestrator's view of the +//! self-update domain. +//! +//! Read-only wrapper around [`crate::openhuman::update::rpc::update_check`]. +//! Lets the orchestrator answer "is there a new version?" in chat without +//! routing the user to Settings → Developer Options. Implementation is a +//! thin pass-through — release URL discovery and version comparison stay +//! in the `update` domain. + +use async_trait::async_trait; +use serde_json::{json, Value}; + +use crate::openhuman::tools::traits::{PermissionLevel, Tool, ToolResult}; +use crate::openhuman::update; + +pub struct UpdateCheckTool; + +impl UpdateCheckTool { + pub fn new() -> Self { + Self + } +} + +impl Default for UpdateCheckTool { + fn default() -> Self { + Self::new() + } +} + +#[async_trait] +impl Tool for UpdateCheckTool { + fn name(&self) -> &str { + "update_check" + } + + fn description(&self) -> &str { + "Check whether a newer OpenHuman core build is available on GitHub \ + Releases. Read-only — performs one HTTPS request to the releases \ + feed and returns version info plus, if a newer build exists, its \ + download URL, asset name, and release notes. Use this when the \ + user asks 'am I up to date', 'is there a new version', or before \ + offering to apply an update with `update_apply`. Does NOT download \ + or install anything." + } + + fn parameters_schema(&self) -> Value { + json!({ + "type": "object", + "properties": {}, + "additionalProperties": false + }) + } + + fn permission_level(&self) -> PermissionLevel { + PermissionLevel::ReadOnly + } + + async fn execute(&self, _args: Value) -> anyhow::Result { + tracing::debug!("[update_check] execute start"); + let outcome = update::rpc::update_check().await; + let body = serde_json::to_string_pretty(&outcome.value)?; + for log in &outcome.logs { + tracing::debug!(target: "update_check", "{log}"); + } + // `update_check` is read-only — both "newer release available" + // and "you're already on the latest version" are normal success + // outcomes here, so we only error out when the underlying RPC + // explicitly surfaces an `error` key (e.g. release feed + // unreachable, asset metadata malformed). The applied=false + // tightening that `update_apply` performs is intentionally NOT + // mirrored — read-only callers must keep treating "no update" + // as a happy answer. + let is_error = outcome.value.get("error").is_some(); + tracing::debug!( + is_error, + body_len = body.len(), + "[update_check] execute done" + ); + Ok(if is_error { + ToolResult::error(body) + } else { + ToolResult::success(body) + }) + } +} + +#[cfg(test)] +#[path = "update_check_tests.rs"] +mod tests; diff --git a/src/openhuman/tools/impl/system/update_check_tests.rs b/src/openhuman/tools/impl/system/update_check_tests.rs new file mode 100644 index 000000000..06b212017 --- /dev/null +++ b/src/openhuman/tools/impl/system/update_check_tests.rs @@ -0,0 +1,36 @@ +use super::*; + +#[test] +fn name_and_permission() { + let tool = UpdateCheckTool::new(); + assert_eq!(tool.name(), "update_check"); + assert_eq!(tool.permission_level(), PermissionLevel::ReadOnly); +} + +#[test] +fn schema_is_closed_object_with_no_properties() { + let schema = UpdateCheckTool::new().parameters_schema(); + assert_eq!(schema["type"], "object"); + assert_eq!(schema["additionalProperties"], false); + let props = schema["properties"].as_object().expect("properties object"); + assert!( + props.is_empty(), + "update_check takes no arguments — schema must not advertise any" + ); +} + +#[test] +fn description_mentions_safety_and_companion_tool() { + let tool = UpdateCheckTool::new(); + let desc = tool.description(); + assert!(desc.contains("Read-only")); + assert!(desc.contains("update_apply")); + assert!(desc.contains("Does NOT download")); +} + +#[test] +fn default_constructs_same_as_new() { + let a = UpdateCheckTool::default(); + let b = UpdateCheckTool::new(); + assert_eq!(a.name(), b.name()); +} diff --git a/src/openhuman/tools/ops.rs b/src/openhuman/tools/ops.rs index a06af4d29..4c0d4ad0d 100644 --- a/src/openhuman/tools/ops.rs +++ b/src/openhuman/tools/ops.rs @@ -144,6 +144,8 @@ pub fn all_tools_with_runtime( Box::new(WhatsAppDataSearchMessagesTool), Box::new(ScheduleTool::new(security.clone(), root_config.clone())), Box::new(ProxyConfigTool::new(config.clone(), security.clone())), + Box::new(UpdateCheckTool::new()), + Box::new(UpdateApplyTool::new(security.clone())), Box::new(GitOperationsTool::new( security.clone(), workspace_dir.to_path_buf(), diff --git a/src/openhuman/tools/user_filter.rs b/src/openhuman/tools/user_filter.rs index f448b1ade..1467ba86b 100644 --- a/src/openhuman/tools/user_filter.rs +++ b/src/openhuman/tools/user_filter.rs @@ -29,6 +29,11 @@ const TOOL_ID_TO_RUST_NAMES: &[(&str, &[&str])] = &[ ], ), ("schedule", &["schedule"]), + // Self-update tools (issue #1435). Filterable so the onboarding + // tool-toggle surface can default them off and let users opt in. + // `update_check` is read-only; `update_apply` is gated by both the + // tool-level autonomy check and `config.update.rpc_mutations_enabled`. + ("update", &["update_check", "update_apply"]), ]; /// All Rust tool names that are filterable (union of all mapping values). diff --git a/src/openhuman/update/ops.rs b/src/openhuman/update/ops.rs index 86d8823c0..2a311a856 100644 --- a/src/openhuman/update/ops.rs +++ b/src/openhuman/update/ops.rs @@ -488,8 +488,20 @@ mod tests { // ── update_apply rejection paths ────────────────────────────── + // `update_apply` reads the mutation-policy config from disk, whose + // path is resolved through the process-global `OPENHUMAN_WORKSPACE` + // env var. Tests that don't lock against the disabled-mutations + // case can race with it: the disabled test sets the env var, the + // sibling test (running on another thread) clears or shadows it + // between `WorkspaceEnvGuard::set` and the await inside + // `update_apply`, and the disabled test then loads a default + // policy (where `rpc_mutations_enabled = true`), proceeds past the + // gate, and fails its `contains("rpc_mutations_enabled=false")` + // assertion. Take `TEST_ENV_LOCK` in every test that calls + // `update_apply` so the three cases serialise on the same mutex. #[tokio::test] async fn update_apply_rejects_non_github_url_before_network_call() { + let _guard = TEST_ENV_LOCK.lock().unwrap(); let outcome = update_apply( "https://evil.example.com/asset".to_string(), "openhuman-core-x86_64.tar.gz".to_string(), @@ -505,6 +517,7 @@ mod tests { #[tokio::test] async fn update_apply_rejects_unsafe_asset_name() { + let _guard = TEST_ENV_LOCK.lock().unwrap(); let outcome = update_apply( "https://github.com/owner/repo/releases/download/v1/x".to_string(), "../etc/passwd".to_string(),