diff --git a/.github/workflows/release.yml b/.github/workflows/release.yml index 6b81154f45..34eb554dfc 100644 --- a/.github/workflows/release.yml +++ b/.github/workflows/release.yml @@ -413,6 +413,9 @@ jobs: - build-wasm-extensions if: ${{ always() && needs.host.result == 'success' && needs.build-wasm-extensions.result == 'success' }} runs-on: "ubuntu-22.04" + permissions: + contents: write + pull-requests: write env: GH_TOKEN: ${{ secrets.GITHUB_TOKEN }} steps: @@ -445,7 +448,7 @@ jobs: fi done done < "$CHECKSUMS" - - name: Commit updated manifests + - name: Create PR with updated manifests run: | git config user.name "github-actions[bot]" git config user.email "github-actions[bot]@users.noreply.github.com" @@ -453,8 +456,15 @@ jobs: if git diff --cached --quiet; then echo "No manifest changes to commit" else + BRANCH="chore/update-checksums-$(date +%s)" + git checkout -b "$BRANCH" git commit -m "chore: update WASM artifact SHA256 checksums [skip ci]" - git push + git push origin "$BRANCH" + gh pr create \ + --title "chore: update WASM artifact SHA256 checksums" \ + --body "Auto-generated by release CI. Updates SHA256 checksums in registry manifests to match the released WASM artifacts." \ + --base main \ + --head "$BRANCH" fi announce: diff --git a/src/agent/dispatcher.rs b/src/agent/dispatcher.rs index 79ac4821dc..452a9a8273 100644 --- a/src/agent/dispatcher.rs +++ b/src/agent/dispatcher.rs @@ -15,6 +15,7 @@ use crate::channels::{IncomingMessage, StatusUpdate}; use crate::context::JobContext; use crate::error::Error; use crate::llm::{ChatMessage, Reasoning, ReasoningContext, RespondResult}; +use crate::tools::redact_params; /// Result of the agentic loop execution. pub(super) enum AgenticLoopResult { @@ -321,14 +322,25 @@ impl Agent { ) .await; - // Record tool calls in the thread + // Record tool calls in the thread with sensitive params redacted. + // Look up each tool's sensitive_params before acquiring the session lock. { + let mut redacted_args: Vec = + Vec::with_capacity(tool_calls.len()); + for tc in &tool_calls { + let safe = if let Some(tool) = self.tools().get(&tc.name).await { + redact_params(&tc.arguments, tool.sensitive_params()) + } else { + tc.arguments.clone() + }; + redacted_args.push(safe); + } let mut sess = session.lock().await; if let Some(thread) = sess.threads.get_mut(&thread_id) && let Some(turn) = thread.last_turn_mut() { - for tc in &tool_calls { - turn.record_tool_call(&tc.name, tc.arguments.clone()); + for (tc, safe_args) in tool_calls.iter().zip(redacted_args) { + turn.record_tool_call(&tc.name, safe_args); } } } @@ -357,11 +369,22 @@ impl Agent { for (idx, original_tc) in tool_calls.iter().enumerate() { let mut tc = original_tc.clone(); + // Fetch the tool upfront so we can redact sensitive params + // before they touch hooks or approval display. + let tool_opt = self.tools().get(&tc.name).await; + let sensitive = tool_opt + .as_ref() + .map(|t| t.sensitive_params()) + .unwrap_or(&[]); + // Hook: BeforeToolCall (runs before approval so hooks can - // modify parameters — approval is checked on final params) + // modify parameters — approval is checked on final params). + // Hooks receive redacted params so sensitive values are not + // exposed to hook handlers or their logs. + let hook_params = redact_params(&tc.arguments, sensitive); let event = crate::hooks::HookEvent::ToolCall { tool_name: tc.name.clone(), - parameters: tc.arguments.clone(), + parameters: hook_params, user_id: message.user_id.clone(), context: "chat".to_string(), }; @@ -388,8 +411,20 @@ impl Agent { } Ok(crate::hooks::HookOutcome::Continue { modified: Some(new_params), - }) => match serde_json::from_str(&new_params) { - Ok(parsed) => tc.arguments = parsed, + }) => match serde_json::from_str::(&new_params) { + Ok(mut parsed) => { + // Restore original sensitive param values so a hook + // cannot overwrite them (they were sent as [REDACTED]). + if let Some(obj) = parsed.as_object_mut() { + for key in sensitive { + if let Some(orig_val) = original_tc.arguments.get(*key) + { + obj.insert((*key).to_string(), orig_val.clone()); + } + } + } + tc.arguments = parsed; + } Err(e) => { tracing::warn!( tool = %tc.name, @@ -404,7 +439,7 @@ impl Agent { // Check if tool requires approval on the final (post-hook) // parameters. Skipped when auto_approve_tools is set. if !self.config.auto_approve_tools - && let Some(tool) = self.tools().get(&tc.name).await + && let Some(tool) = tool_opt { use crate::tools::ApprovalRequirement; let needs_approval = match tool.requires_approval(&tc.arguments) { @@ -451,14 +486,17 @@ impl Agent { .execute_chat_tool(&tc.name, &tc.arguments, &job_ctx) .await; + let disp_tool = self.tools().get(&tc.name).await; let _ = self .channels .send_status( &message.channel, - StatusUpdate::ToolCompleted { - name: tc.name.clone(), - success: result.is_ok(), - }, + StatusUpdate::tool_completed( + tc.name.clone(), + &result, + &tc.arguments, + disp_tool.as_deref(), + ), &message.metadata, ) .await; @@ -499,13 +537,16 @@ impl Agent { ) .await; + let par_tool = tools.get(&tc.name).await; let _ = channels .send_status( &channel, - StatusUpdate::ToolCompleted { - name: tc.name.clone(), - success: result.is_ok(), - }, + StatusUpdate::tool_completed( + tc.name.clone(), + &result, + &tc.arguments, + par_tool.as_deref(), + ), &metadata, ) .await; @@ -675,10 +716,15 @@ impl Agent { // Handle approval if a tool needed it if let Some((approval_idx, tc, tool)) = approval_needed { + // Show redacted params in the approval UI — the user already knows + // the sensitive value (they provided it); showing it again is + // unnecessary and creates a leakage path through channel logs. + let display_params = redact_params(&tc.arguments, tool.sensitive_params()); let pending = PendingApproval { request_id: Uuid::new_v4(), tool_name: tc.name.clone(), parameters: tc.arguments.clone(), + display_parameters: display_params, description: tool.description().to_string(), tool_call_id: tc.id.clone(), context_messages: context_messages.clone(), @@ -738,9 +784,10 @@ pub(super) async fn execute_chat_tool_standalone( .into()); } + let safe_params = redact_params(params, tool.sensitive_params()); tracing::debug!( tool = %tool_name, - params = %params, + params = %safe_params, "Tool call started" ); @@ -1122,6 +1169,7 @@ mod tests { request_id: uuid::Uuid::new_v4(), tool_name: "shell".to_string(), parameters: serde_json::json!({"command": "echo hi"}), + display_parameters: serde_json::json!({"command": "echo hi"}), description: "Run shell command".to_string(), tool_call_id: "call_1".to_string(), context_messages: vec![], diff --git a/src/agent/session.rs b/src/agent/session.rs index 070a0ad4cd..4c3dbd67d7 100644 --- a/src/agent/session.rs +++ b/src/agent/session.rs @@ -148,8 +148,12 @@ pub struct PendingApproval { pub request_id: Uuid, /// Tool name requiring approval. pub tool_name: String, - /// Tool parameters. + /// Tool parameters (original values, used for execution). pub parameters: serde_json::Value, + /// Redacted tool parameters (sensitive values replaced with `[REDACTED]`). + /// Used for display in approval UI, logs, and SSE broadcasts. + #[serde(default)] + pub display_parameters: serde_json::Value, /// Description of what the tool will do. pub description: String, /// Tool call ID from LLM (for proper context continuation). @@ -950,6 +954,7 @@ mod tests { request_id: Uuid::new_v4(), tool_name: "shell".to_string(), parameters: serde_json::json!({"command": "rm -rf /"}), + display_parameters: serde_json::json!({"command": "rm -rf /"}), description: "dangerous command".to_string(), tool_call_id: "call_123".to_string(), context_messages: vec![ChatMessage::user("do it")], @@ -974,6 +979,7 @@ mod tests { request_id: Uuid::new_v4(), tool_name: "http".to_string(), parameters: serde_json::json!({}), + display_parameters: serde_json::json!({}), description: "test".to_string(), tool_call_id: "call_456".to_string(), context_messages: vec![], diff --git a/src/agent/thread_ops.rs b/src/agent/thread_ops.rs index 91eb0e5539..39cd22ed66 100644 --- a/src/agent/thread_ops.rs +++ b/src/agent/thread_ops.rs @@ -21,6 +21,7 @@ use crate::channels::{IncomingMessage, StatusUpdate}; use crate::context::JobContext; use crate::error::Error; use crate::llm::ChatMessage; +use crate::tools::redact_params; impl Agent { /// Hydrate a historical thread from DB into memory if not already present. @@ -357,7 +358,7 @@ impl Agent { let request_id = pending.request_id; let tool_name = pending.tool_name.clone(); let description = pending.description.clone(); - let parameters = pending.parameters.clone(); + let parameters = pending.display_parameters.clone(); thread.await_approval(pending); let _ = self .channels @@ -751,14 +752,17 @@ impl Agent { .execute_chat_tool(&pending.tool_name, &pending.parameters, &job_ctx) .await; + let tool_ref = self.tools().get(&pending.tool_name).await; let _ = self .channels .send_status( &message.channel, - StatusUpdate::ToolCompleted { - name: pending.tool_name.clone(), - success: tool_result.is_ok(), - }, + StatusUpdate::tool_completed( + pending.tool_name.clone(), + &tool_result, + &pending.display_parameters, + tool_ref.as_deref(), + ), &message.metadata, ) .await; @@ -908,14 +912,17 @@ impl Agent { .execute_chat_tool(&tc.name, &tc.arguments, &job_ctx) .await; + let deferred_tool = self.tools().get(&tc.name).await; let _ = self .channels .send_status( &message.channel, - StatusUpdate::ToolCompleted { - name: tc.name.clone(), - success: result.is_ok(), - }, + StatusUpdate::tool_completed( + tc.name.clone(), + &result, + &tc.arguments, + deferred_tool.as_deref(), + ), &message.metadata, ) .await; @@ -957,13 +964,16 @@ impl Agent { ) .await; + let par_tool = tools.get(&tc.name).await; let _ = channels .send_status( &channel, - StatusUpdate::ToolCompleted { - name: tc.name.clone(), - success: result.is_ok(), - }, + StatusUpdate::tool_completed( + tc.name.clone(), + &result, + &tc.arguments, + par_tool.as_deref(), + ), &metadata, ) .await; @@ -1086,6 +1096,7 @@ impl Agent { request_id: Uuid::new_v4(), tool_name: tc.name.clone(), parameters: tc.arguments.clone(), + display_parameters: redact_params(&tc.arguments, tool.sensitive_params()), description: tool.description().to_string(), tool_call_id: tc.id.clone(), context_messages: context_messages.clone(), @@ -1095,7 +1106,7 @@ impl Agent { let request_id = new_pending.request_id; let tool_name = new_pending.tool_name.clone(); let description = new_pending.description.clone(); - let parameters = new_pending.parameters.clone(); + let parameters = new_pending.display_parameters.clone(); { let mut sess = session.lock().await; @@ -1162,7 +1173,7 @@ impl Agent { let request_id = new_pending.request_id; let tool_name = new_pending.tool_name.clone(); let description = new_pending.description.clone(); - let parameters = new_pending.parameters.clone(); + let parameters = new_pending.display_parameters.clone(); thread.await_approval(new_pending); let _ = self .channels diff --git a/src/agent/worker.rs b/src/agent/worker.rs index 70454e7e33..f8017cb89c 100644 --- a/src/agent/worker.rs +++ b/src/agent/worker.rs @@ -18,8 +18,8 @@ use crate::llm::{ ActionPlan, ChatMessage, LlmProvider, Reasoning, ReasoningContext, RespondResult, ToolSelection, }; use crate::safety::SafetyLayer; -use crate::tools::ToolRegistry; use crate::tools::rate_limiter::RateLimitResult; +use crate::tools::{ToolRegistry, redact_params}; /// Shared dependencies for worker execution. /// @@ -700,9 +700,10 @@ Report when the job is complete or if you encounter issues you cannot resolve."# // Run BeforeToolCall hook let params = { use crate::hooks::{HookError, HookEvent, HookOutcome}; + let hook_params = redact_params(params, tool.sensitive_params()); let event = HookEvent::ToolCall { tool_name: tool_name.to_string(), - parameters: params.clone(), + parameters: hook_params, user_id: job_ctx.user_id.clone(), context: format!("job:{}", job_id), }; @@ -758,9 +759,12 @@ Report when the job is complete or if you encounter issues you cannot resolve."# .into()); } + // Redact sensitive parameter values (e.g. secret_save's "value") before + // they touch any observability or audit path. + let safe_params = redact_params(¶ms, tool.sensitive_params()); tracing::debug!( tool = %tool_name, - params = %params, + params = %safe_params, job = %job_id, "Tool call started" ); @@ -812,7 +816,7 @@ Report when the job is complete or if you encounter issues you cannot resolve."# match deps .context_manager .update_memory(job_id, |mem| { - let rec = mem.create_action(tool_name, params.clone()).succeed( + let rec = mem.create_action(tool_name, safe_params.clone()).succeed( output_str.clone(), output.result.clone(), elapsed, @@ -834,7 +838,7 @@ Report when the job is complete or if you encounter issues you cannot resolve."# .context_manager .update_memory(job_id, |mem| { let rec = mem - .create_action(tool_name, params.clone()) + .create_action(tool_name, safe_params.clone()) .fail(e.to_string(), elapsed); mem.record_action(rec.clone()); rec @@ -853,7 +857,7 @@ Report when the job is complete or if you encounter issues you cannot resolve."# .context_manager .update_memory(job_id, |mem| { let rec = mem - .create_action(tool_name, params.clone()) + .create_action(tool_name, safe_params.clone()) .fail("Execution timeout", elapsed); mem.record_action(rec.clone()); rec diff --git a/src/app.rs b/src/app.rs index 8c6a5bbda3..3d21c641b6 100644 --- a/src/app.rs +++ b/src/app.rs @@ -331,6 +331,10 @@ impl AppBuilder { }; tools.register_builtin_tools(); + if let Some(ref ss) = self.secrets_store { + tools.register_secrets_tools(Arc::clone(ss)); + } + // Create embeddings provider using the unified method let embeddings = self .config diff --git a/src/channels/channel.rs b/src/channels/channel.rs index e993bb0a56..46fbc9ca0f 100644 --- a/src/channels/channel.rs +++ b/src/channels/channel.rs @@ -117,7 +117,20 @@ pub enum StatusUpdate { /// Tool execution started. ToolStarted { name: String }, /// Tool execution completed. - ToolCompleted { name: String, success: bool }, + /// + /// Use [`StatusUpdate::tool_completed`] to construct this variant — it + /// handles redaction of sensitive parameters and keeps the 9-line pattern + /// in one place. + ToolCompleted { + name: String, + success: bool, + /// Error message when success is false. + error: Option, + /// Tool input parameters (JSON string) for display on failure. + /// Only populated when `success` is `false`. Values listed in the + /// tool's `sensitive_params()` are replaced with `"[REDACTED]"`. + parameters: Option, + }, /// Brief preview of tool execution output. ToolResult { name: String, preview: String }, /// Streaming text chunk. @@ -152,6 +165,38 @@ pub enum StatusUpdate { }, } +impl StatusUpdate { + /// Build a `ToolCompleted` status with redacted parameters. + /// + /// On failure, serializes the tool's input parameters as pretty JSON after + /// replacing any keys listed in the tool's `sensitive_params()` with + /// `"[REDACTED]"`. On success, no parameters or error are included. + /// + /// Pass the resolved `Tool` reference (if available) so this method can + /// query `sensitive_params()` directly — callers don't need to manage the + /// borrow lifetime of the sensitive slice. + pub fn tool_completed( + name: String, + result: &Result, + params: &serde_json::Value, + tool: Option<&dyn crate::tools::Tool>, + ) -> Self { + let success = result.is_ok(); + let sensitive = tool.map(|t| t.sensitive_params()).unwrap_or(&[]); + Self::ToolCompleted { + name, + success, + error: result.as_ref().err().map(|e| e.to_string()), + parameters: if !success { + let safe = crate::tools::redact_params(params, sensitive); + Some(serde_json::to_string_pretty(&safe).unwrap_or_else(|_| safe.to_string())) + } else { + None + }, + } + } +} + /// Trait for message channels. /// /// Channels receive messages from external sources and convert them to @@ -223,3 +268,131 @@ pub trait Channel: Send + Sync { Ok(()) } } + +#[cfg(test)] +mod tests { + use super::*; + + /// Stub tool that marks `"value"` as sensitive. + struct SecretTool; + + #[async_trait] + impl crate::tools::Tool for SecretTool { + fn name(&self) -> &str { + "secret_save" + } + fn description(&self) -> &str { + "stub" + } + fn parameters_schema(&self) -> serde_json::Value { + serde_json::json!({"type": "object", "properties": {}}) + } + async fn execute( + &self, + _params: serde_json::Value, + _ctx: &crate::context::JobContext, + ) -> Result { + unreachable!() + } + fn sensitive_params(&self) -> &[&str] { + &["value"] + } + } + + #[test] + fn tool_completed_redacts_sensitive_params_on_failure() { + let params = serde_json::json!({"name": "api_key", "value": "sk-secret-123"}); + let err: Result = + Err(crate::error::ToolError::ExecutionFailed { + name: "secret_save".into(), + reason: "db error".into(), + } + .into()); + let tool = SecretTool; + + let status = StatusUpdate::tool_completed( + "secret_save".into(), + &err, + ¶ms, + Some(&tool as &dyn crate::tools::Tool), + ); + + if let StatusUpdate::ToolCompleted { + success, + error, + parameters, + .. + } = &status + { + assert!(!success); + let err_msg = error.as_deref().expect("should have error"); + assert!(err_msg.contains("db error"), "error: {}", err_msg); + let param_str = parameters + .as_ref() + .expect("should have parameters on failure"); + assert!( + param_str.contains("[REDACTED]"), + "sensitive value should be redacted: {}", + param_str + ); + assert!( + !param_str.contains("sk-secret-123"), + "raw secret should not appear: {}", + param_str + ); + assert!( + param_str.contains("api_key"), + "non-sensitive params should be preserved: {}", + param_str + ); + } else { + panic!("expected ToolCompleted variant"); + } + } + + #[test] + fn tool_completed_no_params_on_success() { + let params = serde_json::json!({"name": "key", "value": "secret"}); + let ok: Result = Ok("done".into()); + + let status = StatusUpdate::tool_completed("secret_save".into(), &ok, ¶ms, None); + + if let StatusUpdate::ToolCompleted { + success, + error, + parameters, + .. + } = &status + { + assert!(success); + assert!(error.is_none()); + assert!(parameters.is_none(), "no params should be sent on success"); + } else { + panic!("expected ToolCompleted variant"); + } + } + + #[test] + fn tool_completed_no_tool_passes_params_unredacted() { + let params = serde_json::json!({"cmd": "ls -la"}); + let err: Result = + Err(crate::error::ToolError::ExecutionFailed { + name: "shell".into(), + reason: "timeout".into(), + } + .into()); + + let status = StatusUpdate::tool_completed("shell".into(), &err, ¶ms, None); + + if let StatusUpdate::ToolCompleted { parameters, .. } = &status { + let param_str = parameters.as_ref().expect("should have parameters"); + assert!( + param_str.contains("ls -la"), + "non-sensitive params should pass through: {}", + param_str + ); + } else { + panic!("expected ToolCompleted variant"); + } + } +} diff --git a/src/channels/repl.rs b/src/channels/repl.rs index 99f327b85b..f4cc7d3f92 100644 --- a/src/channels/repl.rs +++ b/src/channels/repl.rs @@ -466,7 +466,7 @@ impl Channel for ReplChannel { StatusUpdate::ToolStarted { name } => { eprintln!(" \x1b[33m\u{25CB} {name}\x1b[0m"); } - StatusUpdate::ToolCompleted { name, success } => { + StatusUpdate::ToolCompleted { name, success, .. } => { if success { eprintln!(" \x1b[32m\u{25CF} {name}\x1b[0m"); } else { diff --git a/src/channels/signal.rs b/src/channels/signal.rs index 09f19d2397..cc07b079a7 100644 --- a/src/channels/signal.rs +++ b/src/channels/signal.rs @@ -974,7 +974,7 @@ impl Channel for SignalChannel { // Send tool completed notification (debug mode only) if self.is_debug() - && let StatusUpdate::ToolCompleted { name, success } = &status + && let StatusUpdate::ToolCompleted { name, success, .. } = &status && let Some(target_str) = metadata.get("signal_target").and_then(|v| v.as_str()) { let (icon, color) = if *success { diff --git a/src/channels/wasm/wrapper.rs b/src/channels/wasm/wrapper.rs index 9d4d298a66..e559aca412 100644 --- a/src/channels/wasm/wrapper.rs +++ b/src/channels/wasm/wrapper.rs @@ -2479,7 +2479,7 @@ fn status_to_wit(status: &StatusUpdate, metadata: &serde_json::Value) -> wit_cha message: format!("Tool started: {}", name), metadata_json, }, - StatusUpdate::ToolCompleted { name, success } => wit_channel::StatusUpdate { + StatusUpdate::ToolCompleted { name, success, .. } => wit_channel::StatusUpdate { status: wit_channel::StatusType::ToolCompleted, message: format!( "Tool completed: {} ({})", @@ -3387,6 +3387,8 @@ mod tests { &crate::channels::StatusUpdate::ToolCompleted { name: "http_request".to_string(), success: true, + error: None, + parameters: None, }, &metadata, ); @@ -3407,6 +3409,8 @@ mod tests { &crate::channels::StatusUpdate::ToolCompleted { name: "http_request".to_string(), success: false, + error: Some("connection refused".to_string()), + parameters: None, }, &metadata, ); diff --git a/src/channels/web/mod.rs b/src/channels/web/mod.rs index 2fbb4dcabc..9c41777028 100644 --- a/src/channels/web/mod.rs +++ b/src/channels/web/mod.rs @@ -304,9 +304,16 @@ impl Channel for GatewayChannel { name, thread_id: thread_id.clone(), }, - StatusUpdate::ToolCompleted { name, success } => SseEvent::ToolCompleted { + StatusUpdate::ToolCompleted { name, success, + error, + parameters, + } => SseEvent::ToolCompleted { + name, + success, + error, + parameters, thread_id: thread_id.clone(), }, StatusUpdate::ToolResult { name, preview } => SseEvent::ToolResult { diff --git a/src/channels/web/server.rs b/src/channels/web/server.rs index 4f8916784d..bada142ab7 100644 --- a/src/channels/web/server.rs +++ b/src/channels/web/server.rs @@ -1592,6 +1592,13 @@ async fn extensions_setup_submit_handler( match ext_mgr.save_setup_secrets(&name, &req.secrets).await { Ok(result) => { + // Broadcast auth_completed so the chat UI can dismiss any in-progress + // auth card or setup modal that was triggered by tool_auth/tool_activate. + state.sse.broadcast(SseEvent::AuthCompleted { + extension_name: name.clone(), + success: true, + message: result.message.clone(), + }); let mut resp = ActionResponse::ok(result.message); resp.activated = Some(result.activated); resp.auth_url = result.auth_url; diff --git a/src/channels/web/static/app.js b/src/channels/web/static/app.js index e1eb8b4584..138b0dee37 100644 --- a/src/channels/web/static/app.js +++ b/src/channels/web/static/app.js @@ -180,7 +180,7 @@ function connectSSE() { eventSource.addEventListener('tool_completed', (e) => { const data = JSON.parse(e.data); if (!isCurrentThread(data.thread_id)) return; - completeToolCard(data.name, data.success); + completeToolCard(data.name, data.success, data.error, data.parameters); }); eventSource.addEventListener('tool_result', (e) => { @@ -222,17 +222,22 @@ function connectSSE() { eventSource.addEventListener('auth_required', (e) => { const data = JSON.parse(e.data); - showAuthCard(data); + if (data.auth_url) { + // OAuth flow: show the auth card with an OAuth button + optional token paste field. + showAuthCard(data); + } else { + // Setup flow: fetch the extension's credential schema and show the multi-field + // configure modal (the same UI used by the Extensions tab "Setup" button). + showConfigureModal(data.extension_name); + } }); eventSource.addEventListener('auth_completed', (e) => { const data = JSON.parse(e.data); + // Dismiss whichever UI path was active: auth card (OAuth) or configure modal (setup). removeAuthCard(data.extension_name); - if (data.success) { - showToast(data.message, 'success'); - } else { - showToast(data.message, 'error'); - } + closeConfigureModal(); + showToast(data.message, data.success ? 'success' : 'error'); // Refresh extensions list so status indicators update if (currentTab === 'extensions') loadExtensions(); enableChatInput(); @@ -590,7 +595,7 @@ function addToolCard(name) { container.scrollTop = container.scrollHeight; } -function completeToolCard(name, success) { +function completeToolCard(name, success, error, parameters) { const entries = _activeToolCards[name]; if (!entries || entries.length === 0) return; // Find first running card @@ -611,6 +616,27 @@ function completeToolCard(name, success) { ? '✓' : '✗'; entry.card.setAttribute('data-status', success ? 'success' : 'fail'); + + // For failed tools, populate the body with error details and auto-expand + if (!success && (error || parameters)) { + const output = entry.card.querySelector('.activity-tool-output'); + if (output) { + let detail = ''; + if (parameters) { + detail += 'Input:\n' + parameters + '\n\n'; + } + if (error) { + detail += 'Error:\n' + error; + } + output.textContent = detail; + + // Auto-expand so the error is immediately visible + const body = entry.card.querySelector('.activity-tool-body'); + const chevron = entry.card.querySelector('.activity-tool-chevron'); + if (body) body.style.display = 'block'; + if (chevron) chevron.classList.add('expanded'); + } + } } function setToolCardOutput(name, preview) { @@ -2176,18 +2202,18 @@ function submitConfigureModal(name, fields) { closeConfigureModal(); if (res.success) { if (res.auth_url) { - // OAuth flow started — open consent popup + // OAuth flow started — open consent popup. The auth_completed SSE will + // not arrive immediately (it fires after OAuth callback), so show a toast now. showToast('Opening OAuth authorization for ' + name, 'info'); window.open(res.auth_url, '_blank', 'width=600,height=700'); - } else if (res.activated) { - showToast('Configured and activated ' + name, 'success'); - } else { - showToast(res.message || 'Configuration saved but activation failed', 'warning'); + loadExtensions(); } + // For non-OAuth success: the server always broadcasts auth_completed SSE, + // which will show the toast and refresh extensions — no need to do it here too. } else { showToast(res.message || 'Configuration failed', 'error'); + loadExtensions(); } - loadExtensions(); }) .catch((err) => { btns.forEach(function(b) { b.disabled = false; }); diff --git a/src/channels/web/static/style.css b/src/channels/web/static/style.css index fff0223159..d0bf514e3a 100644 --- a/src/channels/web/static/style.css +++ b/src/channels/web/static/style.css @@ -553,6 +553,10 @@ body { border-color: rgba(230, 76, 76, 0.3); } +.activity-tool-card[data-status="fail"] .activity-tool-name { + color: var(--danger); +} + .activity-tool-header { display: flex; align-items: center; diff --git a/src/channels/web/types.rs b/src/channels/web/types.rs index 33c8425e16..0e74e26e49 100644 --- a/src/channels/web/types.rs +++ b/src/channels/web/types.rs @@ -123,6 +123,10 @@ pub enum SseEvent { name: String, success: bool, #[serde(skip_serializing_if = "Option::is_none")] + error: Option, + #[serde(skip_serializing_if = "Option::is_none")] + parameters: Option, + #[serde(skip_serializing_if = "Option::is_none")] thread_id: Option, }, #[serde(rename = "tool_result")] diff --git a/src/registry/catalog.rs b/src/registry/catalog.rs index 64b03704f6..8cf99aaa22 100644 --- a/src/registry/catalog.rs +++ b/src/registry/catalog.rs @@ -47,6 +47,9 @@ pub enum RegistryError { actual_sha256: String, }, + #[error("Missing SHA256 checksum for '{name}' artifact. Use --build to build from source.")] + MissingChecksum { name: String }, + #[error( "Source fallback unavailable for '{name}' after artifact install failed. Retry artifact download or run from a repository checkout." )] diff --git a/src/registry/installer.rs b/src/registry/installer.rs index 7f46a5dc23..e4ae785ce4 100644 --- a/src/registry/installer.rs +++ b/src/registry/installer.rs @@ -20,6 +20,10 @@ const ALLOWED_ARTIFACT_HOSTS: &[&str] = &[ ]; fn should_attempt_source_fallback(err: &RegistryError) -> bool { + // MissingChecksum is intentionally allowed here — it's a bootstrapping issue + // (no release has populated checksums yet), not a security concern. Source + // builds use local trusted code. ChecksumMismatch (tampered artifact) and + // InvalidManifest (structural problem) remain blocked. !matches!( err, RegistryError::AlreadyInstalled { .. } @@ -367,15 +371,15 @@ impl RegistryInstaller { // Require SHA256 — refuse to install unverified binaries. Check before // downloading to avoid wasting bandwidth on manifests that are missing - // checksums. + // checksums. Uses MissingChecksum (not InvalidManifest) so that + // install_with_source_fallback can fall back to building from source + // when checksums haven't been populated yet (bootstrapping). let expected_sha = artifact .sha256 .as_ref() - .ok_or_else(|| RegistryError::InvalidManifest { + .ok_or_else(|| RegistryError::MissingChecksum { name: manifest.name.clone(), - field: "artifacts.wasm32-wasip2.sha256", - reason: "sha256 is required for artifact downloads".to_string(), })?; let target_dir = match manifest.kind { @@ -500,7 +504,7 @@ impl RegistryInstaller { if prefer_build || !has_artifact { self.install_from_source(manifest, force).await } else { - self.install_from_artifact(manifest, force).await + self.install_with_source_fallback(manifest, force).await } } @@ -905,9 +909,8 @@ mod tests { let result = installer.install_from_artifact(&manifest, false).await; match result { - Err(RegistryError::InvalidManifest { field, reason, .. }) => { - assert_eq!(field, "artifacts.wasm32-wasip2.sha256"); - assert!(reason.contains("required"), "reason: {}", reason); + Err(RegistryError::MissingChecksum { name }) => { + assert_eq!(name, "demo"); } other => panic!("unexpected result: {:?}", other), } @@ -942,6 +945,12 @@ mod tests { reason: "host not allowed".to_string(), }; assert!(!should_attempt_source_fallback(&invalid)); + + // MissingChecksum SHOULD allow source fallback (bootstrapping) + let missing = RegistryError::MissingChecksum { + name: "demo".to_string(), + }; + assert!(should_attempt_source_fallback(&missing)); } #[test] diff --git a/src/tools/builtin/mod.rs b/src/tools/builtin/mod.rs index 517cf3fdba..6373a87647 100644 --- a/src/tools/builtin/mod.rs +++ b/src/tools/builtin/mod.rs @@ -10,6 +10,7 @@ mod memory; mod message; pub mod path_utils; pub mod routine; +pub mod secrets_tools; pub(crate) mod shell; pub mod skill_tools; mod time; @@ -31,6 +32,7 @@ pub use message::MessageTool; pub use routine::{ RoutineCreateTool, RoutineDeleteTool, RoutineHistoryTool, RoutineListTool, RoutineUpdateTool, }; +pub use secrets_tools::{SecretDeleteTool, SecretListTool}; pub use shell::ShellTool; pub use skill_tools::{SkillInstallTool, SkillListTool, SkillRemoveTool, SkillSearchTool}; pub use time::TimeTool; diff --git a/src/tools/builtin/secrets_tools.rs b/src/tools/builtin/secrets_tools.rs new file mode 100644 index 0000000000..8d5c8d62e2 --- /dev/null +++ b/src/tools/builtin/secrets_tools.rs @@ -0,0 +1,222 @@ +//! Agent-callable tools for inspecting user secrets. +//! +//! These tools allow the LLM to query and manage secrets on behalf of the +//! user. The zero-exposure model is preserved throughout: +//! +//! - `secret_list` returns only names and metadata (no values). +//! - `secret_delete` removes a secret by name. +//! +//! Storing secrets is handled via the extensions setup flow — the user types +//! values directly into the secure UI, which submits them to +//! `/api/extensions/{name}/setup`. Values never appear in the LLM conversation, +//! logs, or ActionRecords. + +use std::sync::Arc; + +use async_trait::async_trait; + +use crate::context::JobContext; +use crate::secrets::SecretsStore; +use crate::tools::tool::{ApprovalRequirement, Tool, ToolError, ToolOutput, require_str}; + +// ── secret_list ────────────────────────────────────────────────────────────── + +pub struct SecretListTool { + store: Arc, +} + +impl SecretListTool { + pub fn new(store: Arc) -> Self { + Self { store } + } +} + +#[async_trait] +impl Tool for SecretListTool { + fn name(&self) -> &str { + "secret_list" + } + + fn description(&self) -> &str { + "List all stored secrets by name. Never returns values — only names and \ + optional provider metadata. Use this to check what credentials are available \ + before attempting a task that requires them." + } + + fn parameters_schema(&self) -> serde_json::Value { + serde_json::json!({ + "type": "object", + "properties": {} + }) + } + + async fn execute( + &self, + _params: serde_json::Value, + ctx: &JobContext, + ) -> Result { + let start = std::time::Instant::now(); + + let refs = self + .store + .list(&ctx.user_id) + .await + .map_err(|e| ToolError::ExecutionFailed(e.to_string()))?; + + let secrets: Vec = refs + .into_iter() + .map(|r| { + serde_json::json!({ + "name": r.name, + "provider": r.provider, + }) + }) + .collect(); + + let count = secrets.len(); + let output = serde_json::json!({ + "secrets": secrets, + "count": count, + }); + + Ok(ToolOutput::success(output, start.elapsed())) + } +} + +// ── secret_delete ───────────────────────────────────────────────────────────── + +pub struct SecretDeleteTool { + store: Arc, +} + +impl SecretDeleteTool { + pub fn new(store: Arc) -> Self { + Self { store } + } +} + +#[async_trait] +impl Tool for SecretDeleteTool { + fn name(&self) -> &str { + "secret_delete" + } + + fn description(&self) -> &str { + "Permanently delete a stored secret by name. This cannot be undone." + } + + fn parameters_schema(&self) -> serde_json::Value { + serde_json::json!({ + "type": "object", + "properties": { + "name": { + "type": "string", + "description": "Name of the secret to delete." + } + }, + "required": ["name"] + }) + } + + async fn execute( + &self, + params: serde_json::Value, + ctx: &JobContext, + ) -> Result { + let start = std::time::Instant::now(); + + let name = require_str(¶ms, "name")?; + + let deleted = self + .store + .delete(&ctx.user_id, name) + .await + .map_err(|e| ToolError::ExecutionFailed(e.to_string()))?; + + let output = if deleted { + serde_json::json!({ + "status": "deleted", + "name": name, + }) + } else { + serde_json::json!({ + "status": "not_found", + "name": name, + "message": format!("No secret named '{}' found.", name), + }) + }; + + Ok(ToolOutput::success(output, start.elapsed())) + } + + fn requires_approval(&self, _params: &serde_json::Value) -> ApprovalRequirement { + ApprovalRequirement::UnlessAutoApproved + } +} + +#[cfg(test)] +mod tests { + use std::sync::Arc; + + use secrecy::SecretString; + + use super::*; + use crate::context::JobContext; + use crate::secrets::{CreateSecretParams, InMemorySecretsStore, SecretsCrypto}; + + fn test_store() -> Arc { + let key = "0123456789abcdef0123456789abcdef"; + let crypto = Arc::new(SecretsCrypto::new(SecretString::from(key.to_string())).unwrap()); + Arc::new(InMemorySecretsStore::new(crypto)) + } + + fn test_ctx() -> JobContext { + JobContext::new("test", "test job") + } + + #[tokio::test] + async fn test_secret_list() { + let store = test_store(); + let list = SecretListTool::new(Arc::clone(&store) as Arc); + let ctx = test_ctx(); + + store + .create( + &ctx.user_id, + CreateSecretParams::new("openai_key", "sk-test"), + ) + .await + .unwrap(); + + let list_result = list.execute(serde_json::json!({}), &ctx).await.unwrap(); + assert_eq!(list_result.result["count"], 1); + assert_eq!(list_result.result["secrets"][0]["name"], "openai_key"); + assert!(list_result.result["secrets"][0].get("value").is_none()); + } + + #[tokio::test] + async fn test_secret_delete() { + let store = test_store(); + let delete = + SecretDeleteTool::new(Arc::clone(&store) as Arc); + let ctx = test_ctx(); + + store + .create(&ctx.user_id, CreateSecretParams::new("to_delete", "secret")) + .await + .unwrap(); + + let result = delete + .execute(serde_json::json!({"name": "to_delete"}), &ctx) + .await + .unwrap(); + assert_eq!(result.result["status"], "deleted"); + + // Deleting again returns not_found + let result2 = delete + .execute(serde_json::json!({"name": "to_delete"}), &ctx) + .await + .unwrap(); + assert_eq!(result2.result["status"], "not_found"); + } +} diff --git a/src/tools/mod.rs b/src/tools/mod.rs index 2590ec9d2d..cd225bd1f4 100644 --- a/src/tools/mod.rs +++ b/src/tools/mod.rs @@ -26,5 +26,5 @@ pub use rate_limiter::RateLimiter; pub use registry::ToolRegistry; pub use tool::{ ApprovalRequirement, Tool, ToolDomain, ToolError, ToolOutput, ToolRateLimitConfig, - validate_tool_schema, + redact_params, validate_tool_schema, }; diff --git a/src/tools/registry.rs b/src/tools/registry.rs index 5b72a3e9d4..c86f34bd52 100644 --- a/src/tools/registry.rs +++ b/src/tools/registry.rs @@ -346,6 +346,20 @@ impl ToolRegistry { tracing::info!("Registered {} job management tools", job_tool_count); } + /// Register secret management tools (list, delete). + /// + /// These allow the LLM to persist API keys and tokens encrypted in the database. + /// Values are never returned to the LLM; only names and metadata are exposed. + pub fn register_secrets_tools( + &self, + store: Arc, + ) { + use crate::tools::builtin::{SecretDeleteTool, SecretListTool}; + self.register_sync(Arc::new(SecretListTool::new(Arc::clone(&store)))); + self.register_sync(Arc::new(SecretDeleteTool::new(store))); + tracing::info!("Registered 2 secret management tools (list, delete)"); + } + /// Register extension management tools (search, install, auth, activate, list, remove). /// /// These allow the LLM to manage MCP servers and WASM tools through conversation. diff --git a/src/tools/tool.rs b/src/tools/tool.rs index 68980da416..78728cf352 100644 --- a/src/tools/tool.rs +++ b/src/tools/tool.rs @@ -239,6 +239,23 @@ pub trait Tool: Send + Sync { ToolDomain::Orchestrator } + /// Parameter names whose values must be redacted before logging, hooks, and approvals. + /// + /// The agent framework replaces these parameter values with `"[REDACTED]"` before: + /// - Writing to debug logs + /// - Storing in `ActionRecord` (in-memory job history) + /// - Recording in `TurnToolCall` (session state) + /// - Sending to `BeforeToolCall` hooks + /// - Displaying in the approval UI + /// + /// **The `execute()` method still receives the original, unredacted parameters.** + /// Redaction only applies to the observability and audit paths, not execution. + /// + /// Use this for tools that accept plaintext secrets as parameters (e.g. `secret_save`). + fn sensitive_params(&self) -> &[&str] { + &[] + } + /// Per-invocation rate limit for this tool. /// /// Return `Some(config)` to throttle how often this tool can be called per user. @@ -287,6 +304,33 @@ pub fn require_param<'a>( .ok_or_else(|| ToolError::InvalidParameters(format!("missing '{}' parameter", name))) } +/// Replace sensitive parameter values with `"[REDACTED]"`. +/// +/// Returns a new JSON value with the specified keys replaced. Non-object params +/// and unknown keys are passed through unchanged. The original value is cloned +/// only if there are sensitive params to redact; otherwise it is cloned once +/// (cheap — callers own the result). +/// +/// Used by the agent framework before logging, hook dispatch, approval display, +/// and `ActionRecord` storage so plaintext secrets never reach those paths. +pub fn redact_params(params: &serde_json::Value, sensitive: &[&str]) -> serde_json::Value { + if sensitive.is_empty() { + return params.clone(); + } + let mut redacted = params.clone(); + if let Some(obj) = redacted.as_object_mut() { + for key in sensitive { + if obj.contains_key(*key) { + obj.insert( + (*key).to_string(), + serde_json::Value::String("[REDACTED]".into()), + ); + } + } + } + redacted +} + /// Lenient runtime validation of a tool's `parameters_schema()`. /// /// Use this function at tool-registration time to catch structural mistakes @@ -500,6 +544,37 @@ mod tests { assert!(ApprovalRequirement::Always.is_required()); } + #[test] + fn test_redact_params_replaces_sensitive_key() { + let params = serde_json::json!({"name": "openai_key", "value": "sk-secret"}); + let redacted = redact_params(¶ms, &["value"]); + assert_eq!(redacted["name"], "openai_key"); + assert_eq!(redacted["value"], "[REDACTED]"); + // Original unchanged + assert_eq!(params["value"], "sk-secret"); + } + + #[test] + fn test_redact_params_empty_sensitive_is_noop() { + let params = serde_json::json!({"name": "key", "value": "secret"}); + let redacted = redact_params(¶ms, &[]); + assert_eq!(redacted, params); + } + + #[test] + fn test_redact_params_missing_key_is_noop() { + let params = serde_json::json!({"name": "key"}); + let redacted = redact_params(¶ms, &["value"]); + assert_eq!(redacted, params); + } + + #[test] + fn test_redact_params_non_object_is_passthrough() { + let params = serde_json::json!("just a string"); + let redacted = redact_params(¶ms, &["value"]); + assert_eq!(redacted, params); + } + #[test] fn test_validate_schema_valid() { let schema = serde_json::json!({ diff --git a/tests/ws_gateway_integration.rs b/tests/ws_gateway_integration.rs index 307271d37a..0016ba4e65 100644 --- a/tests/ws_gateway_integration.rs +++ b/tests/ws_gateway_integration.rs @@ -312,6 +312,8 @@ async fn test_ws_multiple_events_in_sequence() { state.sse.broadcast(SseEvent::ToolCompleted { name: "shell".to_string(), success: true, + error: None, + parameters: None, thread_id: None, }); state.sse.broadcast(SseEvent::Response {