[rust] configure the Chrome sandbox after unpacking a Chrome for Testing build on Windows

[titusfortner]

🔗 Related Issues

Same cause as #14438 (the sandbox error it reports was harmless until Chrome 151 sandboxed the network service)

💥 What does this PR do?

On Windows, Selenium Manager now runs Setup executable after unpacking Chrome for Testing so it can properly use sandboxed services when not in a standard install location

🔧 Implementation Notes

• Since Chrome 151 the network service runs sandboxed on Windows; without the AppContainer ACL that step grants, it crashes and restarts about half a second after launch, and any navigation in flight is silently dropped.
• setup.exe ships in the CfT win64 zip for this purpose and is what Chrome's installer and @puppeteer/browsers run after install; Selenium Manager never did.
• The command is setup.exe --configure-browser-in-directory=<dir>; it only writes the ACLs, nothing else is installed.
• Runs once, right after uncompress, while the download lock is held. Caches unpacked before this change are not repaired; they roll over at the next Chrome milestone.
• A failure is logged and ignored so the browser is still returned.

🤖 AI assistance

• AI assisted
  • Tool(s): Claude Code (Claude Fable 5.1)
  • What was generated: the CI log analysis that identified the cause, the fix, this description
  • I reviewed all AI output and can explain the change

💡 Additional Considerations

• Alternatives considered: having the bindings pass --disable-features=NetworkServiceSandbox or --no-sandbox; rejected because it treats the symptom, weakens every session, and the feature flag is likely to go away.
• The 1s startup sleeps in the Ruby (c1f500a) and JS (81ef99c) test harnesses work around this and can be removed once a Selenium Manager with this change is pinned.
• A hand-unzipped CfT pointed at via binary_location is not covered; the user must run the same command once.

🔄 Types of changes

• Bug fix (backwards compatible)

[qodo-code-review]

PR Summary by Qodo

Configure the Chrome sandbox after Windows CfT downloads

🐞 Bug fix 🕐 10-20 Minutes

AI Description

• Run Chrome for Testing’s setup executable after extraction to grant required Windows sandbox
 permissions.
• Keep setup inside the download lock and warn without blocking browser use if execution fails.

Diagram

graph TD
  Manager["Browser download"] --> Unpack["Extract CfT"] --> Hook["Configuration hook"] --> Check{"Windows and setup?"}
  Check -->|Yes| Setup["Run setup.exe"] --> Return["Return browser"]
  Check -->|No| Return
  Setup -->|Execution error| Warn["Log warning"] --> Return

Loading

High-Level Assessment

The following are alternative approaches to this PR:

1. Disable the network-service sandbox at launch

• ➕ Avoids post-extraction configuration.
• ➖ Weakens every affected session and depends on a launch flag that may disappear.

2. Repair existing cached Chrome builds

• ➕ Could fix browsers downloaded before this change without waiting for cache turnover.
• ➖ Requires tracking configuration state or rerunning setup on cache hits, broadening the change.

Recommendation: Keep the post-extraction hook: it applies Chrome’s intended permission setup once while the download lock is held, without weakening the sandbox. Consider a separate cache-repair change if support for already-downloaded builds is required. Note that the existing command helper does not treat a nonzero exit status as an error, so the warning currently covers launch failures, not every setup failure.

Files changed (2) +25 / -1

Bug fix (2) +25 / -1

chrome.rsConfigure the sandbox for newly extracted Windows Chrome builds +22/-1

Configure the sandbox for newly extracted Windows Chrome builds

• Overrides the new hook to run the bundled setup.exe against the cached Chrome directory when on Windows and the executable exists. Logs an execution error as a warning rather than failing browser acquisition.

rust/src/chrome.rs

lib.rsAdd a post-extraction browser configuration hook +3/-0

Add a post-extraction browser configuration hook

• Adds a default no-op SeleniumManager hook and calls it after browser extraction, before releasing the download lock. Other browser managers retain their existing behavior.

rust/src/lib.rs

[qodo-code-review]

Code Review by Qodo

🐞 Bugs (0) 📘 Rule violations (0) 📜 Skill insights (0)

1. Sandbox setup failures stay untested ✗ Dismissed

Description

The new exit-status match in configure_browser_in_cache has no test covering a nonzero exit or a
failure to start setup.exe. Existing browser-download tests check successful resolution only, so a
change to how Windows setup failures are logged or ignored would not be caught.

Code

rust/src/chrome.rs[R283-286]

