diff --git a/tests/conftest.py b/tests/conftest.py index c25f7b27..8b993538 100644 --- a/tests/conftest.py +++ b/tests/conftest.py @@ -205,8 +205,6 @@ import socket as _hermes_test_socket _REAL_CREATE_CONNECTION = _hermes_test_socket.create_connection _REAL_SOCKET_CONNECT = _hermes_test_socket.socket.connect -_ALLOW_OUTBOUND = False # toggled by the allow_outbound_network fixture - def _hermes_addr_is_local(host: str) -> bool: """Return True for loopback / RFC1918 / link-local / reserved-TLD hosts.""" @@ -258,8 +256,6 @@ def _hermes_addr_is_local(host: str) -> bool: def _hermes_blocked_create_connection(address, *a, **kw): - if _ALLOW_OUTBOUND: - return _REAL_CREATE_CONNECTION(address, *a, **kw) try: host = address[0] except (TypeError, IndexError): @@ -274,8 +270,6 @@ def _hermes_blocked_create_connection(address, *a, **kw): def _hermes_blocked_socket_connect(self, address): - if _ALLOW_OUTBOUND: - return _REAL_SOCKET_CONNECT(self, address) try: host = address[0] except (TypeError, IndexError): @@ -292,19 +286,22 @@ _hermes_test_socket.socket.connect = _hermes_blocked_socket_connect @pytest.fixture -def allow_outbound_network(): +def allow_outbound_network(monkeypatch): """Opt-in to real outbound network for the duration of one test. - Use sparingly. Today only `test_dns_resolution_failure` needs this — - it intentionally exercises a real DNS lookup on a reserved-TLD hostname - to verify the probe maps `socket.gaierror` to `error='dns'`. + Swaps `socket.create_connection` and `socket.socket.connect` back to the + real (unwrapped) implementations for this test only, then monkeypatch + teardown restores the wrapped versions. Direct swap is more reliable + than a module-global toggle on CI runners where wrapper-closure + lookup semantics can surprise. + + Use sparingly. Today zero tests in the repo call this — the previous + test_dns_resolution_failure case was rewritten to mock socket.getaddrinfo + instead, which is fully hermetic. """ - global _ALLOW_OUTBOUND - _ALLOW_OUTBOUND = True - try: - yield - finally: - _ALLOW_OUTBOUND = False + monkeypatch.setattr(_hermes_test_socket, "create_connection", _REAL_CREATE_CONNECTION) + monkeypatch.setattr(_hermes_test_socket.socket, "connect", _REAL_SOCKET_CONNECT) + yield diff --git a/tests/test_conftest_network_isolation.py b/tests/test_conftest_network_isolation.py index 204c0671..8326a6cd 100644 --- a/tests/test_conftest_network_isolation.py +++ b/tests/test_conftest_network_isolation.py @@ -97,38 +97,37 @@ def test_public_ipv4_is_blocked(): assert _conftest._hermes_addr_is_local("204.0.113.0") is False # outside -def test_allow_outbound_network_fixture_disables_the_block_for_a_public_ip(allow_outbound_network): - """When a test opts in to the fixture, the block does NOT fire on a - destination that is otherwise blocked. +def test_allow_outbound_network_fixture_unswaps_the_wrappers(allow_outbound_network): + """When a test opts in to the fixture, socket.create_connection and + socket.socket.connect are restored to their real (unwrapped) implementations + for this test only. - Uses 8.8.8.8 (Google DNS, a real public IPv4 not in any allow-list) - so we can prove the fixture actually disabled the wrapper. Without - the fixture, the wrapper would raise OSError("hermes test network - isolation: ..."). With the fixture, the real socket.create_connection - runs and we either succeed (port 53 is genuinely open) or fail with - a *real* connect error — never with our wrapper's message. + Direct identity check is safer than a behavioral test: it doesn't depend + on whether the CI runner has outbound network access, only on whether the + fixture's monkeypatch actually swapped the symbols. """ - err_msg = "" - try: - sock = socket.create_connection(("8.8.8.8", 53), timeout=2) - sock.close() - # Success is fine — proves the wrapper got out of the way. - return - except OSError as e: - err_msg = str(e) - - # If we did get an OSError, it must NOT be from our wrapper. - assert "hermes test network isolation" not in err_msg, ( - f"allow_outbound_network fixture should disable the block, but got: {err_msg}" + import tests.conftest as _conftest + # Inside the fixture, socket.create_connection should be the REAL one + # (the one captured before the module-level wrap), not the blocked wrapper. + assert socket.create_connection is _conftest._REAL_CREATE_CONNECTION, ( + "allow_outbound_network fixture did not restore the real create_connection" + ) + assert socket.socket.connect is _conftest._REAL_SOCKET_CONNECT, ( + "allow_outbound_network fixture did not restore the real socket.connect" ) def test_block_is_active_outside_the_fixture(): - """Sanity: a test that does NOT request the fixture has the block active. + """Sanity: a test that does NOT request the fixture has the wrapped + socket.create_connection installed (not the real one). - Pairs with the test above to prove the fixture toggle is real — without - this paired test the fixture test would self-pass even if the toggle - didn't work (since the block is on by default and the wrapper-or-real - distinction is what matters).""" - with pytest.raises(OSError, match="hermes test network isolation"): - socket.create_connection(("8.8.8.8", 53), timeout=1) + Pairs with the test above to prove the fixture swap is real — the + fixture test confirms the unswap; this test confirms the default-on + state.""" + import tests.conftest as _conftest + assert socket.create_connection is _conftest._hermes_blocked_create_connection, ( + "default state should have the blocked wrapper installed on socket.create_connection" + ) + assert socket.socket.connect is _conftest._hermes_blocked_socket_connect, ( + "default state should have the blocked wrapper installed on socket.socket.connect" + )