scriptoriumadmin/fabro
1
0
Fork 0
mirror of https://github.com/fabro-sh/fabro.git synced 2026-09-15 23:32:46 +00:00
Code Issues Projects Releases Packages Wiki Activity Actions 5

Let the driver strip terminal control sequences from command output

Browse source 
fabro-sandbox kept its own sanitize_exec_output, a character walker that
removed ANSI escape sequences and control characters from a command's
output tail after redaction. The sandbox driver already offers this as
ExecSpec::output_sanitization, applied chunk-safely to buffered and
streaming output, so fabro carried a second, weaker copy of the same
logic that only ran on the rendered tail and never on the streams the
agent, the command stage, or the sink consumers read.

SandboxExec::apply_policy now sets OutputSanitization::StripAll on any
spec still at the driver's raw default, so every run and run_streaming
call through fabro's exec policy returns text with escape sequences and
stray control characters already removed. A caller that chose another
policy keeps it. spawn_stdio is untouched: long-lived stdio processes
stay raw, as the driver requires. redacted_tail now only redacts secrets
and applies the byte cap, which remain fabro's knowledge, and the
private sanitizer is gone. The tail test that built an ExecResult by
hand now runs a printf through the Host provider and checks that the
stripped output reaches both the result and the tail, and a new test
pins the policy's default and its respect for an explicit choice.

Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
This commit is contained in:
Bryan Helmkamp 2026-09-11 14:47:07 -06:00
parent 705f411c56
commit ab3ef9371a
No known key found for this signature in database
2 changed files with 69 additions and 72 deletions
Show stats Download patch file Download diff file Expand all files Collapse all files

82
lib/components/fabro-sandbox/src/exec.rs
View file

