Skip to content

Commit 9eee92e

Browse files
authored
fix(security): round command-log truncation to UTF-8 boundary (tinyhumansai#1817)
1 parent 5411f19 commit 9eee92e

2 files changed

Lines changed: 63 additions & 5 deletions

File tree

‎src/openhuman/security/policy.rs‎

Lines changed: 7 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -4,6 +4,8 @@ use serde::{Deserialize, Serialize};
44
use std::path::{Path, PathBuf};
55
use std::time::Instant;
66

7+
use crate::openhuman::util::floor_char_boundary;
8+
79
/// How much autonomy the agent has
810
#[derive(Debug, Clone, Copy, Default, PartialEq, Eq, Serialize, Deserialize, JsonSchema)]
911
#[serde(rename_all = "lowercase")]
@@ -498,7 +500,7 @@ impl SecurityPolicy {
498500
if !self.is_command_allowed(command) {
499501
log::warn!(
500502
"[openhuman:policy] Command blocked by allowlist: {}",
501-
&command[..command.len().min(80)]
503+
&command[..floor_char_boundary(command, 80)]
502504
);
503505
return Err(format!("Command not allowed by security policy: {command}"));
504506
}
@@ -509,14 +511,14 @@ impl SecurityPolicy {
509511
if self.block_high_risk_commands {
510512
log::warn!(
511513
"[openhuman:policy] High-risk command blocked: {}",
512-
&command[..command.len().min(80)]
514+
&command[..floor_char_boundary(command, 80)]
513515
);
514516
return Err("Command blocked: high-risk command is disallowed by policy".into());
515517
}
516518
if self.autonomy == AutonomyLevel::Supervised && !approved {
517519
log::warn!(
518520
"[openhuman:policy] High-risk command needs approval: {}",
519-
&command[..command.len().min(80)]
521+
&command[..floor_char_boundary(command, 80)]
520522
);
521523
return Err(
522524
"Command requires explicit approval (approved=true): high-risk operation"
@@ -532,7 +534,7 @@ impl SecurityPolicy {
532534
{
533535
log::info!(
534536
"[openhuman:policy] Medium-risk command needs approval: {}",
535-
&command[..command.len().min(80)]
537+
&command[..floor_char_boundary(command, 80)]
536538
);
537539
return Err(
538540
"Command requires explicit approval (approved=true): medium-risk operation".into(),
@@ -543,7 +545,7 @@ impl SecurityPolicy {
543545
"[openhuman:policy] Command validated: risk={:?}, approved={}, cmd={}",
544546
risk,
545547
approved,
546-
&command[..command.len().min(80)]
548+
&command[..floor_char_boundary(command, 80)]
547549
);
548550
Ok(risk)
549551
}

‎src/openhuman/security/policy_tests.rs‎

Lines changed: 56 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -251,6 +251,62 @@ fn validate_command_rejects_background_chain_bypass() {
251251
assert!(result.unwrap_err().contains("not allowed"));
252252
}
253253

254+
// Regression: OPENHUMAN-TAURI-GW (#1813). A multi-byte UTF-8 char straddling
255+
// byte 80 of the command string used to panic the log truncator with
256+
// `byte index 80 is not a char boundary`, killing the core thread. All five
257+
// `&command[..80]` log sites must now round down to a UTF-8 boundary.
258+
#[test]
259+
fn validate_command_does_not_panic_on_multibyte_char_at_log_truncation_boundary() {
260+
// Real-world Sentry repro: `cmd /c "dir /b "%USERPROFILE%\Desktop\*.lnk"
261+
// 2>nul | findstr /i "Warcraft WoW 魔兽 Battle"` — the 3-byte `'魔'`
262+
// occupies bytes 78..81, so a naked `&command[..80]` panics.
263+
let cmd = "cmd /c \"dir /b \"%USERPROFILE%\\Desktop\\*.lnk\" 2>nul | findstr /i \"Warcraft WoW 魔兽 Battle\"";
264+
assert!(
265+
cmd.len() > 80,
266+
"test fixture must be long enough to trigger truncation"
267+
);
268+
assert!(
269+
!cmd.is_char_boundary(80),
270+
"test fixture must place a multi-byte char across byte 80"
271+
);
272+
273+
// Exercise the allowlist-deny path (cmd starts with "cmd" which is not on
274+
// the default allowlist), which fires the truncating warn! at policy.rs.
275+
let p = default_policy();
276+
let result = p.validate_command_execution(cmd, false);
277+
assert!(
278+
result.is_err(),
279+
"command should be blocked, but did not panic"
280+
);
281+
282+
// And the high-risk-blocked path: allowlist passes (curl is allowed), then
283+
// risk gate fires (curl is a high-risk command), exercising the truncating
284+
// warn! site at the block_high_risk_commands branch.
285+
let prefix = "curl https://example.com/";
286+
let filler = "a".repeat(80 - prefix.len() - 1);
287+
let high_risk_cmd = format!("{prefix}{filler}魔");
288+
assert!(
289+
!high_risk_cmd.is_char_boundary(80),
290+
"fixture must straddle byte 80 with a multi-byte char"
291+
);
292+
let high_risk_policy = SecurityPolicy {
293+
allowed_commands: vec!["curl".into()],
294+
..SecurityPolicy::default()
295+
};
296+
let blocked = high_risk_policy.validate_command_execution(&high_risk_cmd, true);
297+
assert!(blocked.is_err());
298+
assert!(blocked.unwrap_err().contains("high-risk"));
299+
}
300+
301+
// Pathological short multi-byte command — exercises the boundary logic at the
302+
// edge case where `cmd.len() < 80`.
303+
#[test]
304+
fn validate_command_handles_short_multibyte_command() {
305+
let p = default_policy();
306+
// 6 bytes (two 3-byte CJK chars) — well under the 80-byte log cap.
307+
let _ = p.validate_command_execution("魔兽", false);
308+
}
309+
254310
// -- is_path_allowed ----------------------------------------------
255311

256312
#[test]

0 commit comments

Comments
 (0)