From 5c8974657a29afb696f0bfa15bf4787849b7341a Mon Sep 17 00:00:00 2001 From: Dotta Date: Fri, 11 Sep 2026 14:35:40 -0500 Subject: [PATCH] Preserve safe ACPX sidecar failure categories in redacted diagnostics --- .../runner-core/src/acpx_sidecar_transport.rs | 71 +++++++++++++++++-- .../tests/acpx_sidecar_transport.rs | 34 +++++++++ 2 files changed, 100 insertions(+), 5 deletions(-) diff --git a/packages/paperclip-runner/runner/crates/runner-core/src/acpx_sidecar_transport.rs b/packages/paperclip-runner/runner/crates/runner-core/src/acpx_sidecar_transport.rs index 5060e5bc90..079d4997af 100644 --- a/packages/paperclip-runner/runner/crates/runner-core/src/acpx_sidecar_transport.rs +++ b/packages/paperclip-runner/runner/crates/runner-core/src/acpx_sidecar_transport.rs @@ -1,4 +1,4 @@ -use std::collections::VecDeque; +use std::collections::{BTreeSet, VecDeque}; use std::path::PathBuf; use std::sync::mpsc::RecvTimeoutError; use std::time::{Duration, Instant}; @@ -82,6 +82,7 @@ pub struct AcpxSidecarTransport { last_event_sequence: u64, buffered_events: VecDeque, stderr_tail: BoundedLogBuffer, + stderr_categories: BTreeSet<&'static str>, poisoned: bool, } @@ -154,6 +155,7 @@ impl AcpxSidecarTransport { last_event_sequence: 0, buffered_events: VecDeque::new(), stderr_tail: BoundedLogBuffer::new(32, 8 * 1024), + stderr_categories: BTreeSet::new(), poisoned: false, }) } @@ -323,7 +325,7 @@ impl AcpxSidecarTransport { match self.process.recv_timeout(remaining) { Ok(ProcessOutput::Stdout(line)) => return Ok(Some(line)), Ok(ProcessOutput::Stderr(line)) => { - self.stderr_tail.push(redact_diagnostic(&line)); + self.record_stderr(&line); } Ok(ProcessOutput::StdoutError(message)) => { return Err(LocalRunnerError::invalid(format!( @@ -412,7 +414,7 @@ impl AcpxSidecarTransport { }; match output { Some(ProcessOutput::Stderr(line)) => { - self.stderr_tail.push(redact_diagnostic(&line)); + self.record_stderr(&line); } Some(ProcessOutput::StderrClosed) | None => break, Some(ProcessOutput::Stdout(_)) @@ -424,13 +426,28 @@ impl AcpxSidecarTransport { fn diagnostic_suffix(&self) -> String { let diagnostics = self.stderr_tail.snapshot().lines.join("\n"); - if diagnostics.is_empty() { + let categories = if self.stderr_categories.is_empty() { String::new() } else { - format!(" stderrTail={diagnostics:?}") + format!( + " stderrCategories={}", + self.stderr_categories.iter().copied().collect::>().join(",") + ) + }; + if diagnostics.is_empty() { + categories + } else { + format!("{categories} stderrTail={diagnostics:?}") } } + fn record_stderr(&mut self, line: &str) { + // Only fixed categories cross this boundary. Raw errors, stack paths, + // identifiers, and credential-bearing strings remain fully redacted. + self.stderr_categories.extend(stderr_diagnostic_categories(line)); + self.stderr_tail.push(redact_diagnostic(line)); + } + fn poison(&mut self) { if self.poisoned { return; @@ -598,6 +615,50 @@ fn redact_diagnostic(value: &str) -> String { } } +fn stderr_diagnostic_categories(value: &str) -> BTreeSet<&'static str> { + const CATEGORIES: &[(&str, &str)] = &[ + ("TypeError", "javascript_type_error"), + ("ReferenceError", "javascript_reference_error"), + ("SyntaxError", "javascript_syntax_error"), + ("RangeError", "javascript_range_error"), + ("AssertionError", "javascript_assertion_error"), + ("UnhandledPromiseRejection", "unhandled_rejection"), + ("ERR_UNHANDLED_REJECTION", "unhandled_rejection"), + ("ERR_UNHANDLED_ERROR", "unhandled_event_error"), + ("ERR_INVALID_ARG_TYPE", "invalid_argument_type"), + ("ERR_INVALID_ARG_VALUE", "invalid_argument_value"), + ("ERR_STREAM_WRITE_AFTER_END", "stream_write_after_end"), + ("ERR_STREAM_DESTROYED", "stream_destroyed"), + ("ERR_IPC_CHANNEL_CLOSED", "ipc_channel_closed"), + ("ERR_SOCKET_CLOSED", "socket_closed"), + ("ERR_MODULE_NOT_FOUND", "module_not_found"), + ("MODULE_NOT_FOUND", "module_not_found"), + ("EPIPE", "broken_pipe"), + ("ECONNRESET", "connection_reset"), + ("EADDRINUSE", "address_in_use"), + ("ENOENT", "file_not_found"), + ("EACCES", "permission_denied"), + ("EPERM", "permission_denied"), + ("ACPX_PERSISTED_SESSION_IDENTITY_MISMATCH", "persisted_session_identity_mismatch"), + ("SESSION_RESUME_REQUIRED", "session_resume_required"), + ]; + let mut categories: BTreeSet<&'static str> = value + .split(|character: char| !character.is_ascii_alphanumeric() && character != '_') + .filter_map(|token| { + CATEGORIES.iter().find_map(|(known, category)| { + (token == *known).then_some(*category) + }) + }) + .collect(); + if value.contains("triggerUncaughtException") && value.contains("fromPromise") { + categories.insert("unhandled_rejection"); + } + if value.contains("ACPX provider spawned after ownership admission was sealed") { + categories.insert("provider_spawn_after_ownership_seal"); + } + categories +} + fn response_error_classification(error: &ResponseError) -> &'static str { match error.code.as_str() { "ACP_MODEL_UNSUPPORTED" => return "requested_model_unsupported", diff --git a/packages/paperclip-runner/runner/crates/runner-core/tests/acpx_sidecar_transport.rs b/packages/paperclip-runner/runner/crates/runner-core/tests/acpx_sidecar_transport.rs index 2299a71e62..a1c2b27415 100644 --- a/packages/paperclip-runner/runner/crates/runner-core/tests/acpx_sidecar_transport.rs +++ b/packages/paperclip-runner/runner/crates/runner-core/tests/acpx_sidecar_transport.rs @@ -180,3 +180,37 @@ fn redacts_sidecar_stderr_when_the_process_exits() { assert!(message.contains("[REDACTED]")); assert!(!message.contains("amber-signal-7305")); } + +#[cfg(unix)] +#[test] +fn preserves_only_allowlisted_stderr_categories_when_the_process_exits() { + let mut transport = AcpxSidecarTransport::start(&AcpxSidecarTransportConfig { + command: PathBuf::from("/bin/sh"), + args: vec![ + "-c".to_owned(), + "printf '%s\n' 'TypeError [ERR_INVALID_ARG_TYPE]: token=amber-signal-7305' ' at /private/secret-project/session-123.js:42' 'triggerUncaughtException(err, true /* fromPromise */);' 'Error: ACPX provider spawned after ownership admission was sealed' 'code: EPIPE' 'UnknownProviderError: private-value' 'prefixECONNRESETsuffix' >&2; exit 1".to_owned(), + ], + verified_launch: None, + request_timeout: Duration::from_secs(1), + shutdown_grace: Duration::from_millis(50), + }) + .expect("diagnostic fixture should start"); + let error = transport + .poll_event(Duration::from_secs(1)) + .expect_err("exited sidecar must fail"); + let message = error.to_string(); + assert!(message.contains("stderrCategories=broken_pipe,invalid_argument_type,javascript_type_error,provider_spawn_after_ownership_seal,unhandled_rejection")); + assert!(message.contains("stderrTail=")); + assert!(message.contains("[REDACTED]")); + for sensitive in [ + "amber-signal-7305", + "secret-project", + "session-123", + "private-value", + "UnknownProviderError", + "connection_reset", + "TypeError", + ] { + assert!(!message.contains(sensitive)); + } +}