Improve Windows sandbox error details and registry cleanup (#46237)

## Why

Provisioning errors omitted underlying causes, and removing a legacy installation record could leave an empty registry key behind.

## What changed

- Include error chains in provisioning responses and failure logs, retaining control-character replacement and the response byte limit.
- Attempt to delete the legacy installation registry key after removing and flushing its record, preserving any remaining values or subkeys.

## Testing

Add a regression test for nested error causes, control-character replacement, and UTF-8 truncation within the response byte limit.

GitOrigin-RevId: 8fccfb49c708be5f5d5be7180289cb74a66498a2
(cherry picked from commit c11fdc944f)
This commit is contained in:
chess
2026-09-17 18:43:45 -07:00
committed by Ian MacLeod
parent 434535bddf
commit 9c8e46d32d
4 changed files with 32 additions and 3 deletions
@@ -10,6 +10,7 @@ use serde::Deserialize;
use serde::Serialize;
use windows_sys::Win32::Foundation as foundation;
use windows_sys::Win32::System::Registry as registry;
use windows_sys::Win32::UI::Shell::SHDeleteEmptyKeyW;
/// Selects legacy helper materialization or verified app-contained Core for setup.
#[derive(Clone, Copy, Debug, Default, Deserialize, Eq, PartialEq, Serialize)]
@@ -188,7 +189,17 @@ pub fn remove_installation() -> Result<()> {
match status {
foundation::ERROR_SUCCESS
| foundation::ERROR_FILE_NOT_FOUND
| foundation::ERROR_PATH_NOT_FOUND => flush_installation(INSTALLATION_KEY),
| foundation::ERROR_PATH_NOT_FOUND => {
flush_installation(INSTALLATION_KEY)?;
// Best-effort pruning preserves any remaining values or subkeys.
let _ = unsafe {
SHDeleteEmptyKeyW(
registry::HKEY_LOCAL_MACHINE,
to_wide(INSTALLATION_KEY).as_ptr(),
)
};
Ok(())
}
status => Err(io::Error::from_raw_os_error(status as i32))
.context("remove protected legacy sandbox installation record"),
}
+1 -1
View File
@@ -190,7 +190,7 @@ fn write_response(
fn response_error_message(error: &anyhow::Error) -> String {
let mut message = String::new();
for character in error.to_string().chars() {
for character in format!("{error:#}").chars() {
let character = if character.is_control() {
' '
} else {
@@ -1,3 +1,4 @@
use super::MAX_RESPONSE_MESSAGE_BYTES;
use super::OwnedHandle;
use super::PipeConnection;
use super::ServiceRequest;
@@ -8,6 +9,7 @@ use super::pin_existing_ancestors;
use super::pipe_security_descriptor;
use super::refresh_session;
use super::request::ProvisioningRequest;
use super::response_error_message;
use super::validate_request;
use super::wake;
use codex_windows_sandbox::DirectoryOpenDisposition;
@@ -37,6 +39,22 @@ use windows_sys::Win32::Storage::FileSystem as filesystem;
use windows_sys::Win32::Storage::Packaging::Appx;
use windows_sys::Win32::System::Pipes as pipes;
#[test]
fn provisioning_error_response_preserves_causes_within_message_limits() {
let error = anyhow::anyhow!("Windows error\t5\n")
.context("load profile")
.context("registered sandbox provisioning failed");
assert_eq!(
response_error_message(&error),
"registered sandbox provisioning failed: load profile: Windows error 5 "
);
let error = anyhow::anyhow!("é".repeat(MAX_RESPONSE_MESSAGE_BYTES)).context("setup");
let message = response_error_message(&error);
assert!(message.starts_with("setup: é"));
assert_eq!(message.len(), MAX_RESPONSE_MESSAGE_BYTES - 1);
}
#[test]
fn session_refresh_observes_cleanup_completion_before_dispatch() {
let shutdown = AtomicBool::new(false);
@@ -115,7 +115,7 @@ pub(super) fn run(
.inspect_err(|error| {
crate::service::log_error(
crate::service::EVENT_PROVISIONING_FAILED,
&format!("Codex sandbox provisioning failed: {error}"),
&format!("Codex sandbox provisioning failed: {error:#}"),
);
})
.context("registered sandbox provisioning failed")?;