From d4dcef3315be6c236fc9087b9cc282a8b50b41fd Mon Sep 17 00:00:00 2001 From: Claude Date: Mon, 31 Aug 2026 02:12:15 +0000 Subject: [PATCH 1/3] fix(sandboxed-web-e2e): probe explicit port 0, not the scheme default require_unoccupied_readiness_port() derived the probe port with `parsed.port or (443 if https else 80)`. urllib.parse's .port returns the int 0 for a URL with an explicit :0 port, and `0 or X` evaluates to X in Python, so an explicitly-requested port 0 was silently replaced with the scheme's default port (80/443) instead of actually being probed. Devin's review on PR #1347 flagged this pattern but it was out of scope for that PR's authorized task. Switch to an explicit None check so port 0 is honored, and add a regression test that monkeypatches socket.create_connection to record the probed address and assert it names port 0. Co-Authored-By: Claude Sonnet 5 Claude-Session: https://claude.ai/code/session_01KPmJErfkcHer4UVEgrQxUX --- scripts/ci/sandboxed_web_e2e.py | 2 +- tests/test_sandboxed_web_e2e.py | 25 +++++++++++++++++++++++++ 2 files changed, 26 insertions(+), 1 deletion(-) diff --git a/scripts/ci/sandboxed_web_e2e.py b/scripts/ci/sandboxed_web_e2e.py index 1be073104a..196681edfa 100644 --- a/scripts/ci/sandboxed_web_e2e.py +++ b/scripts/ci/sandboxed_web_e2e.py @@ -546,7 +546,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..faf67abc67 100644 --- a/tests/test_sandboxed_web_e2e.py +++ b/tests/test_sandboxed_web_e2e.py @@ -464,6 +464,31 @@ 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_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" From d8ccd5ef1754a3d3ce7c6356af21abbdfde8525e Mon Sep 17 00:00:00 2001 From: Seongho Bae Date: Mon, 31 Aug 2026 11:25:55 +0900 Subject: [PATCH 2/3] fix(security): scope Bandit tmpfs waivers to isolation targets Bandit B108 correctly began running after the central file-detection repair and identified the two literal /tmp arguments passed to bubblewrap. These are tmpfs mount targets inside a new isolated namespace, not host temporary-file paths. Add B108-only waivers with adjacent rationale and a regression that pins both the scope and count, preserving every other Bandit rule and host-path finding. --- scripts/ci/sandboxed_web_e2e.py | 6 ++++-- tests/test_sandboxed_web_e2e.py | 18 ++++++++++++++++++ 2 files changed, 22 insertions(+), 2 deletions(-) diff --git a/scripts/ci/sandboxed_web_e2e.py b/scripts/ci/sandboxed_web_e2e.py index 196681edfa..db9328c7d3 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 "--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 "--bind", str(sandbox_root), SANDBOX_MOUNT, diff --git a/tests/test_sandboxed_web_e2e.py b/tests/test_sandboxed_web_e2e.py index faf67abc67..4f27e02ac8 100644 --- a/tests/test_sandboxed_web_e2e.py +++ b/tests/test_sandboxed_web_e2e.py @@ -489,6 +489,24 @@ def fake_create_connection(address, timeout): assert recorded == [("127.0.0.1", 0)] +def test_bubblewrap_tmpfs_targets_have_only_targeted_bandit_waivers(): + """B108 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 real host-path checks enabled everywhere + else while preventing these two deliberate mount targets from blocking the + Python security gate. + """ + source = Path(sandboxed_web_e2e.__file__).read_text(encoding="utf-8") + waiver = '"/tmp", # nosec B108' + 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(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" From ef75ce93732e5706aa51f73a5f466d65aef2c1ff Mon Sep 17 00:00:00 2001 From: Claude Date: Mon, 31 Aug 2026 02:34:20 +0000 Subject: [PATCH 3/3] fix(lint): pair Ruff S108 waivers with the existing Bandit B108 ones CodeRabbit ran `ruff check --select S108` against this file and found the two bubblewrap tmpfs `"/tmp"` mount-target arguments still fail Ruff's own insecure-temp-path rule -- `# nosec B108` only silences Bandit, not Ruff. Add `# noqa: S108` alongside each existing waiver and extend the regression test that already pins the Bandit waiver's exact text/count/rationale to also pin the Ruff waiver, so this scoped exception can't silently broaden to other paths or rules. Co-Authored-By: Claude Sonnet 5 Claude-Session: https://claude.ai/code/session_01KPmJErfkcHer4UVEgrQxUX --- scripts/ci/sandboxed_web_e2e.py | 4 ++-- tests/test_sandboxed_web_e2e.py | 13 +++++++------ 2 files changed, 9 insertions(+), 8 deletions(-) diff --git a/scripts/ci/sandboxed_web_e2e.py b/scripts/ci/sandboxed_web_e2e.py index db9328c7d3..b0376c0822 100644 --- a/scripts/ci/sandboxed_web_e2e.py +++ b/scripts/ci/sandboxed_web_e2e.py @@ -222,7 +222,7 @@ def _probe_isolation_capability(backend: str) -> None: "/dev", "--tmpfs", # This is the isolated namespace's tmpfs target, not a host temp path. - "/tmp", # nosec B108 + "/tmp", # nosec B108 # noqa: S108 "--bind", probe_workspace, SANDBOX_MOUNT, @@ -434,7 +434,7 @@ def isolated_command( "/dev", "--tmpfs", # This is the isolated namespace's tmpfs target, not a host temp path. - "/tmp", # nosec B108 + "/tmp", # nosec B108 # noqa: S108 "--bind", str(sandbox_root), SANDBOX_MOUNT, diff --git a/tests/test_sandboxed_web_e2e.py b/tests/test_sandboxed_web_e2e.py index 4f27e02ac8..1b1cdf3722 100644 --- a/tests/test_sandboxed_web_e2e.py +++ b/tests/test_sandboxed_web_e2e.py @@ -490,20 +490,21 @@ def fake_create_connection(address, timeout): def test_bubblewrap_tmpfs_targets_have_only_targeted_bandit_waivers(): - """B108 waivers cover only bubblewrap's isolated tmpfs mount targets. + """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 real host-path checks enabled everywhere - else while preventing these two deliberate mount targets from blocking the - Python security gate. + 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' + 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