Fix browser sandbox detection in containers - #29080
Conversation
|
Fixes #28960. Competing with open PRs #22582, #20014, #23517, #24198 — all addressing browser sandbox detection / |
teknium1
left a comment
There was a problem hiding this comment.
Thanks for chasing the container browser failure. The core premise still holds on current main: tools/browser_tool.py:2012-2044 only auto-injects --no-sandbox for root and AppArmor-restricted user namespaces, and a broad grep on origin/main found no unshare -U probe.
Problems
- The Dockerfile hunk is stale against current main. PR #29080 targets the old
apt-get install ... nodejs npm ... tiniline, while current main's Dockerfile now uses Debian 13, copies Node 22 from a source image, and runsnpx playwright install --with-deps chromium --only-shellatDockerfile:132-133. - Part of the PR is already present on current main:
tools/browser_tool.py:2017-2023already preserves user-providedAGENT_BROWSER_ARGSand legacyAGENT_BROWSER_CHROME_FLAGSvia commit4695d2716f.
Suggested changes
- Salvage the unique
unshare -U /bin/trueprobe into the current sandbox-injection path, with the added unit coverage for root, AppArmor, failed unshare, non-Linux skip, and user-provided args preservation. - Re-check whether explicit
libnspr4 libnss3is still needed in the current Dockerfile after the existing Playwright--with-depslayer.
Automated hermes-sweeper review.
| RUN apt-get update && \ | ||
| apt-get install -y --no-install-recommends \ | ||
| build-essential curl nodejs npm python3 ripgrep ffmpeg gcc python3-dev libffi-dev procps git openssh-client docker-cli tini && \ | ||
| build-essential curl nodejs npm python3 ripgrep ffmpeg gcc python3-dev libffi-dev procps git openssh-client docker-cli tini libnspr4 libnss3 && \ |
There was a problem hiding this comment.
This hunk targets the old Dockerfile dependency line. On current main, the image has moved to Debian 13, copies Node 22 from a source stage, and runs npx playwright install --with-deps chromium --only-shell, so this dependency addition needs to be re-evaluated against the current Dockerfile rather than applied verbatim.
|
Thanks for the focused container reproduction and dependency investigation. This is an automated hermes-sweeper review; the requested behavior is already implemented on
|
Summary
Fixes #28960.
Testing
ode:22.22.3-bookworm: \unshare -U /bin/true\ fails with \Operation not permitted, and the new logic injects --no-sandbox,--disable-dev-shm-usage\
Note: I did not run a full Docker image build locally to avoid unnecessary disk usage; the package install and container userns behavior were validated directly.