diff --git a/lib/crates/fabro-acp/src/command.rs b/lib/crates/fabro-acp/src/command.rs index 834c0daab..0244654e6 100644 --- a/lib/crates/fabro-acp/src/command.rs +++ b/lib/crates/fabro-acp/src/command.rs @@ -37,11 +37,7 @@ impl AcpCommand { #[must_use] pub fn to_shell_command(&self) -> String { - std::iter::once(self.program.to_string_lossy().into_owned()) - .chain(self.args.iter().cloned()) - .map(|part| fabro_sandbox::shell_quote(&part)) - .collect::>() - .join(" ") + render_command(&self.program, &self.args) } } @@ -123,11 +119,15 @@ fn parse_acp_command(raw: &str) -> Result { return Err(AcpCommandError::UnsupportedTransport); }; + let program = stdio.command; + let args = stdio.args; + let display = render_command(&program, &args); + Ok(AcpCommand { - display: raw.to_string(), - program: stdio.command, - args: stdio.args, - env: stdio + display, + program, + args, + env: stdio .env .into_iter() .map(|env| (env.name, env.value)) @@ -135,6 +135,14 @@ fn parse_acp_command(raw: &str) -> Result { }) } +fn render_command(program: &Path, args: &[String]) -> String { + std::iter::once(program.to_string_lossy().into_owned()) + .chain(args.iter().cloned()) + .map(|part| fabro_sandbox::shell_quote(&part)) + .collect::>() + .join(" ") +} + fn reject_non_stdio_json_transport(raw: &str) -> Result<(), AcpCommandError> { let trimmed = raw.trim_start(); if !trimmed.starts_with('{') { @@ -228,6 +236,20 @@ mod tests { assert_eq!(command.env().get("MODE").map(String::as_str), Some("test")); } + #[test] + fn json_stdio_acp_command_display_omits_env_contents() { + let raw = r#"{"type":"stdio","name":"fake","command":"agent","args":["--flag","two words"],"env":[{"name":"OPENAI_API_KEY","value":"secret-key"}]}"#; + let command = resolve_acp_command(Provider::OpenAi, Some(raw)).unwrap(); + + assert_eq!( + command.env().get("OPENAI_API_KEY").map(String::as_str), + Some("secret-key") + ); + assert_eq!(command.to_string(), "agent --flag 'two words'"); + assert!(!command.to_string().contains("secret-key")); + assert!(!command.to_string().contains("OPENAI_API_KEY")); + } + #[test] fn non_stdio_acp_command_is_rejected() { let raw = r#"{"type":"http","name":"remote","url":"https://example.test/acp"}"#; diff --git a/lib/crates/fabro-workflow/src/handler/llm/acp.rs b/lib/crates/fabro-workflow/src/handler/llm/acp.rs index aecbea6c7..f43379147 100644 --- a/lib/crates/fabro-workflow/src/handler/llm/acp.rs +++ b/lib/crates/fabro-workflow/src/handler/llm/acp.rs @@ -337,11 +337,12 @@ fn acp_error_to_workflow(error: AcpError) -> Error { #[cfg(test)] mod tests { use std::collections::HashMap; - use std::sync::Arc; + use std::sync::{Arc, Mutex}; use fabro_agent::{LocalSandbox, Sandbox, shell_quote}; use fabro_graphviz::graph::{AttrValue, Node}; use fabro_model::Provider; + use fabro_types::EventBody; use tokio_util::sync::CancellationToken; use super::AgentAcpBackend; @@ -504,6 +505,71 @@ mod tests { assert!(matches!(err, crate::error::Error::Cancelled)); } + #[tokio::test] + async fn acp_started_event_omits_json_command_env_values() { + let tempdir = tempfile::tempdir().unwrap(); + let script_path = tempdir.path().join("fake_acp_agent.py"); + tokio::fs::write(&script_path, fake_agent_script()) + .await + .unwrap(); + + let raw_command = serde_json::json!({ + "type": "stdio", + "name": "fake", + "command": "python3", + "args": [script_path.to_string_lossy()], + "env": [ + {"name": "OPENAI_API_KEY", "value": "secret-key"} + ], + }) + .to_string(); + let mut node = Node::new("work"); + node.attrs.insert( + "provider".to_string(), + AttrValue::String("openai".to_string()), + ); + node.attrs + .insert("backend".to_string(), AttrValue::String("acp".to_string())); + node.attrs + .insert("acp_command".to_string(), AttrValue::String(raw_command)); + + let backend = AgentAcpBackend::new_from_env("fake-acp".to_string(), Provider::OpenAi); + let sandbox: Arc = Arc::new(LocalSandbox::new(tempdir.path().to_path_buf())); + let emitter = Arc::new(Emitter::default()); + let events = Arc::new(Mutex::new(Vec::new())); + emitter.on_event({ + let events = Arc::clone(&events); + move |event| events.lock().unwrap().push(event.clone()) + }); + + backend + .run( + &node, + "write hello", + &Context::new(), + None, + &emitter, + &sandbox, + None, + CancellationToken::new(), + ) + .await + .unwrap(); + + let events = events.lock().unwrap(); + let command = events + .iter() + .find_map(|event| match &event.body { + EventBody::AgentAcpStarted(props) => Some(props.command.as_str()), + _ => None, + }) + .expect("ACP started event should be emitted"); + assert!(command.contains("python3")); + assert!(command.contains("fake_acp_agent.py")); + assert!(!command.contains("OPENAI_API_KEY")); + assert!(!command.contains("secret-key")); + } + fn fake_agent_script() -> &'static str { r#" import json