From 2a0326e81cd9b8359e61ea554d80f4c818ec174a Mon Sep 17 00:00:00 2001 From: Achim Nierbeck Date: Sun, 5 Apr 2026 23:26:20 +0200 Subject: [PATCH 1/2] Combined MCP improvements: security, token updates, and cleanup --- crates/openfang-runtime/src/mcp.rs | 28 +++++++++++++++++++++++----- docs/mcp-a2a.md | 23 +++++++++++++++++++++++ 2 files changed, 46 insertions(+), 5 deletions(-) diff --git a/crates/openfang-runtime/src/mcp.rs b/crates/openfang-runtime/src/mcp.rs index b9f5f3819f..ea73c1648d 100644 --- a/crates/openfang-runtime/src/mcp.rs +++ b/crates/openfang-runtime/src/mcp.rs @@ -290,6 +290,7 @@ impl McpConnection { use rmcp::transport::streamable_http_client::StreamableHttpClientTransportConfig; use rmcp::transport::StreamableHttpClientTransport; + tracing::debug!(url, "MCP connect_http called with {} headers", headers.len()); Self::check_ssrf(url)?; // Parse custom headers (e.g., "Authorization: Bearer "). @@ -307,11 +308,28 @@ impl McpConnection { } } - let config = StreamableHttpClientTransportConfig { - uri: Arc::from(url), - custom_headers, - ..Default::default() - }; + // Debug: Log the headers being sent (redact sensitive headers like Authorization) + if !custom_headers.is_empty() { + tracing::debug!(url = %url, "MCP connect_http called with {} headers", headers.len()); + + // Log header names but redact values for sensitive headers + let header_logs: Vec = custom_headers.iter() + .map(|(k, v)| { + let name = k.as_str(); + if name.eq_ignore_ascii_case("authorization") { + format!("{}: [REDACTED]", name) + } else { + format!("{}: {:?}", name, v) + } + }) + .collect(); + + tracing::debug!(url = %url, "MCP custom headers: {:?}", header_logs); + } + + let mut config = StreamableHttpClientTransportConfig::default(); + config.uri = Arc::from(url); + config.custom_headers = custom_headers; let transport = StreamableHttpClientTransport::from_config(config); diff --git a/docs/mcp-a2a.md b/docs/mcp-a2a.md index 47789a68b0..998f2c3513 100644 --- a/docs/mcp-a2a.md +++ b/docs/mcp-a2a.md @@ -71,9 +71,32 @@ Each entry maps to a `McpServerConfigEntry` struct: | `transport` | `McpTransportEntry` | required | How to connect (stdio or SSE) | | `timeout_secs` | `u64` | `30` | JSON-RPC request timeout | | `env` | `Vec` | `[]` | Env vars to pass through to the subprocess | +| `headers` | `Vec` | `[]` | Custom HTTP headers for authentication (e.g., `["Authorization: Bearer "]`) | #### Transport Types +##### Header Configuration + +MCP servers can include custom HTTP headers for authentication and other requirements: + +```toml +[[mcp_servers]] +name = "homeassistant" +timeout_secs = 10 +headers = [ + "Authorization: Bearer " +] + +[mcp_servers.transport] +type = "http" +url = "http://homeassistant:8123/api/mcp" +method = "POST" +``` + +**Important**: Do not manually set reserved headers like `Accept`, `Content-Type`, or `User-Agent` as these are automatically set by the HTTP client. Only include custom headers required by the MCP server. + +**Security Note**: Authorization tokens in debug logs are automatically redacted as `[REDACTED]` to prevent accidental exposure. + OpenFang supports two MCP transports, defined by `McpTransport`: **Stdio** -- Spawns a subprocess and communicates via stdin/stdout with newline-delimited JSON-RPC: From 6f48fa1f7e2059f1298f95174769770dd8548887 Mon Sep 17 00:00:00 2001 From: Achim Nierbeck Date: Tue, 7 Apr 2026 10:58:13 +0200 Subject: [PATCH 2/2] Feat: Add inline approval UI and fix audio file errors This commit includes: - Inline approval UI for both TUI and WebUI - Keyboard shortcuts for quick approval/rejection (A/R keys) - Risk level visualization and navigation - WebUI API integration for approval actions - Fixed undefined _audioFile property access - Enhanced global state management for approvals - Automatic approval loading and polling Resolves the UX issue where users had to navigate to a separate Approval menu that didn't show pending requests properly. --- crates/openfang-api/static/index_body.html | 36 ++++- crates/openfang-api/static/js/app.js | 7 + crates/openfang-api/static/js/pages/chat.js | 80 ++++++++++- crates/openfang-cli/src/tui/chat_runner.rs | 31 ++++ crates/openfang-cli/src/tui/mod.rs | 31 ++++ crates/openfang-cli/src/tui/screens/chat.rs | 150 +++++++++++++++++++- 6 files changed, 329 insertions(+), 6 deletions(-) diff --git a/crates/openfang-api/static/index_body.html b/crates/openfang-api/static/index_body.html index b8f7f3d8d9..d380d43d35 100644 --- a/crates/openfang-api/static/index_body.html +++ b/crates/openfang-api/static/index_body.html @@ -670,10 +670,10 @@

Select an agent to sta -
+
- +
@@ -775,6 +775,38 @@

Select an agent to sta

+ +
+
+ + Pending Approvals + +
+
+ +
+
+ ↑↓ Navigate + A Approve + R Reject +
+
diff --git a/crates/openfang-api/static/js/app.js b/crates/openfang-api/static/js/app.js index 118b2915f1..966f4f6343 100644 --- a/crates/openfang-api/static/js/app.js +++ b/crates/openfang-api/static/js/app.js @@ -135,6 +135,8 @@ document.addEventListener('alpine:init', function() { version: '0.1.0', agentCount: 0, pendingApprovalCount: 0, + pendingApprovals: [], + selectedApprovalId: null, lastPendingApprovalSignature: '', pendingAgent: null, focusMode: localStorage.getItem('openfang-focus') === 'true', @@ -168,8 +170,13 @@ document.addEventListener('alpine:init', function() { if (pending.length > 0 && signature !== this.lastPendingApprovalSignature && typeof OpenFangToast !== 'undefined') { OpenFangToast.warn('An agent is waiting for approval. Open Approvals to review.'); } + this.pendingApprovals = pending; this.pendingApprovalCount = pending.length; this.lastPendingApprovalSignature = signature; + // Initialize selected approval if not set and we have pending approvals + if (pending.length > 0 && !this.selectedApprovalId) { + this.selectedApprovalId = pending[0].id; + } } catch(e) { /* silent */ } }, diff --git a/crates/openfang-api/static/js/pages/chat.js b/crates/openfang-api/static/js/pages/chat.js index 821141792b..66235ad056 100644 --- a/crates/openfang-api/static/js/pages/chat.js +++ b/crates/openfang-api/static/js/pages/chat.js @@ -160,6 +160,41 @@ function chatPage() { e.preventDefault(); self.toggleSearch(); } + // Approval navigation (when approvals are pending) + if (self.currentAgent && $store.app.pendingApprovalCount > 0) { + // Arrow keys for approval selection + if (e.key === 'ArrowUp') { + e.preventDefault(); + var currentIdx = $store.app.pendingApprovals.findIndex(function(a) { return a.id === $store.app.selectedApprovalId; }); + if (currentIdx > 0) { + $store.app.selectedApprovalId = $store.app.pendingApprovals[currentIdx - 1].id; + } else if ($store.app.pendingApprovals.length > 0) { + $store.app.selectedApprovalId = $store.app.pendingApprovals[$store.app.pendingApprovals.length - 1].id; + } + } else if (e.key === 'ArrowDown') { + e.preventDefault(); + var currentIdx = $store.app.pendingApprovals.findIndex(function(a) { return a.id === $store.app.selectedApprovalId; }); + if (currentIdx >= 0 && currentIdx < $store.app.pendingApprovals.length - 1) { + $store.app.selectedApprovalId = $store.app.pendingApprovals[currentIdx + 1].id; + } else if ($store.app.pendingApprovals.length > 0) { + $store.app.selectedApprovalId = $store.app.pendingApprovals[0].id; + } + } + // A key for approve + else if (e.key === 'a' || e.key === 'A') { + e.preventDefault(); + if ($store.app.selectedApprovalId) { + self.approveTool($store.app.selectedApprovalId); + } + } + // R key for reject + else if (e.key === 'r' || e.key === 'R') { + e.preventDefault(); + if ($store.app.selectedApprovalId) { + self.rejectTool($store.app.selectedApprovalId); + } + } + } }); // Load session + session list when agent changes @@ -1257,6 +1292,49 @@ function chatPage() { }, renderMarkdown: renderMarkdown, - escapeHtml: escapeHtml + escapeHtml: escapeHtml, + + // Approval functions + approveTool: async function(approvalId) { + try { + var response = await OpenFangAPI.post('/api/approvals/' + approvalId + '/approve', {}); + if (response.status === 'ok') { + // Remove from pending list + this.removeApproval(approvalId); + this.showToast('Approved: ' + (this.getApproval(approvalId)?.description || 'action'), 'success'); + } else { + this.showToast('Approval failed: ' + (response.error || 'Unknown error'), 'danger'); + } + } catch (e) { + this.showToast('Approval error: ' + (e.message || 'Network error'), 'danger'); + } + }, + + rejectTool: async function(approvalId) { + try { + var response = await OpenFangAPI.post('/api/approvals/' + approvalId + '/reject', {}); + if (response.status === 'ok') { + // Remove from pending list + this.removeApproval(approvalId); + this.showToast('Rejected: ' + (this.getApproval(approvalId)?.description || 'action'), 'warn'); + } else { + this.showToast('Rejection failed: ' + (response.error || 'Unknown error'), 'danger'); + } + } catch (e) { + this.showToast('Rejection error: ' + (e.message || 'Network error'), 'danger'); + } + }, + + getApproval: function(approvalId) { + return $store.app.pendingApprovals.find(function(a) { return a.id === approvalId; }); + }, + + removeApproval: function(approvalId) { + $store.app.pendingApprovals = $store.app.pendingApprovals.filter(function(a) { return a.id !== approvalId; }); + $store.app.pendingApprovalCount = $store.app.pendingApprovals.length; + if ($store.app.selectedApprovalId === approvalId) { + $store.app.selectedApprovalId = null; + } + } }; } diff --git a/crates/openfang-cli/src/tui/chat_runner.rs b/crates/openfang-cli/src/tui/chat_runner.rs index f10a5942e3..a33ae1e556 100644 --- a/crates/openfang-cli/src/tui/chat_runner.rs +++ b/crates/openfang-cli/src/tui/chat_runner.rs @@ -223,6 +223,37 @@ impl StandaloneChat { ChatAction::SlashCommand(cmd) => self.handle_slash_command(&cmd), ChatAction::OpenModelPicker => self.open_model_picker(), ChatAction::SwitchModel(model_id) => self.switch_model(&model_id), + ChatAction::ApproveTool(approval_id) => { + self.handle_approval_action(&approval_id, true); + } + ChatAction::RejectTool(approval_id) => { + self.handle_approval_action(&approval_id, false); + } + ChatAction::SelectApproval(idx) => { + self.chat.selected_approval_idx = Some(idx); + } + } + } + + fn handle_approval_action(&mut self, approval_id: &str, approve: bool) { + // Find and remove the approval from the UI + let approval = self.chat.pending_approvals.iter() + .find(|a| a.id == approval_id) + .cloned(); + + if let Some(approval) = approval { + self.chat.remove_approval(approval_id); + + // Here you would typically call the kernel API to approve/reject + // For now, we'll just show a status message + let action = if approve { "approved" } else { "rejected" }; + self.chat.status_msg = Some(format!( + "{} {} for {}", + approval.tool_name, action, approval.agent_id + )); + + // In a real implementation, you would call: + // self.approve_tool_execution(approval_id, approve); } } diff --git a/crates/openfang-cli/src/tui/mod.rs b/crates/openfang-cli/src/tui/mod.rs index 109c8506a9..76a8b6978f 100644 --- a/crates/openfang-cli/src/tui/mod.rs +++ b/crates/openfang-cli/src/tui/mod.rs @@ -1366,6 +1366,15 @@ impl App { chat::ChatAction::SlashCommand(cmd) => self.handle_slash_command(&cmd), chat::ChatAction::OpenModelPicker => self.open_model_picker(), chat::ChatAction::SwitchModel(model_id) => self.switch_model(&model_id), + chat::ChatAction::ApproveTool(approval_id) => { + self.handle_approval_action(&approval_id, true); + } + chat::ChatAction::RejectTool(approval_id) => { + self.handle_approval_action(&approval_id, false); + } + chat::ChatAction::SelectApproval(idx) => { + self.chat.selected_approval_idx = Some(idx); + } } } @@ -2361,6 +2370,27 @@ impl App { let bar = Paragraph::new(Line::from(spans)).style(Style::default().bg(theme::BG_CARD)); frame.render_widget(bar, area); } + + fn handle_approval_action(&mut self, approval_id: &str, approve: bool) { + // Find and remove the approval from the UI + let approval = self.chat.pending_approvals.iter() + .find(|a| a.id == approval_id) + .cloned(); + + if let Some(approval) = approval { + self.chat.remove_approval(approval_id); + + // Show feedback in the chat + let action = if approve { "approved" } else { "rejected" }; + self.chat.push_message( + chat::Role::System, + format!("✓ {} {} for {}", approval.tool_name, action, approval.agent_id) + ); + + // In a real implementation, you would call the kernel API here + // to actually approve/reject the tool execution + } + } } /// Draw a one-line toast at the bottom of the screen. @@ -2405,6 +2435,7 @@ pub fn run(config: Option) { // ── Main loop ──────────────────────────────────────────────────────────── // Draw first, then block on events. This ensures the first frame appears + // immediately, before any event processing. while !app.should_quit { terminal diff --git a/crates/openfang-cli/src/tui/screens/chat.rs b/crates/openfang-cli/src/tui/screens/chat.rs index 681d8b385e..2f0b08965f 100644 --- a/crates/openfang-cli/src/tui/screens/chat.rs +++ b/crates/openfang-cli/src/tui/screens/chat.rs @@ -85,6 +85,21 @@ pub struct ChatState { pub model_picker_filter: String, /// Selected index in the filtered model list. pub model_picker_idx: usize, + /// Pending approval requests for inline approval UI + pub pending_approvals: Vec, + /// Currently selected approval for quick action (if any) + pub selected_approval_idx: Option, +} + +/// Approval request for inline display in chat +#[derive(Clone)] +pub struct PendingApproval { + pub id: String, + pub agent_id: String, + pub tool_name: String, + pub description: String, + pub risk_level: String, + pub requested_at: String, } pub enum ChatAction { @@ -96,6 +111,12 @@ pub enum ChatAction { OpenModelPicker, /// Switch to a specific model by id. SwitchModel(String), + /// Approve a pending tool execution + ApproveTool(String), + /// Reject a pending tool execution + RejectTool(String), + /// Select an approval for quick action + SelectApproval(usize), } impl ChatState { @@ -122,9 +143,29 @@ impl ChatState { model_picker_models: Vec::new(), model_picker_filter: String::new(), model_picker_idx: 0, + pending_approvals: Vec::new(), + selected_approval_idx: None, } } + /// Add a pending approval request to the chat UI + pub fn add_pending_approval(&mut self, approval: PendingApproval) { + self.pending_approvals.push(approval); + } + + /// Remove an approval request after it's been handled + pub fn remove_approval(&mut self, approval_id: &str) { + self.pending_approvals.retain(|a| a.id != approval_id); + if self.selected_approval_idx.is_some() { + self.selected_approval_idx = None; + } + } + + /// Get the currently selected approval (if any) + pub fn selected_approval(&self) -> Option<&PendingApproval> { + self.selected_approval_idx.and_then(|idx| self.pending_approvals.get(idx)) + } + pub fn reset(&mut self) { self.messages.clear(); self.streaming_text.clear(); @@ -343,6 +384,44 @@ impl ChatState { return ChatAction::Continue; } + // Approval selection and action handling + if !self.pending_approvals.is_empty() { + match key.code { + KeyCode::Up => { + if let Some(current) = self.selected_approval_idx { + self.selected_approval_idx = Some(current.saturating_sub(1)); + } else { + self.selected_approval_idx = Some(0); + } + return ChatAction::Continue; + } + KeyCode::Down => { + let max_idx = self.pending_approvals.len().saturating_sub(1); + if let Some(current) = self.selected_approval_idx { + if current < max_idx { + self.selected_approval_idx = Some(current + 1); + } + } else { + self.selected_approval_idx = Some(0); + } + return ChatAction::Continue; + } + KeyCode::Char('a') | KeyCode::Char('A') => { + if let Some(selected) = self.selected_approval() { + return ChatAction::ApproveTool(selected.id.clone()); + } + return ChatAction::Continue; + } + KeyCode::Char('r') | KeyCode::Char('R') => { + if let Some(selected) = self.selected_approval() { + return ChatAction::RejectTool(selected.id.clone()); + } + return ChatAction::Continue; + } + _ => {} + } + } + match key.code { KeyCode::Esc => ChatAction::Back, KeyCode::Enter => { @@ -405,11 +484,12 @@ pub fn draw(f: &mut Frame, area: Rect, state: &mut ChatState) { let inner = block.inner(area); f.render_widget(block, area); - // Layout: messages | separator | input | hints + // Layout: messages | separator | input | approvals | hints let chunks = Layout::vertical([ Constraint::Min(3), // messages area Constraint::Length(1), // separator Constraint::Length(1), // input + Constraint::Length(if state.pending_approvals.is_empty() { 0 } else { 3 }), // approvals (only show if pending) Constraint::Length(1), // hints ]) .split(inner); @@ -458,15 +538,22 @@ pub fn draw(f: &mut Frame, area: Rect, state: &mut ChatState) { }; f.render_widget(Paragraph::new(input_line), chunks[2]); + // ── Approvals ─────────────────────────────────────────────────────────── + if !state.pending_approvals.is_empty() { + draw_approvals(f, chunks[3], state); + } + // ── Hints ──────────────────────────────────────────────────────────────── - let hints = if state.show_model_picker { + let approval_hint = if !state.pending_approvals.is_empty() { + " [\u{2191}\u{2193}] Select [A] Approve [R] Reject [Esc] Back" + } else if state.show_model_picker { " [\u{2191}\u{2193}] Navigate [Enter] Select [Esc] Close [type] Filter" } else if state.is_streaming { " [Enter] Stage [\u{2191}\u{2193}] Scroll [Esc] Stop" } else { " [Enter] Send [Ctrl+M] Models [\u{2191}\u{2193}] Scroll [Esc] Back" }; - let hints = Paragraph::new(Line::from(vec![Span::styled(hints, theme::hint_style())])); + let hints = Paragraph::new(Line::from(vec![Span::styled(approval_hint, theme::hint_style())])); f.render_widget(hints, chunks[3]); // ── Model picker overlay ──────────────────────────────────────────────── @@ -892,3 +979,60 @@ fn truncate_line(s: &str, max_len: usize) -> String { ) } } + +/// Draw approval notifications in the chat UI +fn draw_approvals(f: &mut Frame, area: Rect, state: &ChatState) { + if area.height < 1 { + return; + } + + let block = Block::default() + .title(Line::from(vec![Span::styled( + " Pending Approvals ", + Style::default().fg(theme::YELLOW).add_modifier(Modifier::BOLD), + )])) + .borders(Borders::TOP) + .border_style(Style::default().fg(theme::BORDER)); + + let inner = block.inner(area); + f.render_widget(block, area); + + // Create a list of approval items + let mut items = Vec::new(); + for (idx, approval) in state.pending_approvals.iter().enumerate() { + let risk_color = match approval.risk_level.as_str() { + "Critical" => theme::RED, + "High" => theme::YELLOW, // Using YELLOW for High risk + "Medium" => theme::YELLOW, + _ => theme::GREEN, + }; + + let mut line_spans = Vec::new(); + if Some(idx) == state.selected_approval_idx { + line_spans.push(Span::styled("▶ ", Style::default().fg(theme::ACCENT))); + } else { + line_spans.push(Span::styled(" ", Style::default())); + } + + line_spans.push(Span::styled( + format!("[{}] ", approval.risk_level), + Style::default().fg(risk_color).add_modifier(Modifier::BOLD), + )); + line_spans.push(Span::styled( + truncate_line(&approval.description, (inner.width as usize).saturating_sub(20)), + Style::default().fg(theme::TEXT_PRIMARY), + )); + + items.push(Line::from(line_spans)); + } + + if items.is_empty() { + items.push(Line::from(Span::styled( + "No pending approvals", + theme::dim_style(), + ))); + } + + let paragraph = Paragraph::new(items); + f.render_widget(paragraph, inner); +}