From 488d35fc1a5e92dc1e718febc7169d4a6377f9b1 Mon Sep 17 00:00:00 2001 From: memeshee Date: Sun, 8 Nov 2026 02:49:26 +0700 Subject: [PATCH] =?UTF-8?q?fix(tests):=20run=20offline=20without=20dev=20d?= =?UTF-8?q?eps=20=E2=80=94=20the=20guard=20moved=20into=20conftest?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit A reviewer's sandbox installs `requirements.txt` and runs whatever `pytest` the machine has. Our pytest.ini addopts passed `--disable-socket`, which only exists when the `pytest-socket` plugin from requirements-dev.txt is installed, so on that install pytest refused to start at all: ERROR: usage: python -m pytest [options] [file_or_dir] [...] python -m pytest: error: unrecognized arguments: --disable-socket --allow-hosts=127.0.0.1,localhost,::1 Not one test ran. CI stayed green because it installs requirements-dev.txt, so the review reported "still failing tests" on a commit whose suite was fine. The offline guarantee is now stdlib code in conftest.py: `socket.connect`, `connect_ex` and `getaddrinfo` are wrapped and refuse anything outside loopback until a `network`-marked test lifts them. Deps: pytest-socket is gone, nothing in pytest.ini needs a dev-only package. Two things that fix alone would not have caught: - the wrappers are re-asserted in pytest_runtest_setup, because pytest-socket's per-test teardown restores the real `socket.connect` and had silently disarmed the guard partway through the run (found by testing the guard, not the suite); - `reviewer-path` in ci.yml installs runtime requirements + pytest only and runs both `pytest` and `python -m pytest` with an empty $HOME, so this exact configuration can never go untested again. tests/test_offline_guard.py tests the mechanism: DNS and connect are both refused for an unmarked test, loopback still works, the marker lifts the guard, and a clobbered wrapper is put back. Verified with 275 tests passing on Python 3.13 with only requirements.txt + pytest, empty HOME, no .env, read-only checkout; plus ruff and the 79% coverage floor on the dev install. --- .github/workflows/ci.yml | 38 ++++++++++ README.md | 52 +++++++++----- conftest.py | 135 +++++++++++++++++++++++++++++++++--- pytest.ini | 16 ++--- requirements-dev.txt | 1 - tests/test_offline_guard.py | 83 ++++++++++++++++++++++ 6 files changed, 290 insertions(+), 35 deletions(-) create mode 100644 tests/test_offline_guard.py diff --git a/.github/workflows/ci.yml b/.github/workflows/ci.yml index d0718aa..6c41ebf 100644 --- a/.github/workflows/ci.yml +++ b/.github/workflows/ci.yml @@ -45,3 +45,41 @@ jobs: run: | pip install pip-audit pip-audit -r requirements.txt + + # The reviewer path, as a blocking job. Every automated review of this repo + # installs the *runtime* requirements and runs whatever `pytest` the machine + # has — no requirements-dev.txt, no plugins. That is the one configuration the + # job above never tested, and it is how `--disable-socket` in pytest.ini broke + # a whole run with `unrecognized arguments` while CI stayed green. + reviewer-path: + runs-on: ubuntu-latest + steps: + - uses: actions/checkout@v7 + - uses: actions/setup-python@v7 + with: + python-version: "3.13" + + - name: Install runtime requirements + pytest only + run: | + python -m pip install --upgrade pip + pip install -r requirements.txt + pip install pytest + + - name: Bare `pytest` from the repo root (no .env, no keys, no dev deps) + env: + HOME: ${{ runner.temp }}/empty-home + run: | + mkdir -p "$HOME" + pytest + + - name: Same suite via `python -m pytest` + env: + HOME: ${{ runner.temp }}/empty-home + run: python -m pytest -q + + # The guard itself, not just the suite: an unmarked test that opens a + # socket has to fail here rather than pass on a machine with wifi. + - name: The offline guard actually blocks + env: + HOME: ${{ runner.temp }}/empty-home + run: pytest tests/test_offline_guard.py -q diff --git a/README.md b/README.md index 4bf4677..9df31e6 100644 --- a/README.md +++ b/README.md @@ -189,25 +189,43 @@ bash scripts/publish_site.sh # rebuild + redeploy the public hos ## Tests ```bash -python -m pytest # 246 tests, offline: no network, no keys, no .env -python -m pytest -m network # live facilitator, Textile, agent402, Typesafe Jev -python -m pytest -m live # reads real Celo mainnet history -ruff check . # lint (E, F, W, I, B) +pip install -r requirements.txt # runtime deps only — that is enough +pytest # 275 tests, offline: no network, no keys, no .env +pytest -m network # live facilitator, Textile, agent402, Typesafe Jev +pytest -m live # reads real Celo mainnet history +ruff check . # lint (E, F, W, I, B), from requirements-dev.txt ``` -The default run passes on a clean checkout with nothing configured, and that is enforced -rather than asserted: `pytest.ini` passes `--disable-socket`, so a test that quietly -opens a connection fails here instead of passing on a laptop with wifi and hanging in a -stranger's sandbox. Localhost stays open for the HTTP-surface tests, and anything marked -`network` gets its sockets back. Adding that flag found two real leaks — a paid-call -receipt verified against a live RPC, and a settle notification fired at -`api.telegram.org` from a test — and one genuinely offline-broken path: a refused invoice -page was priced (and so needed an FX fetch) before it could say "Dropped". - -CI runs exactly that, plus ruff, byte-compilation, a 75% coverage floor (currently 79%) -and an advisory dependency audit. The densest tests are where silence costs money — -every refusal path in `bot/verify.py`, the x402 spend budgets, the screening gate, and -the public HTTP surface over a real socket. +The default run passes on a clean checkout with nothing configured, and it does so with +**only the runtime requirements installed** — no dev extras, no pytest plugins, no +secrets. That is the configuration a reviewer's sandbox uses (`pip install -r +requirements.txt`, then `pytest`), so it is the configuration CI checks: the +`reviewer-path` job installs that way and runs both `pytest` and `python -m pytest` +against an empty `$HOME`. + +Offline is enforced rather than asserted. `conftest.py` wraps `socket.connect`, +`connect_ex` and `getaddrinfo` in stdlib code that refuses anything outside +loopback, so an unmarked test that reaches for the network fails with +`OfflineViolation: blocked DNS lookup for 'api.x402.celo.org': this test is not marked +network` instead of passing on a laptop with wifi and hanging in a sandbox. The +wrappers are re-asserted before every test, because anything that saves and restores +`socket.connect` around a test — the `pytest-socket` plugin's teardown does — would +otherwise disarm the guard halfway through a run. `tests/test_offline_guard.py` tests +the guard itself, and a CI job runs it on a runtime-only install. + +That guard has now caught three real leaks and one false promise. The leaks: a +paid-call receipt verified against a live RPC, a settle notification fired at +`api.telegram.org` from a test, and a refused invoice page that was priced (and so +needed an FX fetch) before it could say "Dropped". The false promise: the guard used +to live in `pytest.ini` as `addopts = --disable-socket`, which needs the +`pytest-socket` plugin from `requirements-dev.txt`; on any install without it, pytest +refused to start at all — `error: unrecognized arguments: --disable-socket` — and the +whole suite looked broken while CI, which installs dev requirements, stayed green. + +CI also runs ruff, byte-compilation, a 75% coverage floor (currently 79%) and an +advisory dependency audit. The densest tests are where silence costs money — every +refusal path in `bot/verify.py`, the x402 spend budgets, the screening gate, and the +public HTTP surface over a real socket. ## Tracks this targets diff --git a/conftest.py b/conftest.py index e8260e4..38bee48 100644 --- a/conftest.py +++ b/conftest.py @@ -1,3 +1,16 @@ +"""Suite-wide wiring: repo-root imports, an offline guard, and test isolation. + +A plain `pytest` has to pass on a clean checkout with no network, no `.env` and +no API keys — that is how a reviewer's sandbox runs it: the *runtime* +requirements plus whatever pytest the machine already has. So the offline +guarantee is implemented here, in the repo, rather than as a pytest.ini flag +that only exists when an optional plugin is installed. `addopts = +--disable-socket` did exactly that, and wherever `pytest-socket` was missing +pytest refused to start at all — the whole run errored with `unrecognized +arguments`, which reads as "the tests fail". +""" + +import socket import sys from pathlib import Path @@ -5,20 +18,125 @@ sys.path.insert(0, str(Path(__file__).resolve().parent)) +# Loopback and bind-all stay legal: the public HTTP surface is exercised over a +# real socket on 127.0.0.1, and binding a port is not "reaching the network". +_ALLOWED_HOSTS = {"127.0.0.1", "localhost", "::1", "0.0.0.0", ""} + +# How many `network`-marked frames are currently open (0 = suite is offline). +_online = {"depth": 0} +_installed = {} + + +class OfflineViolation(RuntimeError): + """A test tried to reach the network without being marked `network`.""" + + +def _describe(address): + host = address[0] if isinstance(address, tuple) and address else address + return repr(host) + + +def _install_offline_guard(): + """Block outbound connections unless a `network` test has lifted the guard. + + Stdlib only, so the guarantee does not depend on which dev requirements a + sandbox happens to install. The real functions are captured once and the + wrappers are re-asserted before every test: anything that saves and restores + `socket.socket.connect` around a test (pytest-socket's teardown does exactly + that) would otherwise disarm the guard halfway through the suite. + """ + if not _installed: + _installed["connect"] = socket.socket.connect + _installed["connect_ex"] = socket.socket.connect_ex + _installed["getaddrinfo"] = socket.getaddrinfo + + if "wrapper_connect" not in _installed: + _installed["wrapper_connect"] = _make_connect(_installed["connect"]) + _installed["wrapper_connect_ex"] = _make_connect_ex(_installed["connect_ex"]) + _installed["wrapper_getaddrinfo"] = _make_getaddrinfo(_installed["getaddrinfo"]) + + socket.socket.connect = _installed["wrapper_connect"] + socket.socket.connect_ex = _installed["wrapper_connect_ex"] + socket.getaddrinfo = _installed["wrapper_getaddrinfo"] + + +def _check_allowed(sock, address): + if _online["depth"] > 0: + return + if getattr(sock, "family", None) == getattr(socket, "AF_UNIX", object()): + return # local IPC, not the network + if _describe(address).strip("'") in _ALLOWED_HOSTS: + return + raise OfflineViolation( + f"blocked outbound connection to {_describe(address)}: this test is not " + "marked `network`. Mark it and run `pytest -m network`, or stub the call." + ) + + +def _make_connect(real): + def connect(self, address, *args, **kwargs): + _check_allowed(self, address) + return real(self, address, *args, **kwargs) + + return connect + + +def _make_connect_ex(real): + def connect_ex(self, address, *args, **kwargs): + _check_allowed(self, address) + return real(self, address, *args, **kwargs) + + return connect_ex + + +def _make_getaddrinfo(real): + def getaddrinfo(host, *args, **kwargs): + # host=None is a local bind; a hostname has to resolve before it can be + # dialled, so refusing here stops the lookup itself from hanging. + if _online["depth"] == 0 and host is not None and str(host) not in _ALLOWED_HOSTS: + raise OfflineViolation( + f"blocked DNS lookup for {host!r}: this test is not marked `network`." + ) + return real(host, *args, **kwargs) + + return getaddrinfo + + +def _uninstall_offline_guard(): + """Put the real socket functions back (used by this module's own tests).""" + if _installed: + socket.socket.connect = _installed["connect"] + socket.socket.connect_ex = _installed["connect_ex"] + socket.getaddrinfo = _installed["getaddrinfo"] + _installed.clear() + + +def pytest_configure(config): + _install_offline_guard() + + +def pytest_runtest_setup(item): + # Re-assert per test: see _install_offline_guard. + _install_offline_guard() + @pytest.fixture(autouse=True) -def _network_marker_reopens_sockets(request): +def _network_marker_opens_the_guard(request): """`-m network` runs are deliberately online. - pytest.ini blocks sockets for the default suite so an accidental outbound - call fails loudly instead of passing on a laptop and hanging in a sandbox. - A test carrying the `network` marker asked for the network, so it gets it - back for the duration of that test. + A test that asked for the network gets its sockets back for its own + duration; everything else stays offline, so an accidental outbound call + fails here instead of passing on a laptop with wifi and hanging in a + stranger's sandbox. """ if request.node.get_closest_marker("network"): - from pytest_socket import enable_socket - enable_socket() - yield + _online["depth"] += 1 + try: + yield + finally: + _online["depth"] -= 1 + else: + yield @pytest.fixture(autouse=True) @@ -32,4 +150,3 @@ def _claims_snapshot_goes_to_tmp(tmp_path, monkeypatch): """ monkeypatch.setenv("CLAIMS_PATH", str(tmp_path / "claims.json")) yield - diff --git a/pytest.ini b/pytest.ini index 50a313c..84df12a 100644 --- a/pytest.ini +++ b/pytest.ini @@ -1,13 +1,13 @@ [pytest] # A plain `pytest` run must pass on a clean checkout with no network, no .env -# and no API keys — that is how a judge (or a reviewer's sandbox) will run it. -# Anything that reaches outside the process is marked `network` and excluded by -# default. Run them deliberately: pytest -m network -# -# `--disable-socket` makes that a property instead of a promise: a test that -# quietly opens a socket fails here instead of passing on a laptop with wifi and -# timing out in a reviewer's sandbox. Localhost stays open for the server tests. -addopts = -m "not network" --disable-socket --allow-hosts=127.0.0.1,localhost,::1 +# and no API keys — that is how a judge (or a reviewer's sandbox) will run it: +# `pip install -r requirements.txt`, then `pytest`. So nothing here may require a +# package that lives only in requirements-dev.txt. An earlier version passed +# `--disable-socket` in addopts, which made pytest itself refuse to start +# wherever pytest-socket was not installed — the run died with `unrecognized +# arguments` before collecting a single test. The offline guard is now stdlib +# code in conftest.py and is always on. +addopts = -m "not network" testpaths = tests markers = network: needs an outbound HTTP call and/or an API key/secret diff --git a/requirements-dev.txt b/requirements-dev.txt index bb929a7..3349924 100644 --- a/requirements-dev.txt +++ b/requirements-dev.txt @@ -1,5 +1,4 @@ -r requirements.txt pytest>=9.1.1 pytest-cov>=7.1.0 -pytest-socket>=0.7.1 ruff>=0.17.3 diff --git a/tests/test_offline_guard.py b/tests/test_offline_guard.py new file mode 100644 index 0000000..1d52941 --- /dev/null +++ b/tests/test_offline_guard.py @@ -0,0 +1,83 @@ +"""The offline guard is a load-bearing claim, so it gets its own tests. + +The README promises that the default suite cannot reach the network. These tests +check the mechanism rather than trusting it: an unmarked test is refused at both +the DNS and the connect layer, loopback still works, and a `network` test gets a +real socket back. If any of that stops being true, it fails here. +""" + +import socket + +import pytest + +import conftest + + +def test_guard_is_installed_for_the_default_suite(): + assert conftest._installed, "pytest_configure never installed the offline guard" + assert conftest._online["depth"] == 0, "default suite should be offline" + + +def test_outbound_dns_is_refused(): + with pytest.raises(conftest.OfflineViolation) as excinfo: + socket.getaddrinfo("api.x402.celo.org", 443) + assert "not marked `network`" in str(excinfo.value) + + +def test_outbound_connect_is_refused_even_without_dns(): + # A literal address skips getaddrinfo, so the connect wrapper has to catch it + # on its own — otherwise an IP-literal call would slip through unmarked. + sock = socket.socket(socket.AF_INET, socket.SOCK_STREAM) + sock.settimeout(0.5) + try: + with pytest.raises(conftest.OfflineViolation): + sock.connect(("192.0.2.1", 80)) + finally: + sock.close() + + +def test_loopback_and_local_bind_stay_legal(): + socket.getaddrinfo("localhost", 0) + socket.getaddrinfo(None, 0) + socket.getaddrinfo("127.0.0.1", 0) + + +def test_loopback_connection_still_works(): + server = socket.socket(socket.AF_INET, socket.SOCK_STREAM) + server.bind(("127.0.0.1", 0)) + server.listen(1) + try: + client = socket.create_connection(server.getsockname(), timeout=2) + client.close() + finally: + server.close() + + +def test_guard_reasserts_itself_after_something_restores_the_real_connect(): + # Anything that saves and restores socket.socket.connect around a test (the + # pytest-socket plugin's teardown did) disarms the guard halfway through a + # run. The next test setup has to put it back. + real = conftest._installed["connect"] + socket.socket.connect = real + try: + conftest._install_offline_guard() + assert socket.socket.connect is conftest._installed["wrapper_connect"] + with pytest.raises(conftest.OfflineViolation): + socket.socket().connect(("192.0.2.1", 80)) + finally: + conftest._install_offline_guard() + + +def test_network_marker_lifts_the_guard(monkeypatch): + # `-m network` runs are deliberately online. Prove the lift without needing a + # working internet connection: a TEST-NET address is unroutable, so the call + # fails at the OS level, and what matters is that it is *not* our guard. + monkeypatch.setitem(conftest._online, "depth", 1) + sock = socket.socket(socket.AF_INET, socket.SOCK_STREAM) + sock.settimeout(0.2) + try: + with pytest.raises(OSError) as excinfo: + sock.connect(("192.0.2.1", 80)) + assert not isinstance(excinfo.value, conftest.OfflineViolation) + finally: + sock.close()