feat(solana): harden media surface + wire mid-poll re-sign payment guard - #19
Merged
Conversation
Follow-up hardening on the Solana media methods added in #16, addressing a two-model review (correctness/money-path + parity) and completing the in-progress security work. Security / money-path: - Wire _assert_same_payment_terms into the sync mid-poll re-sign: a fresh 402 challenge that reprices or redirects the payment vs. what the job originally authorized now raises PaymentError instead of signing an unbounded, unrelated payment. The guard was defined but never called (dead orig_amount/orig_pay_to capture); now enforced and unit-tested. - Sync + async re-sign: guard the challenge GET and signing so a network or signing error surfaces the gateway's real 402 reason instead of masking it (async challenge GET was unwrapped before). - _safe_path_segment on every network/symbol/market/wallet URL segment (LLM-controlled values can no longer escape the path). Correctness / parity vs the Base clients: - list_voices returns the gateway's {"data":[...]} list, not the whole envelope dict. - price(): data.get("price") so a paid body missing "price" surfaces a clean validation error, not a raw KeyError after the charge settled. - RealFace group_id validated with the shared _GROUP_ID_RE (was truthy-only). - RPC/music/speech settlement receipt + gateway metadata plumbed via _attach_receipt / _last_raw_headers / _rpc_response. - MEDIA_POLL_MAX_RESIGNS 3 -> 2 to match Base VideoClient. Tests: - New tests/unit/test_solana_media.py: the payment-terms guard, media dispatch (music/speech/sound-effects body + endpoint), list_voices envelope, local validation (lyrics+instrumental, video exclusivity, face-id prefix, portrait url), price KeyError-safety, and path-segment injection rejection. - Fixed the timeout-test payment fake to carry pay_to (the guard reads it). Known follow-up: the async re-sign does not yet run the re-price guard (needs submit-time payload threading through _sign_payment_from_response); async is otherwise unchanged from Base. validate_resource_url import was dropped as unused — wiring poll_url redirect validation is a separate change. 286 passed, 15 skipped; ruff + black clean.
VickyXAI
pushed a commit
that referenced
this pull request
Jul 5, 2026
…>=3.10) test_solana_media.py (added in #19) had no version guard and isn't in CI's 3.9 ignore list, so on 3.9 — where x402[svm] isn't installed — the autouse codec-stub fixture's monkeypatch.setattr(...decode_payment_required_header) raised AttributeError and errored the whole module. This is why main went red on 3.9 after #19. Add pytest.importorskip("x402"/"solders"), matching test_solana_timeout_routing.py.
VickyXAI
added a commit
that referenced
this pull request
Jul 5, 2026
* fix(solana): async re-sign payment-terms guard + poll_url host pinning Closes the two follow-ups flagged in #19: - Async mid-poll re-sign now runs the same _assert_same_payment_terms guard as the sync path. The async media helper inlines the submit-time signing so it can capture the original amount/pay_to, and the re-sign block mirrors the sync structure (guard runs OUTSIDE the try/except so a re-price PaymentError propagates instead of being masked as a generic 402). - _absolute_url now host+scheme-pins an absolute poll_url to the API origin (sync + async). The poll loop sends and re-signs the wallet PAYMENT-SIGNATURE against poll_url, so a gateway response redirecting it off-host would leak the signed payment; reject it. Tests (tests/unit/test_solana_media.py): end-to-end re-sign through the poll loop for sync AND async (same-terms completes; re-price propagates), plus poll_url host-pin cases (relative resolved, same-host ok, cross-host and http-downgrade rejected). * test(solana): skip test_solana_media on Python 3.9 (x402 extras need >=3.10) test_solana_media.py (added in #19) had no version guard and isn't in CI's 3.9 ignore list, so on 3.9 — where x402[svm] isn't installed — the autouse codec-stub fixture's monkeypatch.setattr(...decode_payment_required_header) raised AttributeError and errored the whole module. This is why main went red on 3.9 after #19. Add pytest.importorskip("x402"/"solders"), matching test_solana_timeout_routing.py. --------- Co-authored-by: 1bcMax <viewitter@gmail.com>
VickyXAI
added a commit
that referenced
this pull request
Jul 21, 2026
…y max_tokens clamping (1.8.2) (#32) * fix(payments): disclose gateway max_tokens clamping, stop re-paying after settlement The comment justifying MAX_TOKENS_SANITY_LIMIT claimed the gateway "rejects with that model's own number". It does not. Probed against the live 402 leg 2026-07-21 (unpaid quote leg only): opus-4.8 sent 262144 and 1000000 both quote the 128000 price, and gpt-5.2 sent 1e12 returns a quote rather than a 400. The gateway silently clamps to the model ceiling and charges for the clamped value, so there is no server-side rejection to fall back on. The only disclosure is the 402's resource.description, which was read solely to feed resource_description into the signature and then discarded — the one string that would tell a caller "you asked for 500000, you are paying for 128000" was thrown away at the moment it was in hand. _warn_if_clamped now surfaces it on every body-bearing payment path before the caller pays. Separately: _should_fallback returned True for any timeout, including one raised after the PAYMENT-SIGNATURE had gone out. The fallback chain then signed a fresh payment per model, so smart_chat PREMIUM COMPLEX could settle six times and return nothing — the "CHARGED BUT REQUEST FAILED" outcome the CHANGELOG already documents. Errors raised past the settlement boundary are now tagged and refused for fallback. Tagging via attribute rather than a new exception type keeps callers catching httpx.TimeoutException working. Also drops the Raises: docstring promising "PaymentError: If budget is set and would be exceeded" — no budget parameter exists anywhere in the SDK. * style: normalize line endings repo-wide via .gitattributes The repository had no .gitattributes and mixed endings: 17 tracked files were CRLF while the rest were LF. PR #27 converted 3 of them as a side effect of an unrelated fix, which inflated that diff from 51 real lines to 1045 and buried the behavioural change under formatting noise — `git diff` was ~95% churn and `--ignore-cr-at-eol` was needed to review it at all. Converting only 3 left the other 14 to regenerate the same problem on the next contributor's machine. `* text=auto` plus `git add --renormalize .` converts the remaining 14 in one formatting-only commit, so blame stays readable and endings stop depending on who checked out the repo. Verified content-free: `git diff --ignore-cr-at-eol` against the staged tree is empty. * release: 1.8.2 — correct the version the package reports about itself 1.8.1 shipped to PyPI with VERSION and __init__.py still reading 1.8.0, so the installed package reports __version__ == '1.8.0' while PyPI says 1.8.1. pyproject.toml was the only one of three locations I bumped. tests/unit/test_version_consistency.py exists precisely to catch this and did catch it — on CI, after the release had already been cut. I bumped, committed, tagged and released without re-running the suite in between, so a known-failing test never had a chance to stop the publish. Fixing the artifact needs a new version; main alone doesn't repair a wheel already on PyPI. * fix(payments): close the paid-5xx hole in the settlement guard, harden the clamp parse The settlement guard shipped incomplete. _mark_settled was applied only to httpx.TimeoutException and NetworkError, but the dominant post-settlement failure is a paid 5xx, which surfaces as APIError(503) — precisely a status _should_fallback treats as retriable. So the six-settlements-for-zero-tokens path the previous commit claimed to close was still fully open, and the CHANGELOG entry asserting otherwise was wrong. Reproduced before the fix: a 402-then-503 chain across three models sent six signatures. Every exception escaping the paid leg is now tagged. Over-tagging is the safe direction: those handlers begin at an already-read 402 response and create_payment_payload is local EIP-712 signing with no network I/O, so the only exceptions taggable without a settlement are ones _should_fallback already refuses. _warn_if_clamped also had two ways to break the request it was diagnosing. It runs on resource.description, a server-controlled string, immediately before signing: a non-string value raised TypeError, and `(\d[\d,]*)` backtracked super-linearly on a digit run (measured 2.78s at 16k digits). Now a bounded pattern over a 512-char slice, silent when the description is ambiguous rather than reporting a per-unit rate as the ceiling, and wrapped so nothing escapes. Tests: 20 new, covering the tag classification, one-settlement-per-call for both timeout and paid-5xx, that unpaid failures still walk the chain, and the parse. Mutation-verified — reverting the guard fails 5, narrowing it back to timeouts fails 1, restoring the old regex fails 1. Counts distinct signatures rather than signed requests, since the paid leg replays one signature on 502/503. * fix(payments): extend the settlement guard to Solana, gate publish on tests The guard was Base-only. _should_fallback_solana never consulted the settled tag and no Solana paid leg set it, so the double-payment path stayed fully open on one of the two chains while the CHANGELOG read as though both were covered. SPL USDC leaves the wallet on signing exactly like USDC on Base does. Both Solana paid stream phases now tag anything that escapes, and _should_fallback_solana refuses tagged exceptions ahead of its existing checks, so the issue #6 permanent-reason guard is untouched. Tests mirror the Base ones and assert both chains agree on what the tag means; mutation-verified by removing the guard (4 fail). Guarded with importorskip so the 3.9 CI job, which installs without the solana extra, stays green (see #19/#20). publish.yml built and published on release with no test step. That is how 1.8.1 reached PyPI with VERSION and __init__.py still at 1.8.0: the guard test caught it on the push-triggered run, after the release was cut, and PyPI does not allow overwriting a published file. The build job now installs the dev+solana extras and runs the suite before it builds. Promotes the changelog's Unreleased section to 1.8.2 so the four version declarations agree, and says plainly that 1.8.2 exists to supersede a wheel that misreports its own version. * fix(payments): narrow the settled tag to payment outcomes, close abandoned paid streams Review of #32 found three defects in the previous two commits. `except Exception` was too wide. It tagged a paid-leg 402 — which means the facilitator REJECTED the payment and the funds did not move — as blockrun_payment_settled, the one case where the name is exactly backwards. It also relabeled SDK bugs (AttributeError, KeyError, a PEP-479 RuntimeError) as payment outcomes, and `from None` erased the __context__ that explained them: x402.parse_payment_required deliberately raises an opaque "invalid format" whose only diagnostic value is its cause. Narrowed to `(httpx.HTTPError, APIError)`, which is provably exactly the fallback-eligible set — TimeoutException and NetworkError are both HTTPError subclasses, APIError is caught directly, and PaymentError is deliberately not an APIError subclass — so nothing that could trigger a second settlement escapes untagged, while rejections and bugs propagate as themselves. Re-raises bare instead of `from None`, preserving traceback and context. `async for chunk in self._astream_paid_phase(...)` did not close the inner generator when the outer was closed, so an abandoned paid stream stranded the `async with self._client.stream(...)` and its connection until GC finalization. Extracting the paid phase introduced it; the sync path never had it because `yield from` propagates close(). The same defect was already present at the `chat_completion_stream` fallback boundary, so a caller that breaks mid-stream stranded two generators. Both now aclose explicitly. The ReDoS timings in the code comment were wrong — 8k/16k were cited as 1.07s/11.88s, never measured by the author. Re-measured on CPython 3.13: 4k 0.13s, 8k 0.49s, 16k 1.95s. Corrected in the comment, the CHANGELOG and the test docstring so all three agree. Tests: 9 new, and the first drafts of two were rewritten after mutation testing showed they passed for the wrong reason — they asserted isolated shapes rather than driving the client, and the async one could not distinguish a deterministic close from CPython's asyncgen finalizer running at loop teardown. All 7 mutations now fail: removing either aclose, widening or narrowing the handlers, removing either chain's guard, restoring the old regex. * fix(release): make the Solana guard match its CHANGELOG claim, harden the publish gate The CHANGELOG said the settlement guard covers "both Base and Solana", but on Solana only the two streaming legs were tagged. The three non-stream paid legs were unwrapped. Not exploitable today, because _should_fallback_solana is only consulted from the streaming fallback loops and Solana non-stream chat exposes no fallback_models — but the claim was wrong, and the hole would reopen silently the day someone adds a fallback chain there. publish.yml claimed to gate on "the same suite" as CI while running only pytest on 3.11. Now runs black and ruff too, and the comment says plainly what it does and does not cover: the 3.9 and 3.12 legs still only run on push, so version-incompatible syntax is caught there, not here. Also asserts the release tag matches VERSION before anything is built. Cutting v1.8.3 from a 1.8.2 tree passed all 407 tests; PyPI's duplicate-version check was the only thing standing between that and a wrong publish, and it only fires after the release is cut. Same family as the 1.8.1 incident, closed at the same place. --------- Co-authored-by: 1bcMax <viewitter@gmail.com>
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.
Follow-up hardening on the Solana media methods added in #16, from a two-model review (correctness/money-path + Base parity) plus completion of the in-progress payment-terms guard.
Security / money-path
_assert_same_payment_termswas defined but never called (theorig_amount/orig_pay_tocapture was dead, and it crashed the image test fakes). It's now enforced on every sync mid-poll re-sign: a fresh 402 challenge that changes the amount or recipient raisesPaymentErrorinstead of authorizing an unbounded, unrelated payment. ThisPaymentErrordeliberately propagates — it is not swallowed by the fall-through that surfaces the original 402._safe_path_segmentvalidates everynetwork/symbol/market/walletvalue f-string'd into a paid endpoint path — these often come from LLM output.Correctness / Base parity
list_voicesreturns the gateway's{"data":[...]}list, not the whole envelope dict.price()usesdata.get("price")so a paid body missingpriceraises a clean validation error, not a rawKeyErrorafter the charge already settled.group_idvalidated with the shared_GROUP_ID_RE(was truthy-only)._attach_receipt/_last_raw_headers/_rpc_response.MEDIA_POLL_MAX_RESIGNS3 → 2 to match BaseVideoClient.Tests
New
tests/unit/test_solana_media.py(15 tests): the payment-terms guard (pass / amount-change / recipient-change / type-coercion), media dispatch (music/speech/sfx body + endpoint), thelist_voicesenvelope regression, local validation (lyrics+instrumental, video mutual-exclusivity, face-id prefix, portrait URL),price()KeyError-safety, and path-segment injection rejection. Also fixed the timeout-test payment fake to carrypay_to.286 passed, 15 skipped; ruff + black clean.
Known follow-ups (not in this PR)
_sign_payment_from_response(7 callers), which I didn't want to reshape here. Async is otherwise unchanged from Base.validate_resource_urlwas imported but unused; I dropped it. Wiringpoll_urlredirect validation is a separate, deliberate change (it swaps in a "safe default" on host mismatch, which needs its own testing).