+        let outcome = match Command::new(&setup).arg(&argument).status() {
+            Ok(status) if status.success() => return,
+            Ok(status) => format!("setup.exe exited with {status}"),
+            Err(err) => err.to_string(),

Evidence

Rule 4 requires appropriate coverage for changed behavior. The changed match introduces
failure-handling branches, while the existing download tests assert successful resolution and
contain no sandbox-setup failure case.

AGENTS.md: Prefer Small Unit Tests and Avoid Misleading Mocks
rust/src/chrome.rs[283-289]
rust/tests/browser_download_tests.rs[19-45]

Agent prompt

The issue below was found during a code review. Follow the provided context and guidance below and implement a solution

## Issue description
The new exit-status handling for Chrome sandbox setup has no test for a nonzero exit or process-start failure.

## Fix Focus Areas
- rust/src/chrome.rs[283-289]
- rust/tests/browser_download_tests.rs[19-45]

## Recommended Fix
Add focused tests using a controlled setup process to verify that each failure produces a warning and browser download continues.

ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools

---

2. Failed sandbox setup goes unreported ✗ Dismissed

Description

configure_browser_in_cache warns only when run_shell_command_with_log returns an error, but the
command helper returns success without checking setup.exe's exit status. When setup fails to grant
directory permissions, the browser is still returned without a warning, and the download proceeds
despite the sandbox configuration failure.

Code

rust/src/chrome.rs[R284-286]

+        if let Err(err) = run_shell_command_with_log(self.get_logger(), command) {
+            self.get_logger()
+                .warn(format!("Unable to configure the Chrome sandbox: {err}"));

Evidence

The warning handles only an Err, while run_shell_command returns Ok(stdout) after
process.output() without inspecting the exit status or stderr. Chromium reports configuration
success with exit code 78 and failure with 79, so a failed ACL operation cannot reach the warning
through the current helper.

AGENTS.md: Log Behavior Users May Need to Diagnose
rust/src/chrome.rs[283-287]
rust/src/shell.rs[46-57]
rust/src/lib.rs[397-402]
🌐 Chromium assigns exit codes 78 and 79 to successful and failed AppContainer sandbox configuration, respectively.

Agent prompt

The issue below was found during a code review. Follow the provided context and guidance below and implement a solution

## Issue description
A failed Chrome sandbox configuration can be treated as successful because the command helper ignores `setup.exe`'s exit status. Selenium Manager should warn about configuration failure while still returning the browser.

## Fix Focus Areas
- rust/src/chrome.rs[283-287]
- rust/src/shell.rs[46-57]

## Recommended Fix
Capture `setup.exe`'s exit status and distinguish its documented success code 78 from failure code 79 rather than treating every nonzero exit as failure. Warn when configuration fails, including the exit status and available stderr, while preserving the behavior of returning the browser.

ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools

---

Context sources

✅ Web pages:

  🌐 chrome/installer/setup/setup_main.cc

  🌐 chrome/installer/setup/setup_main.cc - chromium/src - Git at Google

  🌐 chrome/installer/setup/setup_main.cc - chromium/src - Git at Google

  +17 more

Review mode: Auto: ⚖️ Balanced: Sandbox ACL configuration changes involve permissions and process execution behavior.

Tip of the day

💡 Did you know, you can keep summaries lean with Findings visible per group, which tucks the rest behind a View link

More tips ↗ | Customize Qodo ↗ | Qodo docs ↗

Previous reviews

Review updated until commit 21ee485

Results up to commit dd24b79 ⚖️ Balanced

🐞 Bugs (0) 📘 Rule violations (0) 📎 Requirement gaps (0) 🎨 UX issues (0) 🔗 Cross-repo conflicts (0) 📜 Skill insights (0)

1. Failed sandbox setup goes unreported ✗ Dismissed

Description

configure_browser_in_cache warns only when run_shell_command_with_log returns an error, but the
command helper returns success without checking setup.exe's exit status. When setup fails to grant
directory permissions, the browser is still returned without a warning, and the download proceeds
despite the sandbox configuration failure.

Code

rust/src/chrome.rs[R284-286]

+        if let Err(err) = run_shell_command_with_log(self.get_logger(), command) {
+            self.get_logger()
+                .warn(format!("Unable to configure the Chrome sandbox: {err}"));

Evidence

The warning handles only an Err, while run_shell_command returns Ok(stdout) after
process.output() without inspecting the exit status or stderr. Chromium reports configuration
success with exit code 78 and failure with 79, so a failed ACL operation cannot reach the warning
through the current helper.

AGENTS.md: Log Behavior Users May Need to Diagnose
rust/src/chrome.rs[283-287]
rust/src/shell.rs[46-57]
rust/src/lib.rs[397-402]
🌐 Chromium assigns exit codes 78 and 79 to successful and failed AppContainer sandbox configuration, respectively.

Agent prompt

The issue below was found during a code review. Follow the provided context and guidance below and implement a solution

## Issue description
A failed Chrome sandbox configuration can be treated as successful because the command helper ignores `setup.exe`'s exit status. Selenium Manager should warn about configuration failure while still returning the browser.

## Fix Focus Areas
- rust/src/chrome.rs[283-287]
- rust/src/shell.rs[46-57]

## Recommended Fix
Capture `setup.exe`'s exit status and distinguish its documented success code 78 from failure code 79 rather than treating every nonzero exit as failure. Warn when configuration fails, including the exit status and available stderr, while preserving the behavior of returning the browser.

ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools

---

[qodo-code-review]

Code review by qodo was updated up to the latest commit 21ee485