fix(video): close the three money-path holes 0.38.1 left documented (0.39.1) - #101
Merged
Conversation
All three were flagged by the 0.38.1 review, recorded in that CHANGELOG as "known and unfixed", and pre-existing since well before the Seedance work. 1. UNREADABLE 402 -> FAIL CLOSED. amountToUsd returning null used to skip the re-reserve, sign the raw unvalidated amount, and book only the estimate — the last way past the budget cap. Now it aborts before createPaymentPayload with "no charge was made". Pinned: nothing signed, one fetch (the quote), reservation released, for six malformed shapes. 2. SSRF GUARD on image_url / last_frame_url, mirroring blockrun_image: http(s)-only schemes, then isBlockedFetchHostResolved on the hostname — resolved, not literal, so 127.0.0.1.nip.io-style names are caught. The MCP never fetches these itself; the gateway does. Defense-in-depth plus a saved paid round trip. Pinned: four hostile shapes refused with ZERO network calls; a public https URL still flows. 3. BOOK ON SETTLEMENT OBSERVATION. The gateway settles on the first poll it answers "completed"; the old path validated the payload first, so a completed body missing its clip URL threw, and finally released the reservation — real USDC spent, ledger unchanged, cap silently raised. Booking now happens the instant "completed" is observed. Pinned: the malformed-completed case keeps $0.40 in the ledger while still erroring; "failed" still books nothing. The first draft of fix 3 double-booked the happy path (poll site + old tail call — a replace hit the wrong occurrence). The new suite's exactly-once ledger assertion caught it before commit, which is the whole argument for asserting on money, not messages. 309 tests pass, typecheck green.
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
1 participant
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.
Closes the three pre-existing
blockrun_videoholes the 0.38.1 review documented as "known and unfixed". All are money-path: each one either lets USDC move unvalidated or lets the ledger lie about USDC that moved.amount→ re-reserve skipped, raw amount signed, estimate booked — the last way past the budget capimage_url/last_frame_urlhad no SSRF guard (blockrun_imagehas one)isBlockedFetchHostResolved(resolved, not literal — catches127.0.0.1.nip.io) before paymentcompletedpoll → throw →finallyreleases reservation while the USDC already settled — ledger silently under-counts, cap silently risescompletedis observed, before payload validation; caller still gets the errorfailedbooks nothing; happy path books exactly onceHonest note: the first draft of fix 3 double-booked the happy path (a replace guarded the wrong
recordActualSpendoccurrence). The new suite's exactly-once ledger assertion caught it pre-commit — the tests assert on money, not messages, for exactly this reason.Verification: 309/309 tests · typecheck green · no estimator or guard-table changes (verify:prices unaffected).
Merging publishes 0.39.1 to npm + the MCP registry.