diff --git a/CHANGELOG.md b/CHANGELOG.md index 8683422..26c572d 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -8,6 +8,22 @@ once a `1.0.0` line is cut. ## [Unreleased] +### Added + +- `policy.shell.allow = ["*"]` allows every program, mirroring the wildcard + `policy.network.allowed_hosts` already accepts. Deployments that want + arbitrary shell execution no longer have to allowlist `bash` and route every + command through `bash -c`. `policy.shell.mode` remains the only shell + restriction under a wildcard, and an empty list still denies everything. +- `policy.allowed_read_roots`, `policy.sensitive_path_patterns`, and + `policy.shell.mode` — the last `PolicySettings` fields the builder pinned to + their defaults — are now configurable. Unset read roots keep the built-in + working-directory and temporary-directory roots; an explicit list replaces + them, and configured `allowed_write_roots` stay readable either way. Unset + sensitive patterns keep the built-in globs; an explicit list replaces them + and an empty list disables the check. `mode` is `"strict"` (default) or + `"relaxed"`. + ## [0.5.0] - 2026-07-27 The OpenAI Responses adapter no longer fails a turn when the upstream sends a diff --git a/README.md b/README.md index 29be61a..277dbef 100644 --- a/README.md +++ b/README.md @@ -518,6 +518,12 @@ prune_signal_threshold = "low" [policy] allowed_write_roots = ["./", "/tmp/halter"] +# Optional. Empty keeps the built-in read roots (working directory + temp +# directory); an explicit list replaces them. Write roots stay readable either way. +# allowed_read_roots = ["/srv/checkouts"] +# Optional. Absent keeps the built-in globs denied to every file tool; an +# explicit list replaces them and `[]` disables the check entirely. +# sensitive_path_patterns = ["**/.ssh/**", "**/.aws/**", "**/.secrets", "/etc/shadow", "/etc/shadow.*"] max_read_bytes = 1048576 max_subagent_depth = 3 max_concurrent_subagents = 8 @@ -528,7 +534,10 @@ allowed_hosts = [] [policy.shell] enabled = true +# `["*"]` allows every program; `[]` denies every external command. allow = ["git", "cargo", "rg", "ls", "find", "true", "cd", "python", "pwd", "cwd", "echo"] +# "strict" (default) also rejects function definitions and eval/exec/source/. +mode = "strict" timeout_secs = 30 [sessions] diff --git a/crates/halter-config/README.md b/crates/halter-config/README.md index 15a12b4..4bb5571 100644 --- a/crates/halter-config/README.md +++ b/crates/halter-config/README.md @@ -435,13 +435,16 @@ Example: ```toml [policy] allowed_write_roots = ["./", "/tmp/halter"] +allowed_read_roots = [] # empty keeps the built-in read roots max_read_bytes = 1048576 max_subagent_depth = 3 max_concurrent_subagents = 8 +# sensitive_path_patterns = ["**/.ssh/**"] # absent keeps the built-in globs [policy.shell] enabled = true allow = ["git", "cargo", "rg", "ls", "find", "true", "cd"] +mode = "strict" timeout_secs = 30 [policy.network] @@ -452,13 +455,21 @@ allowed_hosts = [] Defaults: - `allowed_write_roots = [".", "/tmp/halter"]` -- runtime read roots start from `[ ".", $TMPDIR | "/tmp" ]` and also include - configured `allowed_write_roots` +- `allowed_read_roots = []` — runtime read roots then start from + `[ ".", $TMPDIR | "/tmp" ]`. A non-empty list replaces those built-ins. + Either way the runtime also reads under every configured + `allowed_write_roots` entry +- `sensitive_path_patterns` unset — the runtime denies the built-in globs + (`**/.ssh/**`, `**/.aws/**`, `**/.secrets`, `/etc/shadow`, `/etc/shadow.*`) + to every file tool. An explicit list replaces them; `[]` disables the check - `max_read_bytes = 1_048_576` - `max_subagent_depth = 3` - `max_concurrent_subagents = 8` - shell enabled by default -- shell allowlist defaults to `git`, `cargo`, `rg`, `ls`, `find`, `true`, `cd` +- shell allowlist defaults to `git`, `cargo`, `rg`, `ls`, `find`, `true`, `cd`. + `["*"]` allows every program; `[]` denies every external command +- `policy.shell.mode = "strict"` — also rejects function definitions and + `eval`/`exec`/`source`/`.`. `"relaxed"` applies only the allowlist - network disabled by default Validation rules include: diff --git a/crates/halter-config/src/lib.rs b/crates/halter-config/src/lib.rs index f0f18b1..52d451d 100644 --- a/crates/halter-config/src/lib.rs +++ b/crates/halter-config/src/lib.rs @@ -31,8 +31,8 @@ pub use schema::{ RequestRetryConfig, RequestRetryOverrideConfig, ResilienceConfig, ResilienceOverrideConfig, ResilienceTimeoutsConfig, ResilienceTimeoutsOverrideConfig, ResolvedProviderAuth, ResolvedProviderConfig, ResourcesConfig, RuntimeConfig, SMALL_MODEL_ID, SUBAGENT_MODEL_ID, - SearchRoots, SessionBackend, SessionsConfig, ShellPolicyConfig, SystemPromptPreset, - ToolsConfig, resolve_provider_runtime_config, + SearchRoots, SessionBackend, SessionsConfig, ShellModeConfig, ShellPolicyConfig, + SystemPromptPreset, ToolsConfig, resolve_provider_runtime_config, }; #[cfg(feature = "remote-plugins")] diff --git a/crates/halter-config/src/loader.rs b/crates/halter-config/src/loader.rs index f5e21fb..1306473 100644 --- a/crates/halter-config/src/loader.rs +++ b/crates/halter-config/src/loader.rs @@ -156,7 +156,10 @@ max_concurrent_subagents = 8 [policy.shell] enabled = true +# `["*"]` allows every program; `[]` denies every external command. allow = ["git", "cargo", "rg", "ls", "find", "true", "cd"] +# "strict" also rejects function definitions and eval/exec/source/. +mode = "strict" timeout_secs = 30 [policy.network] diff --git a/crates/halter-config/src/schema.rs b/crates/halter-config/src/schema.rs index fc40f6f..5f9662b 100644 --- a/crates/halter-config/src/schema.rs +++ b/crates/halter-config/src/schema.rs @@ -1232,6 +1232,18 @@ pub struct ToolsConfig { pub struct PolicyConfig { #[serde(default = "default_write_roots")] pub allowed_write_roots: Vec, + /// Roots the read tools may resolve under. Empty (the default) keeps the + /// built-in roots — the working directory and the temporary directory — + /// and an explicit list replaces them. Either way the runtime also reads + /// under every entry in `allowed_write_roots`. + #[serde(default)] + pub allowed_read_roots: Vec, + /// Globs denied to both read and write tools regardless of the configured + /// roots. `None` keeps the built-in patterns (`**/.ssh/**`, `**/.aws/**`, + /// `**/.secrets`, `/etc/shadow*`); an explicit list replaces them, and an + /// empty list disables the check for deliberately unguarded deployments. + #[serde(default)] + pub sensitive_path_patterns: Option>, #[serde(default = "default_max_read_bytes")] pub max_read_bytes: usize, #[serde(default = "default_max_subagent_depth")] @@ -1248,6 +1260,8 @@ impl Default for PolicyConfig { fn default() -> Self { Self { allowed_write_roots: default_write_roots(), + allowed_read_roots: Vec::new(), + sensitive_path_patterns: None, max_read_bytes: default_max_read_bytes(), max_subagent_depth: default_max_subagent_depth(), max_concurrent_subagents: default_max_concurrent_subagents(), @@ -1279,8 +1293,12 @@ const fn default_max_concurrent_subagents() -> usize { pub struct ShellPolicyConfig { #[serde(default = "default_shell_enabled")] pub enabled: bool, + /// Program names the shell tool may run. A single `*` entry allows every + /// program; an empty list denies every external command. #[serde(default = "default_shell_allowlist")] pub allow: Vec, + #[serde(default)] + pub mode: ShellModeConfig, #[serde(default = "default_shell_timeout_secs")] pub timeout_secs: u64, } @@ -1290,11 +1308,25 @@ impl Default for ShellPolicyConfig { Self { enabled: default_shell_enabled(), allow: default_shell_allowlist(), + mode: ShellModeConfig::default(), timeout_secs: default_shell_timeout_secs(), } } } +#[derive(Debug, Clone, Copy, Serialize, Deserialize, JsonSchema, PartialEq, Eq, Default)] +#[serde(rename_all = "snake_case")] +/// How strictly the shell tool parses commands before the allowlist applies. +pub enum ShellModeConfig { + /// Reject function definitions and `eval`/`exec`/`source`/`.` commands, + /// then apply the allowlist. + #[default] + Strict, + /// Apply only the allowlist. Documented as *not* a complete isolation + /// boundary; for workflows that need function definitions or `eval`. + Relaxed, +} + const fn default_shell_enabled() -> bool { true } diff --git a/crates/halter-tools/README.md b/crates/halter-tools/README.md index c7279c3..1cf6665 100644 --- a/crates/halter-tools/README.md +++ b/crates/halter-tools/README.md @@ -142,11 +142,14 @@ Defaults: - `allowed_write_roots = [".", "/tmp/halter"]` - `allowed_read_roots = [".", $TMPDIR | "/tmp"]` -- `sensitive_path_patterns = ["**/.ssh/**", "**/.aws/**", "**/.env", "**/.env.*", "/etc/shadow", "/etc/shadow.*"]` +- `sensitive_path_patterns = ["**/.ssh/**", "**/.aws/**", "**/.secrets", "/etc/shadow", "/etc/shadow.*"]` - `max_read_bytes = 1_048_576` - shell enabled = `true` - `shell_mode = Strict` (rejects `eval`, `exec`, `source`, `.`, and function definitions at the AST level) -- shell allowlist = `git`, `cargo`, `rg`, `ls`, `find`, `true`, `cd` +- shell allowlist = `git`, `cargo`, `rg`, `ls`, `find`, `true`, `cd`. A single + `*` entry short-circuits to allow — like `allowed_hosts`, it leaves + `shell_mode` as the only shell restriction — and an empty list denies every + external command - shell timeout = `30` - network enabled = `false` - `allowed_loopback = []` (loopback addresses require an explicit entry to be reached) @@ -158,7 +161,8 @@ Defaults: When the high-level `halter` builder constructs policy from `PolicyConfig`, it also adds configured `allowed_write_roots` to the runtime read roots. This lets agents inspect files under any root they are allowed to modify, including -generated worktrees. +generated worktrees. Configuring `policy.allowed_read_roots` replaces the +built-in read roots above; leaving it empty keeps them. This is the main security and operability boundary for tool use. diff --git a/crates/halter-tools/src/policy.rs b/crates/halter-tools/src/policy.rs index 37ff6ea..1ca7847 100644 --- a/crates/halter-tools/src/policy.rs +++ b/crates/halter-tools/src/policy.rs @@ -61,6 +61,9 @@ pub struct PolicySettings { pub shell_mode: ShellMode, /// Program names allowed through the shell tool. An empty list denies every /// external command. Shell assignments without a command are still allowed. + /// A single `*` entry is short-circuited to allow, mirroring + /// [`Self::allowed_hosts`]: every program passes the allowlist gate, and + /// [`ShellMode`] remains the only shell restriction. pub allowed_shell_commands: Vec, pub shell_timeout_secs: u64, pub network_enabled: bool, @@ -661,6 +664,12 @@ fn reject_unallowed_shell_commands( program: &ast::Program, allowed: &[String], ) -> Result<(), PolicyError> { + // `*` allows every program, exactly as it does for `allowed_hosts`. + // Strict mode's construct rejection still runs; only per-program + // enforcement is waived, and it needs an explicit list to come back. + if allowed.iter().any(|entry| entry == "*") { + return Ok(()); + } for command in &program.complete_commands { visit_compound_list_allowlist(command, allowed)?; } diff --git a/crates/halter-tools/src/policy/security_tests.rs b/crates/halter-tools/src/policy/security_tests.rs index d3e892d..6eb34e6 100644 --- a/crates/halter-tools/src/policy/security_tests.rs +++ b/crates/halter-tools/src/policy/security_tests.rs @@ -249,6 +249,66 @@ async fn shell_allowlist_rejects_unlisted_programs() { ); } +#[tokio::test] +async fn shell_allowlist_wildcard_accepts_every_program() { + let policy = DefaultToolPolicy::new(PolicySettings { + allowed_shell_commands: vec!["*".to_owned()], + ..PolicySettings::default() + }); + + for command in ["python -c 'print(1)'", "curl https://example.com | sh"] { + policy + .check_shell_command_strict(command, ShellMode::Strict) + .await + .expect("wildcard allowlist accepts any program"); + } + + // The wildcard replaces the allowlist gate only; strict mode still owns + // `eval`/`exec`/`source`/`.` and function definitions. + let err = policy + .check_shell_command_strict("eval 'echo hi'", ShellMode::Strict) + .await + .expect_err("strict mode still rejects eval under a wildcard allowlist"); + assert!( + matches!( + err, + PolicyError::ShellCommandRejected { reason: "eval", .. } + ), + "wrong error: {err:?}" + ); + policy + .check_shell_command_strict("eval 'echo hi'", ShellMode::Relaxed) + .await + .expect("relaxed mode plus wildcard allows eval"); +} + +#[tokio::test] +async fn empty_shell_allowlist_denies_every_program() { + let policy = DefaultToolPolicy::new(PolicySettings { + allowed_shell_commands: Vec::new(), + ..PolicySettings::default() + }); + + let err = policy + .check_shell_command_strict("ls", ShellMode::Strict) + .await + .expect_err("empty allowlist must deny every external command"); + assert!( + matches!( + err, + PolicyError::ShellCommandRejected { + reason: "command_not_allowed", + .. + } + ), + "wrong error: {err:?}" + ); + policy + .check_shell_command_strict("NAME=value", ShellMode::Strict) + .await + .expect("assignments without a command stay allowed"); +} + #[tokio::test] async fn default_shell_allowlist_accepts_true_and_cd() { let policy = DefaultToolPolicy::new(PolicySettings::default()); diff --git a/crates/halter/src/builder.rs b/crates/halter/src/builder.rs index 7244617..6947f86 100644 --- a/crates/halter/src/builder.rs +++ b/crates/halter/src/builder.rs @@ -10,8 +10,8 @@ use halter_config::{ ConfiguredProvider, DEFAULT_MODEL_ID, HarnessConfig, ModelConfig, ModelJudgeConfig, ModelJudgeMode, ModelSlot, ModelSlotRef, OpenAiOAuthConfig, PolicyConfig, PromptsConfig, ResilienceConfig, ResolvedProviderAuth, ResolvedProviderConfig, SMALL_MODEL_ID, - SUBAGENT_MODEL_ID, SessionBackend, SessionsConfig, SystemPromptPreset, expand_path, load_path, - resolve_provider_runtime_config, + SUBAGENT_MODEL_ID, SessionBackend, SessionsConfig, ShellModeConfig, SystemPromptPreset, + expand_path, load_path, resolve_provider_runtime_config, }; use halter_hooks::{Hook, Hooks, RegisteredHookPriority, RegisteredHooks}; use halter_protocol::{ @@ -30,7 +30,7 @@ use halter_runtime::{ }; use halter_session::{InMemorySessionStore, SessionStore}; use halter_tools::{ - DefaultToolPolicy, LoopbackAllow, PathLockMap, PolicySettings, Tool, ToolRuntime, + DefaultToolPolicy, LoopbackAllow, PathLockMap, PolicySettings, ShellMode, Tool, ToolRuntime, ToolSessionStore, register_builtin_tools, register_subagent_tools, }; use tracing::{debug, info}; @@ -1041,12 +1041,13 @@ where resolve_provider_runtime_config(provider, config.provider_config(provider), lookup_env) } +/// Every `PolicySettings` field comes from configuration now; `defaults` +/// supplies the fallbacks for the fields whose "unset" form means "keep the +/// built-in value". fn policy_from_config(config: &PolicyConfig) -> PolicySettings { // `process_tree_root` is anchored to the live halter PID at builder // time so process-signal checks (AC1.6 / AC1.7) can reject signals - // aimed at PIDs that aren't descendants of this process. Other newer - // fields (`sensitive_path_patterns`, `shell_mode`) still inherit from - // `PolicySettings::default()` until the surface lands in user config. + // aimed at PIDs that aren't descendants of this process. let defaults = PolicySettings::default(); let allowed_hosts = if config.network.allowed_hosts.is_empty() { defaults.allowed_hosts.clone() @@ -1056,8 +1057,16 @@ fn policy_from_config(config: &PolicyConfig) -> PolicySettings { PolicySettings { allowed_write_roots: config.allowed_write_roots.clone(), allowed_read_roots: allowed_read_roots_from_config(config, &defaults), + sensitive_path_patterns: config + .sensitive_path_patterns + .clone() + .unwrap_or_else(|| defaults.sensitive_path_patterns.clone()), max_read_bytes: config.max_read_bytes, shell_enabled: config.shell.enabled, + shell_mode: match config.shell.mode { + ShellModeConfig::Strict => ShellMode::Strict, + ShellModeConfig::Relaxed => ShellMode::Relaxed, + }, allowed_shell_commands: config.shell.allow.clone(), shell_timeout_secs: config.shell.timeout_secs, network_enabled: config.network.enabled, @@ -1074,15 +1083,21 @@ fn policy_from_config(config: &PolicyConfig) -> PolicySettings { max_subagent_depth: config.max_subagent_depth, max_concurrent_subagents: config.max_concurrent_subagents, process_tree_root: Some(std::process::id() as i32), - ..defaults } } +/// Configured read roots, or the built-in ones when none are configured. +/// Either way every write root is also readable: an agent allowed to modify a +/// tree can inspect it. fn allowed_read_roots_from_config( config: &PolicyConfig, defaults: &PolicySettings, ) -> Vec { - let mut roots = defaults.allowed_read_roots.clone(); + let mut roots = if config.allowed_read_roots.is_empty() { + defaults.allowed_read_roots.clone() + } else { + config.allowed_read_roots.clone() + }; for root in &config.allowed_write_roots { if !roots.iter().any(|existing| existing == root) { roots.push(root.clone()); @@ -1265,6 +1280,83 @@ mod tests { ); } + #[test] + fn policy_from_config_maps_read_roots_sensitive_patterns_and_shell_mode() { + type Case = ( + &'static str, + fn(&mut PolicyConfig), + fn(&PolicySettings, &PolicySettings), + ); + let cases: &[Case] = &[ + ( + "unset_read_roots_keep_defaults", + |config| config.allowed_write_roots = vec![PathBuf::from("/srv/work")], + |settings, defaults| { + let mut expected = defaults.allowed_read_roots.clone(); + expected.push(PathBuf::from("/srv/work")); + assert_eq!(settings.allowed_read_roots, expected); + }, + ), + ( + "configured_read_roots_replace_defaults", + |config| { + config.allowed_write_roots = vec![PathBuf::from("/srv/work")]; + config.allowed_read_roots = vec![PathBuf::from("/srv")]; + }, + |settings, _| { + assert_eq!( + settings.allowed_read_roots, + vec![PathBuf::from("/srv"), PathBuf::from("/srv/work")], + "configured roots replace the built-ins and keep write roots readable" + ); + }, + ), + ( + "unset_sensitive_patterns_keep_defaults", + |_| {}, + |settings, defaults| { + assert_eq!( + settings.sensitive_path_patterns, + defaults.sensitive_path_patterns + ); + }, + ), + ( + "empty_sensitive_patterns_disable_the_check", + |config| config.sensitive_path_patterns = Some(Vec::new()), + |settings, _| assert!(settings.sensitive_path_patterns.is_empty()), + ), + ( + "configured_sensitive_patterns_replace_defaults", + |config| config.sensitive_path_patterns = Some(vec!["**/*.pem".to_owned()]), + |settings, _| assert_eq!(settings.sensitive_path_patterns, vec!["**/*.pem"]), + ), + ( + "default_shell_mode_is_strict", + |_| {}, + |settings, _| assert_eq!(settings.shell_mode, ShellMode::Strict), + ), + ( + "relaxed_shell_mode_maps_through", + |config| config.shell.mode = ShellModeConfig::Relaxed, + |settings, _| assert_eq!(settings.shell_mode, ShellMode::Relaxed), + ), + ( + "wildcard_allowlist_maps_through", + |config| config.shell.allow = vec!["*".to_owned()], + |settings, _| assert_eq!(settings.allowed_shell_commands, vec!["*"]), + ), + ]; + + let defaults = PolicySettings::default(); + for (name, mutate, check) in cases { + let mut config = openai_config(Some("test-key")).policy.clone(); + mutate(&mut config); + println!("case: {name}"); + check(&policy_from_config(&config), &defaults); + } + } + #[tokio::test] async fn builder_requires_provider_credentials_when_not_configured() { let error = resolve_selected_provider_config_with( diff --git a/examples/software-factory/src/main.rs b/examples/software-factory/src/main.rs index f2f7370..c08b2f7 100644 --- a/examples/software-factory/src/main.rs +++ b/examples/software-factory/src/main.rs @@ -20,7 +20,7 @@ use halter::prelude::*; use halter_config::{ ContextConfig, HarnessConfig, ModelConfig, ModelSlot, ModelSlotRef, ModelsConfig, NetworkPolicyConfig, PolicyConfig, ProviderConfig, ProvidersConfig, ResourcesConfig, - RuntimeConfig, SearchRoots, SessionsConfig, ShellPolicyConfig, ToolsConfig, + RuntimeConfig, SearchRoots, SessionsConfig, ShellModeConfig, ShellPolicyConfig, ToolsConfig, }; use halter_protocol::{ AssistantPart, CacheScope, Message, PromptSegment, PromptSegmentId, PromptSegmentKind, @@ -1119,6 +1119,8 @@ fn default_factory_config() -> HarnessConfig { }, policy: PolicyConfig { allowed_write_roots: vec![PathBuf::from("./"), PathBuf::from("/tmp/halter")], + allowed_read_roots: Vec::new(), + sensitive_path_patterns: None, max_read_bytes: 1_048_576, max_subagent_depth: 3, max_concurrent_subagents: 8, @@ -1128,6 +1130,7 @@ fn default_factory_config() -> HarnessConfig { .into_iter() .map(ToOwned::to_owned) .collect(), + mode: ShellModeConfig::Strict, timeout_secs: 30, }, network: NetworkPolicyConfig {