@ -16,10 +16,14 @@
//! [`Termination::Cancelled`].
//! - **exit**: the process ended on its own.
//!
//! Output is drained regardless of the retention cap, redacted only when a
//! tail is rendered for events or logs ([`ExecResultExt`]), and delivered
//! live through the caller's [`sandbox_driver::OutputSink`]. Explicit
//! environment variables pass through a fail-closed secret filter under
//! Output is drained regardless of the retention cap and delivered live
//! through the caller's [`sandbox_driver::OutputSink`]. Fabro reads command
//! output as text, so the policy asks the driver for
//! [`OutputSanitization::StripAll`]: terminal escape sequences and stray
//! control characters never reach a result, a sink chunk, or a tail. Secret
//! redaction stays fabro's job and happens only when a tail is rendered for
//! events or logs ([`ExecResultExt`]). Explicit environment variables pass
//! through a fail-closed secret filter under
//! [`ExplicitEnvPolicy::FilterSensitive`], matching what the Host provider
//! already does for inherited variables.
@ -29,8 +33,8 @@ use std::time::Duration;
use fabro_static::EnvVars;
use fabro_types::{CommandTermination, ExecOutputTail};
use sandbox_driver::{
Exec, ExecControls, ExecFailure, ExecResult, ExecSpec, ExecStreamingResult, SpawnSpec,
StdioProcess, Termination,
Exec, ExecControls, ExecFailure, ExecResult, ExecSpec, ExecStreamingResult, OutputSanitization,
SpawnSpec, StdioProcess, Termination,
};
use tokio_util::sync::CancellationToken;
@ -161,9 +165,9 @@ impl<'a> SandboxExec<'a> {
/// `controls.sink` as it arrives.
///
/// The policy fills what the spec leaves open: the stop grace, the
/// working directory, and the explicit environment filter. The
/// caller's `controls.term` is the `term` stop; the provider runs the
/// grace and the `kill` itself. Output beyond
/// working directory, the text output policy, and the explicit
/// environment filter. The caller's `controls.term` is the `term` stop;
/// the provider runs the grace and the `kill` itself. Output beyond
/// `controls.retained_output_limit` (fabro's default when unset) is
/// drained and counted, not kept.
pub async fn run_streaming(
@ -200,6 +204,12 @@ impl<'a> SandboxExec<'a> {
Ok(self.exec.spawn_stdio(&spec).await?)
}
/// Fills what a spec leaves open. The output policy has no "unset"
/// state: the driver's default is raw, and fabro reads command output
/// as text, so a spec still at that default gets
/// [`OutputSanitization::StripAll`]; a caller that chose another policy
/// keeps it. Long-lived stdio processes ([`Self::spawn_stdio`]) and PTY
/// sessions stay raw, as the driver requires.
fn apply_policy(&self, mut spec: ExecSpec) -> ExecSpec {
if spec.stop_grace.is_none() {
spec.stop_grace = Some(self.stop_grace);
@ -207,6 +217,9 @@ impl<'a> SandboxExec<'a> {
if spec.working_dir.is_none() {
spec.working_dir.clone_from(&self.working_dir);
}
if spec.output_sanitization == OutputSanitization::default() {
spec.output_sanitization = OutputSanitization::StripAll;
}
self.apply_env_policy(&mut spec.env);
spec
}
@ -258,8 +271,10 @@ pub trait ExecResultExt {
/// [`program_exit_code`].
fn program_exit_code(&self) -> Option<i32>;
/// Redacted, sanitized tails of both streams, each bounded to
/// `max_bytes_per_stream`. `None` when both streams are empty.
/// Redacted tails of both streams, each bounded to
/// `max_bytes_per_stream`. `None` when both streams are empty. Terminal
/// control sequences were already stripped by the driver under
/// [`SandboxExec`]'s output policy.
fn redacted_output_tail(&self, max_bytes_per_stream: usize) -> Option<ExecOutputTail>;
/// [`Self::redacted_output_tail`] at fabro's event budget.
@ -732,22 +747,43 @@ mod tests {
assert!(tail.stdout_truncated);
}
#[test]
fn output_tail_sanitizes_terminal_control_sequences() {
let result = exec_result(
"\u{1b}[31mred\u{1b}[0m \u{1b}]0;window-title\u{7}shown \
\u{1b}(Bset \u{1b}Mtwo-byte \u{8}backspace",
"",
Some(1),
Termination::Exited,
1,
);
#[tokio::test]
async fn command_output_arrives_stripped_of_terminal_control_sequences() {
let fixture = HostFixture::new().await;
let result = run(
&fixture,
"printf '\\033[31mred\\033[0m \\033]0;window-title\\007shown \\033(Bset \\033Mtwo-byte \
\\bbackspace'",
)
.await;
assert!(result.success(), "{result:?}");
assert_eq!(result.stdout_lossy(), "red shown set two-byte backspace");
let tail = result
.redacted_output_tail(1024)
.expect("redacted output tail");
let stdout = tail.stdout.expect("stdout tail");
assert_eq!(stdout, "red shown set two-byte backspace");
assert_eq!(
tail.stdout.as_deref(),
Some("red shown set two-byte backspace")
);
}
#[tokio::test]
async fn policy_strips_output_unless_the_caller_chose_another_policy() {
let fixture = HostFixture::new().await;
let exec = fixture.exec(ExplicitEnvPolicy::FilterSensitive);
assert_eq!(
exec.apply_policy(ExecSpec::bash("true"))
.output_sanitization,
OutputSanitization::StripAll
);
assert_eq!(
exec.apply_policy(
ExecSpec::bash("true").output_sanitization(OutputSanitization::StripAnsi)
)
.output_sanitization,
OutputSanitization::StripAnsi
);
}
#[test]

59
lib/components/fabro-sandbox/src/sandbox.rs
View file

@ -69,9 +69,13 @@ pub fn format_lines_numbered(content: &str, offset: Option<usize>, limit: Option
result
}
/// Build a redacted `ExecOutputTail` from raw stdout/stderr without
/// fabricating a synthetic `ExecResult`. Pass `""` for either stream that
/// isn't relevant. Returns `None` when both streams are empty.
/// Build a redacted `ExecOutputTail` from stdout/stderr text without
/// fabricating a synthetic `ExecResult`. Each stream is redacted, then
/// capped to its newest `max_bytes_per_stream`. Terminal control sequences
/// are not stripped here: command output reaches fabro with them already
/// removed by the driver under [`crate::exec::SandboxExec`]'s output policy.
/// Pass `""` for either stream that isn't relevant. Returns `None` when both
/// streams are empty.
#[must_use]
pub fn redacted_output_tail(
stdout: &str,
@ -95,59 +99,16 @@ fn redacted_tail(text: &str, max_bytes: usize) -> (Option<String>, bool) {
}
let redacted = fabro_redact::redact_string(text);
let sanitized = sanitize_exec_output(&redacted);
let truncated = sanitized.len() > max_bytes;
let truncated = redacted.len() > max_bytes;
let start = if truncated {
sanitized.floor_char_boundary(sanitized.len() - max_bytes)
redacted.floor_char_boundary(redacted.len() - max_bytes)
} else {
0
};
let tail = sanitized[start..].to_string();
let tail = redacted[start..].to_string();
((!tail.is_empty()).then_some(tail), truncated)
}
fn sanitize_exec_output(text: &str) -> String {
let mut sanitized = String::with_capacity(text.len());
let mut chars = text.chars().peekable();
while let Some(ch) = chars.next() {
if ch == '\u{1b}' {
match chars.peek().copied() {
Some('[') => {
chars.next();
for next in chars.by_ref() {
if ('@'..='~').contains(&next) {
break;
}
}
}
Some(']') => {
chars.next();
let mut saw_esc = false;
for next in chars.by_ref() {
if next == '\u{7}' || (saw_esc && next == '\\') {
break;
}
saw_esc = next == '\u{1b}';
}
}
Some('(' | ')' | '*' | '+' | '-' | '.' | '/') => {
chars.next();
chars.next();
}
Some('@'..='_') => {
chars.next();
}
_ => {}
}
continue;
}
if ch == '\n' || ch == '\r' || ch == '\t' || !ch.is_control() {
sanitized.push(ch);
}
}
sanitized
}
/// A regular file discovered inside a sandbox.
#[derive(Debug, Clone, PartialEq, Eq)]
pub struct SandboxFile {