Compare commits

...

12 Commits

Author SHA1 Message Date
simianastronaut a1af84d992 fix(security): enforce approval policy for channel-driven runs
Channel-driven runs (Telegram, Matrix, Discord, etc.) previously bypassed
the ApprovalManager entirely — `None` was passed into the tool-call loop,
so `auto_approve`, `always_ask`, and supervised approval checks were
silently skipped for all non-CLI execution paths.

Add a non-interactive mode to ApprovalManager that enforces the same
autonomy config policies but auto-denies tools requiring interactive
approval (since no operator is present on channel runs). Specifically:

- Add `ApprovalManager::for_non_interactive()` constructor that creates
  a manager which auto-denies tools needing approval instead of prompting
- Add `is_non_interactive()` method so the tool-call loop can distinguish
  interactive (CLI prompt) from non-interactive (auto-deny) managers
- Update tool-call loop: non-interactive managers auto-deny instead of
  the previous auto-approve behavior for non-CLI channels
- Wire the non-interactive approval manager into ChannelRuntimeContext
  so channel runs enforce the full approval policy
- Add 8 tests covering non-interactive approval behavior

Security implications:
- `always_ask` tools are now denied on channels (previously bypassed)
- Supervised-mode unknown tools are now denied on channels (previously
  bypassed)
- `auto_approve` tools continue to work on channels unchanged
- `full` autonomy mode is unaffected (no approval needed regardless)
- `read_only` mode is unaffected (blocks execution elsewhere)

Closes #3487

Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
2026-03-15 15:56:57 -04:00
SimianAstronaut7 e34a804255 Merge pull request #3632 from zeroclaw-labs/work-issues/3544-fix-codex-sse-buffering
fix(provider): use incremental SSE stream reading for openai-codex responses
2026-03-15 15:34:39 -04:00
SimianAstronaut7 6120b3f705 Merge pull request #3630 from zeroclaw-labs/work-issues/3567-allow-commands-bypass-high-risk
fix(security): let explicit allowed_commands bypass high-risk block
2026-03-15 15:34:37 -04:00
SimianAstronaut7 f175261e32 Merge pull request #3631 from zeroclaw-labs/work-issues/3486-fix-matrix-image-marker
fix(channels): use canonical IMAGE marker in Matrix channel
2026-03-15 15:34:31 -04:00
simianastronaut fd9f66cad7 fix(provider): use incremental SSE stream reading for openai-codex responses
Replace full-body buffering (`response.text().await`) in
`decode_responses_body()` with incremental `bytes_stream()` chunk
processing.  The previous approach held the HTTP connection open until
every byte had arrived; on high-latency links the long-lived connection
would frequently drop mid-read, producing the "error decoding response
body" failure on the first attempt (succeeding only after retry).

Reading chunks incrementally lets each network segment complete within
its own timeout window, eliminating the systematic first-attempt failure.

Closes #3544

Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
2026-03-15 15:22:55 -04:00
simianastronaut d928ebc92e fix(channels): use canonical IMAGE marker in Matrix channel
Matrix image messages used lowercase `[image: ...]` format instead of
the canonical `[IMAGE:...]` marker used by all other channels (Telegram,
Slack, Discord, QQ, LinQ). This caused Matrix image attachments to
bypass the multimodal vision pipeline which looks for `[IMAGE:...]`.

Closes #3486

Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
2026-03-15 15:14:53 -04:00
simianastronaut 9fca9f478a fix(security): let explicit allowed_commands bypass high-risk block
When `block_high_risk_commands = true`, commands like `curl` and `wget`
were unconditionally blocked even if explicitly listed in
`allowed_commands`. This made it impossible to use legitimate API calls
in full autonomy mode.

Now, if a command is explicitly named in `allowed_commands` (not via
the wildcard `*`), it is exempt from the `block_high_risk_commands`
gate. The wildcard entry intentionally does NOT grant this exemption,
preserving the safety net for broad allowlists.

Other security gates (supervised-mode approval, rate limiting, path
policy, argument validation) remain fully enforced.

Closes #3567

Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
2026-03-15 15:13:32 -04:00
SimianAstronaut7 7106632b51 Merge pull request #3627 from zeroclaw-labs/work-issues/3533-fix-utf8-slice-panic
fix(agent): use char-boundary-safe slicing to prevent CJK text panic
2026-03-15 14:36:49 -04:00
SimianAstronaut7 b834278754 Merge pull request #3626 from zeroclaw-labs/work-issues/3563-fix-cron-add-nl-security
fix(cron): add --agent flag so natural language prompts bypass shell security
2026-03-15 14:36:46 -04:00
SimianAstronaut7 186f6d9797 Merge pull request #3625 from zeroclaw-labs/work-issues/3568-http-request-private-hosts
feat(tool): add allow_private_hosts option to http_request tool
2026-03-15 14:36:44 -04:00
simianastronaut 6cdc92a256 fix(agent): use char-boundary-safe slicing to prevent CJK text panic
Replace unsafe byte-index string slicing (`&text[..N]`) with
char-boundary-safe alternatives in memory consolidation and security
redaction to prevent panics when multi-byte UTF-8 characters (e.g.
Chinese/Japanese/Korean) span the slice boundary.

