diff --git a/scripts/ci/sandboxed_web_e2e.py b/scripts/ci/sandboxed_web_e2e.py index 1be073104a..b0376c0822 100644 --- a/scripts/ci/sandboxed_web_e2e.py +++ b/scripts/ci/sandboxed_web_e2e.py @@ -221,7 +221,8 @@ def _probe_isolation_capability(backend: str) -> None: "--dev", "/dev", "--tmpfs", - "/tmp", + # This is the isolated namespace's tmpfs target, not a host temp path. + "/tmp", # nosec B108 # noqa: S108 "--bind", probe_workspace, SANDBOX_MOUNT, @@ -432,7 +433,8 @@ def isolated_command( "--dev", "/dev", "--tmpfs", - "/tmp", + # This is the isolated namespace's tmpfs target, not a host temp path. + "/tmp", # nosec B108 # noqa: S108 "--bind", str(sandbox_root), SANDBOX_MOUNT, @@ -546,7 +548,7 @@ def require_unoccupied_readiness_port(url: str) -> None: """ parsed = urllib.parse.urlparse(url) hostname = parsed.hostname or "127.0.0.1" - port = parsed.port or (443 if parsed.scheme.lower() == "https" else 80) + port = parsed.port if parsed.port is not None else (443 if parsed.scheme.lower() == "https" else 80) try: with socket.create_connection((hostname, port), timeout=0.2): pass diff --git a/tests/test_sandboxed_web_e2e.py b/tests/test_sandboxed_web_e2e.py index 4ab489c179..1b1cdf3722 100644 --- a/tests/test_sandboxed_web_e2e.py +++ b/tests/test_sandboxed_web_e2e.py @@ -464,6 +464,50 @@ def test_require_unoccupied_readiness_port_allows_a_free_port(): sandboxed_web_e2e.require_unoccupied_readiness_port(f"http://127.0.0.1:{port}/health") +def test_require_unoccupied_readiness_port_probes_explicit_port_zero(monkeypatch): + """An explicit ``:0`` port must be probed as port 0, not the scheme default. + + ``urllib.parse``'s ``.port`` returns the int ``0`` for a URL with an + explicit ``:0`` port, and ``0 or 80`` evaluates to ``80`` in Python, so a + naive ``parsed.port or `` derivation silently probes the + scheme's default port instead of the requested port 0. Devin's review on + PR #1347 flagged this pattern. This regression test records the actual + address ``require_unoccupied_readiness_port`` probes and asserts it names + port 0, not port 80, pinning the ``parsed.port if parsed.port is not + None else `` fix. + """ + recorded = [] + + def fake_create_connection(address, timeout): + recorded.append(address) + raise OSError("nothing listening") + + monkeypatch.setattr(sandboxed_web_e2e.socket, "create_connection", fake_create_connection) + + sandboxed_web_e2e.require_unoccupied_readiness_port("http://127.0.0.1:0/health") + + assert recorded == [("127.0.0.1", 0)] + + +def test_bubblewrap_tmpfs_targets_have_only_targeted_bandit_waivers(): + """B108/S108 waivers cover only bubblewrap's isolated tmpfs mount targets. + + These strings are command arguments naming the mount point created inside + the new bubblewrap namespace; they are not host temporary-file paths. A + targeted waiver keeps Bandit's and Ruff's real host-path checks enabled + everywhere else while preventing these two deliberate mount targets from + blocking the Python security gate or the lint gate. + """ + source = Path(sandboxed_web_e2e.__file__).read_text(encoding="utf-8") + waiver = '"/tmp", # nosec B108 # noqa: S108' + rationale = "isolated namespace's tmpfs target, not a host temp path" + + assert source.count(waiver) == 2 + assert source.count("nosec B108") == 2 + assert source.count("noqa: S108") == 2 + assert source.count(rationale) == 2 + + def test_main_reports_occupied_readiness_port_before_starting_services(monkeypatch, tmp_path, capsys): """A readiness port already occupied by another process fails closed with exit 125.""" repo = tmp_path / "repo"