fix: redact ACP command env from metadata

This commit is contained in:
Bryan Helmkamp 2026-05-11 13:36:20 -04:00
parent e1d5b925e4
commit cd8a968b4a
No known key found for this signature in database
2 changed files with 98 additions and 10 deletions

View file

@ -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::<Vec<_>>()
.join(" ")
render_command(&self.program, &self.args)
}
}
@ -123,11 +119,15 @@ fn parse_acp_command(raw: &str) -> Result<AcpCommand, AcpCommandError> {
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<AcpCommand, AcpCommandError> {
})
}
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::<Vec<_>>()
.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"}"#;

View file

@ -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<dyn Sandbox> = 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