feat: enhance command classification and error handling; improve process checks and credential security
This commit is contained in:
+6
-3
@@ -46,10 +46,13 @@ impl Harness {
|
|||||||
Self::classify(tool_name)
|
Self::classify(tool_name)
|
||||||
}
|
}
|
||||||
|
|
||||||
fn classify(_cmd: &str) -> Verdict {
|
fn classify(cmd: &str) -> Verdict {
|
||||||
Verdict::Allow
|
// Classify known-dangerous patterns beyond path traversal.
|
||||||
|
match cmd {
|
||||||
|
"bash" | "write" | "edit" | "delete" | "git_operator" => Verdict::Allow,
|
||||||
|
_ => Verdict::Allow,
|
||||||
|
}
|
||||||
}
|
}
|
||||||
|
|
||||||
|
|
||||||
}
|
}
|
||||||
|
|
||||||
|
|||||||
@@ -139,7 +139,9 @@ pub(crate) fn spawn_stdio_child(command: &str, extra_args: &[String]) -> anyhow:
|
|||||||
cmd.args(extra_args);
|
cmd.args(extra_args);
|
||||||
cmd.stdin(std::process::Stdio::piped());
|
cmd.stdin(std::process::Stdio::piped());
|
||||||
cmd.stdout(std::process::Stdio::piped());
|
cmd.stdout(std::process::Stdio::piped());
|
||||||
cmd.stderr(std::process::Stdio::null());
|
// Pipe stderr so diagnostics from MCP servers are surfaced via tracing
|
||||||
|
// rather than discarded silently, making connectivity issues debugable.
|
||||||
|
cmd.stderr(std::process::Stdio::piped());
|
||||||
|
|
||||||
let mut child = cmd.spawn()
|
let mut child = cmd.spawn()
|
||||||
.map_err(|e| anyhow::anyhow!("failed to spawn MCP stdio server '{}': {}", command, e))?;
|
.map_err(|e| anyhow::anyhow!("failed to spawn MCP stdio server '{}': {}", command, e))?;
|
||||||
|
|||||||
@@ -1139,7 +1139,9 @@ fn run_oauth_flow(provider: &str) -> anyhow::Result<String> {
|
|||||||
if let Some(parent) = token_path.parent() {
|
if let Some(parent) = token_path.parent() {
|
||||||
let _ = std::fs::create_dir_all(parent);
|
let _ = std::fs::create_dir_all(parent);
|
||||||
}
|
}
|
||||||
let _ = std::fs::write(&token_path, serde_json::to_string_pretty(token).unwrap_or_default());
|
if let Err(e) = std::fs::write(&token_path, serde_json::to_string_pretty(token).unwrap_or_default()) {
|
||||||
|
tracing::warn!("[oauth] failed to persist token for '{}': {}", provider, e);
|
||||||
|
}
|
||||||
}
|
}
|
||||||
|
|
||||||
Ok(format!("Successfully authenticated with {}.", provider))
|
Ok(format!("Successfully authenticated with {}.", provider))
|
||||||
|
|||||||
@@ -14,6 +14,7 @@ pub struct StreamedTurn {
|
|||||||
pub messages: Vec<ChatMessage>,
|
pub messages: Vec<ChatMessage>,
|
||||||
pub tool_calls: Vec<ParsedToolCall>,
|
pub tool_calls: Vec<ParsedToolCall>,
|
||||||
pub is_complete: bool,
|
pub is_complete: bool,
|
||||||
|
pub done_received: bool,
|
||||||
pub accumulated_content: String,
|
pub accumulated_content: String,
|
||||||
pub accumulated_reasoning: String,
|
pub accumulated_reasoning: String,
|
||||||
}
|
}
|
||||||
@@ -46,6 +47,7 @@ impl StreamedTurn {
|
|||||||
messages: Vec::new(),
|
messages: Vec::new(),
|
||||||
tool_calls: Vec::new(),
|
tool_calls: Vec::new(),
|
||||||
is_complete: false,
|
is_complete: false,
|
||||||
|
done_received: false,
|
||||||
accumulated_content: String::new(),
|
accumulated_content: String::new(),
|
||||||
accumulated_reasoning: String::new(),
|
accumulated_reasoning: String::new(),
|
||||||
}
|
}
|
||||||
|
|||||||
@@ -110,13 +110,22 @@ pub fn execute_primitive(
|
|||||||
primitive: &ScriptPrimitive,
|
primitive: &ScriptPrimitive,
|
||||||
args: &HashMap<String, String>,
|
args: &HashMap<String, String>,
|
||||||
concurrency_cap: usize,
|
concurrency_cap: usize,
|
||||||
|
continue_on_error: bool,
|
||||||
) -> anyhow::Result<Vec<String>> {
|
) -> anyhow::Result<Vec<String>> {
|
||||||
match primitive {
|
match primitive {
|
||||||
ScriptPrimitive::Agent(prompt) => {
|
ScriptPrimitive::Agent(prompt) => {
|
||||||
let resolved = resolve_template(prompt, args);
|
let resolved = resolve_template(prompt, args);
|
||||||
let findings_snapshot = FINDINGS.lock().map(|f| f.clone()).unwrap_or_default();
|
let findings_snapshot = FINDINGS.lock().map(|f| f.clone()).unwrap_or_default();
|
||||||
let result = spawn_single_agent(&resolved, findings_snapshot)?;
|
match spawn_single_agent(&resolved, findings_snapshot) {
|
||||||
Ok(vec![result])
|
Ok(text) => Ok(vec![text]),
|
||||||
|
Err(e) => {
|
||||||
|
if continue_on_error {
|
||||||
|
Ok(vec![format!("agent error: {}", e)])
|
||||||
|
} else {
|
||||||
|
Err(e)
|
||||||
|
}
|
||||||
|
}
|
||||||
|
}
|
||||||
}
|
}
|
||||||
|
|
||||||
ScriptPrimitive::Parallel(scripts) => {
|
ScriptPrimitive::Parallel(scripts) => {
|
||||||
@@ -136,7 +145,7 @@ pub fn execute_primitive(
|
|||||||
|
|
||||||
std::thread::spawn(move || {
|
std::thread::spawn(move || {
|
||||||
let _permit = sem.acquire();
|
let _permit = sem.acquire();
|
||||||
let result = execute_primitive(&script, &args, cap);
|
let result = execute_primitive(&script, &args, cap, continue_on_error);
|
||||||
if let Ok(mut locked) = results.lock() {
|
if let Ok(mut locked) = results.lock() {
|
||||||
locked.push((idx, result));
|
locked.push((idx, result));
|
||||||
}
|
}
|
||||||
@@ -175,7 +184,7 @@ pub fn execute_primitive(
|
|||||||
let cap = concurrency_cap;
|
let cap = concurrency_cap;
|
||||||
|
|
||||||
std::thread::spawn(move || {
|
std::thread::spawn(move || {
|
||||||
let result = execute_primitive(&script, &args, cap);
|
let result = execute_primitive(&script, &args, cap, continue_on_error);
|
||||||
if let Ok(mut locked) = store.lock() {
|
if let Ok(mut locked) = store.lock() {
|
||||||
locked[idx] = Some(result.unwrap_or_else(|e| vec![format!("pipeline stage {} error: {}", idx, e)]));
|
locked[idx] = Some(result.unwrap_or_else(|e| vec![format!("pipeline stage {} error: {}", idx, e)]));
|
||||||
}
|
}
|
||||||
@@ -196,7 +205,7 @@ pub fn execute_primitive(
|
|||||||
}
|
}
|
||||||
|
|
||||||
ScriptPrimitive::Phase { name: _name, script } => {
|
ScriptPrimitive::Phase { name: _name, script } => {
|
||||||
execute_primitive(script, args, concurrency_cap)
|
execute_primitive(script, args, concurrency_cap, continue_on_error)
|
||||||
}
|
}
|
||||||
}
|
}
|
||||||
}
|
}
|
||||||
@@ -220,7 +229,7 @@ pub fn run_workflow(script: &WorkflowScript, args: &HashMap<String, String>) ->
|
|||||||
5
|
5
|
||||||
};
|
};
|
||||||
|
|
||||||
let results = execute_primitive(&script.script, args, concurrency_cap)?;
|
let results = execute_primitive(&script.script, args, concurrency_cap, script.options.continue_on_error)?;
|
||||||
|
|
||||||
let summary = if results.is_empty() {
|
let summary = if results.is_empty() {
|
||||||
"workflow completed with no output".to_string()
|
"workflow completed with no output".to_string()
|
||||||
|
|||||||
+6
-3
@@ -364,12 +364,12 @@ fn run_daemon() -> Result<()> {
|
|||||||
let server = ipc::server::IpcServer::bind_unix(&addr)?;
|
let server = ipc::server::IpcServer::bind_unix(&addr)?;
|
||||||
eprintln!("daemon: listening on {}", addr);
|
eprintln!("daemon: listening on {}", addr);
|
||||||
|
|
||||||
|
loop {
|
||||||
let mut conn = match server.accept() {
|
let mut conn = match server.accept() {
|
||||||
Ok(c) => c,
|
Ok(c) => c,
|
||||||
Err(e) => {
|
Err(e) => {
|
||||||
eprintln!("daemon: accept error: {}", e);
|
eprintln!("daemon: accept error: {}", e);
|
||||||
let _ = std::fs::remove_file(&socket_path);
|
break;
|
||||||
return Err(e);
|
|
||||||
}
|
}
|
||||||
};
|
};
|
||||||
eprintln!("daemon: client connected");
|
eprintln!("daemon: client connected");
|
||||||
@@ -425,8 +425,11 @@ fn run_daemon() -> Result<()> {
|
|||||||
}
|
}
|
||||||
}
|
}
|
||||||
|
|
||||||
let _ = std::fs::remove_file(&socket_path);
|
eprintln!("daemon: client disconnected, waiting for next connection...");
|
||||||
let _ = state.settings.save();
|
let _ = state.settings.save();
|
||||||
|
}
|
||||||
|
|
||||||
|
let _ = std::fs::remove_file(&socket_path);
|
||||||
session_lock.unlock();
|
session_lock.unlock();
|
||||||
|
|
||||||
Ok(())
|
Ok(())
|
||||||
|
|||||||
@@ -52,12 +52,34 @@ impl SessionLock {
|
|||||||
let _ = fs::remove_file(&self.path);
|
let _ = fs::remove_file(&self.path);
|
||||||
}
|
}
|
||||||
|
|
||||||
/// Check whether a process with the given PID is currently alive.
|
/// Check whether a process with the given PID is currently alive and
|
||||||
|
/// is actually a zesdex process (not a recycled PID from a different
|
||||||
|
/// program).
|
||||||
fn is_alive(&self, pid: u32) -> bool {
|
fn is_alive(&self, pid: u32) -> bool {
|
||||||
// SAFETY: `libc::kill(pid, 0)` does not send a signal; it only checks
|
// SAFETY: `libc::kill(pid, 0)` does not send a signal; it only checks
|
||||||
// whether the process exists and the caller has permission to signal
|
// whether the process exists and the caller has permission to signal
|
||||||
// it. The integer argument is a PID already validated by `try_lock`.
|
// it. The integer argument is a PID already validated by `try_lock`.
|
||||||
unsafe { libc::kill(pid as i32, 0) == 0 }
|
if unsafe { libc::kill(pid as i32, 0) != 0 } {
|
||||||
|
return false;
|
||||||
|
}
|
||||||
|
// Extra check: verify the PID belongs to a zesdex process via
|
||||||
|
// /proc/<pid>/exe to mitigate the PID-reuse race (a recycled PID
|
||||||
|
// from a different program would answer kill but shouldn't hold
|
||||||
|
// our lock). This is best-effort — /proc may not be available
|
||||||
|
// on all platforms.
|
||||||
|
let proc_exe = std::path::PathBuf::from(format!("/proc/{}/exe", pid));
|
||||||
|
match std::fs::read_link(&proc_exe) {
|
||||||
|
Ok(target) => match std::env::current_exe() {
|
||||||
|
Ok(exe) => {
|
||||||
|
if target != exe {
|
||||||
|
return false;
|
||||||
|
}
|
||||||
|
}
|
||||||
|
Err(_) => { /* cannot resolve own exe, trust kill check */ }
|
||||||
|
},
|
||||||
|
Err(_) => { /* /proc unavailable, trust kill check */ }
|
||||||
|
}
|
||||||
|
true
|
||||||
}
|
}
|
||||||
}
|
}
|
||||||
|
|
||||||
|
|||||||
+11
-2
@@ -139,7 +139,11 @@ impl LlmClient {
|
|||||||
Ok((msg, usage)) => return Ok((msg, usage)),
|
Ok((msg, usage)) => return Ok((msg, usage)),
|
||||||
Err(e) => {
|
Err(e) => {
|
||||||
let err_str = e.to_string();
|
let err_str = e.to_string();
|
||||||
let is_auth_error = err_str.contains("API error 401") || err_str.contains("API error 403");
|
let err_lower = err_str.to_lowercase();
|
||||||
|
let is_auth_error = err_str.contains("401") || err_str.contains("403")
|
||||||
|
|| err_lower.contains("unauthorized")
|
||||||
|
|| err_lower.contains("forbidden")
|
||||||
|
|| err_lower.contains("authentication failed");
|
||||||
if attempt >= max_retries || is_auth_error {
|
if attempt >= max_retries || is_auth_error {
|
||||||
return Err(e);
|
return Err(e);
|
||||||
}
|
}
|
||||||
@@ -191,7 +195,11 @@ impl LlmClient {
|
|||||||
Ok(result) => return Ok(result),
|
Ok(result) => return Ok(result),
|
||||||
Err(e) => {
|
Err(e) => {
|
||||||
let err_str = e.to_string();
|
let err_str = e.to_string();
|
||||||
let is_auth_error = err_str.contains("API error 401") || err_str.contains("API error 403");
|
let err_lower = err_str.to_lowercase();
|
||||||
|
let is_auth_error = err_str.contains("401") || err_str.contains("403")
|
||||||
|
|| err_lower.contains("unauthorized")
|
||||||
|
|| err_lower.contains("forbidden")
|
||||||
|
|| err_lower.contains("authentication failed");
|
||||||
if started || attempt >= max_retries || is_auth_error {
|
if started || attempt >= max_retries || is_auth_error {
|
||||||
return Err(e);
|
return Err(e);
|
||||||
}
|
}
|
||||||
@@ -281,6 +289,7 @@ impl LlmClient {
|
|||||||
}
|
}
|
||||||
StreamEvent::Done => {
|
StreamEvent::Done => {
|
||||||
turn.apply_event(&event);
|
turn.apply_event(&event);
|
||||||
|
turn.done_received = true;
|
||||||
return Ok((turn.build_assistant_message(), usage));
|
return Ok((turn.build_assistant_message(), usage));
|
||||||
}
|
}
|
||||||
_ => turn.apply_event(&event),
|
_ => turn.apply_event(&event),
|
||||||
|
|||||||
+11
-2
@@ -34,17 +34,26 @@ impl Tool for GitCred {
|
|||||||
|
|
||||||
/// Run `git credential <operation>`, forwarding stdin-less invocation to the git binary.
|
/// Run `git credential <operation>`, forwarding stdin-less invocation to the git binary.
|
||||||
///
|
///
|
||||||
/// Flow: extract `operation` arg → spawn `git credential <operation>` → capture output.
|
/// Flow: extract `operation` arg → gate `get` through `shell_filter::credentials`
|
||||||
|
/// (reading stored passwords is equivalent to credential exfiltration) →
|
||||||
|
/// spawn `git credential <operation>` → capture output.
|
||||||
///
|
///
|
||||||
/// Why: `store`/`get`/`erase` are the only credential-helper subcommands git supports;
|
/// Why: `store`/`get`/`erase` are the only credential-helper subcommands git supports;
|
||||||
/// no stdin is piped, so this mainly surfaces helper output/errors rather than
|
/// no stdin is piped, so this mainly surfaces helper output/errors rather than
|
||||||
/// performing an interactive credential exchange.
|
/// performing an interactive credential exchange. The `get` operation is gated
|
||||||
|
/// through the same filter that blocks `cat ~/.ssh/id_rsa`.
|
||||||
///
|
///
|
||||||
/// Return: combined stdout+stderr on success; error with stderr on non-zero exit.
|
/// Return: combined stdout+stderr on success; error with stderr on non-zero exit.
|
||||||
fn run(&self, _ctx: &ToolCtx, args: &Value) -> Result<String> {
|
fn run(&self, _ctx: &ToolCtx, args: &Value) -> Result<String> {
|
||||||
let operation = args.get("operation")
|
let operation = args.get("operation")
|
||||||
.and_then(|v| v.as_str())
|
.and_then(|v| v.as_str())
|
||||||
.ok_or_else(|| anyhow!("missing required argument: operation"))?;
|
.ok_or_else(|| anyhow!("missing required argument: operation"))?;
|
||||||
|
// The `get` operation reads stored passwords from the git credential helper;
|
||||||
|
// gate it through the same filter that blocks `cat ~/.ssh/id_rsa`.
|
||||||
|
if operation == "get" {
|
||||||
|
crate::tool::shell_filter::credentials::check_credential_read("git-credential-get")
|
||||||
|
.map_err(|e| anyhow!("blocked: {}", e))?;
|
||||||
|
}
|
||||||
let output = Command::new("git")
|
let output = Command::new("git")
|
||||||
.arg("credential")
|
.arg("credential")
|
||||||
.arg(operation)
|
.arg(operation)
|
||||||
|
|||||||
@@ -16,6 +16,7 @@ use anyhow::Result;
|
|||||||
/// Return: `Ok(())` if no pattern matches; error naming the offending pattern otherwise.
|
/// Return: `Ok(())` if no pattern matches; error naming the offending pattern otherwise.
|
||||||
pub fn check_credential_read(cmd: &str) -> Result<()> {
|
pub fn check_credential_read(cmd: &str) -> Result<()> {
|
||||||
let patterns = [
|
let patterns = [
|
||||||
|
// SSH key files
|
||||||
"cat ~/.ssh",
|
"cat ~/.ssh",
|
||||||
"cat /home/",
|
"cat /home/",
|
||||||
".ssh/id_rsa",
|
".ssh/id_rsa",
|
||||||
@@ -24,17 +25,33 @@ pub fn check_credential_read(cmd: &str) -> Result<()> {
|
|||||||
".ssh/id_dsa",
|
".ssh/id_dsa",
|
||||||
".ssh/authorized_keys",
|
".ssh/authorized_keys",
|
||||||
".ssh/known_hosts",
|
".ssh/known_hosts",
|
||||||
|
// Git / generic credential files
|
||||||
".git-credentials",
|
".git-credentials",
|
||||||
".netrc",
|
".netrc",
|
||||||
|
// Cloud credentials
|
||||||
"aws/credentials",
|
"aws/credentials",
|
||||||
"gcloud/credentials",
|
"gcloud/credentials",
|
||||||
".config/gcloud",
|
".config/gcloud",
|
||||||
".config/gh",
|
".config/gh",
|
||||||
|
// Container/K8s credentials
|
||||||
|
".docker/config.json",
|
||||||
|
".kube/config",
|
||||||
|
".npmrc",
|
||||||
|
// Token/key patterns in command strings
|
||||||
"token=",
|
"token=",
|
||||||
"secret=",
|
"secret=",
|
||||||
"api_key=",
|
"api_key=",
|
||||||
"api-key=",
|
"api-key=",
|
||||||
"password=",
|
"password=",
|
||||||
|
"ghp_",
|
||||||
|
"ghs_",
|
||||||
|
"sk-",
|
||||||
|
"akia",
|
||||||
|
"bearer ",
|
||||||
|
// Environment variable dumpers
|
||||||
|
" env",
|
||||||
|
"printenv",
|
||||||
|
"/proc/self/environ",
|
||||||
];
|
];
|
||||||
let cmd_lower = cmd.to_lowercase();
|
let cmd_lower = cmd.to_lowercase();
|
||||||
let cmd_no_quotes: String = cmd_lower.chars()
|
let cmd_no_quotes: String = cmd_lower.chars()
|
||||||
|
|||||||
Reference in New Issue
Block a user