feat: add Tempo charge sender validation hook

[brendanjryan]

Summary

Added a Tempo charge sender validation hook for accepting authorized third-party transfer senders while preserving core payment verification checks.

[pkg-pr-new]

Open in StackBlitz

npm i https://pkg.pr.new/mppx@452

commit: 4562f26

[tempoxyz-cyclops-bot]

👁️ Cyclops Review

PR #452 introduces an opt-in validateSender hook on tempo.server.charge() so operators can accept TIP-20 hash credentials whose on-chain Transfer.from differs from the address mppx would otherwise require. The default no-hook behavior remains bit-for-bit equivalent to the prior strict matcher, and replay protection / memo binding / source-DID validation continue to run.

The hook is well scoped, but it surfaces a pre-existing weakness in assertTransferLogs(): it consumes decoded log entries by array index instead of by economic TIP-20 payment effect, so a permissive validateSender can let one underlying debit satisfy two distinct expected transfers. Two manifestations were verified end-to-end with PoC tests (TIP-1022 virtual-address forwarding hop, and same-recipient split with one transferWithMemo). Inline comments below.

Full consolidated report: findings/pr-452/pr-452-consolidated.md.

Reviewer Callouts

• ⚡ credential.source trust boundary: With validateSender, credential.source can intentionally differ from the actual Transfer.from. onPaymentSuccess handlers, custom transports, and any merchant code that reads credential.source should not treat it as authenticated payer identity unless their hook explicitly verifies the source -> sender relationship.
• ⚡ Hook context is narrow: The hook sees only { expectedSender, sender, source } — no log kind, recipient, amount, memo, receipt hash, or block number. This makes it impossible for policy code to distinguish real third-party debits from synthetic/upstream attribution events.
• ⚡ Memo binding is "at least one matched memo": Safe for normal client flows but fragile when one upstream movement produces multiple matchable logs and validateSender is permissive — the memo half satisfies binding while the paired plain log carries no challenge attribution.
• ⚡ Explicit-memo + permissive validateSender: When the server sets an explicit static memo, no challenge binding is enforced. Combined with a permissive validateSender, any on-chain Transfer matching (currency, recipient, amount, memo) from any sender can be claimed by any hash credential. Worth amplifying in user-facing docs.
• ⚡ Greedy matching with looser from predicate: Consider adding a test that exercises N splits + N+M logs with mixed senders to confirm the algorithm still picks a valid combination when one exists.

[tempoxyz-cyclops-bot]

🚨 [SECURITY] assertTransferLogs matches by raw decoded log entries instead of by economic payment effects

Severity: Medium

The matcher decodes Transfer and TransferWithMemo events as independent entries (logs = [...memoLogs, ...transferLogs]) and tracks consumption via used keyed by the concatenated array index. A single TIP-20 movement can produce multiple decoded entries in that array, so two distinct expected transfers can be satisfied by one underlying debit — but only after this new validateSender hook lets the mismatched Transfer.from through. Two PoC-verified manifestations:

1. TIP-1022 virtual-address forwarding. A transferWithMemo to a virtual address emits Transfer(payer, virtual, amount) + TransferWithMemo(payer, virtual, amount, memo) + the synthetic forwarding log Transfer(virtual, master, amount). With a permissive hook (e.g. an allowlist that approves the virtual address), the primary expected transfer to the virtual address consumes the memo log and the split to the master address consumes the forwarding log. One paid token satisfies a challenge requiring two.
2. Same-recipient split with one transferWithMemo. A schema-valid charge with amount: '2' and splits: [{ recipient: merchant, amount: '1' }] produces two indistinguishable expected transfers (merchant, 1). One transferWithMemo(thirdParty -> merchant, 1, challengeMemo) emits one Transfer and one TransferWithMemo for the same debit. After the hook approves thirdParty, the first expected transfer consumes the memo log and the second consumes the paired plain Transfer. assertChallengeBoundMemo() (Charge.ts:974-985) only requires one matched log to be challenge-bound, so the memo half is sufficient.

In both cases the hook itself cannot detect the issue — it receives no log kind, recipient, amount, memo, or log index. Both flows were reproduced with bun test PoCs on this commit; without validateSender the strict equality check still rejects the same receipts.

Recommended Fix:

• Collapse adjacent Transfer(from, to, amount) and TransferWithMemo(from, to, amount, memo) for the same debit into one matchable effect carrying optional memo metadata; mark both indices used when either is matched.
• For TIP-1022 virtual-address forwarding, refuse to invoke validateSender on the synthetic Transfer(virtual, master, amount) forwarding hop (or pair it with the preceding payer -> virtual log as one effect).
• Either reject duplicate (recipient, amount, allowAnyMemo) expected transfers in getExpectedTransfers(), or require the receipt contains that many distinct collapsed effects before accepting.
• Optionally broaden ValidateSenderParameters so policy code can see log kind / recipient / amount / receipt context.

[brendanjryan]

will fix in another pr

[tempoxyz-cyclops-bot]

💡 [SUGGESTION] Tighten validateSender JSDoc to match actual trigger conditions

Severity: Low

The doc says the hook validates a sender "when it differs from the credential source," but the actual trigger is "differs from source?.address ?? receipt.from." The hook also fires when no source field is present (smart-account / EOA-only flows), and it can be invoked multiple times per credential — once per candidate matching log — without that being documented. An operator writing validateSender: ({source, sender}) => isAuthorized(source!.address, sender) will throw at runtime when a credential omits source; operators implementing metrics/rate-limiting inside the hook will observe duplicate invocations.

Additionally, the hash-credential source field is a self-asserted string and is not cryptographically authenticated. The doc should warn that validateSender cannot be used to restrict otherwise valid payers (an attacker can supply their own address as source to bypass an app-level allowlist implemented in the hook); it can only expand the set of accepted senders for a given source.

Recommended Fix: Update the JSDoc on charge.Parameters.validateSender and on ValidateSenderParameters.source to call out (1) the actual trigger condition including source = undefined, (2) potential multi-invocation per verification (hook should be side-effect-free / idempotent), and (3) that source is a client-supplied claim, so the hook is expand-only and not usable as a deny-list.

[tempoxyz-cyclops-bot]

🛡️ [DEFENSE-IN-DEPTH] Pull-mode assertTransferLogs call omits validateSender and source

Severity: Low (not currently exploitable)

This pull-mode (transaction credential) call site does not thread validateSender / source into assertTransferLogs(). Today this is not exploitable: pre-broadcast assertTransferCalls() / getTransferCalls() (Charge.ts:650-690) restrict accepted calldata to direct transfer / transferWithMemo (and the approve + swapExactAmountOut prefix) on the configured currency, so Transfer.from == transaction.from is structurally forced and the hook would never be consulted.

If Tempo later supports operations where msg.sender differs from the signing EOA (session-key relays, smart-account batched ops, account-abstraction calls, vault-mediated transfers), the omission becomes a real "user paid on-chain but server returns 402" bug because the on-chain Transfer.from would no longer equal transaction.from.

Recommended Fix: For API symmetry and forward-compatibility, thread validateSender (and source when available) into this call as well. Mechanical change that prevents a future regression once richer transaction-credential flows are supported.