mirror of
https://github.com/tinyhumansai/openhuman.git
synced 2026-07-27 21:08:00 +00:00
feat(security): wire AuditLogger into shell tool execution (#2342)
This commit is contained in:
@@ -818,6 +818,11 @@ impl Agent {
|
||||
&config.autonomy,
|
||||
&config.workspace_dir,
|
||||
));
|
||||
// Phase 1 of #1401: see comment in channels/runtime/startup.rs.
|
||||
let audit = crate::openhuman::security::get_or_create_workspace_audit_logger(
|
||||
crate::openhuman::config::AuditConfig::default(),
|
||||
config.workspace_dir.clone(),
|
||||
)?;
|
||||
|
||||
let local_embedding = config.workload_local_model("embeddings");
|
||||
let memory: Arc<dyn Memory> = Arc::from(memory::create_memory_with_local_ai(
|
||||
@@ -832,6 +837,7 @@ impl Agent {
|
||||
Arc::new(config.clone()),
|
||||
&security,
|
||||
runtime,
|
||||
audit,
|
||||
memory.clone(),
|
||||
&config.browser,
|
||||
&config.http_request,
|
||||
|
||||
@@ -180,6 +180,16 @@ pub async fn start_channels(config: Config) -> Result<()> {
|
||||
&config.autonomy,
|
||||
&config.workspace_dir,
|
||||
));
|
||||
// Phase 1 of #1401: audit logger is wired with defaults so emission paths
|
||||
// are exercised at runtime. A follow-up promotes `SecurityConfig` (and
|
||||
// therefore the `audit` knob) onto the runtime `Config` schema so users
|
||||
// can override `enabled`, `log_path`, and `max_size_mb` via TOML. The
|
||||
// logger is workspace-scoped and shared, so concurrent sessions append to
|
||||
// one `audit.log` without racing on rotation.
|
||||
let audit = crate::openhuman::security::get_or_create_workspace_audit_logger(
|
||||
crate::openhuman::config::AuditConfig::default(),
|
||||
config.workspace_dir.clone(),
|
||||
)?;
|
||||
let model = config
|
||||
.default_model
|
||||
.clone()
|
||||
@@ -199,6 +209,7 @@ pub async fn start_channels(config: Config) -> Result<()> {
|
||||
Arc::new(config.clone()),
|
||||
&security,
|
||||
runtime,
|
||||
audit,
|
||||
Arc::clone(&mem),
|
||||
&config.browser,
|
||||
&config.http_request,
|
||||
|
||||
@@ -39,6 +39,12 @@ fn build_runtime_tools(config: &Config) -> Result<Vec<Box<dyn Tool>>, String> {
|
||||
&config.autonomy,
|
||||
&config.workspace_dir,
|
||||
));
|
||||
// Phase 1 of #1401: see comment in channels/runtime/startup.rs.
|
||||
let audit = crate::openhuman::security::get_or_create_workspace_audit_logger(
|
||||
crate::openhuman::config::AuditConfig::default(),
|
||||
config.workspace_dir.clone(),
|
||||
)
|
||||
.map_err(|e| e.to_string())?;
|
||||
let runtime: Arc<dyn RuntimeAdapter> = Arc::new(NativeRuntime::new());
|
||||
let local_embedding = config.workload_local_model("embeddings");
|
||||
trace!("[runtime_node::ops] build_runtime_tools: create_memory_with_local_ai");
|
||||
@@ -63,6 +69,7 @@ fn build_runtime_tools(config: &Config) -> Result<Vec<Box<dyn Tool>>, String> {
|
||||
Arc::new(config.clone()),
|
||||
&security,
|
||||
runtime,
|
||||
audit,
|
||||
memory,
|
||||
&config.browser,
|
||||
&config.http_request,
|
||||
|
||||
@@ -5,9 +5,11 @@ use anyhow::Result;
|
||||
use chrono::{DateTime, Utc};
|
||||
use parking_lot::Mutex;
|
||||
use serde::{Deserialize, Serialize};
|
||||
use std::collections::HashMap;
|
||||
use std::fs::OpenOptions;
|
||||
use std::io::Write;
|
||||
use std::path::PathBuf;
|
||||
use std::sync::{Arc, OnceLock};
|
||||
use uuid::Uuid;
|
||||
|
||||
/// Audit event types
|
||||
@@ -148,6 +150,63 @@ pub struct AuditLogger {
|
||||
log_path: PathBuf,
|
||||
config: AuditConfig,
|
||||
buffer: Mutex<Vec<AuditEvent>>,
|
||||
/// Serializes the rotate + append + fsync critical section so concurrent
|
||||
/// callers sharing one logger cannot interleave partial JSON lines.
|
||||
write_lock: Mutex<()>,
|
||||
}
|
||||
|
||||
/// Process-global cache of one [`AuditLogger`] per workspace directory.
|
||||
///
|
||||
/// Multiple agent sessions in the same process share a workspace and therefore
|
||||
/// the same `<workspace>/audit.log`. Handing each session its own logger would
|
||||
/// let concurrent appends race on rotation and interleave partial lines; this
|
||||
/// registry guarantees a single `Arc<AuditLogger>` per workspace so all writes
|
||||
/// serialize through one instance's `write_lock`.
|
||||
static WORKSPACE_AUDIT_LOGGERS: OnceLock<Mutex<HashMap<PathBuf, Arc<AuditLogger>>>> =
|
||||
OnceLock::new();
|
||||
|
||||
/// Return the shared [`AuditLogger`] for `openhuman_dir`, creating it on first
|
||||
/// use. All callers targeting the same workspace receive the same `Arc`, so log
|
||||
/// rotation and appends coordinate through one logger. The `config` of the first
|
||||
/// caller for a given workspace wins; later callers reuse the cached logger.
|
||||
pub fn get_or_create_workspace_audit_logger(
|
||||
config: AuditConfig,
|
||||
openhuman_dir: PathBuf,
|
||||
) -> Result<Arc<AuditLogger>> {
|
||||
// Normalize the key: `PathBuf` equality is lexical, so `/ws` vs `/ws/` vs a
|
||||
// symlinked spelling would otherwise cache distinct loggers for one physical
|
||||
// workspace and reopen the rotate/append race this registry prevents.
|
||||
//
|
||||
// A canonicalize failure falls back to the raw path rather than propagating:
|
||||
// audit-logger creation must never block agent startup. `NotFound` (the
|
||||
// workspace dir not created yet) is expected and logged at debug; other
|
||||
// errors (permission, I/O) are unexpected and logged at warn so real
|
||||
// filesystem problems stay observable.
|
||||
let openhuman_dir = match std::fs::canonicalize(&openhuman_dir) {
|
||||
Ok(path) => path,
|
||||
Err(err) if err.kind() == std::io::ErrorKind::NotFound => {
|
||||
log::debug!(
|
||||
"[openhuman:audit] workspace path not yet created; keying registry on raw path: {}",
|
||||
openhuman_dir.display()
|
||||
);
|
||||
openhuman_dir
|
||||
}
|
||||
Err(err) => {
|
||||
log::warn!(
|
||||
"[openhuman:audit] failed to canonicalize workspace path {} ({err}); keying registry on raw path",
|
||||
openhuman_dir.display()
|
||||
);
|
||||
openhuman_dir
|
||||
}
|
||||
};
|
||||
let registry = WORKSPACE_AUDIT_LOGGERS.get_or_init(|| Mutex::new(HashMap::new()));
|
||||
let mut map = registry.lock();
|
||||
if let Some(existing) = map.get(&openhuman_dir) {
|
||||
return Ok(Arc::clone(existing));
|
||||
}
|
||||
let logger = Arc::new(AuditLogger::new(config, openhuman_dir.clone())?);
|
||||
map.insert(openhuman_dir, Arc::clone(&logger));
|
||||
Ok(logger)
|
||||
}
|
||||
|
||||
/// Structured command execution details for audit logging.
|
||||
@@ -163,6 +222,23 @@ pub struct CommandExecutionLog<'a> {
|
||||
}
|
||||
|
||||
impl AuditLogger {
|
||||
/// Build a disabled `Arc<AuditLogger>` for tests and contexts that need a
|
||||
/// handle but should not write to disk. The `enabled = false` flag
|
||||
/// short-circuits `log()` before any filesystem I/O, so the sentinel
|
||||
/// `log_path` is never touched.
|
||||
pub fn disabled() -> Arc<Self> {
|
||||
Arc::new(Self {
|
||||
log_path: PathBuf::new(),
|
||||
config: AuditConfig {
|
||||
enabled: false,
|
||||
log_path: String::new(),
|
||||
max_size_mb: 0,
|
||||
},
|
||||
buffer: Mutex::new(Vec::new()),
|
||||
write_lock: Mutex::new(()),
|
||||
})
|
||||
}
|
||||
|
||||
/// Create a new audit logger
|
||||
pub fn new(config: AuditConfig, openhuman_dir: PathBuf) -> Result<Self> {
|
||||
let log_path = openhuman_dir.join(&config.log_path);
|
||||
@@ -175,6 +251,7 @@ impl AuditLogger {
|
||||
log_path,
|
||||
config,
|
||||
buffer: Mutex::new(Vec::new()),
|
||||
write_lock: Mutex::new(()),
|
||||
})
|
||||
}
|
||||
|
||||
@@ -190,11 +267,14 @@ impl AuditLogger {
|
||||
event.event_id
|
||||
);
|
||||
|
||||
// Check log size and rotate if needed
|
||||
self.rotate_if_needed()?;
|
||||
|
||||
// Serialize and write
|
||||
// Serialize the event outside the write lock — formatting is pure.
|
||||
let line = serde_json::to_string(event)?;
|
||||
|
||||
// Hold `write_lock` across rotate + append + fsync so concurrent
|
||||
// callers sharing this logger cannot interleave partial lines or
|
||||
// race on rotation renames.
|
||||
let _guard = self.write_lock.lock();
|
||||
self.rotate_if_needed()?;
|
||||
let mut file = OpenOptions::new()
|
||||
.create(true)
|
||||
.append(true)
|
||||
@@ -331,6 +411,28 @@ mod tests {
|
||||
assert!(parsed.result.is_some());
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn audit_logger_disabled_helper_is_noop() -> Result<()> {
|
||||
let logger = AuditLogger::disabled();
|
||||
let event = AuditEvent::new(AuditEventType::CommandExecution);
|
||||
logger.log(&event)?;
|
||||
assert!(!logger.config.enabled);
|
||||
Ok(())
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn workspace_audit_logger_is_shared_per_workspace() -> Result<()> {
|
||||
let tmp = TempDir::new()?;
|
||||
let cfg = AuditConfig::default();
|
||||
let first = get_or_create_workspace_audit_logger(cfg.clone(), tmp.path().to_path_buf())?;
|
||||
let second = get_or_create_workspace_audit_logger(cfg, tmp.path().to_path_buf())?;
|
||||
assert!(
|
||||
Arc::ptr_eq(&first, &second),
|
||||
"same workspace must yield the same shared logger instance"
|
||||
);
|
||||
Ok(())
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn audit_logger_disabled_does_not_create_file() -> Result<()> {
|
||||
let tmp = TempDir::new()?;
|
||||
|
||||
@@ -14,7 +14,10 @@ pub mod secrets;
|
||||
pub mod traits;
|
||||
|
||||
#[allow(unused_imports)]
|
||||
pub use audit::{AuditEvent, AuditEventType, AuditLogger};
|
||||
pub use audit::{
|
||||
get_or_create_workspace_audit_logger, AuditEvent, AuditEventType, AuditLogger,
|
||||
CommandExecutionLog,
|
||||
};
|
||||
pub use core::*;
|
||||
#[allow(unused_imports)]
|
||||
pub use detect::create_sandbox;
|
||||
|
||||
@@ -1,11 +1,11 @@
|
||||
use crate::openhuman::agent::host_runtime::RuntimeAdapter;
|
||||
use crate::openhuman::javascript::NodeBootstrap;
|
||||
use crate::openhuman::security::SecurityPolicy;
|
||||
use crate::openhuman::security::{AuditLogger, CommandExecutionLog, SecurityPolicy};
|
||||
use crate::openhuman::tools::traits::{Tool, ToolResult};
|
||||
use async_trait::async_trait;
|
||||
use serde_json::json;
|
||||
use std::sync::Arc;
|
||||
use std::time::Duration;
|
||||
use std::time::{Duration, Instant};
|
||||
|
||||
/// Maximum shell command execution time before kill.
|
||||
const SHELL_TIMEOUT_SECS: u64 = 60;
|
||||
@@ -21,6 +21,7 @@ const SAFE_ENV_VARS: &[&str] = &[
|
||||
pub struct ShellTool {
|
||||
security: Arc<SecurityPolicy>,
|
||||
runtime: Arc<dyn RuntimeAdapter>,
|
||||
audit: Arc<AuditLogger>,
|
||||
/// Optional managed Node.js bootstrap. When provided **and** a prior
|
||||
/// `NodeBootstrap::resolve()` has already succeeded, every shell invocation
|
||||
/// transparently prepends the managed `bin/` dir to `PATH` — so skills
|
||||
@@ -31,10 +32,15 @@ pub struct ShellTool {
|
||||
}
|
||||
|
||||
impl ShellTool {
|
||||
pub fn new(security: Arc<SecurityPolicy>, runtime: Arc<dyn RuntimeAdapter>) -> Self {
|
||||
pub fn new(
|
||||
security: Arc<SecurityPolicy>,
|
||||
runtime: Arc<dyn RuntimeAdapter>,
|
||||
audit: Arc<AuditLogger>,
|
||||
) -> Self {
|
||||
Self {
|
||||
security,
|
||||
runtime,
|
||||
audit,
|
||||
node_bootstrap: None,
|
||||
}
|
||||
}
|
||||
@@ -45,14 +51,45 @@ impl ShellTool {
|
||||
pub fn with_node_bootstrap(
|
||||
security: Arc<SecurityPolicy>,
|
||||
runtime: Arc<dyn RuntimeAdapter>,
|
||||
audit: Arc<AuditLogger>,
|
||||
bootstrap: Arc<NodeBootstrap>,
|
||||
) -> Self {
|
||||
Self {
|
||||
security,
|
||||
runtime,
|
||||
audit,
|
||||
node_bootstrap: Some(bootstrap),
|
||||
}
|
||||
}
|
||||
|
||||
/// Emit a single `CommandExecution` audit event. A write failure is logged
|
||||
/// as a structured warning but not propagated — audit must never block or
|
||||
/// fail a tool call, yet a silently broken audit trail must not go
|
||||
/// unnoticed.
|
||||
fn emit_audit(
|
||||
&self,
|
||||
command: &str,
|
||||
approved: bool,
|
||||
allowed: bool,
|
||||
success: bool,
|
||||
duration_ms: u64,
|
||||
) {
|
||||
if let Err(error) = self.audit.log_command_event(CommandExecutionLog {
|
||||
channel: "tool:shell",
|
||||
command,
|
||||
risk_level: "unknown",
|
||||
approved,
|
||||
allowed,
|
||||
success,
|
||||
duration_ms,
|
||||
}) {
|
||||
tracing::warn!(
|
||||
error = %error,
|
||||
channel = "tool:shell",
|
||||
"[shell] failed to persist command execution audit event"
|
||||
);
|
||||
}
|
||||
}
|
||||
}
|
||||
|
||||
#[async_trait]
|
||||
@@ -102,23 +139,35 @@ impl Tool for ShellTool {
|
||||
.and_then(|v| v.as_bool())
|
||||
.unwrap_or(false);
|
||||
|
||||
let start = Instant::now();
|
||||
let (allowed, result) = self.run_with_security(command, approved).await;
|
||||
let duration_ms = u64::try_from(start.elapsed().as_millis()).unwrap_or(u64::MAX);
|
||||
self.emit_audit(command, approved, allowed, !result.is_error, duration_ms);
|
||||
Ok(result)
|
||||
}
|
||||
}
|
||||
|
||||
impl ShellTool {
|
||||
/// Run the command through the security policy and runtime. Returns
|
||||
/// `(allowed, result)` where `allowed=false` means the policy or rate
|
||||
/// limiter blocked execution before the command was launched.
|
||||
async fn run_with_security(&self, command: &str, approved: bool) -> (bool, ToolResult) {
|
||||
if self.security.is_rate_limited() {
|
||||
return Ok(ToolResult::error(
|
||||
"Rate limit exceeded: too many actions in the last hour",
|
||||
));
|
||||
return (
|
||||
false,
|
||||
ToolResult::error("Rate limit exceeded: too many actions in the last hour"),
|
||||
);
|
||||
}
|
||||
|
||||
match self.security.validate_command_execution(command, approved) {
|
||||
Ok(_) => {}
|
||||
Err(reason) => {
|
||||
return Ok(ToolResult::error(reason));
|
||||
}
|
||||
if let Err(reason) = self.security.validate_command_execution(command, approved) {
|
||||
return (false, ToolResult::error(reason));
|
||||
}
|
||||
|
||||
if !self.security.record_action() {
|
||||
return Ok(ToolResult::error(
|
||||
"Rate limit exceeded: action budget exhausted",
|
||||
));
|
||||
return (
|
||||
false,
|
||||
ToolResult::error("Rate limit exceeded: action budget exhausted"),
|
||||
);
|
||||
}
|
||||
|
||||
// Execute with timeout to prevent hanging commands.
|
||||
@@ -130,9 +179,10 @@ impl Tool for ShellTool {
|
||||
{
|
||||
Ok(cmd) => cmd,
|
||||
Err(e) => {
|
||||
return Ok(ToolResult::error(format!(
|
||||
"Failed to build runtime command: {e}"
|
||||
)));
|
||||
return (
|
||||
true,
|
||||
ToolResult::error(format!("Failed to build runtime command: {e}")),
|
||||
);
|
||||
}
|
||||
};
|
||||
cmd.env_clear();
|
||||
@@ -168,7 +218,7 @@ impl Tool for ShellTool {
|
||||
let result =
|
||||
tokio::time::timeout(Duration::from_secs(SHELL_TIMEOUT_SECS), cmd.output()).await;
|
||||
|
||||
match result {
|
||||
let tool_result = match result {
|
||||
Ok(Ok(output)) => {
|
||||
let mut stdout = String::from_utf8_lossy(&output.stdout).to_string();
|
||||
let mut stderr = String::from_utf8_lossy(&output.stderr).to_string();
|
||||
@@ -191,21 +241,22 @@ impl Tool for ShellTool {
|
||||
|
||||
if output.status.success() {
|
||||
if stderr.is_empty() {
|
||||
Ok(ToolResult::success(stdout))
|
||||
ToolResult::success(stdout)
|
||||
} else {
|
||||
// Successful exit but stderr present — attach stderr as output suffix
|
||||
Ok(ToolResult::success(format!("{stdout}\n[stderr]\n{stderr}")))
|
||||
ToolResult::success(format!("{stdout}\n[stderr]\n{stderr}"))
|
||||
}
|
||||
} else {
|
||||
let err_msg = if stderr.is_empty() { stdout } else { stderr };
|
||||
Ok(ToolResult::error(err_msg))
|
||||
ToolResult::error(err_msg)
|
||||
}
|
||||
}
|
||||
Ok(Err(e)) => Ok(ToolResult::error(format!("Failed to execute command: {e}"))),
|
||||
Err(_) => Ok(ToolResult::error(format!(
|
||||
Ok(Err(e)) => ToolResult::error(format!("Failed to execute command: {e}")),
|
||||
Err(_) => ToolResult::error(format!(
|
||||
"Command timed out after {SHELL_TIMEOUT_SECS}s and was killed"
|
||||
))),
|
||||
}
|
||||
)),
|
||||
};
|
||||
(true, tool_result)
|
||||
}
|
||||
}
|
||||
|
||||
@@ -227,21 +278,99 @@ mod tests {
|
||||
Arc::new(NativeRuntime::new())
|
||||
}
|
||||
|
||||
fn test_audit() -> Arc<AuditLogger> {
|
||||
AuditLogger::disabled()
|
||||
}
|
||||
|
||||
fn audit_with_tempdir() -> (Arc<AuditLogger>, tempfile::TempDir) {
|
||||
use crate::openhuman::config::AuditConfig;
|
||||
let tmp = tempfile::tempdir().expect("create tempdir");
|
||||
let logger = AuditLogger::new(
|
||||
AuditConfig {
|
||||
enabled: true,
|
||||
log_path: "audit.log".into(),
|
||||
max_size_mb: 10,
|
||||
},
|
||||
tmp.path().to_path_buf(),
|
||||
)
|
||||
.expect("create audit logger");
|
||||
(Arc::new(logger), tmp)
|
||||
}
|
||||
|
||||
#[cfg(not(windows))]
|
||||
#[tokio::test]
|
||||
async fn shell_emits_audit_line_on_success() {
|
||||
use crate::openhuman::security::AuditEvent;
|
||||
let (audit, tmp) = audit_with_tempdir();
|
||||
let tool = ShellTool::new(
|
||||
test_security(AutonomyLevel::Supervised),
|
||||
test_runtime(),
|
||||
audit,
|
||||
);
|
||||
let _ = tool
|
||||
.execute(json!({"command": "echo hello"}))
|
||||
.await
|
||||
.unwrap();
|
||||
let log = std::fs::read_to_string(tmp.path().join("audit.log"))
|
||||
.expect("audit log file should exist");
|
||||
assert!(!log.is_empty(), "audit log should not be empty");
|
||||
let parsed: AuditEvent = serde_json::from_str(log.trim()).expect("audit event JSON parses");
|
||||
let action = parsed.action.expect("action present");
|
||||
assert_eq!(action.command, Some("echo hello".to_string()));
|
||||
assert!(action.allowed, "allowed command should set allowed=true");
|
||||
let result = parsed.result.expect("result present");
|
||||
assert!(result.success, "echo hello should succeed");
|
||||
let actor = parsed.actor.expect("actor present");
|
||||
assert_eq!(actor.channel, "tool:shell");
|
||||
}
|
||||
|
||||
#[tokio::test]
|
||||
async fn shell_emits_audit_line_on_denial() {
|
||||
use crate::openhuman::security::AuditEvent;
|
||||
let (audit, tmp) = audit_with_tempdir();
|
||||
let tool = ShellTool::new(
|
||||
test_security(AutonomyLevel::ReadOnly),
|
||||
test_runtime(),
|
||||
audit,
|
||||
);
|
||||
let _ = tool.execute(json!({"command": "ls"})).await.unwrap();
|
||||
let log = std::fs::read_to_string(tmp.path().join("audit.log"))
|
||||
.expect("audit log file should exist");
|
||||
let parsed: AuditEvent = serde_json::from_str(log.trim()).expect("audit event JSON parses");
|
||||
let action = parsed.action.expect("action present");
|
||||
assert!(
|
||||
!action.allowed,
|
||||
"denied command should set allowed=false on the audit event"
|
||||
);
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn shell_tool_name() {
|
||||
let tool = ShellTool::new(test_security(AutonomyLevel::Supervised), test_runtime());
|
||||
let tool = ShellTool::new(
|
||||
test_security(AutonomyLevel::Supervised),
|
||||
test_runtime(),
|
||||
test_audit(),
|
||||
);
|
||||
assert_eq!(tool.name(), "shell");
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn shell_tool_description() {
|
||||
let tool = ShellTool::new(test_security(AutonomyLevel::Supervised), test_runtime());
|
||||
let tool = ShellTool::new(
|
||||
test_security(AutonomyLevel::Supervised),
|
||||
test_runtime(),
|
||||
test_audit(),
|
||||
);
|
||||
assert!(!tool.description().is_empty());
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn shell_tool_schema_has_command() {
|
||||
let tool = ShellTool::new(test_security(AutonomyLevel::Supervised), test_runtime());
|
||||
let tool = ShellTool::new(
|
||||
test_security(AutonomyLevel::Supervised),
|
||||
test_runtime(),
|
||||
test_audit(),
|
||||
);
|
||||
let schema = tool.parameters_schema();
|
||||
assert!(schema["properties"]["command"].is_object());
|
||||
assert!(schema["required"]
|
||||
@@ -254,7 +383,11 @@ mod tests {
|
||||
#[cfg(not(windows))]
|
||||
#[tokio::test]
|
||||
async fn shell_executes_allowed_command() {
|
||||
let tool = ShellTool::new(test_security(AutonomyLevel::Supervised), test_runtime());
|
||||
let tool = ShellTool::new(
|
||||
test_security(AutonomyLevel::Supervised),
|
||||
test_runtime(),
|
||||
test_audit(),
|
||||
);
|
||||
let result = tool
|
||||
.execute(json!({"command": "echo hello"}))
|
||||
.await
|
||||
@@ -266,7 +399,11 @@ mod tests {
|
||||
|
||||
#[tokio::test]
|
||||
async fn shell_blocks_disallowed_command() {
|
||||
let tool = ShellTool::new(test_security(AutonomyLevel::Supervised), test_runtime());
|
||||
let tool = ShellTool::new(
|
||||
test_security(AutonomyLevel::Supervised),
|
||||
test_runtime(),
|
||||
test_audit(),
|
||||
);
|
||||
let result = tool.execute(json!({"command": "rm -rf /"})).await.unwrap();
|
||||
assert!(result.is_error);
|
||||
let error = result.output();
|
||||
@@ -275,7 +412,11 @@ mod tests {
|
||||
|
||||
#[tokio::test]
|
||||
async fn shell_blocks_readonly() {
|
||||
let tool = ShellTool::new(test_security(AutonomyLevel::ReadOnly), test_runtime());
|
||||
let tool = ShellTool::new(
|
||||
test_security(AutonomyLevel::ReadOnly),
|
||||
test_runtime(),
|
||||
test_audit(),
|
||||
);
|
||||
let result = tool.execute(json!({"command": "ls"})).await.unwrap();
|
||||
assert!(result.is_error);
|
||||
assert!(&result.output().contains("not allowed"));
|
||||
@@ -283,7 +424,11 @@ mod tests {
|
||||
|
||||
#[tokio::test]
|
||||
async fn shell_missing_command_param() {
|
||||
let tool = ShellTool::new(test_security(AutonomyLevel::Supervised), test_runtime());
|
||||
let tool = ShellTool::new(
|
||||
test_security(AutonomyLevel::Supervised),
|
||||
test_runtime(),
|
||||
test_audit(),
|
||||
);
|
||||
let result = tool.execute(json!({})).await;
|
||||
assert!(result.is_err());
|
||||
assert!(result.unwrap_err().to_string().contains("command"));
|
||||
@@ -291,14 +436,22 @@ mod tests {
|
||||
|
||||
#[tokio::test]
|
||||
async fn shell_wrong_type_param() {
|
||||
let tool = ShellTool::new(test_security(AutonomyLevel::Supervised), test_runtime());
|
||||
let tool = ShellTool::new(
|
||||
test_security(AutonomyLevel::Supervised),
|
||||
test_runtime(),
|
||||
test_audit(),
|
||||
);
|
||||
let result = tool.execute(json!({"command": 123})).await;
|
||||
assert!(result.is_err());
|
||||
}
|
||||
|
||||
#[tokio::test]
|
||||
async fn shell_captures_exit_code() {
|
||||
let tool = ShellTool::new(test_security(AutonomyLevel::Supervised), test_runtime());
|
||||
let tool = ShellTool::new(
|
||||
test_security(AutonomyLevel::Supervised),
|
||||
test_runtime(),
|
||||
test_audit(),
|
||||
);
|
||||
let result = tool
|
||||
.execute(json!({"command": "ls /nonexistent_dir_xyz"}))
|
||||
.await
|
||||
@@ -344,7 +497,7 @@ mod tests {
|
||||
async fn shell_does_not_leak_api_key() {
|
||||
let _g1 = EnvGuard::set("API_KEY", "sk-test-secret-12345");
|
||||
|
||||
let tool = ShellTool::new(test_security_with_env_cmd(), test_runtime());
|
||||
let tool = ShellTool::new(test_security_with_env_cmd(), test_runtime(), test_audit());
|
||||
let result = tool
|
||||
.execute(json!({"command": "echo $API_KEY"}))
|
||||
.await
|
||||
@@ -359,7 +512,7 @@ mod tests {
|
||||
#[cfg(not(windows))]
|
||||
#[tokio::test]
|
||||
async fn shell_preserves_path_and_home() {
|
||||
let tool = ShellTool::new(test_security_with_env_cmd(), test_runtime());
|
||||
let tool = ShellTool::new(test_security_with_env_cmd(), test_runtime(), test_audit());
|
||||
|
||||
let result = tool
|
||||
.execute(json!({"command": "echo $HOME"}))
|
||||
@@ -392,7 +545,7 @@ mod tests {
|
||||
..SecurityPolicy::default()
|
||||
});
|
||||
|
||||
let tool = ShellTool::new(security.clone(), test_runtime());
|
||||
let tool = ShellTool::new(security.clone(), test_runtime(), test_audit());
|
||||
let command = if cfg!(windows) {
|
||||
"mkdir openhuman_shell_approval_test"
|
||||
} else {
|
||||
@@ -471,7 +624,7 @@ mod tests {
|
||||
workspace_dir: std::env::temp_dir(),
|
||||
..SecurityPolicy::default()
|
||||
});
|
||||
let tool = ShellTool::new(security, test_runtime());
|
||||
let tool = ShellTool::new(security, test_runtime(), test_audit());
|
||||
let result = tool.execute(json!({"command": "echo test"})).await.unwrap();
|
||||
assert!(result.is_error);
|
||||
assert!(result.output().contains("Rate limit"));
|
||||
|
||||
@@ -4,7 +4,7 @@ use crate::openhuman::agent::host_runtime::{NativeRuntime, RuntimeAdapter};
|
||||
use crate::openhuman::config::{Config, DelegateAgentConfig};
|
||||
use crate::openhuman::javascript::NodeBootstrap;
|
||||
use crate::openhuman::memory::Memory;
|
||||
use crate::openhuman::security::SecurityPolicy;
|
||||
use crate::openhuman::security::{AuditLogger, SecurityPolicy};
|
||||
use std::collections::HashMap;
|
||||
use std::sync::Arc;
|
||||
|
||||
@@ -14,12 +14,18 @@ pub fn default_tools(security: Arc<SecurityPolicy>) -> Vec<Box<dyn Tool>> {
|
||||
}
|
||||
|
||||
/// Create the default tool registry with explicit runtime adapter.
|
||||
///
|
||||
/// Convenience entry point used by tests and the lightweight CLI surface.
|
||||
/// Production assembly sites use [`all_tools_with_runtime`] and pass a real
|
||||
/// [`AuditLogger`]; this wrapper substitutes [`AuditLogger::disabled`] so
|
||||
/// existing test callers do not need to plumb one through.
|
||||
pub fn default_tools_with_runtime(
|
||||
security: Arc<SecurityPolicy>,
|
||||
runtime: Arc<dyn RuntimeAdapter>,
|
||||
) -> Vec<Box<dyn Tool>> {
|
||||
let audit = AuditLogger::disabled();
|
||||
vec![
|
||||
Box::new(ShellTool::new(security.clone(), runtime)),
|
||||
Box::new(ShellTool::new(security.clone(), runtime, audit)),
|
||||
Box::new(FileReadTool::new(security.clone())),
|
||||
Box::new(FileWriteTool::new(security)),
|
||||
]
|
||||
@@ -30,6 +36,7 @@ pub fn default_tools_with_runtime(
|
||||
pub fn all_tools(
|
||||
config: Arc<Config>,
|
||||
security: &Arc<SecurityPolicy>,
|
||||
audit: Arc<AuditLogger>,
|
||||
memory: Arc<dyn Memory>,
|
||||
browser_config: &crate::openhuman::config::BrowserConfig,
|
||||
http_config: &crate::openhuman::config::HttpRequestConfig,
|
||||
@@ -41,6 +48,7 @@ pub fn all_tools(
|
||||
config,
|
||||
security,
|
||||
Arc::new(NativeRuntime::new()),
|
||||
audit,
|
||||
memory,
|
||||
browser_config,
|
||||
http_config,
|
||||
@@ -56,6 +64,7 @@ pub fn all_tools_with_runtime(
|
||||
config: Arc<Config>,
|
||||
security: &Arc<SecurityPolicy>,
|
||||
runtime: Arc<dyn RuntimeAdapter>,
|
||||
audit: Arc<AuditLogger>,
|
||||
memory: Arc<dyn Memory>,
|
||||
browser_config: &crate::openhuman::config::BrowserConfig,
|
||||
http_config: &crate::openhuman::config::HttpRequestConfig,
|
||||
@@ -89,10 +98,15 @@ pub fn all_tools_with_runtime(
|
||||
Box::new(ShellTool::with_node_bootstrap(
|
||||
security.clone(),
|
||||
Arc::clone(&runtime),
|
||||
Arc::clone(&audit),
|
||||
Arc::clone(bootstrap),
|
||||
))
|
||||
} else {
|
||||
Box::new(ShellTool::new(security.clone(), Arc::clone(&runtime)))
|
||||
Box::new(ShellTool::new(
|
||||
security.clone(),
|
||||
Arc::clone(&runtime),
|
||||
Arc::clone(&audit),
|
||||
))
|
||||
};
|
||||
|
||||
let mut tools: Vec<Box<dyn Tool>> = vec![
|
||||
|
||||
@@ -1,6 +1,7 @@
|
||||
use super::*;
|
||||
use crate::openhuman::config::{BrowserConfig, Config, MemoryConfig};
|
||||
use crate::openhuman::credentials::{AuthService, APP_SESSION_PROVIDER, DEFAULT_AUTH_PROFILE_NAME};
|
||||
use crate::openhuman::security::AuditLogger;
|
||||
use tempfile::TempDir;
|
||||
|
||||
#[path = "../integrations/test_support.rs"]
|
||||
@@ -67,6 +68,7 @@ fn integration_tools_for_config(tmp: &TempDir, cfg: &Config) -> Vec<Box<dyn Tool
|
||||
all_tools(
|
||||
Arc::new(cfg.clone()),
|
||||
&security,
|
||||
AuditLogger::disabled(),
|
||||
mem,
|
||||
&browser,
|
||||
&http,
|
||||
@@ -118,6 +120,7 @@ fn all_tools_includes_spawn_subagent() {
|
||||
let tools = all_tools(
|
||||
Arc::new(Config::default()),
|
||||
&security,
|
||||
AuditLogger::disabled(),
|
||||
mem,
|
||||
&browser,
|
||||
&http,
|
||||
@@ -154,6 +157,7 @@ fn all_tools_includes_spawn_parallel_agents() {
|
||||
let tools = all_tools(
|
||||
Arc::new(Config::default()),
|
||||
&security,
|
||||
AuditLogger::disabled(),
|
||||
mem,
|
||||
&browser,
|
||||
&http,
|
||||
@@ -190,6 +194,7 @@ fn all_tools_always_registers_curl() {
|
||||
let tools = all_tools(
|
||||
Arc::new(cfg.clone()),
|
||||
&security,
|
||||
AuditLogger::disabled(),
|
||||
mem,
|
||||
&browser,
|
||||
&http,
|
||||
@@ -222,6 +227,7 @@ fn all_tools_registers_gitbooks_when_enabled() {
|
||||
let tools = all_tools(
|
||||
Arc::new(cfg.clone()),
|
||||
&security,
|
||||
AuditLogger::disabled(),
|
||||
mem,
|
||||
&browser,
|
||||
&http,
|
||||
@@ -288,6 +294,7 @@ fn all_tools_skips_gitbooks_when_disabled() {
|
||||
let tools = all_tools(
|
||||
Arc::new(cfg.clone()),
|
||||
&security,
|
||||
AuditLogger::disabled(),
|
||||
mem,
|
||||
&browser,
|
||||
&http,
|
||||
@@ -327,6 +334,7 @@ fn all_tools_includes_complete_onboarding() {
|
||||
let tools = all_tools(
|
||||
Arc::new(Config::default()),
|
||||
&security,
|
||||
AuditLogger::disabled(),
|
||||
mem,
|
||||
&browser,
|
||||
&http,
|
||||
@@ -363,6 +371,7 @@ fn all_tools_includes_current_time() {
|
||||
let tools = all_tools(
|
||||
Arc::new(Config::default()),
|
||||
&security,
|
||||
AuditLogger::disabled(),
|
||||
mem,
|
||||
&browser,
|
||||
&http,
|
||||
@@ -392,6 +401,7 @@ fn all_tools_default_registry_contains_expected_baseline_surface() {
|
||||
let tools = all_tools(
|
||||
Arc::new(Config::default()),
|
||||
&security,
|
||||
AuditLogger::disabled(),
|
||||
mem,
|
||||
&browser,
|
||||
&http,
|
||||
@@ -469,6 +479,7 @@ fn all_tools_default_registry_has_no_duplicate_tool_names() {
|
||||
let tools = all_tools(
|
||||
Arc::new(Config::default()),
|
||||
&security,
|
||||
AuditLogger::disabled(),
|
||||
mem,
|
||||
&browser,
|
||||
&http,
|
||||
@@ -508,6 +519,7 @@ fn all_tools_excludes_browser_when_disabled() {
|
||||
let tools = all_tools(
|
||||
Arc::new(Config::default()),
|
||||
&security,
|
||||
AuditLogger::disabled(),
|
||||
mem,
|
||||
&browser,
|
||||
&http,
|
||||
@@ -545,6 +557,7 @@ fn all_tools_includes_browser_when_enabled() {
|
||||
let tools = all_tools(
|
||||
Arc::new(Config::default()),
|
||||
&security,
|
||||
AuditLogger::disabled(),
|
||||
mem,
|
||||
&browser,
|
||||
&http,
|
||||
@@ -672,6 +685,7 @@ fn all_tools_includes_delegate_when_agents_configured() {
|
||||
let tools = all_tools(
|
||||
Arc::new(Config::default()),
|
||||
&security,
|
||||
AuditLogger::disabled(),
|
||||
mem,
|
||||
&browser,
|
||||
&http,
|
||||
@@ -701,6 +715,7 @@ fn all_tools_excludes_delegate_when_no_agents() {
|
||||
let tools = all_tools(
|
||||
Arc::new(Config::default()),
|
||||
&security,
|
||||
AuditLogger::disabled(),
|
||||
mem,
|
||||
&browser,
|
||||
&http,
|
||||
@@ -734,6 +749,7 @@ fn all_tools_registers_node_exec_when_node_enabled() {
|
||||
let tools = all_tools(
|
||||
Arc::new(Config::default()),
|
||||
&security,
|
||||
AuditLogger::disabled(),
|
||||
mem,
|
||||
&browser,
|
||||
&http,
|
||||
@@ -771,6 +787,7 @@ fn all_tools_excludes_node_exec_when_node_disabled() {
|
||||
let tools = all_tools(
|
||||
Arc::new(Config::default()),
|
||||
&security,
|
||||
AuditLogger::disabled(),
|
||||
mem,
|
||||
&browser,
|
||||
&http,
|
||||
@@ -808,6 +825,7 @@ fn all_tools_excludes_computer_control_when_disabled() {
|
||||
let tools = all_tools(
|
||||
Arc::new(Config::default()),
|
||||
&security,
|
||||
AuditLogger::disabled(),
|
||||
mem,
|
||||
&browser,
|
||||
&http,
|
||||
@@ -845,6 +863,7 @@ fn all_tools_includes_computer_control_when_enabled() {
|
||||
let tools = all_tools(
|
||||
Arc::new(Config::default()),
|
||||
&security,
|
||||
AuditLogger::disabled(),
|
||||
mem,
|
||||
&browser,
|
||||
&http,
|
||||
@@ -884,6 +903,7 @@ fn all_tools_registers_integration_families_when_enabled_and_signed_in() {
|
||||
let tools = all_tools(
|
||||
Arc::new(cfg.clone()),
|
||||
&security,
|
||||
AuditLogger::disabled(),
|
||||
mem,
|
||||
&browser,
|
||||
&http,
|
||||
@@ -951,6 +971,7 @@ fn all_tools_registers_optional_search_lsp_and_tool_stats_when_enabled() {
|
||||
let tools = all_tools(
|
||||
Arc::new(cfg.clone()),
|
||||
&security,
|
||||
AuditLogger::disabled(),
|
||||
mem,
|
||||
&browser,
|
||||
&http,
|
||||
|
||||
Reference in New Issue
Block a user