From 9eee92e336ce8b21ee35678339c6b7cf9ab21bbc Mon Sep 17 00:00:00 2001 From: sanil-23 Date: Sat, 16 May 2026 01:00:09 +0530 Subject: [PATCH] fix(security): round command-log truncation to UTF-8 boundary (#1817) --- src/openhuman/security/policy.rs | 12 +++--- src/openhuman/security/policy_tests.rs | 56 ++++++++++++++++++++++++++ 2 files changed, 63 insertions(+), 5 deletions(-) diff --git a/src/openhuman/security/policy.rs b/src/openhuman/security/policy.rs index 7b3cb4bc5..24705d427 100644 --- a/src/openhuman/security/policy.rs +++ b/src/openhuman/security/policy.rs @@ -4,6 +4,8 @@ use serde::{Deserialize, Serialize}; use std::path::{Path, PathBuf}; use std::time::Instant; +use crate::openhuman::util::floor_char_boundary; + /// How much autonomy the agent has #[derive(Debug, Clone, Copy, Default, PartialEq, Eq, Serialize, Deserialize, JsonSchema)] #[serde(rename_all = "lowercase")] @@ -498,7 +500,7 @@ impl SecurityPolicy { if !self.is_command_allowed(command) { log::warn!( "[openhuman:policy] Command blocked by allowlist: {}", - &command[..command.len().min(80)] + &command[..floor_char_boundary(command, 80)] ); return Err(format!("Command not allowed by security policy: {command}")); } @@ -509,14 +511,14 @@ impl SecurityPolicy { if self.block_high_risk_commands { log::warn!( "[openhuman:policy] High-risk command blocked: {}", - &command[..command.len().min(80)] + &command[..floor_char_boundary(command, 80)] ); return Err("Command blocked: high-risk command is disallowed by policy".into()); } if self.autonomy == AutonomyLevel::Supervised && !approved { log::warn!( "[openhuman:policy] High-risk command needs approval: {}", - &command[..command.len().min(80)] + &command[..floor_char_boundary(command, 80)] ); return Err( "Command requires explicit approval (approved=true): high-risk operation" @@ -532,7 +534,7 @@ impl SecurityPolicy { { log::info!( "[openhuman:policy] Medium-risk command needs approval: {}", - &command[..command.len().min(80)] + &command[..floor_char_boundary(command, 80)] ); return Err( "Command requires explicit approval (approved=true): medium-risk operation".into(), @@ -543,7 +545,7 @@ impl SecurityPolicy { "[openhuman:policy] Command validated: risk={:?}, approved={}, cmd={}", risk, approved, - &command[..command.len().min(80)] + &command[..floor_char_boundary(command, 80)] ); Ok(risk) } diff --git a/src/openhuman/security/policy_tests.rs b/src/openhuman/security/policy_tests.rs index 70bef472a..07d174f22 100644 --- a/src/openhuman/security/policy_tests.rs +++ b/src/openhuman/security/policy_tests.rs @@ -251,6 +251,62 @@ fn validate_command_rejects_background_chain_bypass() { assert!(result.unwrap_err().contains("not allowed")); } +// Regression: OPENHUMAN-TAURI-GW (#1813). A multi-byte UTF-8 char straddling +// byte 80 of the command string used to panic the log truncator with +// `byte index 80 is not a char boundary`, killing the core thread. All five +// `&command[..80]` log sites must now round down to a UTF-8 boundary. +#[test] +fn validate_command_does_not_panic_on_multibyte_char_at_log_truncation_boundary() { + // Real-world Sentry repro: `cmd /c "dir /b "%USERPROFILE%\Desktop\*.lnk" + // 2>nul | findstr /i "Warcraft WoW 魔兽 Battle"` — the 3-byte `'魔'` + // occupies bytes 78..81, so a naked `&command[..80]` panics. + let cmd = "cmd /c \"dir /b \"%USERPROFILE%\\Desktop\\*.lnk\" 2>nul | findstr /i \"Warcraft WoW 魔兽 Battle\""; + assert!( + cmd.len() > 80, + "test fixture must be long enough to trigger truncation" + ); + assert!( + !cmd.is_char_boundary(80), + "test fixture must place a multi-byte char across byte 80" + ); + + // Exercise the allowlist-deny path (cmd starts with "cmd" which is not on + // the default allowlist), which fires the truncating warn! at policy.rs. + let p = default_policy(); + let result = p.validate_command_execution(cmd, false); + assert!( + result.is_err(), + "command should be blocked, but did not panic" + ); + + // And the high-risk-blocked path: allowlist passes (curl is allowed), then + // risk gate fires (curl is a high-risk command), exercising the truncating + // warn! site at the block_high_risk_commands branch. + let prefix = "curl https://example.com/"; + let filler = "a".repeat(80 - prefix.len() - 1); + let high_risk_cmd = format!("{prefix}{filler}魔"); + assert!( + !high_risk_cmd.is_char_boundary(80), + "fixture must straddle byte 80 with a multi-byte char" + ); + let high_risk_policy = SecurityPolicy { + allowed_commands: vec!["curl".into()], + ..SecurityPolicy::default() + }; + let blocked = high_risk_policy.validate_command_execution(&high_risk_cmd, true); + assert!(blocked.is_err()); + assert!(blocked.unwrap_err().contains("high-risk")); +} + +// Pathological short multi-byte command — exercises the boundary logic at the +// edge case where `cmd.len() < 80`. +#[test] +fn validate_command_handles_short_multibyte_command() { + let p = default_policy(); + // 6 bytes (two 3-byte CJK chars) — well under the 80-byte log cap. + let _ = p.validate_command_execution("魔兽", false); +} + // -- is_path_allowed ---------------------------------------------- #[test]