Skip to content
Merged
Show file tree
Hide file tree
Changes from 2 commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
8 changes: 5 additions & 3 deletions scripts/ci/sandboxed_web_e2e.py
Original file line number Diff line number Diff line change
Expand Up @@ -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
Comment thread
seonghobae marked this conversation as resolved.
Outdated
Comment thread
coderabbitai[bot] marked this conversation as resolved.
Outdated
"--bind",
probe_workspace,
SANDBOX_MOUNT,
Expand Down Expand Up @@ -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,
Expand Down Expand Up @@ -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
Expand Down
43 changes: 43 additions & 0 deletions tests/test_sandboxed_web_e2e.py
Original file line number Diff line number Diff line change
Expand Up @@ -464,6 +464,49 @@ 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 <default>`` 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 <default>`` 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 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
Comment thread
seonghobae marked this conversation as resolved.


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"
Expand Down
Loading