Skip to content
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
18 changes: 17 additions & 1 deletion src/backends/isolation_session/common/src/error.rs
Original file line number Diff line number Diff line change
Expand Up @@ -61,8 +61,24 @@ const ERROR_NOT_FOUND_HRESULT: u32 = 0x80070490;
/// Formats an `IsoSessionError` into a typed `IsolationSessionError`.
/// Promotes `ERROR_NOT_FOUND` to `Stale`.
pub(super) fn format_iso_error(op: &str, err: &IsoSessionError) -> IsolationSessionError {
// Read `Code()` first and propagate its failure honestly: it is the
// classification-critical field (it drives the `Stale` promotion below),
// so fabricating 0 on a getter failure would silently downgrade a stale
// sandbox to a generic lifecycle error. `Message`/`Remediation` are
// cosmetic and stay best-effort.
let code = match err.Code() {
Ok(c) => c.0 as u32,
Err(e) => {
// Code() is gone, but Message() may still carry signal — fold in
// the best-effort text so the failure is diagnosable.
let msg = err.Message().map(|h| h.to_string()).unwrap_or_default();
return IsolationSessionError::Lifecycle(format!(
"{} failed: {} (could not read HRESULT code: {})",
op, msg, e
));
}
};
let msg = err.Message().map(|h| h.to_string()).unwrap_or_default();
let code = err.Code().map(|h| h.0 as u32).unwrap_or(0);
let remediation = err.Remediation().map(|h| h.to_string()).unwrap_or_default();
let suffix = if remediation.is_empty() {
String::new()
Expand Down
40 changes: 34 additions & 6 deletions src/backends/isolation_session/common/src/manager.rs
Original file line number Diff line number Diff line change
Expand Up @@ -137,7 +137,9 @@ impl IsolationSessionManager {
let err = user_result
.Error()
.map_err(|e| lifecycle_err(format!("AddUserAsync: get Error failed: {}", e)))?;
let is_error = err.IsError().unwrap_or(false);
let is_error = err
.IsError()
.map_err(|e| lifecycle_err(format!("AddUserAsync: get IsError failed: {}", e)))?;
if is_error {
return Err(format_iso_error("AddUserAsync", &err));
}
Expand Down Expand Up @@ -177,7 +179,9 @@ impl IsolationSessionManager {
let err = user_result
.Error()
.map_err(|e| lifecycle_err(format!("AddUserAsync2: get Error failed: {}", e)))?;
let is_error = err.IsError().unwrap_or(false);
let is_error = err
.IsError()
.map_err(|e| lifecycle_err(format!("AddUserAsync2: get IsError failed: {}", e)))?;
if is_error {
return Err(format_iso_error("AddUserAsync2", &err));
}
Expand Down Expand Up @@ -303,7 +307,12 @@ impl IsolationSessionManager {
e
))
})?;
let is_error = err.IsError().unwrap_or(false);
let is_error = err.IsError().map_err(|e| {
lifecycle_err(format!(
"RunProcessWithOptionsAsync: get IsError failed: {}",
e
))
})?;
if is_error {
return Err(format_iso_error("RunProcessWithOptionsAsync", &err));
}
Expand Down Expand Up @@ -333,9 +342,28 @@ impl IsolationSessionManager {
// Lifetime: relay-param structs are stack-allocated; we wait on
// every spawned thread (INFINITE for stdout/stderr, bounded for
// stdin) before this function returns.
let stdout_handle_val = process.OutputHandle().unwrap_or(0);
let stderr_handle_val = process.ErrorHandle().unwrap_or(0);
let stdin_handle_val = process.InputHandle().unwrap_or(0);
// A getter that errors is a backend failure, not an absent stream:
// propagate it rather than coercing to 0, which downstream treats as
// "no handle" and silently skips the corresponding stdio relay. A
// genuinely returned 0 still means absent and is preserved.
let stdout_handle_val = process.OutputHandle().map_err(|e| {
lifecycle_err(format!(
"RunProcessWithOptionsAsync: get OutputHandle failed: {}",
e
))
})?;
let stderr_handle_val = process.ErrorHandle().map_err(|e| {
lifecycle_err(format!(
"RunProcessWithOptionsAsync: get ErrorHandle failed: {}",
e
))
})?;
let stdin_handle_val = process.InputHandle().map_err(|e| {
lifecycle_err(format!(
"RunProcessWithOptionsAsync: get InputHandle failed: {}",
e
))
})?;

let wxc_stdout = unsafe { GetStdHandle(STD_OUTPUT_HANDLE) }
.map_err(|e| lifecycle_err(format!("GetStdHandle(stdout) failed: {}", e)))?;
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -167,8 +167,12 @@ impl WindowsSandboxScriptRunner {
if response_line.starts_with("RESULT ") {
match DaemonResult::parse(response_line) {
Ok(result) => {
let stdout_text = String::from_utf8(result.stdout).unwrap_or_default();
let stderr_text = String::from_utf8(result.stderr).unwrap_or_default();
// Lossy decode: the daemon transports raw bytes, so a
// single invalid UTF-8 byte must not discard the entire
// stream (which `String::from_utf8(...).unwrap_or_default()`
// would do). Replace invalid sequences with U+FFFD instead.
let stdout_text = String::from_utf8_lossy(&result.stdout).into_owned();
let stderr_text = String::from_utf8_lossy(&result.stderr).into_owned();
ScriptResponse {
exit_code: result.exit_code,
standard_out: stdout_text,
Expand Down Expand Up @@ -236,4 +240,27 @@ mod tests {
daemon_pipe_name_to_port("custom-pipe")
);
}

#[test]
fn parse_daemon_response_preserves_non_utf8_output() {
// A single invalid UTF-8 byte must not discard the whole stream.
let line = DaemonResult {
exit_code: 0,
stdout: vec![b'h', b'i', 0xFF, b'!'],
stderr: vec![0xFE, b'e', b'r', b'r'],
error_message: String::new(),
}
.to_line();

let resp = WindowsSandboxScriptRunner::parse_daemon_response(line.trim());

assert_eq!(resp.exit_code, 0);
// U+FFFD replacement char is kept; the surrounding bytes survive.
assert!(resp.standard_out.contains("hi"));
assert!(resp.standard_out.contains('!'));
assert!(resp.standard_out.contains('\u{FFFD}'));
assert!(resp.standard_err.contains("err"));
// error_message falls back to stderr text when empty.
assert!(resp.error_message.contains("err"));
}
}
Loading