diff --git a/src/openhuman/config/schema/autonomy.rs b/src/openhuman/config/schema/autonomy.rs index 4b49fb3fc..b79054123 100644 --- a/src/openhuman/config/schema/autonomy.rs +++ b/src/openhuman/config/schema/autonomy.rs @@ -60,9 +60,18 @@ fn default_max_cost_per_day_cents() -> u32 { fn default_allowed_commands() -> Vec { vec![ + // Version control "git".into(), + // Package managers / build systems. `make` can run arbitrary recipes, + // but the shell policy still gates execution to this command allow-list + // and Supervised mode approval remains responsible for risky invocations. "npm".into(), + "pnpm".into(), + "yarn".into(), "cargo".into(), + "make".into(), + "cmake".into(), + // Directory / file inspection (read-only) "ls".into(), "cat".into(), "grep".into(), @@ -73,6 +82,26 @@ fn default_allowed_commands() -> Vec { "head".into(), "tail".into(), "date".into(), + "sort".into(), + "uniq".into(), + "diff".into(), + "which".into(), + "uname".into(), + "basename".into(), + "dirname".into(), + "tr".into(), + "cut".into(), + "realpath".into(), + "readlink".into(), + "stat".into(), + "file".into(), + // Filesystem mutations (medium-risk — require approval in Supervised mode) + "mkdir".into(), + "touch".into(), + "cp".into(), + "mv".into(), + "ln".into(), + // Windows read-only equivalents for ls/cat/grep/which "dir".into(), "type".into(), "where".into(), @@ -106,11 +135,15 @@ fn default_forbidden_paths() -> Vec { fn default_auto_approve() -> Vec { vec![ + // Read-only tools — always safe to skip the approval prompt "file_read".into(), "memory_search".into(), "memory_list".into(), "get_time".into(), "list_dir".into(), + // Workspace-scoped search tools — read-only, no side effects + "glob".into(), + "grep".into(), ] } diff --git a/src/openhuman/migrations/expand_autonomy_defaults.rs b/src/openhuman/migrations/expand_autonomy_defaults.rs new file mode 100644 index 000000000..57ff3655b --- /dev/null +++ b/src/openhuman/migrations/expand_autonomy_defaults.rs @@ -0,0 +1,351 @@ +//! Migration 3 → 4: expand autonomy defaults for existing users. +//! +//! PR #2500 expanded the code defaults for `autonomy.allowed_commands` and +//! `autonomy.auto_approve`, and changed `max_actions_per_hour` from 20 to +//! `u32::MAX` (effectively unlimited). Existing users had the old values +//! persisted in their `config.toml` at `schema_version = 3`, so they did +//! **not** pick up the new defaults automatically — their on-disk values +//! shadow the code defaults. +//! +//! ## What this migration does +//! +//! 1. **Merges new commands** into `config.autonomy.allowed_commands`. Only +//! commands not already present are added, so any user customisation +//! (e.g. additional entries, deliberate removals) is fully preserved. +//! 2. **Merges new auto-approve tools** into `config.autonomy.auto_approve` +//! with the same additive-only merge logic. +//! 3. **Bumps `max_actions_per_hour`** from 20 (the old hard-coded default) +//! to `u32::MAX` only when the persisted value is exactly 20. Users who +//! deliberately set a different limit are left untouched. +//! +//! ## Idempotency +//! +//! - Gated externally by [`Config::schema_version`] (`== 3`). Once the bump +//! to version 4 is persisted, future launches skip this migration entirely. +//! - Internally idempotent: merging already-present items is a no-op because +//! the merge logic guards every insert with a `contains` check. + +use crate::openhuman::config::Config; + +/// The old hard-coded default for `max_actions_per_hour`. When this exact +/// value is still persisted, we assume it was never deliberately customised +/// and bump it to the new unlimited sentinel. +const OLD_DEFAULT_MAX_ACTIONS_PER_HOUR: u32 = 20; + +/// Commands to merge into persisted `allowed_commands` during the v3→v4 bump. +/// +/// The target set mirrors the current default more closely than old v3 configs. +/// Some entries may already be present for customized users; the migration is +/// additive and skips duplicates. +const NEW_COMMANDS: &[&str] = &[ + "pnpm", "yarn", "make", "cmake", "sort", "uniq", "diff", "which", "uname", "basename", + "dirname", "tr", "cut", "realpath", "readlink", "stat", "file", "mkdir", "touch", "cp", "mv", + "ln", "date", "dir", "type", "where", "findstr", "more", +]; + +/// New auto-approve tools to merge into `auto_approve`. +/// +/// These were added to the code default in PR #2500 but are absent from any +/// `config.toml` written before that change. +const NEW_AUTO_APPROVE_TOOLS: &[&str] = &["glob", "grep"]; + +/// Counters returned by [`run`] for diagnostics. Logged at INFO once per +/// successful migration run. +#[derive(Debug, Default, Clone, Copy, PartialEq, Eq)] +pub struct MigrationStats { + /// Number of commands added to `allowed_commands`. + pub commands_added: usize, + /// Number of tools added to `auto_approve`. + pub tools_added: usize, + /// `true` when `max_actions_per_hour` was bumped from 20 to `u32::MAX`. + pub max_actions_bumped: bool, +} + +/// Run the autonomy defaults expansion migration on the given `Config`. +/// +/// Synchronous — pure config mutation, no I/O. The caller +/// (`migrations::run_pending`) persists the result via `Config::save()` and +/// bumps `schema_version`. +pub fn run(config: &mut Config) -> anyhow::Result { + let mut stats = MigrationStats::default(); + + log::debug!( + "[migrations][expand-autonomy-defaults] starting \ + allowed_commands.len={} auto_approve.len={} max_actions_per_hour={}", + config.autonomy.allowed_commands.len(), + config.autonomy.auto_approve.len(), + config.autonomy.max_actions_per_hour, + ); + + // Merge new commands (additive only — never remove user entries). + for &cmd in NEW_COMMANDS { + if !config.autonomy.allowed_commands.iter().any(|c| c == cmd) { + log::debug!( + "[migrations][expand-autonomy-defaults] adding command={:?} to allowed_commands", + cmd + ); + config.autonomy.allowed_commands.push(cmd.to_string()); + stats.commands_added += 1; + } + } + + // Merge new auto-approve tools (additive only). + for &tool in NEW_AUTO_APPROVE_TOOLS { + if !config.autonomy.auto_approve.iter().any(|t| t == tool) { + log::debug!( + "[migrations][expand-autonomy-defaults] adding tool={:?} to auto_approve", + tool + ); + config.autonomy.auto_approve.push(tool.to_string()); + stats.tools_added += 1; + } + } + + // Bump max_actions_per_hour only when it still holds the old default. + // Users who deliberately configured a different ceiling keep their value. + if config.autonomy.max_actions_per_hour == OLD_DEFAULT_MAX_ACTIONS_PER_HOUR { + log::info!( + "[migrations][expand-autonomy-defaults] bumping max_actions_per_hour \ + {} -> u32::MAX (old hard-coded default, PR #2500 changed code default)", + OLD_DEFAULT_MAX_ACTIONS_PER_HOUR + ); + config.autonomy.max_actions_per_hour = u32::MAX; + stats.max_actions_bumped = true; + } else { + log::debug!( + "[migrations][expand-autonomy-defaults] max_actions_per_hour={} — \ + not the old default, leaving unchanged", + config.autonomy.max_actions_per_hour + ); + } + + log::info!( + "[migrations][expand-autonomy-defaults] done \ + commands_added={} tools_added={} max_actions_bumped={}", + stats.commands_added, + stats.tools_added, + stats.max_actions_bumped, + ); + + Ok(stats) +} + +#[cfg(test)] +mod tests { + use super::*; + use crate::openhuman::config::Config; + + /// A config representing the old v3 defaults that a user would have + /// persisted before PR #2500 expanded the code defaults. + fn old_v3_config() -> Config { + let mut config = Config::default(); + // Reset to the narrow v3 allowed_commands. + config.autonomy.allowed_commands = vec![ + "git".into(), + "npm".into(), + "cargo".into(), + "ls".into(), + "cat".into(), + "grep".into(), + "find".into(), + "echo".into(), + "pwd".into(), + "wc".into(), + "head".into(), + "tail".into(), + ]; + // Reset to the narrow v3 auto_approve list. + config.autonomy.auto_approve = vec![ + "file_read".into(), + "memory_search".into(), + "memory_list".into(), + "get_time".into(), + "list_dir".into(), + ]; + // Reset to the old numeric default. + config.autonomy.max_actions_per_hour = OLD_DEFAULT_MAX_ACTIONS_PER_HOUR; + config + } + + #[test] + fn adds_new_commands_to_narrow_list() { + let mut config = old_v3_config(); + let stats = run(&mut config).expect("migration should succeed"); + + // Every new command must now be present. + for cmd in NEW_COMMANDS { + assert!( + config.autonomy.allowed_commands.iter().any(|c| c == cmd), + "expected {:?} in allowed_commands after migration", + cmd + ); + } + // Existing commands must be preserved. + for cmd in &["git", "npm", "cargo", "ls", "cat", "grep"] { + assert!( + config.autonomy.allowed_commands.iter().any(|c| c == *cmd), + "expected existing command {:?} preserved", + cmd + ); + } + assert!( + stats.commands_added > 0, + "expected at least one command to be added" + ); + } + + #[test] + fn adds_new_auto_approve_tools_to_narrow_list() { + let mut config = old_v3_config(); + let stats = run(&mut config).expect("migration should succeed"); + + for tool in NEW_AUTO_APPROVE_TOOLS { + assert!( + config.autonomy.auto_approve.iter().any(|t| t == tool), + "expected {:?} in auto_approve after migration", + tool + ); + } + // Write tools must keep Supervised mode's ask-before-edit contract. + for tool in &["file_write", "edit_file"] { + assert!( + !config.autonomy.auto_approve.iter().any(|t| t == *tool), + "expected {:?} to require approval after migration", + tool + ); + } + // Existing tools must be preserved. + for tool in &["file_read", "memory_search", "memory_list"] { + assert!( + config.autonomy.auto_approve.iter().any(|t| t == *tool), + "expected existing tool {:?} preserved", + tool + ); + } + assert!( + stats.tools_added > 0, + "expected at least one tool to be added" + ); + } + + #[test] + fn bumps_max_actions_when_old_default() { + let mut config = old_v3_config(); + assert_eq!( + config.autonomy.max_actions_per_hour, + OLD_DEFAULT_MAX_ACTIONS_PER_HOUR + ); + + let stats = run(&mut config).expect("migration should succeed"); + + assert!(stats.max_actions_bumped); + assert_eq!(config.autonomy.max_actions_per_hour, u32::MAX); + } + + #[test] + fn does_not_bump_max_actions_when_user_customised() { + let mut config = old_v3_config(); + config.autonomy.max_actions_per_hour = 100; + + let stats = run(&mut config).expect("migration should succeed"); + + assert!(!stats.max_actions_bumped); + assert_eq!( + config.autonomy.max_actions_per_hour, 100, + "user-customised ceiling must be preserved" + ); + } + + #[test] + fn idempotent_on_already_expanded_config() { + let mut config = old_v3_config(); + + // First run. + let stats1 = run(&mut config).expect("first run should succeed"); + assert!(stats1.commands_added > 0 || stats1.tools_added > 0 || stats1.max_actions_bumped); + + // Second run — nothing changes. + let snapshot_commands = config.autonomy.allowed_commands.clone(); + let snapshot_tools = config.autonomy.auto_approve.clone(); + let snapshot_max = config.autonomy.max_actions_per_hour; + + let stats2 = run(&mut config).expect("second run should succeed"); + + assert_eq!(stats2.commands_added, 0, "no commands added on second run"); + assert_eq!(stats2.tools_added, 0, "no tools added on second run"); + assert!( + !stats2.max_actions_bumped, + "max_actions not bumped on second run" + ); + assert_eq!(config.autonomy.allowed_commands, snapshot_commands); + assert_eq!(config.autonomy.auto_approve, snapshot_tools); + assert_eq!(config.autonomy.max_actions_per_hour, snapshot_max); + } + + #[test] + fn preserves_user_custom_commands_not_in_new_set() { + let mut config = old_v3_config(); + config + .autonomy + .allowed_commands + .push("my_custom_tool".to_string()); + + run(&mut config).expect("migration should succeed"); + + assert!( + config + .autonomy + .allowed_commands + .iter() + .any(|c| c == "my_custom_tool"), + "user's custom command must be preserved" + ); + } + + #[test] + fn no_duplicate_commands_when_some_already_present() { + let mut config = old_v3_config(); + // Pre-seed a subset of new commands so we can check no duplicates appear. + config.autonomy.allowed_commands.push("pnpm".to_string()); + config.autonomy.allowed_commands.push("yarn".to_string()); + + run(&mut config).expect("migration should succeed"); + + let pnpm_count = config + .autonomy + .allowed_commands + .iter() + .filter(|c| *c == "pnpm") + .count(); + let yarn_count = config + .autonomy + .allowed_commands + .iter() + .filter(|c| *c == "yarn") + .count(); + assert_eq!(pnpm_count, 1, "pnpm must appear exactly once"); + assert_eq!(yarn_count, 1, "yarn must appear exactly once"); + } + + #[test] + fn no_op_on_fresh_install_defaults() { + // A fresh install already has the expanded defaults; the migration + // should be a complete no-op (all guards fire early). + let mut config = Config::default(); + + let stats = run(&mut config).expect("migration should succeed"); + + assert_eq!( + stats.commands_added, 0, + "fresh install: no commands should be added" + ); + assert_eq!( + stats.tools_added, 0, + "fresh install: no tools should be added" + ); + assert!( + !stats.max_actions_bumped, + "fresh install: max_actions already u32::MAX, must not bump again" + ); + } +} diff --git a/src/openhuman/migrations/mod.rs b/src/openhuman/migrations/mod.rs index 0e0e9cf41..38b42e688 100644 --- a/src/openhuman/migrations/mod.rs +++ b/src/openhuman/migrations/mod.rs @@ -23,12 +23,14 @@ use crate::openhuman::config::Config; +mod expand_autonomy_defaults; mod phase_out_profile_md; +mod remove_write_auto_approve; mod retire_chat_v1_model; mod unify_ai_provider_settings; /// Current target schema version. Bumped alongside every new migration. -pub const CURRENT_SCHEMA_VERSION: u32 = 3; +pub const CURRENT_SCHEMA_VERSION: u32 = 5; /// Run any migrations whose `schema_version` gate hasn't yet been /// crossed for this workspace. @@ -176,6 +178,78 @@ pub async fn run_pending(config: &mut Config) { } } } + + // 3 -> 4: expand autonomy defaults for existing users. PR #2500 enlarged + // `autonomy.allowed_commands`, `autonomy.auto_approve`, and changed + // `max_actions_per_hour` from 20 to u32::MAX. Existing workspaces kept + // the old persisted values. This migration merges the new commands/tools + // (additive only) and bumps `max_actions_per_hour` when it still holds + // the old hard-coded default of 20. + // Guard on `== 3` so a failed 2→3 migration doesn't skip this step. + if config.schema_version == 3 { + match expand_autonomy_defaults::run(config) { + Ok(stats) => { + let previous_version = config.schema_version; + config.schema_version = 4; + if let Err(err) = config.save().await { + config.schema_version = previous_version; + log::warn!( + "[migrations] expand_autonomy_defaults ran but config.save failed: \ + {err:#} — rolled in-memory schema_version back to {previous_version}, \ + will retry on next launch" + ); + return; + } + log::info!( + "[migrations] schema_version bumped to 4 (expand_autonomy_defaults \ + commands_added={} tools_added={} max_actions_bumped={})", + stats.commands_added, + stats.tools_added, + stats.max_actions_bumped, + ); + } + Err(err) => { + log::warn!( + "[migrations] expand_autonomy_defaults failed: {err:#} — \ + will retry on next launch" + ); + } + } + } + + // 4 -> 5: remove write tools from `autonomy.auto_approve`. A short-lived + // v4 default/migration let Supervised mode skip prompts for file edits. + // Keep those tools available, but remove the prompt bypass so normal + // approval gating applies again. Guard on `== 4` so earlier failed steps + // do not get skipped. + if config.schema_version == 4 { + match remove_write_auto_approve::run(config) { + Ok(stats) => { + let previous_version = config.schema_version; + config.schema_version = 5; + if let Err(err) = config.save().await { + config.schema_version = previous_version; + log::warn!( + "[migrations] remove_write_auto_approve ran but config.save failed: \ + {err:#} — rolled in-memory schema_version back to {previous_version}, \ + will retry on next launch" + ); + return; + } + log::info!( + "[migrations] schema_version bumped to 5 (remove_write_auto_approve \ + auto_approve_removed={})", + stats.auto_approve_removed, + ); + } + Err(err) => { + log::warn!( + "[migrations] remove_write_auto_approve failed: {err:#} — \ + will retry on next launch" + ); + } + } + } } #[cfg(test)] diff --git a/src/openhuman/migrations/mod_tests.rs b/src/openhuman/migrations/mod_tests.rs index 57d62fd1d..58e26c7fd 100644 --- a/src/openhuman/migrations/mod_tests.rs +++ b/src/openhuman/migrations/mod_tests.rs @@ -7,6 +7,34 @@ use std::fs; use std::path::Path; use tempfile::TempDir; +/// Simulate a v3 user config: narrow allowed_commands, narrow auto_approve, +/// and the old hard-coded `max_actions_per_hour = 20`. +fn simulate_v3_autonomy(config: &mut Config) { + config.schema_version = 3; + config.autonomy.allowed_commands = vec![ + "git".into(), + "npm".into(), + "cargo".into(), + "ls".into(), + "cat".into(), + "grep".into(), + "find".into(), + "echo".into(), + "pwd".into(), + "wc".into(), + "head".into(), + "tail".into(), + ]; + config.autonomy.auto_approve = vec![ + "file_read".into(), + "memory_search".into(), + "memory_list".into(), + "get_time".into(), + "list_dir".into(), + ]; + config.autonomy.max_actions_per_hour = 20; +} + fn tainted_prompt() -> String { "## Identity\n\nYou are an assistant.\n\n\ ### PROFILE.md\n\n\ @@ -74,7 +102,7 @@ async fn run_pending_runs_phase_out_when_version_zero() { assert_eq!(config.schema_version, 0); run_pending(&mut config).await; - assert_eq!(config.schema_version, 3); + assert_eq!(config.schema_version, CURRENT_SCHEMA_VERSION); let session = read_transcript(&path).unwrap(); assert!( !session.messages[0].content.contains("### PROFILE.md"), @@ -84,8 +112,8 @@ async fn run_pending_runs_phase_out_when_version_zero() { let on_disk = std::fs::read_to_string(&config.config_path).unwrap(); assert!( - on_disk.contains("schema_version = 3"), - "saved config.toml must record schema_version=3, got:\n{on_disk}" + on_disk.contains("schema_version = 5"), + "saved config.toml must record schema_version=5, got:\n{on_disk}" ); } @@ -98,9 +126,9 @@ async fn run_pending_bumps_version_on_fresh_install() { let mut config = config_in(&tmp); run_pending(&mut config).await; - assert_eq!(config.schema_version, 3); + assert_eq!(config.schema_version, CURRENT_SCHEMA_VERSION); let on_disk = std::fs::read_to_string(&config.config_path).unwrap(); - assert!(on_disk.contains("schema_version = 3")); + assert!(on_disk.contains("schema_version = 5")); } #[tokio::test] @@ -132,7 +160,7 @@ async fn run_pending_is_a_no_op_on_second_invocation() { let mut config = config_in(&tmp); run_pending(&mut config).await; - assert_eq!(config.schema_version, 3); + assert_eq!(config.schema_version, CURRENT_SCHEMA_VERSION); // Mutate the config file timestamp marker by reading + comparing // before vs after the second invocation. @@ -141,9 +169,144 @@ async fn run_pending_is_a_no_op_on_second_invocation() { run_pending(&mut config).await; let after = fs::metadata(&config.config_path).unwrap().modified().ok(); - assert_eq!(config.schema_version, 3); + assert_eq!(config.schema_version, CURRENT_SCHEMA_VERSION); assert_eq!( before, after, "config.toml must not be re-saved on second run" ); } + +// ── v3 → v4: expand_autonomy_defaults integration test ────────────────────── + +/// Verify that `run_pending` applies the v3→v4 autonomy expansion when +/// starting from a simulated old-default config at schema_version=3. +#[tokio::test] +async fn run_pending_expands_autonomy_defaults_from_v3() { + let tmp = TempDir::new().unwrap(); + fs::create_dir_all(tmp.path().join("workspace")).unwrap(); + + let mut config = config_in(&tmp); + simulate_v3_autonomy(&mut config); + + assert_eq!(config.schema_version, 3); + assert_eq!(config.autonomy.max_actions_per_hour, 20); + assert!(!config.autonomy.allowed_commands.iter().any(|c| c == "pnpm")); + assert!(!config.autonomy.auto_approve.iter().any(|t| t == "glob")); + + run_pending(&mut config).await; + + assert_eq!( + config.schema_version, CURRENT_SCHEMA_VERSION, + "schema_version must be bumped to current" + ); + + // New commands must be present after the migration. + for cmd in &["pnpm", "yarn", "make", "sort", "diff", "mkdir", "cp"] { + assert!( + config.autonomy.allowed_commands.iter().any(|c| c == *cmd), + "expected {:?} in allowed_commands after v3→v4 migration", + cmd + ); + } + // Original commands must be preserved. + for cmd in &["git", "npm", "cargo", "ls", "cat"] { + assert!( + config.autonomy.allowed_commands.iter().any(|c| c == *cmd), + "expected original command {:?} preserved", + cmd + ); + } + + // New read-only auto-approve tools must be present. + for tool in &["glob", "grep"] { + assert!( + config.autonomy.auto_approve.iter().any(|t| t == *tool), + "expected {:?} in auto_approve after v3→v4 migration", + tool + ); + } + // Write tools must keep Supervised mode's ask-before-edit contract. + for tool in &["file_write", "edit_file"] { + assert!( + !config.autonomy.auto_approve.iter().any(|t| t == *tool), + "expected {:?} to require approval after v3→v4 migration", + tool + ); + } + // Original auto-approve tools must be preserved. + for tool in &["file_read", "memory_search", "memory_list"] { + assert!( + config.autonomy.auto_approve.iter().any(|t| t == *tool), + "expected original tool {:?} preserved", + tool + ); + } + + // max_actions_per_hour must be bumped from the old default of 20. + assert_eq!( + config.autonomy.max_actions_per_hour, + u32::MAX, + "max_actions_per_hour must be bumped to u32::MAX" + ); + + // On-disk config must reflect the new schema_version. + let on_disk = fs::read_to_string(&config.config_path).unwrap(); + assert!( + on_disk.contains("schema_version = 5"), + "saved config.toml must record schema_version=5, got:\n{on_disk}" + ); +} + +// ── v4 → v5: remove_write_auto_approve integration test ───────────────────── + +/// Verify that workspaces already migrated to schema_version=4 have write tools +/// removed from `auto_approve` so Supervised mode prompts before file edits. +#[tokio::test] +async fn run_pending_v4_to_v5_removes_write_tools_from_auto_approve() { + let tmp = TempDir::new().unwrap(); + fs::create_dir_all(tmp.path().join("workspace")).unwrap(); + + let mut config = config_in(&tmp); + config.schema_version = 4; + config.autonomy.auto_approve = vec![ + "file_read".into(), + "file_write".into(), + "edit_file".into(), + "glob".into(), + ]; + + run_pending(&mut config).await; + + assert_eq!(config.schema_version, CURRENT_SCHEMA_VERSION); + assert_eq!( + config.autonomy.auto_approve, + vec!["file_read".to_string(), "glob".to_string()] + ); + + let on_disk = fs::read_to_string(&config.config_path).unwrap(); + assert!( + on_disk.contains("schema_version = 5"), + "saved config.toml must record schema_version=5, got:\n{on_disk}" + ); +} + +/// Verify that a user at v3 with a deliberately customised +/// `max_actions_per_hour` does NOT have it reset by the migration. +#[tokio::test] +async fn run_pending_v3_to_v4_preserves_custom_max_actions() { + let tmp = TempDir::new().unwrap(); + fs::create_dir_all(tmp.path().join("workspace")).unwrap(); + + let mut config = config_in(&tmp); + simulate_v3_autonomy(&mut config); + // User has deliberately configured a specific ceiling. + config.autonomy.max_actions_per_hour = 50; + + run_pending(&mut config).await; + + assert_eq!(config.schema_version, CURRENT_SCHEMA_VERSION); + assert_eq!( + config.autonomy.max_actions_per_hour, 50, + "user-customised max_actions_per_hour must not be overwritten" + ); +} diff --git a/src/openhuman/migrations/remove_write_auto_approve.rs b/src/openhuman/migrations/remove_write_auto_approve.rs new file mode 100644 index 000000000..5518dde64 --- /dev/null +++ b/src/openhuman/migrations/remove_write_auto_approve.rs @@ -0,0 +1,70 @@ +//! Migration 4 -> 5: remove write tools from `autonomy.auto_approve`. +//! +//! A short-lived v4 default/migration added `file_write` and `edit_file` to +//! `auto_approve`, which made Supervised mode skip its ask-before-edit prompt. +//! The v3 -> v4 migration no longer adds them, but workspaces that already +//! persisted schema_version 4 still need their config scrubbed. + +use crate::openhuman::config::Config; + +const WRITE_AUTO_APPROVE_TOOLS: &[&str] = &["file_write", "edit_file"]; + +#[derive(Debug, Default, Clone, Copy, PartialEq, Eq)] +pub struct MigrationStats { + pub auto_approve_removed: usize, +} + +pub fn run(config: &mut Config) -> anyhow::Result { + let before_len = config.autonomy.auto_approve.len(); + config.autonomy.auto_approve.retain(|tool| { + !WRITE_AUTO_APPROVE_TOOLS + .iter() + .any(|blocked| tool == blocked) + }); + + let stats = MigrationStats { + auto_approve_removed: before_len.saturating_sub(config.autonomy.auto_approve.len()), + }; + + log::info!( + "[migrations][remove-write-auto-approve] done auto_approve_removed={}", + stats.auto_approve_removed + ); + + Ok(stats) +} + +#[cfg(test)] +mod tests { + use super::*; + use crate::openhuman::config::Config; + + #[test] + fn removes_write_tools_from_auto_approve() { + let mut config = Config::default(); + config.autonomy.auto_approve = vec![ + "file_read".into(), + "file_write".into(), + "edit_file".into(), + "glob".into(), + ]; + + let stats = run(&mut config).expect("migration should succeed"); + + assert_eq!(stats.auto_approve_removed, 2); + assert_eq!( + config.autonomy.auto_approve, + vec!["file_read".to_string(), "glob".to_string()] + ); + } + + #[test] + fn removes_write_tools_even_when_mixed() { + let mut config = Config::default(); + config.autonomy.auto_approve = vec!["file_write".into()]; + + run(&mut config).expect("migration should succeed"); + + assert!(config.autonomy.auto_approve.is_empty()); + } +} diff --git a/src/openhuman/security/policy.rs b/src/openhuman/security/policy.rs index d9443d2cd..c3dfdce18 100644 --- a/src/openhuman/security/policy.rs +++ b/src/openhuman/security/policy.rs @@ -197,9 +197,16 @@ impl Default for SecurityPolicy { workspace_dir: PathBuf::from("."), workspace_only: true, allowed_commands: vec![ + // Version control "git".into(), + // Package managers / build systems "npm".into(), + "pnpm".into(), + "yarn".into(), "cargo".into(), + "make".into(), + "cmake".into(), + // Directory / file inspection (read-only, low-risk) "ls".into(), "cat".into(), "grep".into(), @@ -210,6 +217,25 @@ impl Default for SecurityPolicy { "head".into(), "tail".into(), "date".into(), + "sort".into(), + "uniq".into(), + "diff".into(), + "which".into(), + "uname".into(), + "basename".into(), + "dirname".into(), + "tr".into(), + "cut".into(), + "realpath".into(), + "readlink".into(), + "stat".into(), + "file".into(), + // Filesystem mutations (medium-risk — require approval in Supervised mode) + "mkdir".into(), + "touch".into(), + "cp".into(), + "mv".into(), + "ln".into(), // Windows read-only equivalents for the same basic // inspection workflows as ls/cat/grep/which. "dir".into(), @@ -240,7 +266,10 @@ impl Default for SecurityPolicy { "~/.aws".into(), "~/.config".into(), ], - max_actions_per_hour: 20, + // Effectively unlimited — matches AutonomyConfig::default_max_actions_per_hour(). + // The rate-limiter check is `count <= max`, so u32::MAX is functionally + // infinite without requiring an Option sentinel on the field type. + max_actions_per_hour: u32::MAX, max_cost_per_day_cents: 500, require_approval_for_medium_risk: true, block_high_risk_commands: true, diff --git a/src/openhuman/security/policy_tests.rs b/src/openhuman/security/policy_tests.rs index 0a7bb9393..d8e0c4ae5 100644 --- a/src/openhuman/security/policy_tests.rs +++ b/src/openhuman/security/policy_tests.rs @@ -97,6 +97,64 @@ fn action_budget_error_mentions_limit_and_settings() { // -- is_command_allowed ------------------------------------------- +#[test] +fn default_policy_allowed_commands_expanded() { + // Issue #2486: verify all newly added safe commands are present in the + // default allowlist so agents can use them without manual configuration. + let p = default_policy(); + + // Build tools + for cmd in ["make", "cmake", "pnpm", "yarn"] { + assert!( + p.is_command_allowed(cmd), + "default policy should allow build tool: {cmd}" + ); + } + + // Read-only inspection tools (low-risk) + for cmd in [ + "sort file.txt", + "uniq file.txt", + "diff a.txt b.txt", + "which git", + "uname -a", + "basename /foo/bar.rs", + "dirname /foo/bar.rs", + "tr 'a' 'b'", + "cut -d: -f1 /dev/stdin", + "realpath .", + "readlink file", + "stat file.txt", + "file README.md", + ] { + assert!( + p.is_command_allowed(cmd), + "default policy should allow read-only tool: {cmd}" + ); + } + + // Filesystem mutation tools (medium-risk — allowed on allowlist, + // but require approval in Supervised mode) + for cmd in [ + "mkdir src/new", + "touch Makefile", + "cp src/a.rs src/b.rs", + "mv old.txt new.txt", + "ln -s src/a.rs link.rs", + ] { + assert!( + p.is_command_allowed(cmd), + "default policy should allow medium-risk tool: {cmd}" + ); + // Confirm they are actually medium-risk so the approval gate applies + assert_eq!( + p.command_risk_level(cmd), + CommandRiskLevel::Medium, + "{cmd} should be classified as medium-risk" + ); + } +} + #[test] fn allowed_commands_basic() { let p = default_policy(); @@ -885,6 +943,45 @@ fn write_to_not_yet_existing_path_in_workspace_still_allowed() { assert!(p.is_path_string_allowed("not-yet-existing/subdir/file.txt")); } +// -- auto_approve defaults ---------------------------------------- + +#[test] +fn config_default_auto_approve_includes_expanded_tools() { + // Issue #2486: verify read-only tools are auto-approved by default, + // and write tools are NOT (Supervised mode must prompt for edits). + let cfg = crate::openhuman::config::AutonomyConfig::default(); + + // Pre-existing auto-approved tools must still be present + for tool in [ + "file_read", + "memory_search", + "memory_list", + "get_time", + "list_dir", + ] { + assert!( + cfg.auto_approve.iter().any(|t| t == tool), + "default auto_approve must still include pre-existing tool: {tool}" + ); + } + + // Newly added read-only workspace-scoped tools + for tool in ["glob", "grep"] { + assert!( + cfg.auto_approve.iter().any(|t| t == tool), + "default auto_approve must include newly added tool: {tool}" + ); + } + + // Write tools must NOT be auto-approved (v4→v5 migration strips these) + for tool in ["file_write", "edit_file"] { + assert!( + !cfg.auto_approve.iter().any(|t| t == tool), + "write tool {tool} must NOT be auto-approved by default" + ); + } +} + // -- from_config -------------------------------------------------- #[test] @@ -2137,7 +2234,7 @@ fn full_access_bypasses_command_allowlist() { #[test] fn supervised_still_enforces_command_allowlist() { let p = default_policy(); // Supervised - assert!(!p.is_command_allowed("mkdir -p foo/bar")); // not allow-listed + assert!(p.is_command_allowed("mkdir -p foo/bar")); // allow-listed (expanded in #2486) assert!(!p.is_command_allowed("ls 2>/dev/null")); // redirect blocked assert!(p.is_command_allowed("ls -la")); // allow-listed, no redirect }