Skip to content

Commit 5a714f3

Browse files
author
lpb-docs
committed
fix(browser): bridge LPB_AGENT_BROWSER_* → AGENT_BROWSER_* + repair chrome exec bits
agent-browser reads the bare AGENT_BROWSER_* names, not LPB_ — so the container-safe defaults in lpb.conf.env (notably --no-sandbox) silently never reached the browser, and Chrome could not launch in a container. start.sh: - bridge each LPB_AGENT_BROWSER_* to AGENT_BROWSER_* (shell env > LPB_ > container-safe fallback; ARGS falls back to the full safe set, since Chrome cannot launch without --no-sandbox) lpb.conf.env: - LPB_AGENT_BROWSER_ARGS now comma-separated (agent-browser parses --args as a comma/newline list; was space-separated) install-browser: - extract member-by-member and restore Unix modes (CPython's extractall() drops exec bits, leaving chrome 0644 → PermissionError errno 13 on every launch) - self-heal existing trees: restore exec bits on chrome, chrome_crashpad_handler (its PermissionError kills Chrome at startup before DevTools starts), headless_shell - merge the container-safe arg set into an existing ~/.agent-browser/ config.json (previously skipped when the file existed, so an old or user config silenced the defaults); corrupted configs are replaced - verify_installation reports a clean error instead of crashing with a PermissionError traceback Tests: +6 install-browser (exec-bit restore on extract, self-heal incl. crashpad handler, config create/merge/corrupt, verify non-executable), +4 env-bridge (AGENT_BROWSER_* fallback/promotion/priority/default). Also fixed test isolation: the agent-install success test now mocks Path.home instead of writing to the real ~/.agent-browser. Verified live: re-ran the fixed installer in this container — exec bits restored, config merged, --- AGENT_BROWSER_PAGE_CONTENT nonce=89388e76bdb6c8333382237273b27f84 origin=https://example.com/ --- # Example Domain This domain is for use in documentation examples without needing permission. Avoid use in operations. Learn more --- END_AGENT_BROWSER_PAGE_CONTENT nonce=89388e76bdb6c8333382237273b27f84 --- succeeds. 50/50 + 80/80 + 22/22, pre-commit gate green. doc/env-vars.md: note that LPB_AGENT_BROWSER_* is bridged to AGENT_BROWSER_* at container start.
1 parent 58336a5 commit 5a714f3

6 files changed

Lines changed: 278 additions & 16 deletions

File tree

doc/env-vars.md

Lines changed: 6 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -47,6 +47,12 @@ If a value from `.env` doesn't seem to apply, check shell env first:
4747
| `LPB_PERSIST_GH_CONFIG` || Persist gh CLI auth into the state volume |
4848
| `LPB_OPENROUTER_BASE_URL` || Optional overflow provider (not needed with Lemonade) |
4949

50+
> **Browser var bridging:** agent-browser (CLI / MCP server) reads the
51+
> `AGENT_BROWSER_*` names, not `LPB_*`. `start.sh` bridges each
52+
> `LPB_AGENT_BROWSER_*` to `AGENT_BROWSER_*` (shell env > LPB_ > container-safe
53+
> fallback), so the container-safe Chrome args — notably `--no-sandbox`,
54+
> required to launch Chrome inside a container — always take effect.
55+
5056
## API Keys
5157

5258
| Variable | Used by |

lpb.conf.env

Lines changed: 3 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -52,7 +52,9 @@ LPB_AGENT_BROWSER_SESSION=${PI_WORKTREE_ID}
5252
# --no-first-run : skip first-run setup dialog
5353
# --disable-gpu : avoid GPU issues without GPU passthrough
5454
# --disable-crashpad : crash dumps not useful in containers
55-
LPB_AGENT_BROWSER_ARGS=--no-sandbox --no-first-run --disable-gpu --disable-crashpad
55+
# Comma-separated — agent-browser parses --args as comma/newline lists;
56+
# start.sh bridges this to AGENT_BROWSER_ARGS (the name agent-browser reads).
57+
LPB_AGENT_BROWSER_ARGS=--no-sandbox,--no-first-run,--disable-gpu,--disable-crashpad
5658
# Max output length to prevent context flooding
5759
LPB_AGENT_BROWSER_MAX_OUTPUT=4000
5860
# LLM safety markers in content extraction

scripts/test_env_bridge.sh

Lines changed: 42 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -255,6 +255,48 @@ r=$(_bridge_only '
255255
')
256256
[[ "$r" == "exa1:c71" ]] && pass "7d: multiple LPB_ vars bridged" || fail "7d: expected exa1:c71, got $r"
257257

258+
# ─── 8. AGENT_BROWSER_* Bridge (agent-browser reads bare names) ──────────────
259+
# agent-browser does NOT read LPB_AGENT_BROWSER_* — start.sh bridges the
260+
# LPB_ names to AGENT_BROWSER_* with container-safe fallbacks (notably
261+
# --no-sandbox, required for Chrome to launch in a container).
262+
263+
echo ""
264+
echo "=== 8. AGENT_BROWSER_* Bridge ==="
265+
266+
_agent_bridge_test() {
267+
local agent_block unset_list setup var
268+
agent_block=$(sed -n '/^export AGENT_BROWSER_ARGS=/,/^export AGENT_BROWSER_SESSION=/p' "$SUPPORT_SCRIPT")
269+
[[ -n "$agent_block" ]] || { echo "UNSET-BLOCK-MISSING"; return; }
270+
unset_list='
271+
unset AGENT_BROWSER_ARGS AGENT_BROWSER_MAX_OUTPUT AGENT_BROWSER_CONTENT_BOUNDARIES AGENT_BROWSER_CONFIRM_ACTIONS AGENT_BROWSER_IDLE_TIMEOUT_MS AGENT_BROWSER_SESSION
272+
unset LPB_AGENT_BROWSER_ARGS LPB_AGENT_BROWSER_MAX_OUTPUT LPB_AGENT_BROWSER_CONTENT_BOUNDARIES LPB_AGENT_BROWSER_CONFIRM_ACTIONS LPB_AGENT_BROWSER_IDLE_TIMEOUT_MS LPB_AGENT_BROWSER_SESSION
273+
'
274+
setup="$1"
275+
var="${2:-AGENT_BROWSER_ARGS}"
276+
bash -c "
277+
$unset_list
278+
$setup
279+
$agent_block
280+
echo \"\${$var:-UNSET}\"
281+
"
282+
}
283+
284+
r=$(_agent_bridge_test '')
285+
if [[ "$r" == *"--no-sandbox"* && "$r" == *"--disable-gpu"* ]]; then
286+
pass "8a: container-safe fallback when nothing set"
287+
else
288+
fail "8a: expected container-safe args, got '$r'"
289+
fi
290+
291+
r=$(_agent_bridge_test 'export LPB_AGENT_BROWSER_ARGS=--custom,--flags')
292+
[[ "$r" == "--custom,--flags" ]] && pass "8b: LPB_ bridges to AGENT_BROWSER_" || fail "8b: expected --custom,--flags, got '$r'"
293+
294+
r=$(_agent_bridge_test 'export AGENT_BROWSER_ARGS=shell-wins; export LPB_AGENT_BROWSER_ARGS=lpb-loses')
295+
[[ "$r" == "shell-wins" ]] && pass "8c: shell env > LPB_" || fail "8c: expected shell-wins, got '$r'"
296+
297+
r=$(_agent_bridge_test '' 'AGENT_BROWSER_MAX_OUTPUT')
298+
[[ "$r" == "4000" ]] && pass "8d: MAX_OUTPUT fallback default" || fail "8d: expected 4000, got '$r'"
299+
258300
# ─── Summary ─────────────────────────────────────────────────────────────────
259301

260302
echo ""

scripts/test_localpibox_install_browser.py

Lines changed: 120 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -6,6 +6,9 @@
66
from testharness import run_lpbx_suite, _quiet_console
77

88
import io
9+
import json
10+
import os
11+
import shutil
912
from unittest import mock
1013
import importlib
1114

@@ -24,11 +27,63 @@ def test_install_browser_skips_existing_chrome(tmpdir):
2427
with mock.patch.object(ib, "CHROME_BASE", tmpdir), \
2528
mock.patch.object(ib, "fetch_stable_chrome_version", return_value=version):
2629
(tmpdir / f"chrome-{version}" / "chrome-linux64").mkdir(parents=True)
27-
(tmpdir / f"chrome-{version}" / "chrome-linux64" / "chrome").touch()
30+
chrome = (tmpdir / f"chrome-{version}" / "chrome-linux64" / "chrome")
31+
chrome.touch()
32+
chrome.chmod(0o755) # properly installed → pure skip
2833
assert ib.install_chrome(cons) == 0
2934
assert "already installed" in cons.err.getvalue()
3035

3136

37+
def test_install_browser_chrome_extract_restores_exec_bit(tmpdir):
38+
"""zipfile extraction must leave the chrome binary executable.
39+
40+
CPython's extractall() drops Unix modes, so install_chrome extracts
41+
member-by-member and chmods from the zip entry mode.
42+
"""
43+
import zipfile as _zip
44+
cons = _quiet_console()
45+
version = "99.0.0.1"
46+
zip_path = tmpdir / "chrome-linux64.zip"
47+
with _zip.ZipFile(zip_path, "w") as zf:
48+
zi = _zip.ZipInfo("chrome-linux64/chrome")
49+
zi.external_attr = 0o755 << 16
50+
zf.writestr(zi, "#!/bin/sh\n")
51+
zi2 = _zip.ZipInfo("chrome-linux64/manifest.json")
52+
zi2.external_attr = 0o644 << 16
53+
zf.writestr(zi2, "{}")
54+
55+
def fake_retrieve(url, dest):
56+
shutil.copy(zip_path, dest)
57+
58+
with mock.patch.object(ib, "CHROME_BASE", tmpdir), \
59+
mock.patch.object(ib, "fetch_stable_chrome_version", return_value=version), \
60+
mock.patch.object(ib.urllib.request, "urlretrieve", side_effect=fake_retrieve):
61+
assert ib.install_chrome(cons) == 0
62+
bin_path = tmpdir / f"chrome-{version}" / "chrome-linux64" / "chrome"
63+
assert bin_path.is_file(), "chrome not extracted"
64+
assert os.access(bin_path, os.X_OK), "chrome binary must be executable after extract"
65+
assert os.stat(bin_path).st_mode & 0o777 == 0o755, "exec bit must come from the zip entry"
66+
67+
68+
def test_install_browser_chrome_non_executable_repaired(tmpdir):
69+
"""An existing chrome tree without exec bits (older installs) self-heals —
70+
including chrome_crashpad_handler (its PermissionError 13 kills Chrome
71+
at startup before DevTools starts)."""
72+
cons = _quiet_console()
73+
version = "99.0.0.1"
74+
chrome = tmpdir / f"chrome-{version}" / "chrome-linux64" / "chrome"
75+
crashpad = tmpdir / f"chrome-{version}" / "chrome-linux64" / "chrome_crashpad_handler"
76+
chrome.parent.mkdir(parents=True)
77+
chrome.touch() # 0644 — the broken state
78+
crashpad.touch()
79+
with mock.patch.object(ib, "CHROME_BASE", tmpdir), \
80+
mock.patch.object(ib, "fetch_stable_chrome_version", return_value=version):
81+
assert ib.install_chrome(cons) == 0
82+
assert os.access(chrome, os.X_OK), "chrome exec bit must be restored"
83+
assert os.access(crashpad, os.X_OK), "crashpad handler exec bit must be restored"
84+
assert "exec bits restored" in cons.err.getvalue()
85+
86+
3287
def test_install_browser_verify_no_chrome(tmpdir):
3388
cons = _quiet_console()
3489
with mock.patch.object(ib, "CHROME_BASE", tmpdir), \
@@ -38,6 +93,19 @@ def test_install_browser_verify_no_chrome(tmpdir):
3893
assert "Chrome binary not found" in cons.err.getvalue()
3994

4095

96+
def test_install_browser_verify_non_executable_chrome(tmpdir):
97+
"""verify must report a clean error (not crash) when chrome is 0644."""
98+
cons = _quiet_console()
99+
version = "99.0.0.1"
100+
chrome = tmpdir / f"chrome-{version}" / "chrome-linux64" / "chrome"
101+
chrome.parent.mkdir(parents=True)
102+
chrome.touch() # 0644 — real exec attempt raises PermissionError
103+
with mock.patch.object(ib, "CHROME_BASE", tmpdir), \
104+
mock.patch.object(ib, "which", return_value=None):
105+
assert ib.verify_installation(cons) == 1
106+
assert "not executable" in cons.err.getvalue()
107+
108+
41109
def test_install_browser_agent_install_missing_binary(tmpdir):
42110
cons = _quiet_console()
43111
with mock.patch.object(ib, "which", return_value=None):
@@ -56,12 +124,62 @@ class FakeResult:
56124
return FakeResult()
57125

58126
with mock.patch.object(ib, "which", return_value="/bin/agent-browser"), \
59-
mock.patch.object(ib.subprocess, "run", side_effect=fake_run):
127+
mock.patch.object(ib.subprocess, "run", side_effect=fake_run), \
128+
mock.patch.object(ib.Path, "home", return_value=tmpdir):
60129
assert ib.install_agent_browser(cons) == 0
61130
assert calls == [
62131
["agent-browser", "install"],
63132
["agent-browser", "install", "--with-deps"],
64133
]
134+
# container-safe config created in the (mocked) home
135+
data = json.loads((tmpdir / ".agent-browser" / "config.json").read_text())
136+
for arg in ("--no-sandbox", "--no-first-run", "--disable-gpu", "--disable-crashpad"):
137+
assert arg in data["args"].split(","), f"missing {arg}: {data['args']}"
138+
139+
140+
def test_install_browser_config_merges_existing(tmpdir):
141+
"""An existing config.json must keep user args + other keys, and gain
142+
the missing container-safe args (no duplicates)."""
143+
cons = _quiet_console()
144+
(tmpdir / ".agent-browser").mkdir()
145+
config = tmpdir / ".agent-browser" / "config.json"
146+
config.write_text(json.dumps({"args": "--headless=new", "hideScrollbars": False}))
147+
148+
def fake_run(args, **kwargs):
149+
class FakeResult:
150+
returncode = 0
151+
return FakeResult()
152+
153+
with mock.patch.object(ib, "which", return_value="/bin/agent-browser"), \
154+
mock.patch.object(ib.subprocess, "run", side_effect=fake_run), \
155+
mock.patch.object(ib.Path, "home", return_value=tmpdir):
156+
assert ib.install_agent_browser(cons) == 0
157+
data = json.loads(config.read_text())
158+
args = data["args"].split(",")
159+
assert "--headless=new" in args, f"lost user arg: {data['args']}"
160+
assert "--no-sandbox" in args, f"missing --no-sandbox: {data['args']}"
161+
assert args.count("--no-sandbox") == 1, f"duplicated: {data['args']}"
162+
assert data["hideScrollbars"] is False, "other keys must be preserved"
163+
164+
165+
def test_install_browser_config_replaces_unreadable(tmpdir):
166+
"""A corrupted config.json is overwritten (safe set wins, no crash)."""
167+
cons = _quiet_console()
168+
(tmpdir / ".agent-browser").mkdir()
169+
config = tmpdir / ".agent-browser" / "config.json"
170+
config.write_text("{not json")
171+
172+
def fake_run(args, **kwargs):
173+
class FakeResult:
174+
returncode = 0
175+
return FakeResult()
176+
177+
with mock.patch.object(ib, "which", return_value="/bin/agent-browser"), \
178+
mock.patch.object(ib.subprocess, "run", side_effect=fake_run), \
179+
mock.patch.object(ib.Path, "home", return_value=tmpdir):
180+
assert ib.install_agent_browser(cons) == 0
181+
data = json.loads(config.read_text())
182+
assert "--no-sandbox" in data["args"].split(",")
65183

66184

67185
def main() -> int:

support/install-browser.py

Lines changed: 93 additions & 13 deletions
Original file line numberDiff line numberDiff line change
@@ -16,6 +16,7 @@
1616

1717
import argparse
1818
import json
19+
import os
1920
import sys
2021
import tempfile
2122
import urllib.request
@@ -35,6 +36,9 @@
3536

3637
CHROME_BASE = Path("/home/lpb/.agent-browser/browsers")
3738
SYSTEM_CHROME = Path("/opt/google/chrome/chrome")
39+
# Executable binaries inside the chrome-linux64/ payload (everything else
40+
# is data/libs). Used to self-heal trees extracted without exec bits.
41+
CHROME_EXECUTABLES = {"chrome", "chrome_crashpad_handler", "headless_shell"}
3842
LAST_KNOWN_URL = (
3943
"https://googlechromelabs.github.io/chrome-for-testing/"
4044
"last-known-good-versions-with-downloads.json"
@@ -63,6 +67,36 @@ def chrome_binary(version: str) -> Path:
6367
return chrome_dir_for(version) / "chrome-linux64" / "chrome"
6468

6569

70+
def _ensure_executable(path: Path, cons: Console | None = None) -> None:
71+
"""Restore the exec bit that Python's zipfile extractall drops.
72+
73+
Chrome-for-Testing zips ship 0755 entries, but CPython's extractall()
74+
does not restore Unix modes — left alone, the binary is 0644 and every
75+
launch dies with PermissionError (errno 13).
76+
"""
77+
if path.is_file() and not os.access(path, os.X_OK):
78+
path.chmod(path.stat().st_mode | 0o755)
79+
if cons:
80+
cons.warn(f"Restored missing exec bit on {path}")
81+
82+
83+
def _restore_exec_bits(chrome_root: Path, cons: Console | None = None) -> int:
84+
"""Self-heal an extracted Chrome tree: restore exec bits on the known
85+
binaries. Returns the number of files fixed. Chrome crashes at startup
86+
if chrome_crashpad_handler is not executable (PermissionError 13)."""
87+
fixed = 0
88+
for sub in (chrome_root, chrome_root / "chrome-linux64"):
89+
if not sub.is_dir():
90+
continue
91+
for f in sub.iterdir():
92+
if f.name in CHROME_EXECUTABLES and f.is_file() and not os.access(f, os.X_OK):
93+
f.chmod(f.stat().st_mode | 0o755)
94+
fixed += 1
95+
if fixed and cons:
96+
cons.warn(f"Restored missing exec bits on {fixed} Chrome binary(ies) under {chrome_root}")
97+
return fixed
98+
99+
66100
def install_chrome(cons: Console) -> int:
67101
cons.info("Downloading latest Chrome for Testing...")
68102
try:
@@ -72,8 +106,15 @@ def install_chrome(cons: Console) -> int:
72106
return 1
73107
cons.info(f"Chrome version: {version}")
74108

75-
if chrome_binary(version).is_file():
76-
cons.warn(f"Chrome already installed at {chrome_dir_for(version)}")
109+
bin_path = chrome_binary(version)
110+
if bin_path.is_file():
111+
if os.access(bin_path, os.X_OK) and not _restore_exec_bits(chrome_dir_for(version)):
112+
cons.warn(f"Chrome already installed at {chrome_dir_for(version)}")
113+
return 0
114+
# Self-heal: an earlier install may have extracted without exec bits
115+
# (Python's zipfile drops Unix modes) — restore and reuse.
116+
_restore_exec_bits(chrome_dir_for(version), cons)
117+
cons.warn(f"Chrome already installed at {chrome_dir_for(version)} (exec bits restored)")
77118
return 0
78119

79120
chrome_dir_for(version).mkdir(parents=True, exist_ok=True)
@@ -84,11 +125,19 @@ def install_chrome(cons: Console) -> int:
84125
cons.info(f"Downloading {url} ...")
85126
urllib.request.urlretrieve(url, zip_path)
86127
with zipfile.ZipFile(zip_path) as zf:
87-
zf.extractall(chrome_dir_for(version))
128+
# Extract member-by-member and restore Unix modes: extractall()
129+
# drops exec bits, leaving the chrome binary unrunnable (0644).
130+
for member in zf.infolist():
131+
target = zf.extract(member, chrome_dir_for(version))
132+
if not member.is_dir():
133+
mode = member.external_attr >> 16
134+
if mode:
135+
os.chmod(target, mode & 0o7777)
88136
finally:
89137
zip_path.unlink(missing_ok=True)
90138

91-
if chrome_binary(version).is_file():
139+
if bin_path.is_file():
140+
_restore_exec_bits(chrome_dir_for(version), cons) # belt & braces
92141
cons.info(f"Chrome extracted to {chrome_dir_for(version)}")
93142
return 0
94143
cons.error("Chrome extraction failed")
@@ -123,14 +172,34 @@ def _run_agent(*args: str) -> subprocess.CompletedProcess[str]:
123172

124173
cons.info("agent-browser installed successfully")
125174

126-
# Container-safe defaults: Chrome needs --no-sandbox in Docker/container
175+
# Container-safe defaults: Chrome needs --no-sandbox in a container.
176+
# agent-browser reads ~/.agent-browser/config.json (camelCase keys; the
177+
# "args" value is a comma-separated launch-arg list). MERGE the safe set
178+
# into any existing config instead of skipping it — an existing file
179+
# (user-customized, or from an older stack) must not silence the
180+
# container-safe defaults.
127181
config_path = Path.home() / ".agent-browser" / "config.json"
128-
if not config_path.is_file():
129-
config_path.parent.mkdir(parents=True, exist_ok=True)
130-
config_path.write_text(
131-
json.dumps({"args": "--no-sandbox"}, indent=2)
132-
)
182+
existed = config_path.is_file()
183+
config = {}
184+
if existed:
185+
try:
186+
loaded = json.loads(config_path.read_text())
187+
if isinstance(loaded, dict):
188+
config = loaded
189+
except (json.JSONDecodeError, OSError):
190+
cons.warn(f"Existing {config_path} is unreadable — overwriting")
191+
safe_args = ["--no-sandbox", "--no-first-run", "--disable-gpu", "--disable-crashpad"]
192+
existing = [a.strip() for a in str(config.get("args", "")).replace("\n", ",").split(",") if a.strip()]
193+
merged = existing + [a for a in safe_args if a not in existing]
194+
config["args"] = ",".join(merged)
195+
config_path.parent.mkdir(parents=True, exist_ok=True)
196+
config_path.write_text(json.dumps(config, indent=2) + "\n")
197+
if not existed:
133198
cons.info(f"Created container-safe config: {config_path}")
199+
elif merged != existing:
200+
cons.info(f"Merged container-safe args into {config_path}")
201+
else:
202+
cons.info(f"Container-safe args already present in {config_path}")
134203

135204
return 0
136205

@@ -145,9 +214,20 @@ def verify_installation(cons: Console) -> int:
145214

146215
if chrome_bin:
147216
cons.info(f" Chrome: {chrome_bin}")
148-
out, _err, code = run_cmd([str(chrome_bin), "--version"], timeout=30)
149-
if code == 0 and out.strip():
150-
cons.raw(f" {out.strip()}")
217+
try:
218+
out, _err, code = run_cmd([str(chrome_bin), "--version"], timeout=30)
219+
except (PermissionError, OSError) as exc:
220+
cons.error(
221+
f" Chrome binary not executable ({exc.__class__.__name__}); "
222+
f"re-run install-browser to restore permissions"
223+
)
224+
errors += 1
225+
else:
226+
if code == 0 and out.strip():
227+
cons.raw(f" {out.strip()}")
228+
else:
229+
cons.error(f" Chrome --version failed (exit {code})")
230+
errors += 1
151231
else:
152232
cons.error(" Chrome binary not found")
153233
errors += 1

0 commit comments

Comments
 (0)