Loopback OIDC timeouts and the callback state check have no test coverage #259

New issue

Closed

opened 2026-09-06 15:55:57 +00:00 by stephen · 0 comments

stephen commented

2026-09-06 15:55:57 +00:00

Owner

#258 rewrote the OIDC loopback wait onto async tokio and added
cancelling_wait_for_code_does_not_stall_runtime_shutdown. That test binds, cancels after 50ms and
asserts on runtime shutdown latency. It never connects to the listener, so it exercises
cancellability and nothing else.

Three properties of the rewritten path have no test at all:

CALLBACK_TIMEOUT (300s), the outer budget around the accept loop.
REQUEST_LINE_TIMEOUT (10s), which is what stops one silent connection wedging the sign-in.
The state != expected_state comparison in the loopback callback. parse_callback_* covers the
pure parser and pasted_state_is_csrf_checked_but_bare_code_passes covers the manual flow, which
is a different code path.

So if the rewrite had silently broken the 300s budget or the loopback state check, nothing in the
repo would catch it. That is the same shape the gate on #258 flagged in the other direction: the
evidence for a working timeout and a working CSRF check is currently indistinguishable from the
evidence for a broken one.

The probes exist and pass on 116be94. They were written during the #258 gate, run, and removed
before the verdict so the reviewed tree stayed byte-identical to the head:

PROBE right-state      => Ok("THECODE")
PROBE wrong-state      => Some("OAuth state mismatch on the callback; aborting to avoid a possible CSRF")
PROBE silent-then-real => Ok("LATE") after 9.900777689s

They drive a real connection against the bound listener and run in about ten seconds total. Landing
them as tests is the work.

Separately, and not part of this: connections are still handled one at a time, so
REQUEST_LINE_TIMEOUT bounds a single stall but not the aggregate. Roughly thirty stalled
connections would consume the whole 300s budget in sequence. The listener is loopback-only so this
needs a local process, and it is strictly better than the pre-#258 shape where one silent socket
blocked forever.

`#258` rewrote the OIDC loopback wait onto async tokio and added `cancelling_wait_for_code_does_not_stall_runtime_shutdown`. That test binds, cancels after 50ms and asserts on runtime shutdown latency. It never connects to the listener, so it exercises cancellability and nothing else. Three properties of the rewritten path have no test at all: - `CALLBACK_TIMEOUT` (300s), the outer budget around the accept loop. - `REQUEST_LINE_TIMEOUT` (10s), which is what stops one silent connection wedging the sign-in. - The `state != expected_state` comparison in the loopback callback. `parse_callback_*` covers the pure parser and `pasted_state_is_csrf_checked_but_bare_code_passes` covers the manual flow, which is a different code path. So if the rewrite had silently broken the 300s budget or the loopback state check, nothing in the repo would catch it. That is the same shape the gate on #258 flagged in the other direction: the evidence for a working timeout and a working CSRF check is currently indistinguishable from the evidence for a broken one. The probes exist and pass on `116be94`. They were written during the #258 gate, run, and removed before the verdict so the reviewed tree stayed byte-identical to the head: ``` PROBE right-state => Ok("THECODE") PROBE wrong-state => Some("OAuth state mismatch on the callback; aborting to avoid a possible CSRF") PROBE silent-then-real => Ok("LATE") after 9.900777689s ``` They drive a real connection against the bound listener and run in about ten seconds total. Landing them as tests is the work. Separately, and not part of this: connections are still handled one at a time, so `REQUEST_LINE_TIMEOUT` bounds a single stall but not the aggregate. Roughly thirty stalled connections would consume the whole 300s budget in sequence. The listener is loopback-only so this needs a local process, and it is strictly better than the pre-#258 shape where one silent socket blocked forever.

stephen referenced this issue from a commit

2026-09-06 16:11:57 +00:00

Cover the OIDC loopback callback paths with real-socket tests

stephen referenced this issue from a pull request that will close it,

2026-09-06 16:12:19 +00:00

Cover the OIDC loopback callback paths with real-socket tests #260

stephen closed this issue

2026-09-06 16:27:38 +00:00

stephen referenced this issue from a commit

2026-09-06 16:27:39 +00:00

Cover the OIDC loopback callback paths with real-socket tests (#260)