Fixes the same class of bug as the prior fix in `execute_one_tool`
(commit 8fcbb6eb), applied to two remaining instances:
- `src/memory/consolidation.rs`: truncation at byte 4000 and 200
- `src/security/mod.rs`: `redact()` prefix at byte 4

Adds regression tests with CJK input for both locations.

Closes #3533

Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
2026-03-15 14:27:09 -04:00
simianastronaut fe64d7ef7e feat(tool): add allow_private_hosts option to http_request tool (#3568)
The http_request tool unconditionally blocked all private/LAN hosts with
no opt-out, preventing legitimate use cases like calling a local Home
Assistant instance or internal APIs. This adds an `allow_private_hosts`
config flag (default: false) under `[http_request]` that, when set to
true, skips the private-host SSRF check while still enforcing the domain
allowlist.

Closes #3568

Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
2026-03-15 14:23:54 -04:00
11 changed files with 507 additions and 21 deletions
+6 -4
View File
@@ -2687,11 +2687,13 @@ pub(crate) async fn run_tool_call_loop(
arguments: tool_args.clone(),
};
// Only prompt interactively on CLI; auto-approve on other channels.
let decision = if channel_name == "cli" {
mgr.prompt_cli(&request)
// Interactive CLI: prompt the operator.
// Non-interactive (channels): auto-deny since no operator
// is present to approve.
let decision = if mgr.is_non_interactive() {
ApprovalResponse::No
} else {
ApprovalResponse::Yes
mgr.prompt_cli(&request)
};
mgr.record_decision(&tool_name, &tool_args, decision, channel_name);
+128 -4
View File
@@ -44,11 +44,18 @@ pub struct ApprovalLogEntry {
// ── ApprovalManager ──────────────────────────────────────────────
/// Manages the interactive approval workflow.
/// Manages the approval workflow for tool calls.
///
/// - Checks config-level `auto_approve` / `always_ask` lists
/// - Maintains a session-scoped "always" allowlist
/// - Records an audit trail of all decisions
///
/// Two modes:
/// - **Interactive** (CLI): tools needing approval trigger a stdin prompt.
/// - **Non-interactive** (channels): tools needing approval are auto-denied
/// because there is no interactive operator to approve them. `auto_approve`
/// policy is still enforced, and `always_ask` / supervised-default tools are
/// denied rather than silently allowed.
pub struct ApprovalManager {
/// Tools that never need approval (from config).
auto_approve: HashSet<String>,
@@ -56,6 +63,9 @@ pub struct ApprovalManager {
always_ask: HashSet<String>,
/// Autonomy level from config.
autonomy_level: AutonomyLevel,
/// When `true`, tools that would require interactive approval are
/// auto-denied instead. Used for channel-driven (non-CLI) runs.
non_interactive: bool,
/// Session-scoped allowlist built from "Always" responses.
session_allowlist: Mutex<HashSet<String>>,
/// Audit trail of approval decisions.
@@ -63,17 +73,40 @@ pub struct ApprovalManager {
}
impl ApprovalManager {
/// Create from autonomy config.
/// Create an interactive (CLI) approval manager from autonomy config.
pub fn from_config(config: &AutonomyConfig) -> Self {
Self {
auto_approve: config.auto_approve.iter().cloned().collect(),
always_ask: config.always_ask.iter().cloned().collect(),
autonomy_level: config.level,
non_interactive: false,
session_allowlist: Mutex::new(HashSet::new()),
audit_log: Mutex::new(Vec::new()),
}
}
/// Create a non-interactive approval manager for channel-driven runs.
///
/// Enforces the same `auto_approve` / `always_ask` / supervised policies
/// as the CLI manager, but tools that would require interactive approval
/// are auto-denied instead of prompting (since there is no operator).
pub fn for_non_interactive(config: &AutonomyConfig) -> Self {
Self {
auto_approve: config.auto_approve.iter().cloned().collect(),
always_ask: config.always_ask.iter().cloned().collect(),
autonomy_level: config.level,
non_interactive: true,
session_allowlist: Mutex::new(HashSet::new()),
audit_log: Mutex::new(Vec::new()),
}
}
/// Returns `true` when this manager operates in non-interactive mode
/// (i.e. for channel-driven runs where no operator can approve).
pub fn is_non_interactive(&self) -> bool {
self.non_interactive
}
/// Check whether a tool call requires interactive approval.
///
/// Returns `true` if the call needs a prompt, `false` if it can proceed.
@@ -147,8 +180,8 @@ impl ApprovalManager {
/// Prompt the user on the CLI and return their decision.
///
/// For non-CLI channels, returns `Yes` automatically (interactive
/// approval is only supported on CLI for now).
/// Only called for interactive (CLI) managers. Non-interactive managers
/// auto-deny in the tool-call loop before reaching this point.
pub fn prompt_cli(&self, request: &ApprovalRequest) -> ApprovalResponse {
prompt_cli_interactive(request)
}
@@ -401,6 +434,97 @@ mod tests {
assert!(summary.contains("just a string"));
}
// ── non-interactive (channel) mode ────────────────────────
#[test]
fn non_interactive_manager_reports_non_interactive() {
let mgr = ApprovalManager::for_non_interactive(&supervised_config());
assert!(mgr.is_non_interactive());
}
#[test]
fn interactive_manager_reports_interactive() {
let mgr = ApprovalManager::from_config(&supervised_config());
assert!(!mgr.is_non_interactive());
}
#[test]
fn non_interactive_auto_approve_tools_skip_approval() {
let mgr = ApprovalManager::for_non_interactive(&supervised_config());
// auto_approve tools (file_read, memory_recall) should not need approval.
assert!(!mgr.needs_approval("file_read"));
assert!(!mgr.needs_approval("memory_recall"));
}
#[test]
fn non_interactive_always_ask_tools_need_approval() {
let mgr = ApprovalManager::for_non_interactive(&supervised_config());
// always_ask tools (shell) still report as needing approval,
// so the tool-call loop will auto-deny them in non-interactive mode.
assert!(mgr.needs_approval("shell"));
}
#[test]
fn non_interactive_unknown_tools_need_approval_in_supervised() {
let mgr = ApprovalManager::for_non_interactive(&supervised_config());
// Unknown tools in supervised mode need approval (will be auto-denied
// by the tool-call loop for non-interactive managers).
assert!(mgr.needs_approval("file_write"));
assert!(mgr.needs_approval("http_request"));
}
#[test]
fn non_interactive_full_autonomy_never_needs_approval() {
let mgr = ApprovalManager::for_non_interactive(&full_config());
// Full autonomy means no approval needed, even in non-interactive mode.
assert!(!mgr.needs_approval("shell"));
assert!(!mgr.needs_approval("file_write"));
assert!(!mgr.needs_approval("anything"));
}
#[test]
fn non_interactive_readonly_never_needs_approval() {
let config = AutonomyConfig {
level: AutonomyLevel::ReadOnly,
..AutonomyConfig::default()
};
let mgr = ApprovalManager::for_non_interactive(&config);
// ReadOnly blocks execution elsewhere; approval manager does not prompt.
assert!(!mgr.needs_approval("shell"));
}
#[test]
fn non_interactive_session_allowlist_still_works() {
let mgr = ApprovalManager::for_non_interactive(&supervised_config());
assert!(mgr.needs_approval("file_write"));
// Simulate an "Always" decision (would come from a prior channel run
// if the tool was auto-approved somehow, e.g. via config change).
mgr.record_decision(
"file_write",
&serde_json::json!({"path": "test.txt"}),
ApprovalResponse::Always,
"telegram",
);
assert!(!mgr.needs_approval("file_write"));
}
#[test]
fn non_interactive_always_ask_overrides_session_allowlist() {
let mgr = ApprovalManager::for_non_interactive(&supervised_config());
mgr.record_decision(
"shell",
&serde_json::json!({"command": "ls"}),
ApprovalResponse::Always,
"telegram",
);
// shell is in always_ask, so it still needs approval even after "Always".
assert!(mgr.needs_approval("shell"));
}
// ── ApprovalResponse serde ───────────────────────────────
#[test]
+1 -1
View File
@@ -746,7 +746,7 @@ impl Channel for MatrixChannel {
MessageType::Notice(content) => (content.body.clone(), None),
MessageType::Image(content) => {
let dl = media_info(&content.source, &content.body);
(format!("[image: {}]", content.body), dl)
(format!("[IMAGE:{}]", content.body), dl)
}
MessageType::File(content) => {
let dl = media_info(&content.source, &content.body);
+80 -1
View File
@@ -76,6 +76,7 @@ pub use whatsapp::WhatsAppChannel;
pub use whatsapp_web::WhatsAppWebChannel;
use crate::agent::loop_::{build_tool_instructions, run_tool_call_loop, scrub_credentials};
use crate::approval::ApprovalManager;
use crate::config::Config;
use crate::identity;
use crate::memory::{self, Memory};
@@ -314,6 +315,11 @@ struct ChannelRuntimeContext {
ack_reactions: bool,
show_tool_calls: bool,
session_store: Option<Arc<session_store::SessionStore>>,
/// Non-interactive approval manager for channel-driven runs.
/// Enforces `auto_approve` / `always_ask` / supervised policy from
/// `[autonomy]` config; auto-denies tools that would need interactive
/// approval since no operator is present on channel runs.
approval_manager: Arc<ApprovalManager>,
}
#[derive(Clone)]
@@ -2025,7 +2031,7 @@ async fn process_channel_message(
route.model.as_str(),
runtime_defaults.temperature,
true,
None,
Some(&*ctx.approval_manager),
msg.channel.as_str(),
&ctx.multimodal,
ctx.max_tool_iterations,
@@ -3851,6 +3857,7 @@ pub async fn start_channels(config: Config) -> Result<()> {
} else {
None
},
approval_manager: Arc::new(ApprovalManager::for_non_interactive(&config.autonomy)),
});
// Hydrate in-memory conversation histories from persisted JSONL session files.
@@ -4139,6 +4146,9 @@ mod tests {
ack_reactions: true,
show_tool_calls: true,
session_store: None,
approval_manager: Arc::new(ApprovalManager::for_non_interactive(
&crate::config::AutonomyConfig::default(),
)),
};
assert!(compact_sender_history(&ctx, &sender));
@@ -4243,6 +4253,9 @@ mod tests {
ack_reactions: true,
show_tool_calls: true,
session_store: None,
approval_manager: Arc::new(ApprovalManager::for_non_interactive(
&crate::config::AutonomyConfig::default(),
)),
};
append_sender_turn(&ctx, &sender, ChatMessage::user("hello"));
@@ -4303,6 +4316,9 @@ mod tests {
ack_reactions: true,
show_tool_calls: true,
session_store: None,
approval_manager: Arc::new(ApprovalManager::for_non_interactive(
&crate::config::AutonomyConfig::default(),
)),
};
assert!(rollback_orphan_user_turn(&ctx, &sender, "pending"));
@@ -4821,6 +4837,9 @@ BTC is currently around $65,000 based on latest tool output."#
ack_reactions: true,
show_tool_calls: true,
session_store: None,
approval_manager: Arc::new(ApprovalManager::for_non_interactive(
&crate::config::AutonomyConfig::default(),
)),
});
process_channel_message(
@@ -4889,6 +4908,9 @@ BTC is currently around $65,000 based on latest tool output."#
ack_reactions: true,
show_tool_calls: true,
session_store: None,
approval_manager: Arc::new(ApprovalManager::for_non_interactive(
&crate::config::AutonomyConfig::default(),
)),
});
process_channel_message(
@@ -4971,6 +4993,9 @@ BTC is currently around $65,000 based on latest tool output."#
ack_reactions: true,
show_tool_calls: true,
session_store: None,
approval_manager: Arc::new(ApprovalManager::for_non_interactive(
&crate::config::AutonomyConfig::default(),
)),
});
process_channel_message(
@@ -5038,6 +5063,9 @@ BTC is currently around $65,000 based on latest tool output."#
ack_reactions: true,
show_tool_calls: true,
session_store: None,
approval_manager: Arc::new(ApprovalManager::for_non_interactive(
&crate::config::AutonomyConfig::default(),
)),
});
process_channel_message(
@@ -5115,6 +5143,9 @@ BTC is currently around $65,000 based on latest tool output."#
ack_reactions: true,
show_tool_calls: true,
session_store: None,
approval_manager: Arc::new(ApprovalManager::for_non_interactive(
&crate::config::AutonomyConfig::default(),
)),
});
process_channel_message(
@@ -5212,6 +5243,9 @@ BTC is currently around $65,000 based on latest tool output."#
ack_reactions: true,
show_tool_calls: true,
session_store: None,
approval_manager: Arc::new(ApprovalManager::for_non_interactive(
&crate::config::AutonomyConfig::default(),
)),
});
process_channel_message(
@@ -5291,6 +5325,9 @@ BTC is currently around $65,000 based on latest tool output."#
ack_reactions: true,
show_tool_calls: true,
session_store: None,
approval_manager: Arc::new(ApprovalManager::for_non_interactive(
&crate::config::AutonomyConfig::default(),
)),
});
process_channel_message(
@@ -5385,6 +5422,9 @@ BTC is currently around $65,000 based on latest tool output."#
ack_reactions: true,
show_tool_calls: true,
session_store: None,
approval_manager: Arc::new(ApprovalManager::for_non_interactive(
&crate::config::AutonomyConfig::default(),
)),
});
process_channel_message(
@@ -5464,6 +5504,9 @@ BTC is currently around $65,000 based on latest tool output."#
ack_reactions: true,
show_tool_calls: true,
session_store: None,
approval_manager: Arc::new(ApprovalManager::for_non_interactive(
&crate::config::AutonomyConfig::default(),
)),
});
process_channel_message(
@@ -5533,6 +5576,9 @@ BTC is currently around $65,000 based on latest tool output."#
ack_reactions: true,
show_tool_calls: true,
session_store: None,
approval_manager: Arc::new(ApprovalManager::for_non_interactive(
&crate::config::AutonomyConfig::default(),
)),
});
process_channel_message(
@@ -5713,6 +5759,9 @@ BTC is currently around $65,000 based on latest tool output."#
ack_reactions: true,
show_tool_calls: true,
session_store: None,
approval_manager: Arc::new(ApprovalManager::for_non_interactive(
&crate::config::AutonomyConfig::default(),
)),
});
let (tx, rx) = tokio::sync::mpsc::channel::<traits::ChannelMessage>(4);
@@ -5801,6 +5850,9 @@ BTC is currently around $65,000 based on latest tool output."#
ack_reactions: true,
show_tool_calls: true,
session_store: None,
approval_manager: Arc::new(ApprovalManager::for_non_interactive(
&crate::config::AutonomyConfig::default(),
)),
});
let (tx, rx) = tokio::sync::mpsc::channel::<traits::ChannelMessage>(8);
@@ -5904,6 +5956,9 @@ BTC is currently around $65,000 based on latest tool output."#
non_cli_excluded_tools: Arc::new(Vec::new()),
tool_call_dedup_exempt: Arc::new(Vec::new()),
model_routes: Arc::new(Vec::new()),
approval_manager: Arc::new(ApprovalManager::for_non_interactive(
&crate::config::AutonomyConfig::default(),
)),
});
let (tx, rx) = tokio::sync::mpsc::channel::<traits::ChannelMessage>(8);
@@ -6004,6 +6059,9 @@ BTC is currently around $65,000 based on latest tool output."#
ack_reactions: true,
show_tool_calls: true,
session_store: None,
approval_manager: Arc::new(ApprovalManager::for_non_interactive(
&crate::config::AutonomyConfig::default(),
)),
});
let (tx, rx) = tokio::sync::mpsc::channel::<traits::ChannelMessage>(8);
@@ -6086,6 +6144,9 @@ BTC is currently around $65,000 based on latest tool output."#
ack_reactions: true,
show_tool_calls: true,
session_store: None,
approval_manager: Arc::new(ApprovalManager::for_non_interactive(
&crate::config::AutonomyConfig::default(),
)),
});
process_channel_message(
@@ -6153,6 +6214,9 @@ BTC is currently around $65,000 based on latest tool output."#
ack_reactions: true,
show_tool_calls: true,
session_store: None,
approval_manager: Arc::new(ApprovalManager::for_non_interactive(
&crate::config::AutonomyConfig::default(),
)),
});
process_channel_message(
@@ -6778,6 +6842,9 @@ BTC is currently around $65,000 based on latest tool output."#
ack_reactions: true,
show_tool_calls: true,
session_store: None,
approval_manager: Arc::new(ApprovalManager::for_non_interactive(
&crate::config::AutonomyConfig::default(),
)),
});
process_channel_message(
@@ -6871,6 +6938,9 @@ BTC is currently around $65,000 based on latest tool output."#
ack_reactions: true,
show_tool_calls: true,
session_store: None,
approval_manager: Arc::new(ApprovalManager::for_non_interactive(
&crate::config::AutonomyConfig::default(),
)),
});
process_channel_message(
@@ -6964,6 +7034,9 @@ BTC is currently around $65,000 based on latest tool output."#
ack_reactions: true,
show_tool_calls: true,
session_store: None,
approval_manager: Arc::new(ApprovalManager::for_non_interactive(
&crate::config::AutonomyConfig::default(),
)),
});
process_channel_message(
@@ -7521,6 +7594,9 @@ This is an example JSON object for profile settings."#;
ack_reactions: true,
show_tool_calls: true,
session_store: None,
approval_manager: Arc::new(ApprovalManager::for_non_interactive(
&crate::config::AutonomyConfig::default(),
)),
});
// Simulate a photo attachment message with [IMAGE:] marker.
@@ -7595,6 +7671,9 @@ This is an example JSON object for profile settings."#;
ack_reactions: true,
show_tool_calls: true,
session_store: None,
approval_manager: Arc::new(ApprovalManager::for_non_interactive(
&crate::config::AutonomyConfig::default(),
)),
});
process_channel_message(
+5
View File
@@ -1423,6 +1423,10 @@ pub struct HttpRequestConfig {
/// Request timeout in seconds (default: 30)
#[serde(default = "default_http_timeout_secs")]
pub timeout_secs: u64,
/// Allow requests to private/LAN hosts (RFC 1918, loopback, link-local, .local).
/// Default: false (deny private hosts for SSRF protection).
#[serde(default)]
pub allow_private_hosts: bool,
}
impl Default for HttpRequestConfig {
@@ -1432,6 +1436,7 @@ impl Default for HttpRequestConfig {
allowed_domains: vec![],
max_response_size: default_http_max_response_size(),
timeout_secs: default_http_timeout_secs(),
allow_private_hosts: false,
}
}
}
+24 -2
View File
@@ -43,8 +43,13 @@ pub async fn consolidate_turn(
let turn_text = format!("User: {user_message}\nAssistant: {assistant_response}");
// Truncate very long turns to avoid wasting tokens on consolidation.
// Use char-boundary-safe slicing to prevent panic on multi-byte UTF-8 (e.g. CJK text).
let truncated = if turn_text.len() > 4000 {
format!("{}", &turn_text[..4000])
let mut end = 4000;
while end > 0 && !turn_text.is_char_boundary(end) {
end -= 1;
}
format!("{}", &turn_text[..end])
} else {
turn_text.clone()
};
@@ -92,8 +97,13 @@ fn parse_consolidation_response(raw: &str, fallback_text: &str) -> Consolidation
serde_json::from_str(cleaned).unwrap_or_else(|_| {
// Fallback: use truncated turn text as history entry.
// Use char-boundary-safe slicing to prevent panic on multi-byte UTF-8.
let summary = if fallback_text.len() > 200 {
format!("{}", &fallback_text[..200])
let mut end = 200;
while end > 0 && !fallback_text.is_char_boundary(end) {
end -= 1;
}
format!("{}", &fallback_text[..end])
} else {
fallback_text.to_string()
};
@@ -150,4 +160,16 @@ mod tests {
// 200 bytes + "…" (3 bytes in UTF-8) = 203
assert!(result.history_entry.len() <= 203);
}
#[test]
fn fallback_truncates_cjk_text_without_panic() {
// Each CJK character is 3 bytes in UTF-8; byte index 200 may land
// inside a character. This must not panic.
let cjk_text = "二手书项目".repeat(50); // 250 chars = 750 bytes
let result = parse_consolidation_response("invalid", &cjk_text);
assert!(result
.history_entry
.is_char_boundary(result.history_entry.len()));
assert!(result.history_entry.ends_with('…'));
}
}
+18 -1
View File
@@ -4,6 +4,7 @@ use crate::multimodal;
use crate::providers::traits::{ChatMessage, Provider, ProviderCapabilities};
use crate::providers::ProviderRuntimeOptions;
use async_trait::async_trait;
use futures_util::StreamExt;
use reqwest::Client;
use serde::{Deserialize, Serialize};
use serde_json::Value;
@@ -472,8 +473,24 @@ fn extract_stream_error_message(event: &Value) -> Option<String> {
None
}
/// Read the response body incrementally via `bytes_stream()` to avoid
/// buffering the entire SSE payload in memory. The previous implementation
/// used `response.text().await?` which holds the HTTP connection open until
/// every byte has arrived — on high-latency links the long-lived connection
/// often drops mid-read, producing the "error decoding response body" failure
/// reported in #3544.
async fn decode_responses_body(response: reqwest::Response) -> anyhow::Result<String> {
let body = response.text().await?;
let mut body = String::new();
let mut stream = response.bytes_stream();
while let Some(chunk) = stream.next().await {
let bytes = chunk
.map_err(|err| anyhow::anyhow!("error reading OpenAI Codex response stream: {err}"))?;
let text = std::str::from_utf8(&bytes).map_err(|err| {
anyhow::anyhow!("OpenAI Codex response contained invalid UTF-8: {err}")
})?;
body.push_str(text);
}
if let Some(text) = parse_sse_text(&body)? {
return Ok(text);
+16 -1
View File
@@ -67,7 +67,13 @@ pub fn redact(value: &str) -> String {
if value.len() <= 4 {
"***".to_string()
} else {
format!("{}***", &value[..4])
// Use char-boundary-safe slicing to prevent panic on multi-byte UTF-8.
let prefix = value
.char_indices()
.nth(4)
.map(|(byte_idx, _)| &value[..byte_idx])
.unwrap_or(value);
format!("{}***", prefix)
}
}
@@ -102,4 +108,13 @@ mod tests {
assert_eq!(redact(""), "***");
assert_eq!(redact("12345"), "1234***");
}
#[test]
fn redact_handles_multibyte_utf8_without_panic() {
// CJK characters are 3 bytes each; slicing at byte 4 would panic
// without char-boundary-safe handling.
let result = redact("密码是很长的秘密");
assert!(result.ends_with("***"));
assert!(result.is_char_boundary(result.len()));
}
}
+144 -3
View File
@@ -793,6 +793,8 @@ impl SecurityPolicy {
// 1. Allowlist check (is the base command permitted at all?)
// 2. Risk classification (high / medium / low)
// 3. Policy flags (block_high_risk_commands, require_approval_for_medium_risk)
// — explicit allowlist entries exempt a command from the high-risk block,
// but the wildcard "*" does NOT grant an exemption.
// 4. Autonomy level × approval status (supervised requires explicit approval)
// This ordering ensures deny-by-default: unknown commands are rejected
// before any risk or autonomy logic runs.
@@ -810,7 +812,7 @@ impl SecurityPolicy {
let risk = self.command_risk_level(command);
if risk == CommandRiskLevel::High {
if self.block_high_risk_commands {
if self.block_high_risk_commands && !self.is_command_explicitly_allowed(command) {
return Err("Command blocked: high-risk command is disallowed by policy".into());
}
if self.autonomy == AutonomyLevel::Supervised && !approved {
@@ -834,6 +836,48 @@ impl SecurityPolicy {
Ok(risk)
}
/// Check whether **every** segment of a command is explicitly listed in
/// `allowed_commands` — i.e., matched by a concrete entry rather than by
/// the wildcard `"*"`.
///
/// This is used to exempt explicitly-allowlisted high-risk commands from
/// the `block_high_risk_commands` gate. The wildcard entry intentionally
/// does **not** qualify as an explicit allowlist match, so that operators
/// who set `allowed_commands = ["*"]` still get the high-risk safety net.
fn is_command_explicitly_allowed(&self, command: &str) -> bool {
let segments = split_unquoted_segments(command);
for segment in &segments {
let cmd_part = skip_env_assignments(segment);
let mut words = cmd_part.split_whitespace();
let executable = strip_wrapping_quotes(words.next().unwrap_or("")).trim();
let base_cmd_owned = command_basename(executable).to_ascii_lowercase();
let base_cmd = strip_windows_exe_suffix(&base_cmd_owned);
if base_cmd.is_empty() {
continue;
}
let explicitly_listed = self.allowed_commands.iter().any(|allowed| {
let allowed = strip_wrapping_quotes(allowed).trim();
// Skip wildcard — it does not count as an explicit entry.
if allowed.is_empty() || allowed == "*" {
return false;
}
is_allowlist_entry_match(allowed, executable, base_cmd)
});
if !explicitly_listed {
return false;
}
}
// At least one real command must be present.
segments.iter().any(|s| {
let s = skip_env_assignments(s.trim());
s.split_whitespace().next().is_some_and(|w| !w.is_empty())
})
}
// ── Layered Command Allowlist ──────────────────────────────────────────
// Defence-in-depth: five independent gates run in order before the
// per-segment allowlist check. Each gate targets a specific bypass
@@ -1503,10 +1547,13 @@ mod tests {
}
#[test]
fn validate_command_blocks_high_risk_by_default() {
fn validate_command_blocks_high_risk_via_wildcard() {
// Wildcard allows the command through is_command_allowed, but
// block_high_risk_commands still rejects it because "*" does not
// count as an explicit allowlist entry.
let p = SecurityPolicy {
autonomy: AutonomyLevel::Supervised,
allowed_commands: vec!["rm".into()],
allowed_commands: vec!["*".into()],
..SecurityPolicy::default()
};
@@ -1515,6 +1562,100 @@ mod tests {
assert!(result.unwrap_err().contains("high-risk"));
}
#[test]
fn validate_command_allows_explicitly_listed_high_risk() {
// When a high-risk command is explicitly in allowed_commands, the
// block_high_risk_commands gate is bypassed — the operator has made
// a deliberate decision to permit it.
let p = SecurityPolicy {
autonomy: AutonomyLevel::Full,
allowed_commands: vec!["curl".into()],
block_high_risk_commands: true,
..SecurityPolicy::default()
};
let result = p.validate_command_execution("curl https://api.example.com/data", true);
assert_eq!(result.unwrap(), CommandRiskLevel::High);
}
#[test]
fn validate_command_allows_wget_when_explicitly_listed() {
let p = SecurityPolicy {
autonomy: AutonomyLevel::Full,
allowed_commands: vec!["wget".into()],
block_high_risk_commands: true,
..SecurityPolicy::default()
};
let result =
p.validate_command_execution("wget https://releases.example.com/v1.tar.gz", true);
assert_eq!(result.unwrap(), CommandRiskLevel::High);
}
#[test]
fn validate_command_blocks_non_listed_high_risk_when_another_is_allowed() {
// Allowing curl explicitly should not exempt wget.
let p = SecurityPolicy {
autonomy: AutonomyLevel::Full,
allowed_commands: vec!["curl".into()],
block_high_risk_commands: true,
..SecurityPolicy::default()
};
let result = p.validate_command_execution("wget https://evil.com", true);
assert!(result.is_err());
assert!(result.unwrap_err().contains("not allowed"));
}
#[test]
fn validate_command_explicit_rm_bypasses_high_risk_block() {
// Operator explicitly listed "rm" — they accept the risk.
let p = SecurityPolicy {
autonomy: AutonomyLevel::Full,
allowed_commands: vec!["rm".into()],
block_high_risk_commands: true,
..SecurityPolicy::default()
};
let result = p.validate_command_execution("rm -rf /tmp/test", true);
assert_eq!(result.unwrap(), CommandRiskLevel::High);
}
#[test]
fn validate_command_high_risk_still_needs_approval_in_supervised() {
// Even when explicitly allowed, supervised mode still requires
// approval for high-risk commands (the approval gate is separate
// from the block gate).
let p = SecurityPolicy {
autonomy: AutonomyLevel::Supervised,
allowed_commands: vec!["curl".into()],
block_high_risk_commands: true,
..SecurityPolicy::default()
};
let denied = p.validate_command_execution("curl https://api.example.com", false);
assert!(denied.is_err());
assert!(denied.unwrap_err().contains("requires explicit approval"));
let allowed = p.validate_command_execution("curl https://api.example.com", true);
assert_eq!(allowed.unwrap(), CommandRiskLevel::High);
}
#[test]
fn validate_command_pipe_needs_all_segments_explicitly_allowed() {
// When a pipeline contains a high-risk command, every segment
// must be explicitly allowed for the exemption to apply.
let p = SecurityPolicy {
autonomy: AutonomyLevel::Full,
allowed_commands: vec!["curl".into(), "grep".into()],
block_high_risk_commands: true,
..SecurityPolicy::default()
};
let result = p.validate_command_execution("curl https://api.example.com | grep data", true);
assert_eq!(result.unwrap(), CommandRiskLevel::High);
}
#[test]
fn validate_command_full_mode_skips_medium_risk_approval_gate() {
let p = SecurityPolicy {
+84 -4
View File
@@ -12,6 +12,7 @@ pub struct HttpRequestTool {
allowed_domains: Vec<String>,
max_response_size: usize,
timeout_secs: u64,
allow_private_hosts: bool,
}
impl HttpRequestTool {
@@ -20,12 +21,14 @@ impl HttpRequestTool {
allowed_domains: Vec<String>,
max_response_size: usize,
timeout_secs: u64,
allow_private_hosts: bool,
) -> Self {
Self {
security,
allowed_domains: normalize_allowed_domains(allowed_domains),
max_response_size,
timeout_secs,
allow_private_hosts,
}
}
@@ -52,7 +55,7 @@ impl HttpRequestTool {
let host = extract_host(url)?;
if is_private_or_local_host(&host) {
if !self.allow_private_hosts && is_private_or_local_host(&host) {
anyhow::bail!("Blocked local/private host: {host}");
}
@@ -454,6 +457,13 @@ mod tests {
use crate::security::{AutonomyLevel, SecurityPolicy};
fn test_tool(allowed_domains: Vec<&str>) -> HttpRequestTool {
test_tool_with_private(allowed_domains, false)
}
fn test_tool_with_private(
allowed_domains: Vec<&str>,
allow_private_hosts: bool,
) -> HttpRequestTool {
let security = Arc::new(SecurityPolicy {
autonomy: AutonomyLevel::Supervised,
..SecurityPolicy::default()
@@ -463,6 +473,7 @@ mod tests {
allowed_domains.into_iter().map(String::from).collect(),
1_000_000,
30,
allow_private_hosts,
)
}
@@ -570,7 +581,7 @@ mod tests {
#[test]
fn validate_requires_allowlist() {
let security = Arc::new(SecurityPolicy::default());
let tool = HttpRequestTool::new(security, vec![], 1_000_000, 30);
let tool = HttpRequestTool::new(security, vec![], 1_000_000, 30, false);
let err = tool
.validate_url("https://example.com")
.unwrap_err()
@@ -686,7 +697,7 @@ mod tests {
autonomy: AutonomyLevel::ReadOnly,
..SecurityPolicy::default()
});
let tool = HttpRequestTool::new(security, vec!["example.com".into()], 1_000_000, 30);
let tool = HttpRequestTool::new(security, vec!["example.com".into()], 1_000_000, 30, false);
let result = tool
.execute(json!({"url": "https://example.com"}))
.await
@@ -701,7 +712,7 @@ mod tests {
max_actions_per_hour: 0,
..SecurityPolicy::default()
});
let tool = HttpRequestTool::new(security, vec!["example.com".into()], 1_000_000, 30);
let tool = HttpRequestTool::new(security, vec!["example.com".into()], 1_000_000, 30, false);
let result = tool
.execute(json!({"url": "https://example.com"}))
.await
@@ -724,6 +735,7 @@ mod tests {
vec!["example.com".into()],
10,
30,
false,
);
let text = "hello world this is long";
let truncated = tool.truncate_response(text);
@@ -738,6 +750,7 @@ mod tests {
vec!["example.com".into()],
0, // max_response_size = 0 means no limit
30,
false,
);
let text = "a".repeat(10_000_000);
assert_eq!(tool.truncate_response(&text), text);
@@ -750,6 +763,7 @@ mod tests {
vec!["example.com".into()],
5,
30,
false,
);
let text = "hello world";
let truncated = tool.truncate_response(text);
@@ -935,4 +949,70 @@ mod tests {
.to_string();
assert!(err.contains("IPv6"));
}
// ── allow_private_hosts opt-in tests ────────────────────────
#[test]
fn default_blocks_private_hosts() {
let tool = test_tool(vec!["localhost", "192.168.1.5", "*"]);
assert!(tool
.validate_url("https://localhost:8080")
.unwrap_err()
.to_string()
.contains("local/private"));
assert!(tool
.validate_url("https://192.168.1.5")
.unwrap_err()
.to_string()
.contains("local/private"));
assert!(tool
.validate_url("https://10.0.0.1")
.unwrap_err()
.to_string()
.contains("local/private"));
}
#[test]
fn allow_private_hosts_permits_localhost() {
let tool = test_tool_with_private(vec!["localhost"], true);
assert!(tool.validate_url("https://localhost:8080").is_ok());
}
#[test]
fn allow_private_hosts_permits_private_ipv4() {
let tool = test_tool_with_private(vec!["192.168.1.5"], true);
assert!(tool.validate_url("https://192.168.1.5").is_ok());
}
#[test]
fn allow_private_hosts_permits_rfc1918_with_wildcard() {
let tool = test_tool_with_private(vec!["*"], true);
assert!(tool.validate_url("https://10.0.0.1").is_ok());
assert!(tool.validate_url("https://172.16.0.1").is_ok());
assert!(tool.validate_url("https://192.168.1.1").is_ok());
assert!(tool.validate_url("http://localhost:8123").is_ok());
}
#[test]
fn allow_private_hosts_still_requires_allowlist() {
let tool = test_tool_with_private(vec!["example.com"], true);
let err = tool
.validate_url("https://192.168.1.5")
.unwrap_err()
.to_string();
assert!(
err.contains("allowed_domains"),
"Private host should still need allowlist match, got: {err}"
);
}
#[test]
fn allow_private_hosts_false_still_blocks() {
let tool = test_tool_with_private(vec!["*"], false);
assert!(tool
.validate_url("https://localhost:8080")
.unwrap_err()
.to_string()
.contains("local/private"));
}
}
+1
View File
@@ -314,6 +314,7 @@ pub fn all_tools_with_runtime(
http_config.allowed_domains.clone(),
http_config.max_response_size,
http_config.timeout_secs,
http_config.allow_private_hosts,
)));
}