fix(flows): survive malformed requests in the compliance server#55
Open
0xKarm wants to merge 1 commit into
Open
fix(flows): survive malformed requests in the compliance server#550xKarm wants to merge 1 commit into
0xKarm wants to merge 1 commit into
Conversation
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
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.
compliance-server: one malformed request can kill the whole flow run
Repro
Against the server as it ships today:
JSON.parsethrows inside the async request listener, nothing catches it, andNode's default
unhandled-rejections=throwterminates the process. Because theflow runner starts this server once and reuses it for every adapter, the
crash cascades: every remaining flow case for every remaining adapter fails
with connection errors, and the eventual report points nowhere near the actual
cause.
The listener had several such landmines, all reachable with a single request:
/rpcJSON.parse(raw)challengecredential.challenge.idAuthorizationon adigest_bindingrouteCredential.deserialize(...)Authorizationon abind_request_resourcerouteCredential.deserialize(...)Hostheadernew URL(...)'error'event onreq(plus a forever-pendingreadBodypromise)Notably the
invalid_challenge_idbranch does guard itsChallenge.deserializewith try/catch — the unguarded copies elsewhere looklike oversights rather than intent.
Fix
Three layers, smallest change that makes each input class behave sensibly:
handleRequest(); theexported listener wraps it in try/catch and answers
400with{"ok": false, "error": "malformed request"}(only if headers weren'talready sent), logging the error to stderr. The process no longer dies.
readBodynow rejects onreq.on('error'), so an aborted uploadsurfaces as a caught error instead of a hung response plus process crash.
deserializeCredentialSafely()helper. A garbageAuthorizationheader issemantically "no valid credential", so those routes now respond with the
protocol-correct
402+ fresh challenge rather than a 400 (or, before thispatch, a dead server).
Verified
Before (main):
POST 'not json' -> /rpc→ connection dropped, process exits,subsequent
/freeunreachable.After (this branch), same server instance across all probes:
Regression:
python3 scripts/flow_runner.py --adapter typescript→PASSED: 33 passed, 33 total (golden untouched).
npx tsc --noEmitreportsthe identical pre-existing error set as main (23 = 23, all environmental
@types/noderesolution noise; nothing new from this change).