Repository navigation
Integrate cleaned protocol histories with validated AP2 v0.2 helpers - #8
Merged
Merged
Conversation
Three admission paths accepted input they should have refused: - verifyDelegateSdJwtChain checked audience, nonce and typ inside the delegation loop, which never ran for a one-hop chain. A single closed mandate therefore verified with a stale nonce and the wrong audience, and disclosed no open mandate, so no constraint was evaluated either. Chains now need two hops; minHops opts out, and the terminal audience and nonce are checked even then. splitChain rejects an empty open set. - payment.execution_date was skipped entirely when the closed mandate omitted execution_date, so omitting the field escaped a window the user signed. Verification time is now used as the execution moment. - Checkout status was range-checked but not payability-checked, so a canceled checkout could back a capture. Only ready_for_complete, complete_in_progress and completed qualify; acceptedCheckoutStatuses widens that deliberately. Also: - Pin poolParticipationAuthorizationId against the Soroban vector. It had none, and the pooled contract tests sign in Rust, so nothing proved a TypeScript verifier's signature was verifiable on-chain. - Sort ScMap keys by byte value rather than localeCompare, which is locale- and ICU-build-dependent and deprioritizes underscore. - Refuse to sign a capture authorization whose merchant disagrees with the verified payee, when the payee id is a Stellar address. - Stop rebuilding a credential against the wall clock. An expired credential now reports EXPIRED from the validator's own trusted clock instead of INVALID_CREDENTIAL from a failed rebind. - Share amount, expiry and replay between the v0.1 and v0.2 branches; the schemas stay isolated, the common tail no longer drifts. - Type the signing digests as the full payload they actually hash. 69 package tests, 230 across the monorepo, T2 gate check passing.
0.4.0 could validate an AP2 v0.1 IntentMandate but no longer sign one, which stranded every 0.3.x call site and left the A4 adversarial gate crashing on its first check. Restore the v0.1 authoring path in legacy-v01.ts as signAp2V01Mandate and bindIntentMandate. The output is byte-identical to the published 0.3.0 package: legacy-v01.test.ts pins a credential generated by actually installing @reapp-sdk/ap2@0.3.0, so drift off the released wire format fails the build. The version is never inferred. signAp2Mandate means v0.2, signAp2V01Mandate means v0.1, and the two schemas stay isolated so neither can be read under the other's rules; a 0.3.x call site that upgrades without renaming fails closed on the unknown `intent` key rather than silently switching protocol versions. Rewrite the A4 adversarial script to run its full suite against both profiles. It previously threw at check 1 and, worse, its mustReject helper would have counted a failed mint as a fail-closed win. Rejections now must come from the validator with an expected Ap2ValidationError code, and refusals at signing are asserted separately from refusals at admission. 37 checks pass. 77 AP2 tests, 238 across the monorepo.
…signatures Round-1 review caught that the previous commit fixed only half the problem it described. mustReject was hardened to require an Ap2ValidationError with an expected code; mustRefuseAtSigning was left accepting ANY throw, so a profile-specific TypeError or a malformed fixture could still score as a fail-closed win. It now requires the expected refusal message. It also caught a real evidence gap: proving our own signer refuses unsupported semantics says nothing about an attacker who holds the user key and does not call our signer. A4 now reimplements the signing digest from the documented construction rather than importing it, and uses it to forge correctly signed credentials carrying semantics the package would never emit. Each profile first runs a control that forges an unmodified credential and requires the validator to ADMIT it — without that, a forged-rejection result would prove nothing, since the credential could be failing on a bad signature rather than on the rule under test. Both controls pass, which independently confirms the digest construction matches what the README documents. Add a prepack build hook so publishing from a clean checkout cannot ship an empty dist; packages/cli already used this pattern. Correct doc claims the review found unsupported: the adversarial README's "37 checks ... expected code" (11 of them were signing-time), stale 56/30 test counts in ap2-validator.md, checkoutReference described as unconditionally required when v0.1 ignores it, "byte-identical for the same inputs" without noting an omitted nonce is random, and "cannot collide" stated as absolute rather than as disjoint preimages under SHA-256. 41 A4 checks, 77 AP2 tests, gatecheck:t2 passes.
Round-2 review: the forged-semantics checks accepted three codes, so a regression that started returning BINDING_MISMATCH for an unrelated reason would still have scored as a pass. Both profiles produce INVALID_CREDENTIAL today — v0.1 from its credential schema before rebinding, v0.2 while rebinding refuses the unsupported constraint — so pin them there. Also from the review: make prepack clean dist first so a publish cannot carry orphaned output from an older build, state the adversarial README's code claim precisely rather than as blanket "exact", drop an over-absolute collision comment in the test, and correct repo-inventory's tarball count from eight to the five the gate actually builds.
agentool
changed the title
agentool
marked this pull request as ready for review
October 6, 2026 11:59
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.
Integrates all three cleaned protocol feature histories while preserving the retired wallet deletion and current verified Mainnet V2/CLI behavior. Adds explicit
@ackrate/ap2/v02helpers while keeping the default v0.1 API, signed bytes, hardened SD-JWT verifier, replay identity, package versions and existing payment runtime intact. Historical extension encoders do not add Mainnet V2 methods.The independent review identified inherited fail-open cases. This revision rejects malformed signed date windows, invalid or overflowing usage counters, and pool authorization whose verified checkout merchant differs from the participation terms. Tests use actual signed and verified checkout chains. Current main is merged into the branch, and all three original feature heads remain ancestors.
Validation: 454 tests, clean builds, strict types and the full packed release gate passed. Combined and five minimal consumers passed dependency checks, the explicit export and signing/XDR interoperability. Clean-head candidates and original evidence are retained in canonical project artifacts. Updated exact-head CI and independent Claude Code Opus5.5 follow-up review are pending. Packages remain unpublished and require version bumps before registry publication.