From 95c738bf1a543f6ae08147e632616c18f7510069 Mon Sep 17 00:00:00 2001 From: Cowboy Date: Mon, 27 Jul 2026 22:12:43 +0000 Subject: [PATCH] Let policy config allow every program and set read roots MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit `policy.shell.allow = ["*"]` now allows every program, mirroring the wildcard `policy.network.allowed_hosts` already accepts. Deployments that want arbitrary shell execution had to allowlist `bash` and route every command through `bash -c` — a bypass dressed up as an allowlist entry, which taught agents a worse habit than simply saying "any program". The builder also stopped pinning the last three `PolicySettings` fields to their defaults. `policy.allowed_read_roots`, `policy.sensitive_path_patterns`, and `policy.shell.mode` are configurable, so an embedder can state the exact filesystem an agent sees instead of inheriting the working directory and temp directory forever. Unset keeps today's behavior in every case. Co-Authored-By: Claude Opus 5 --- CHANGELOG.md | 16 +++ README.md | 9 ++ crates/halter-config/README.md | 17 ++- crates/halter-config/src/lib.rs | 4 +- crates/halter-config/src/loader.rs | 3 + crates/halter-config/src/schema.rs | 32 ++++++ crates/halter-tools/README.md | 10 +- crates/halter-tools/src/policy.rs | 9 ++ .../halter-tools/src/policy/security_tests.rs | 60 ++++++++++ crates/halter/src/builder.rs | 108 ++++++++++++++++-- examples/software-factory/src/main.rs | 5 +- 11 files changed, 256 insertions(+), 17 deletions(-) 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 {