pytest: make test_grpc_wrong_auth cross the wires on one port, with deadlines
What the developer wrote
Authored by cdecker
pytest: make test_grpc_wrong_auth cross the wires on one port, with deadlines
After l1 stopped, the old stub dialled l1's now-free gRPC port while l2
listened on a different one, so the test never checked a foreign
certificate against a live node. Its gRPC calls also had no deadline: when
the client keeps retrying a handshake the server rejects, the call blocked
forever and hung the test.
Start l2 on l1's port, connect to 127.0.0.1 explicitly, and give every call
a deadline. The call with the wrong credentials may now end in
DEADLINE_EXCEEDED as well as UNAVAILABLE; both mean it was refused.
Changelog-None
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
What changed, and why it matters
This commit only changes a test file. It fixes a flaky test that was supposed to verify a gRPC client with the wrong certificate gets rejected by a different node, but the test was not actually checking the real scenario and could hang forever. There is no change to production code or any security fix to the software itself.
No security action needed. This is a test-only improvement. Reviewers may optionally verify the test now correctly exercises certificate mismatch against a live node.
Security signals we found
No strong security signals were identified.
Evidence from the diff
The patch modifies tests/test_cln_rs.py’s test_grpc_wrong_auth. Previously, after stopping l1, the reused stub could dial l1’s freed gRPC port while l2 listened elsewhere, so the test never verified a foreign certificate against a live node. It also lacked call deadlines, so retrying a rejected handshake could hang. The fix binds l2 to l1’s former gRPC port, connects explicitly to 127.0.0.1, and adds timeouts to gRPC calls. This is a test-hardening change only.
Changed components
tests/test_cln_rs.pyInspect captured patch +14 / −6
### tests/test_cln_rs.py
@@ -294,27 +294,35 @@ def connect(node):
)
channel = grpc.secure_channel(
- f"localhost:{node.grpc_port}",
+ f"127.0.0.1:{node.grpc_port}",
creds,
options=(('grpc.ssl_target_name_override', 'cln'),)
)
return clnpb.NodeStub(channel)
+ # Calls get a deadline: without one, a connection that never completes
+ # its handshake blocks the test forever instead of failing it.
stub = connect(l1)
# This should work, it's the correct node
- stub.Getinfo(clnpb.GetinfoRequest())
+ stub.Getinfo(clnpb.GetinfoRequest(), timeout=TIMEOUT)
+ # Start l2 on l1's gRPC port, so the old stub really talks to a different
+ # node rather than to a freed port.
l1.stop()
+ l2.grpc_port = l1.grpc_port
+ l2.daemon.opts['grpc-port'] = l1.grpc_port
l2.start()
wait_for_grpc_start(l2)
- # This should not work, it's a different node
- with pytest.raises(Exception, match=r'Socket closed|StatusCode.UNAVAILABLE'):
- stub.Getinfo(clnpb.GetinfoRequest())
+ # This should not work, it's a different node. Depending on timing the
+ # client either gives up on the rejected handshake or keeps retrying it
+ # until the deadline expires; both mean the old credentials are refused.
+ with pytest.raises(Exception, match=r'Socket closed|StatusCode.UNAVAILABLE|StatusCode.DEADLINE_EXCEEDED'):
+ stub.Getinfo(clnpb.GetinfoRequest(), timeout=10)
# Now load the correct ones and we should be good to go
stub = connect(l2)
- stub.Getinfo(clnpb.GetinfoRequest())
+ stub.Getinfo(clnpb.GetinfoRequest(), timeout=TIMEOUT)
def test_cln_plugin_reentrant(node_factory, executor):Why this scored 15/100
Community notes
Notes can correct, qualify, or add evidence to the AI analysis. Every note shown here has been validated by a human moderator.
The AI analysis stands alone for now. Submit a note if you can add evidence or important context.