browser-use--browser-harness
ad39a9500a
Two additional codex findings on 6d412c9:
1. _ipc.identify() accepted any int as pid, including 0 and negatives.
On POSIX, os.kill(0, sig) signals every process in the calling
process group, and os.kill(-1, sig) signals every process the
caller can. A hostile or buggy daemon replying {pid: 0} or
{pid: -1} would have turned restart_daemon() into a process-group
kill. Restrict to pid > 0.
2. _ipc.ping() still did resp.get('pong') without a type check, so a
list/scalar/null reply would raise AttributeError. The previous
commit added that guard to identify() but left ping() bare. With
the new daemon_alive fallback in restart_daemon() that now calls
ping(), an unhandled raise here would abort restart before
cleanup ran. Mirrored the identify() guards: isinstance(resp, dict)
plus AttributeError in the except.
Tests in tests/unit/test_ipc.py now also cover:
- identify() rejects pid=0, pid=-1, pid=-42, pid=-99999
- ping() returns False for non-dict payloads (list/str/int/None)
- ping() requires pong is exactly True (rejects truthy non-True values)
Full suite: 71 passed.
108 行
4.4 KiB
Python
108 行
4.4 KiB
Python
from browser_harness import _ipc as ipc
|
|
|
|
|
|
# --- identify(): ping payload sanitation ---
|
|
|
|
class _FakeConn:
|
|
def close(self): pass
|
|
|
|
|
|
def _patch_identify_response(monkeypatch, response):
|
|
"""Stub connect() and request() so identify() sees `response` as the JSON
|
|
parsed from the daemon's reply, exactly as it would arrive over the wire."""
|
|
monkeypatch.setattr(ipc, "connect", lambda name, timeout=1.0: (_FakeConn(), "tok"))
|
|
monkeypatch.setattr(ipc, "request", lambda conn, tok, msg: response)
|
|
|
|
|
|
def test_identify_returns_pid_for_well_formed_ping_reply(monkeypatch):
|
|
_patch_identify_response(monkeypatch, {"pong": True, "pid": 4242})
|
|
|
|
assert ipc.identify("default", timeout=0.0) == 4242
|
|
|
|
|
|
def test_identify_rejects_boolean_pid(monkeypatch):
|
|
"""isinstance(True, int) is True in Python; a hostile or buggy daemon
|
|
that replies {"pid": True} would otherwise yield PID 1 (init on POSIX),
|
|
which os.kill(1, SIGTERM) would target. Reject it explicitly."""
|
|
_patch_identify_response(monkeypatch, {"pong": True, "pid": True})
|
|
|
|
assert ipc.identify("default", timeout=0.0) is None
|
|
|
|
|
|
def test_identify_rejects_boolean_false_pid(monkeypatch):
|
|
"""False is also an int subclass and would yield PID 0."""
|
|
_patch_identify_response(monkeypatch, {"pong": True, "pid": False})
|
|
|
|
assert ipc.identify("default", timeout=0.0) is None
|
|
|
|
|
|
def test_identify_returns_none_when_pid_field_missing(monkeypatch):
|
|
"""Pre-upgrade daemons reply {pong: True} only — no pid. identify must
|
|
return None so callers know they have no verified PID to signal, while
|
|
still letting alive-checks via ipc.ping() succeed."""
|
|
_patch_identify_response(monkeypatch, {"pong": True})
|
|
|
|
assert ipc.identify("default", timeout=0.0) is None
|
|
|
|
|
|
def test_identify_handles_non_dict_ping_payload(monkeypatch):
|
|
"""request() can deserialize any valid JSON value. A stale or hostile
|
|
endpoint replying with a list / scalar / null would crash a naive
|
|
resp.get() with AttributeError; identify must absorb that and return None."""
|
|
for payload in ([1, 2, 3], "hello", 42, None):
|
|
_patch_identify_response(monkeypatch, payload)
|
|
assert ipc.identify("default", timeout=0.0) is None, (
|
|
f"identify() should reject non-dict ping payload: {payload!r}"
|
|
)
|
|
|
|
|
|
def test_identify_returns_none_when_pong_is_not_true(monkeypatch):
|
|
_patch_identify_response(monkeypatch, {"pong": False, "pid": 4242})
|
|
|
|
assert ipc.identify("default", timeout=0.0) is None
|
|
|
|
|
|
def test_identify_rejects_zero_and_negative_pids(monkeypatch):
|
|
"""os.kill semantics on POSIX: pid=0 signals every process in the calling
|
|
process group; pid=-1 signals every process the caller can; pid<-1 signals
|
|
the corresponding process group. None of these are valid daemon PIDs and
|
|
forwarding any of them to os.kill would be catastrophic."""
|
|
for bad_pid in (0, -1, -42, -99999):
|
|
_patch_identify_response(monkeypatch, {"pong": True, "pid": bad_pid})
|
|
assert ipc.identify("default", timeout=0.0) is None, (
|
|
f"identify() must reject non-positive pid {bad_pid!r}"
|
|
)
|
|
|
|
|
|
# --- ping(): same payload sanitation ---
|
|
|
|
def _patch_ping_response(monkeypatch, response):
|
|
monkeypatch.setattr(ipc, "connect", lambda name, timeout=1.0: (_FakeConn(), "tok"))
|
|
monkeypatch.setattr(ipc, "request", lambda conn, tok, msg: response)
|
|
|
|
|
|
def test_ping_returns_true_for_well_formed_pong(monkeypatch):
|
|
_patch_ping_response(monkeypatch, {"pong": True})
|
|
|
|
assert ipc.ping("default", timeout=0.0) is True
|
|
|
|
|
|
def test_ping_handles_non_dict_payload(monkeypatch):
|
|
"""Same regression class as identify(): if a stale or hostile endpoint
|
|
replies with a list / scalar / null, ping() must return False rather than
|
|
raising AttributeError on resp.get(). restart_daemon() now calls ping() on
|
|
the fallback path, so an unhandled raise here would abort cleanup."""
|
|
for payload in ([1, 2, 3], "hello", 42, None):
|
|
_patch_ping_response(monkeypatch, payload)
|
|
assert ipc.ping("default", timeout=0.0) is False, (
|
|
f"ping() should reject non-dict payload: {payload!r}"
|
|
)
|
|
|
|
|
|
def test_ping_returns_false_when_pong_field_is_missing_or_not_true(monkeypatch):
|
|
for resp in ({}, {"pong": False}, {"pong": "yes"}, {"pong": 1}):
|
|
_patch_ping_response(monkeypatch, resp)
|
|
assert ipc.ping("default", timeout=0.0) is False, (
|
|
f"ping() should require pong is exactly True; got: {resp!r}"
|
|
)
|