From 3a7330884f53cb7685df6aadc5793ea14c8f5747 Mon Sep 17 00:00:00 2001 From: Steven Enamakel <31011319+senamakel@users.noreply.github.com> Date: Mon, 6 Jul 2026 09:30:04 -0700 Subject: [PATCH] feat(agent): surface storage transfer tools (#4577) --- .../agents/code_executor/agent.toml | 4 + .../agents/integrations_agent/agent.toml | 9 +- src/openhuman/agent_registry/agents/loader.rs | 44 +++++- .../agents/orchestrator/agent.toml | 17 ++- .../agents/orchestrator/prompt.md | 10 +- src/openhuman/file_storage/tools.rs | 133 ++++++++++++++++-- src/openhuman/file_storage/tools_tests.rs | 67 ++++++++- .../tools/impl/network/http_request.rs | 15 +- .../tools/impl/network/http_request_tests.rs | 25 ++++ 9 files changed, 295 insertions(+), 29 deletions(-) diff --git a/src/openhuman/agent_registry/agents/code_executor/agent.toml b/src/openhuman/agent_registry/agents/code_executor/agent.toml index 2816bc2a8..c05ce9bfe 100644 --- a/src/openhuman/agent_registry/agents/code_executor/agent.toml +++ b/src/openhuman/agent_registry/agents/code_executor/agent.toml @@ -46,5 +46,9 @@ named = [ "todowrite", "plan_exit", "web_fetch", + "storage_upload_file", + "storage_download_file", + "storage_list_files", + "storage_get_link", "lsp", ] diff --git a/src/openhuman/agent_registry/agents/integrations_agent/agent.toml b/src/openhuman/agent_registry/agents/integrations_agent/agent.toml index 069de0b86..262889bab 100644 --- a/src/openhuman/agent_registry/agents/integrations_agent/agent.toml +++ b/src/openhuman/agent_registry/agents/integrations_agent/agent.toml @@ -14,14 +14,19 @@ omit_skills_catalog = true hint = "burst" [tools] -# Composio-only surface: this agent drives a single Composio toolkit per -# spawn. Workflow discovery/execution is the orchestrator's job now +# Integration surface: this agent drives a single Composio toolkit per +# spawn, with managed file-storage transfer tools for attachments/artifacts. +# Workflow discovery/execution is the orchestrator's job now # (`list_workflows` / `run_workflow`), so the old `workflow_load` / # `workflow_phase` tools (removed with the `agent_workflows` domain) are gone. named = [ "composio_list_tools", "file_read", "composio_execute", + "storage_upload_file", + "storage_download_file", + "storage_list_files", + "storage_get_link", # Inline-in-chat OAuth connect card (#3993). When the bound toolkit is not # connected — or an action fails with a true auth/connection error — raise # the connect card with this tool and await the result, instead of bubbling diff --git a/src/openhuman/agent_registry/agents/loader.rs b/src/openhuman/agent_registry/agents/loader.rs index 53c9993bb..ba7d7a013 100644 --- a/src/openhuman/agent_registry/agents/loader.rs +++ b/src/openhuman/agent_registry/agents/loader.rs @@ -733,10 +733,18 @@ mod tests { // orchestrator must not mutate files or run commands; all // modification is deferred to `run_code` / owning // specialists where edits live next to build/test/verify. - for forbidden in ["shell", "edit", "file_write", "apply_patch"] { + for forbidden in [ + "shell", + "edit", + "file_write", + "apply_patch", + "curl", + "storage_set_visibility", + "storage_delete_file", + ] { assert!( !tools.iter().any(|t| t == forbidden), - "orchestrator must NOT have write/exec tool `{forbidden}`" + "orchestrator must NOT have write/exec/lifecycle tool `{forbidden}`" ); } // Basic READ-ONLY direct surface: quick lookups without @@ -748,6 +756,7 @@ mod tests { "list", "web_search_tool", "web_fetch", + "http_request", ] { assert!( tools.iter().any(|t| t == direct), @@ -815,6 +824,35 @@ mod tests { ); } + #[test] + fn broad_agent_surfaces_expose_storage_transfer_not_lifecycle_tools() { + for id in ["code_executor", "integrations_agent", "orchestrator"] { + let def = find(id); + match &def.tools { + ToolScope::Named(tools) => { + for required in [ + "storage_upload_file", + "storage_download_file", + "storage_list_files", + "storage_get_link", + ] { + assert!( + tools.iter().any(|t| t == required), + "{id} must expose storage transfer tool `{required}`" + ); + } + for forbidden in ["storage_set_visibility", "storage_delete_file"] { + assert!( + !tools.iter().any(|t| t == forbidden), + "{id} must not expose storage lifecycle tool `{forbidden}`" + ); + } + } + ToolScope::Wildcard => panic!("{id} must have Named tool scope"), + } + } + } + #[test] fn tool_maker_is_sandboxed_with_max_2_iterations() { let def = find("tool_maker"); @@ -1478,7 +1516,7 @@ mod tests { fn orchestrator_does_not_get_curl() { // Per design: curl is a `Write` permission tool that writes // to the workspace. The orchestrator delegates rather than - // executing — code_executor / researcher own actual downloads. + // executing — code_executor / tools_agent own actual downloads. let def = find("orchestrator"); if let ToolScope::Named(tools) = &def.tools { assert!( diff --git a/src/openhuman/agent_registry/agents/orchestrator/agent.toml b/src/openhuman/agent_registry/agents/orchestrator/agent.toml index 3cf9ccbbc..cf3076f7f 100644 --- a/src/openhuman/agent_registry/agents/orchestrator/agent.toml +++ b/src/openhuman/agent_registry/agents/orchestrator/agent.toml @@ -216,12 +216,21 @@ named = [ "grep", "glob", "list", - # Quick web lookups. A single fact ("what's the capital of X", "latest - # version of Y") is one `web_search_tool` / `web_fetch` call — spawning - # the `research` worker for that is overkill. Multi-source crawls, - # comparisons, and doc digests still delegate to `research`. + # Quick web/API lookups. A single fact ("what's the capital of X", "latest + # version of Y") is one `web_search_tool` / `web_fetch` / `http_request` + # call — spawning the `research` worker for that is overkill. Multi-source + # crawls, comparisons, and doc digests still delegate to `research`. "web_search_tool", "web_fetch", + "http_request", + # Managed file-storage transfer surface for passing artifacts/attachments + # between chat, users, and workers. Storage lifecycle mutators + # (`storage_set_visibility`, `storage_delete_file`) stay out of the broad + # chat surface. + "storage_upload_file", + "storage_download_file", + "storage_list_files", + "storage_get_link", # NOTE: `spawn_worker_thread` is intentionally absent — the tool is # disabled pending a worker-thread UI (#1624) and is not registered, so # listing it here (and teaching it in prompt.md) only trained the model diff --git a/src/openhuman/agent_registry/agents/orchestrator/prompt.md b/src/openhuman/agent_registry/agents/orchestrator/prompt.md index 7cbc69452..c92244a2f 100644 --- a/src/openhuman/agent_registry/agents/orchestrator/prompt.md +++ b/src/openhuman/agent_registry/agents/orchestrator/prompt.md @@ -1,6 +1,6 @@ # Orchestrator - Staff Engineer -You are the **Orchestrator**, the senior agent in a multi-agent system. Your role is strategic: you decide when to respond directly, when to use direct tools, and when to delegate. You have a small **read-only** direct surface for lookups (`file_read`, `grep`, `glob`, `list`, `web_search_tool`, `web_fetch`). You **never** write or edit files and **never** execute shell commands — every file modification, however small, is delegated to `run_code` (or the owning specialist). +You are the **Orchestrator**, the senior agent in a multi-agent system. Your role is strategic: you decide when to respond directly, when to use direct tools, and when to delegate. You have a small direct surface for lookups (`file_read`, `grep`, `glob`, `list`, `web_search_tool`, `web_fetch`, `http_request`) and managed storage transfer (`storage_upload_file`, `storage_download_file`, `storage_list_files`, `storage_get_link`). You **never** use generic file-write/edit tools and **never** execute shell commands — ordinary file modifications are delegated to `run_code` (or the owning specialist), while managed storage upload/download calls go through their own tool policy gates. ## Core Responsibilities @@ -24,8 +24,8 @@ Follow this sequence for every user message: - If the relevant toolkit is **not** in **Connected Integrations**, call `composio_connect { toolkit: "" }` **directly** to raise an **inline connect card** so the user can authorize in one click, then continue the task once it returns `connected: true`. Do **not** refuse based on the Connected Integrations list (that is only what is *already* connected, not what is *connectable*), do **not** make "go to Settings → Connections" your first move, and do **not** silently fall back to memory retrieval (see "Connecting external services" below). 3. **Can I solve this with direct tools?** - Yes: use direct tools (`retrieve_memory`, `read_workspace_state`, `composio_list_connections`, task tools, etc.). - - **Quick lookups are direct work.** A single web fact (a version number, a definition, one page's contents) is one `web_search_tool` or `web_fetch` call — do it yourself and answer. Reserve `research` for multi-source crawls, comparisons, or doc digests. - - **Read-only file lookups are direct work.** Reading a file the user names, grepping for a string, or listing a directory (`file_read` / `grep` / `glob` / `list`) needs no sub-agent. But you cannot write: the moment the task requires *changing* a file — even a one-line edit — delegate it to `run_code` (see below). Never promise an edit you cannot make yourself. + - **Quick lookups are direct work.** Use `web_search_tool` for quick discovery, `web_fetch` for one URL/body read, and `http_request` for basic API/HTTP semantics (methods, headers, JSON endpoints, status/HEAD checks). Reserve `research` for multi-source crawls, comparisons, deep digests, or uncertain evidence gathering. + - **Read-only file lookups are direct work.** Reading a file the user names, grepping for a string, or listing a directory (`file_read` / `grep` / `glob` / `list`) needs no sub-agent. Managed storage transfer is also direct when the user needs uploaded/downloaded/listed/linked artifacts. But you cannot use generic write/edit tools: the moment the task requires *changing* a file — even a one-line edit — delegate it to `run_code` (see below). Never promise an edit you cannot make yourself. - No: continue. 4. **Does this need other specialised execution?** - If the request is about OpenHuman product behavior, settings, docs, setup, or feature availability, use `ask_docs`. @@ -38,8 +38,8 @@ Follow this sequence for every user message: - **Do not stall after reading code-repo files.** If you (or a worker you spawned) have *read* files in a repo and have not yet *acted* on them — edited, built, tested, run, or pushed — and the user expects an outcome rather than a summary, that's the signal the task should have gone to `run_code` from the start. Re-issue the entire task as one `run_code` call with the full intent and let the code executor own the lifecycle. Do **not** narrate "reading the file…" / "let me check the code…" and then sit idle: in a code-repo task, reading is step zero of execution, not the deliverable. The user does not need to write "use the code executor" — infer it from the request shape (code, repo, file, build, test, run, fix, refactor, push, PR). - If the request is to find, browse, install, or manage agent skills from community registries — or to follow a SKILL.md URL — use `setup_skills`. - If the request is to run or execute an installed agent skill by name, use `run_skill`. The skill runs in an isolated worker, so its instructions never enter this conversation — you get back only its result. If that result contains a `## Handoff Plan` (steps the worker's narrow toolset couldn't perform — e.g. sending email, writing memory), carry out those steps yourself with your full tool set, routing each through the normal delegation path, then report the combined outcome. Treat handoff steps as *proposed* actions: never bypass the approval gate for them, especially for third-party skills. - - If multi-source web/doc crawling is required, use `research`. For a single live fact (weather, one price, one page) prefer your direct `web_search_tool` / `web_fetch` first. - - If the user asks for live/current/time-sensitive facts — weather, forecasts, current temperatures, recent news, fresh web facts, or "use Grok/web/live data" — get them now: one quick fact via direct `web_search_tool`, anything broader via `research` with a prompt that asks for live sources. Do **not** stop at "on it", and do **not** wait for the exact named provider if it is not wired in. Use the available tools and then answer with the result. + - If multi-source web/doc crawling is required, use `research`. For a single live fact (weather, one price, one page) prefer your direct `web_search_tool` / `web_fetch` / `http_request` first. + - If the user asks for live/current/time-sensitive facts — weather, forecasts, current temperatures, recent news, fresh web facts, or "use Grok/web/live data" — get them now: one quick fact via direct `web_search_tool` / `web_fetch` / `http_request`, anything broader via `research` with a prompt that asks for live sources. Do **not** stop at "on it", and do **not** wait for the exact named provider if it is not wired in. Use the available tools and then answer with the result. - If complex multi-step decomposition is required, use `plan`. - If code review is requested, use `review_code`. - If memory archiving or distillation is required, use `archive_session`. diff --git a/src/openhuman/file_storage/tools.rs b/src/openhuman/file_storage/tools.rs index 996c42c22..dd5db49e9 100644 --- a/src/openhuman/file_storage/tools.rs +++ b/src/openhuman/file_storage/tools.rs @@ -22,6 +22,7 @@ use serde_json::{json, Value}; use crate::openhuman::config::Config; use crate::openhuman::integrations::IntegrationClient; +use crate::openhuman::security::SecurityPolicy; use crate::openhuman::tools::traits::{ PermissionLevel, Tool, ToolCallOptions, ToolCategory, ToolResult, }; @@ -214,19 +215,46 @@ fn file_path(file_id: &str, suffix: &str) -> String { format!("{FILES_PATH}/{file_id}{suffix}") } +fn readonly_autonomy_block(security: &SecurityPolicy) -> Option { + if security.can_act() { + None + } else { + Some(ToolResult::error( + "[policy-blocked] Action blocked: autonomy is read-only", + )) + } +} + // ── StorageUploadFileTool ─────────────────────────────────────────── pub struct StorageUploadFileTool { client: Arc, action_dir: PathBuf, + security: Arc, } impl StorageUploadFileTool { pub fn new(client: Arc, action_dir: PathBuf) -> Self { - Self { client, action_dir } + Self::new_with_security(client, action_dir, Arc::new(SecurityPolicy::default())) + } + + pub fn new_with_security( + client: Arc, + action_dir: PathBuf, + security: Arc, + ) -> Self { + Self { + client, + action_dir, + security, + } } async fn run(&self, args: Value, action_dir: &Path) -> anyhow::Result { + if let Some(blocked) = readonly_autonomy_block(&self.security) { + return Ok(blocked); + } + let raw_path = match args.get("path").and_then(|v| v.as_str()) { Some(p) if !p.trim().is_empty() => p.trim(), _ => return Ok(ToolResult::error("path is required")), @@ -384,14 +412,31 @@ impl Tool for StorageUploadFileTool { pub struct StorageDownloadFileTool { client: Arc, action_dir: PathBuf, + security: Arc, } impl StorageDownloadFileTool { pub fn new(client: Arc, action_dir: PathBuf) -> Self { - Self { client, action_dir } + Self::new_with_security(client, action_dir, Arc::new(SecurityPolicy::default())) + } + + pub fn new_with_security( + client: Arc, + action_dir: PathBuf, + security: Arc, + ) -> Self { + Self { + client, + action_dir, + security, + } } async fn run(&self, args: Value, action_dir: &Path) -> anyhow::Result { + if let Some(blocked) = readonly_autonomy_block(&self.security) { + return Ok(blocked); + } + let file_id = match validate_file_id(&args) { Ok(id) => id, Err(e) => return Ok(ToolResult::error(e)), @@ -487,6 +532,14 @@ impl Tool for StorageDownloadFileTool { ToolCategory::Workflow } + fn permission_level(&self) -> PermissionLevel { + PermissionLevel::Write + } + + fn external_effect(&self) -> bool { + true + } + async fn execute(&self, args: Value) -> anyhow::Result { self.run(args, &self.action_dir).await } @@ -576,11 +629,19 @@ impl Tool for StorageListFilesTool { pub struct StorageGetLinkTool { client: Arc, + security: Arc, } impl StorageGetLinkTool { pub fn new(client: Arc) -> Self { - Self { client } + Self::new_with_security(client, Arc::new(SecurityPolicy::default())) + } + + pub fn new_with_security( + client: Arc, + security: Arc, + ) -> Self { + Self { client, security } } } @@ -612,7 +673,19 @@ impl Tool for StorageGetLinkTool { ToolCategory::Workflow } + fn permission_level(&self) -> PermissionLevel { + PermissionLevel::Write + } + + fn external_effect(&self) -> bool { + true + } + async fn execute(&self, args: Value) -> anyhow::Result { + if let Some(blocked) = readonly_autonomy_block(&self.security) { + return Ok(blocked); + } + let file_id = match validate_file_id(&args) { Ok(id) => id, Err(e) => return Ok(ToolResult::error(e)), @@ -653,11 +726,19 @@ impl Tool for StorageGetLinkTool { pub struct StorageSetVisibilityTool { client: Arc, + security: Arc, } impl StorageSetVisibilityTool { pub fn new(client: Arc) -> Self { - Self { client } + Self::new_with_security(client, Arc::new(SecurityPolicy::default())) + } + + pub fn new_with_security( + client: Arc, + security: Arc, + ) -> Self { + Self { client, security } } } @@ -697,6 +778,10 @@ impl Tool for StorageSetVisibilityTool { } async fn execute(&self, args: Value) -> anyhow::Result { + if let Some(blocked) = readonly_autonomy_block(&self.security) { + return Ok(blocked); + } + let file_id = match validate_file_id(&args) { Ok(id) => id, Err(e) => return Ok(ToolResult::error(e)), @@ -735,11 +820,19 @@ impl Tool for StorageSetVisibilityTool { pub struct StorageDeleteFileTool { client: Arc, + security: Arc, } impl StorageDeleteFileTool { pub fn new(client: Arc) -> Self { - Self { client } + Self::new_with_security(client, Arc::new(SecurityPolicy::default())) + } + + pub fn new_with_security( + client: Arc, + security: Arc, + ) -> Self { + Self { client, security } } } @@ -777,6 +870,10 @@ impl Tool for StorageDeleteFileTool { } async fn execute(&self, args: Value) -> anyhow::Result { + if let Some(blocked) = readonly_autonomy_block(&self.security) { + return Ok(blocked); + } + let file_id = match validate_file_id(&args) { Ok(id) => id, Err(e) => return Ok(ToolResult::error(e)), @@ -811,19 +908,35 @@ pub fn build_file_storage_tools(root_config: &Config, action_dir: &Path) -> Vec< }; let action_dir = action_dir.to_path_buf(); + let security = Arc::new(SecurityPolicy::from_config( + &root_config.autonomy, + &root_config.workspace_dir, + &root_config.action_dir, + )); let tools: Vec> = vec![ - Box::new(StorageUploadFileTool::new( + Box::new(StorageUploadFileTool::new_with_security( Arc::clone(&client), action_dir.clone(), + Arc::clone(&security), )), - Box::new(StorageDownloadFileTool::new( + Box::new(StorageDownloadFileTool::new_with_security( Arc::clone(&client), action_dir, + Arc::clone(&security), )), Box::new(StorageListFilesTool::new(Arc::clone(&client))), - Box::new(StorageGetLinkTool::new(Arc::clone(&client))), - Box::new(StorageSetVisibilityTool::new(Arc::clone(&client))), - Box::new(StorageDeleteFileTool::new(Arc::clone(&client))), + Box::new(StorageGetLinkTool::new_with_security( + Arc::clone(&client), + Arc::clone(&security), + )), + Box::new(StorageSetVisibilityTool::new_with_security( + Arc::clone(&client), + Arc::clone(&security), + )), + Box::new(StorageDeleteFileTool::new_with_security( + Arc::clone(&client), + Arc::clone(&security), + )), ]; tracing::debug!( "[file_storage] registered {} file-storage tools", diff --git a/src/openhuman/file_storage/tools_tests.rs b/src/openhuman/file_storage/tools_tests.rs index 184c0cae6..a2406eaa6 100644 --- a/src/openhuman/file_storage/tools_tests.rs +++ b/src/openhuman/file_storage/tools_tests.rs @@ -8,6 +8,7 @@ use super::{ StorageGetLinkTool, StorageListFilesTool, StorageSetVisibilityTool, StorageUploadFileTool, }; use crate::openhuman::integrations::IntegrationClient; +use crate::openhuman::security::{AutonomyLevel, SecurityPolicy}; use crate::openhuman::tools::traits::{PermissionLevel, Tool, ToolCategory}; fn dummy_client() -> Arc { @@ -40,9 +41,9 @@ fn upload_tool_schema_and_metadata() { fn download_tool_schema_and_metadata() { let tool = StorageDownloadFileTool::new(dummy_client(), PathBuf::from("/tmp")); assert_eq!(tool.name(), "storage_download_file"); - assert_eq!(tool.permission_level(), PermissionLevel::ReadOnly); + assert_eq!(tool.permission_level(), PermissionLevel::Write); assert_eq!(tool.category(), ToolCategory::Workflow); - assert!(!tool.external_effect()); + assert!(tool.external_effect()); let schema = tool.parameters_schema(); assert_eq!(schema["required"], json!(["file_id"])); @@ -65,8 +66,9 @@ fn list_tool_metadata() { fn get_link_tool_schema_and_metadata() { let tool = StorageGetLinkTool::new(dummy_client()); assert_eq!(tool.name(), "storage_get_link"); - assert_eq!(tool.permission_level(), PermissionLevel::ReadOnly); + assert_eq!(tool.permission_level(), PermissionLevel::Write); assert_eq!(tool.category(), ToolCategory::Workflow); + assert!(tool.external_effect()); let schema = tool.parameters_schema(); assert_eq!(schema["required"], json!(["file_id"])); @@ -252,6 +254,65 @@ fn client_for(server: &MockServer) -> Arc { Arc::new(IntegrationClient::new(server.uri(), "tok".to_string())) } +fn readonly_security() -> Arc { + Arc::new(SecurityPolicy { + autonomy: AutonomyLevel::ReadOnly, + ..SecurityPolicy::default() + }) +} + +#[tokio::test] +async fn storage_transfer_tools_block_readonly_autonomy_before_network() { + let tmp = tempfile::tempdir().unwrap(); + std::fs::write(tmp.path().join("a.txt"), b"hi").unwrap(); + + let upload = StorageUploadFileTool::new_with_security( + dummy_client(), + tmp.path().to_path_buf(), + readonly_security(), + ); + let upload_res = upload.execute(json!({ "path": "a.txt" })).await.unwrap(); + assert!(upload_res.is_error); + assert!(upload_res.output().contains("[policy-blocked]")); + + let download = StorageDownloadFileTool::new_with_security( + dummy_client(), + tmp.path().to_path_buf(), + readonly_security(), + ); + let download_res = download + .execute(json!({ "file_id": "file_123" })) + .await + .unwrap(); + assert!(download_res.is_error); + assert!(download_res.output().contains("[policy-blocked]")); + + let get_link = StorageGetLinkTool::new_with_security(dummy_client(), readonly_security()); + let link_res = get_link + .execute(json!({ "file_id": "file_123" })) + .await + .unwrap(); + assert!(link_res.is_error); + assert!(link_res.output().contains("[policy-blocked]")); + + let set_visibility = + StorageSetVisibilityTool::new_with_security(dummy_client(), readonly_security()); + let visibility_res = set_visibility + .execute(json!({ "file_id": "file_123", "visibility": "public" })) + .await + .unwrap(); + assert!(visibility_res.is_error); + assert!(visibility_res.output().contains("[policy-blocked]")); + + let delete = StorageDeleteFileTool::new_with_security(dummy_client(), readonly_security()); + let delete_res = delete + .execute(json!({ "file_id": "file_123" })) + .await + .unwrap(); + assert!(delete_res.is_error); + assert!(delete_res.output().contains("[policy-blocked]")); +} + #[tokio::test] async fn upload_tool_posts_multipart_and_reports_file_id() { let server = MockServer::start().await; diff --git a/src/openhuman/tools/impl/network/http_request.rs b/src/openhuman/tools/impl/network/http_request.rs index 9a9d373d2..34dde8aec 100644 --- a/src/openhuman/tools/impl/network/http_request.rs +++ b/src/openhuman/tools/impl/network/http_request.rs @@ -1,7 +1,7 @@ use super::url_guard::{normalize_allowed_domains, validate_url_with_dns_check}; use crate::openhuman::config::HttpRequestConfig; -use crate::openhuman::security::SecurityPolicy; -use crate::openhuman::tools::traits::{Tool, ToolResult}; +use crate::openhuman::security::{CommandClass, GateDecision, SecurityPolicy}; +use crate::openhuman::tools::traits::{PermissionLevel, Tool, ToolResult}; use async_trait::async_trait; use base64::engine::Engine as _; use serde_json::json; @@ -322,6 +322,17 @@ impl Tool for HttpRequestTool { }) } + /// Rich HTTP semantics (methods, headers, request bodies, and x402 retry) + /// are the same Network-class risk as `curl`: read-only autonomy is blocked + /// in `execute`, and supervised/full tiers route through ApprovalGate. + fn external_effect_with_args(&self, _args: &serde_json::Value) -> bool { + self.security.gate_decision(CommandClass::Network) == GateDecision::Prompt + } + + fn permission_level(&self) -> PermissionLevel { + PermissionLevel::Write + } + async fn execute(&self, args: serde_json::Value) -> anyhow::Result { let url = args .get("url") diff --git a/src/openhuman/tools/impl/network/http_request_tests.rs b/src/openhuman/tools/impl/network/http_request_tests.rs index 8ba216da8..9a51b06e0 100644 --- a/src/openhuman/tools/impl/network/http_request_tests.rs +++ b/src/openhuman/tools/impl/network/http_request_tests.rs @@ -180,3 +180,28 @@ fn redirect_policy_is_none() { let tool = test_tool(vec!["example.com"]); assert_eq!(tool.name(), "http_request"); } + +#[test] +fn supervised_http_request_is_external_effect_for_approval_gate() { + let tool = test_tool(vec!["example.com"]); + assert_eq!(tool.permission_level(), PermissionLevel::Write); + assert!(tool.external_effect_with_args(&json!({ + "url": "https://example.com/api", + "method": "POST", + "headers": { "Authorization": "Bearer token" }, + "body": "{}" + }))); +} + +#[test] +fn readonly_http_request_is_not_external_effect_because_execute_blocks() { + let security = Arc::new(SecurityPolicy { + autonomy: AutonomyLevel::ReadOnly, + ..SecurityPolicy::default() + }); + let tool = HttpRequestTool::new(security, vec!["example.com".into()], 1_000_000, 30); + assert!(!tool.external_effect_with_args(&json!({ + "url": "https://example.com/api", + "method": "GET" + }))); +}