fix(api_server): run the cron-fire token verifier off the event loop (salvage #65730) - #76972
Merged
kshitijk4poor merged 1 commit intoAug 2, 2026
Conversation
_handle_cron_fire verified the NAS-minted fire JWT by calling the fire-verifier inline on the event loop. That verifier resolves the NAS signing key from a JWKS URL — a synchronous HTTP GET on a cache miss (a cold PyJWKClient, or a rotated kid the cached client doesn't know) — so a slow or rate-limited portal stalls the whole event loop and starves every other adapter sharing it. NousResearch#64641 already documented this exact symptom (relay 504s on high-job-count instances) and cut the fetch frequency by caching the client per URL, but the residual cache-miss fetch still ran inline on the loop. Dispatch the verifier the same way the platform HTTP event verifier was hardened: await a coroutine verifier directly, run a sync one via asyncio.to_thread so its blocking I/O stays off the loop, and fail closed (reject with 401, never admit the fire) if the verifier raises — this is the only inbound that can trigger remote job execution. The verifier's JWK-client cache is already thread-safe (threading.Lock), so moving the call to a worker thread is safe. Adds regression tests: a sync verifier runs on a worker thread rather than the loop thread, a crashing verifier yields 401 with no fire, and a coroutine verifier is awaited.
kshitijk4poor
enabled auto-merge (rebase)
August 2, 2026 17:38
12 tasks
alt-glitch
added
type/perf
19 tasks
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
3 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Salvages #65730 by @Frowtek — commit cherry-picked to preserve authorship, conflict resolved (test-file-only).
Context — what this fixes, for whom
Anyone using Chronos-scheduled cron jobs:
_handle_cron_fireis the inbound webhook that triggers remote job execution, and it verifies the NAS-minted JWT inline on the event loop. The verifier resolves signing keys from a JWKS URL — a synchronous HTTP GET on cache miss (cold process or key rotation). A slow or rate-limited portal stalls every other adapter sharing the loop. Worse, on current main a crashing verifier (network error, parse error) propagates as an unhandled exception → 500, on the one endpoint that can trigger remote code execution.What the fix does (from #65730, kept verbatim)
asyncio.to_thread(mirrors the platform-event verifier hardening in 14f023c); coroutine verifiers are awaited directly.logger.exceptionand rejects the fire with 401 (instead of today's 500) — the correct posture for an RCE-trigger endpoint.Conflict resolution (ours, content-free)
Cherry-pick anchor drift only: main added
test_fire_does_not_require_api_server_keyat the same insertion point. Both sides kept; zero production-code conflict. 6/6 tests pass.Verification
tests/gateway/test_cron_fire_webhook.py: 6 passed (2 pre-existing + PR's 3 + main's 1)get_fire_verifierat module level with plain lambdas —iscoroutinefunctionroutes them toto_thread, behavior preserved (verified by the 2 pre-existing tests passing)Closes #65730 (superseded by this salvage — original author credited via cherry-pick authorship).