Repository navigation
1.9.79: payment notifications answer no_click and unknown_click in each provider's own field names - #319
Merged
Conversation
Account › API integrations handed out tracking202/static/paykickstart.php as PayKickstart's Affiliate IPN URL under "No setup needed", and GET /system/integrations and `p202 system integrations` listed it. The file never existed, so every IPN sent there was a 404. The Affiliate IPN could not have worked anyway: it is unsigned JSON with PayKickstart's own tracking id and no custom variables, so nothing in it names a click. The URL now takes PayKickstart's product (vendor) IPN. A vendor puts subid=[[subid]] on the checkout link that is the campaign's offer URL, pastes the URL into each product's IPN Integration tab and stores each PayKickstart campaign's IPN secret key (202_users_pref, one per line, at most 20, never served back). Each IPN is verified against every account's stored keys with PayKickstart's published HMAC-SHA1 algorithm before anything in it is read, and is recorded for the account whose key verifies it, on a click of that account (another account's click reads as no click). sales records a conversion (source paykickstart, pixel type 2) under its transaction_id at its amount; refund reverses it; test mode, subscription-payment and every other event record nothing. A retry under a transaction id on file is a duplicate only when it states the same amount, and is refused (409) when it states another. Every outcome is a status PayKickstart's log shows, with the reason, and one log line without a key or buyer data. The receiving side is provider-neutral (Prosper202\PaymentNotification: a ProviderAdapter that verifies and parses, one Processor that records or reverses through p202RecordLegacyConversion(), one Response mapping), so JVZoo and ClickBank can follow as adapters. HandedOutUrlsNameServedFilesTest holds every script a served literal names, and every integration GET /system/integrations lists, to a file in the tree; jvzoo.php, zpn.php and slack.php are listed as known dead until their own changes. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01VRoS2SViuxaucFaKWsFnLr
202-config/changelogs.json kept both: #258's updated lines, then the PayKickstart line. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01VRoS2SViuxaucFaKWsFnLr
#258 made the API answer a 5xx while the files are deployed and the upgrade not run with a 503 naming the upgrade. The IPN endpoint is not in the API's tree, and between those two moments its keys' column is not there yet: it answered 500. The Receiver now asks SchemaVersion::pendingUpgrade() on a failure and answers 503 with its sentence (OutcomeKind::UPGRADE_PENDING); PayKickstart's retry lands once the upgrade has run. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01VRoS2SViuxaucFaKWsFnLr
…ey is "the key" admin-005 asks an agent to store a campaign's PayKickstart IPN secret key and give the URL to paste: the key is stored with user prefs update, the URL is the one system integrations answers, and neither the preferences read nor the reply repeats the key. The reference agent answers it; an agent that stores nothing and repeats the key fails all four checks. Search puts system integrations first for the ask (two tasks in one ask), so it is listed in searchKnownMisses rather than taught by a phrase lifted from the ask. A refusal of a single pasted key says "The key is 5 characters long" rather than "Key 1"; a list still names the key by its place. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01VRoS2SViuxaucFaKWsFnLr
…what the reversal on file took back A refund with no amount (or 0) takes back the whole sale. Its comparison with the reversal on file was skipped when the amount was null, so a full refund after a partial one was answered 200 duplicate: PayKickstart stopped retrying while the rest of the sale stayed credited (CLAUDE.md #15). The Processor now reads the reversal on file with the sale it reverses and compares what the refund takes back, a full refund read as the sale's amount. Any other refund of a sale already reversed is a 409 naming both: "already reversed by 10.5, and 36.5 of its 47 is still credited; this refund states its full amount, 47". A sale is still reversed once. The other duplicate decision, a sale or rebill under a transaction id on file, compares two amounts that are never null (the sale's is required, the stored one NOT NULL). Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01VRoS2SViuxaucFaKWsFnLr
Account › API integrations, GET /system/integrations and p202 system integrations handed out tracking202/static/jvzoo.php as JVZoo's IPN URL, and the file never existed: every JVZIPN sent there was a 404. It now runs the payment-notification Receiver (#268) with a JVZoo adapter. JVZoo's own article ("JVZIPN - How to create your own integrated script", read through its Help Center API) is the source. The cverify is its published validator's: every posted value but cverify's, in field-name order, each followed by "|", the account's JVZIPN secret key, SHA-1, the first 8 hex digits upper-cased; compared with hash_equals(), where the sample's == would take "+12345678" for an all-digit cverify. It is checked against every account's stored key before anything is read, and the account is the one whose key verifies it (CLAUDE.md #16), so a vendor's sale that names no click is still acknowledged. A JVZIPN carries two click fields, caffitid (the tid on an affiliate's link) and cvendthru (the variables on the vendor's buy link), and each is the other party's choice to the one who did not set it. JVZoo asks for the URL in two places, an affiliate's Affiliate IPN and a product's JVZIPN v1 URL, so each gets its own URL: ?role=affiliate reads only caffitid, ?role=vendor only cvendthru's subid; a URL with no role is a 400. SALE and BILL record a conversion (source jvzoo, new) under ctransreceipt at ctransamount; RFND, CGBK and INSF (JVZoo: an eCheck chargeback, so a reversal, not nothing) reverse the whole sale ctransreceipt names, since JVZoo refunds a payment whole; CANCEL-REBILL, UNCANCEL-REBILL and TEST record nothing. JVZoo marks no test notification: a Test Purchase Mode sale is a real sale at a test price. ctransamount must have a decimal point: the field table says pennies and the notifications say dollars. Only the fields JVZIPN v1 documents are taken, because the cverify joins values without names and an added field moves every later value along (a receipt read from ctranstime would record the sale twice). JVZIPN v2, whose cverify covers neither the amount nor the tid, is a 415. One change to #268's Processor: a sale already on file that a live reversal nets (JVZoo sends RFND, then SALE again, when a refund is undone) is a 409 naming the reversal to delete, where it was answered 200 duplicate with the sale still reversed. The key is read and written through JVZooKey (trimmed; a line break or a control character refused) by the page, PUT /users/{id}/preferences and the endpoint, so GET /system/integrations secret_stored says whether a key the endpoint can sign with is stored. The panel gives both sets of steps. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01VRoS2SViuxaucFaKWsFnLr
…er a deleted reversal reverses again A sale stated again under a transaction id whose sale a refund took back was answered 200 duplicate while the sale stayed reversed. JVZoo sends an undone refund as RFND and then SALE again; the sale is now a 409 naming the reversal to delete to count it (ported from the JVZoo branch). Deleting that reversal did not make a later refund work: a deleted row keeps its dedupe key, every refund without an id of its own shared rev:<sale>:1, so the next refund of the sale found the deleted row and was answered 200 duplicate with nothing reversed. Such a refund is now keyed by its body, so only the same notification sent again unchanged finds a reversal already on file, live or deleted; any other is either compared with the reversal that stands (the writer refuses a second one under its lock, and the Processor answers duplicate when it takes back the same) or, when none stands, reverses the sale again. A refund with its own id is still told apart by that id. The 403 names the provider's signature field (ProviderAdapter:: signatureField(); PayKickstart's is hash). Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01VRoS2SViuxaucFaKWsFnLr
cb202.php read the ClickBank secret key and the Slack webhook of account 1 and of no other account, so a second account could not use ClickBank's Instant Notification Service at all. It also decrypted with AES-128-CBC, while ClickBank's INS article says notifications are encrypted with CBC-AES-256 and its PHP and Python samples decrypt them so: AES-128 reads 16 of the derived key's 32 bytes, and a notification encrypted as the article says did not decrypt for any account (executed: the openssl command line's AES-256 ciphertext, which Python's cryptography decrypts keyed as ClickBank's Python sample, is refused by the old code, which accepts only the AES-128 its own tests encrypted with). cb202.php now runs the shared payment-notification Receiver with a ClickBank adapter (#268's shape): - ClickBankCipher decrypts as ClickBank's samples do: AES-256-CBC keyed with the first 32 hex digits of SHA-1(secret), trailing control bytes trimmed, the order a JSON object naming its transactionType. - ClickBankAdapter tries every account's stored cb_key; the account is the one whose key decrypts the notification. A key two accounts store is refused (a new outcome, ambiguous_account, 409) and the log names the accounts. A ciphertext that decrypts to something other than an order answers as one no key decrypts: the old code answered the two with different 400s, which is what a padding-oracle attack needs. - The envelope is read before any key: a legacy cverify form (4.0 and earlier) is a 415 naming the version to choose, anything else that is not {"notification","iv"} a 400. - A SALE is recorded under its receipt at totalAccountAmount, stated as USD (ClickBank: "Total you received for the transaction in USD"), so an account counting in another currency is refused by the Processor. The amount is read exactly whether 8.0 sends it as a string or 7.0 as a number. The click is the first tracking code (tid=[[subid]]), or, with none, the affiliate's extclid that 8.0 sends; no click is 200 no_click rather than the 400 ClickBank resent five times. - TEST marks that account's cb_verified through a new optional interface, ConfirmsTestNotifications, which the Processor calls for a test one account verified; a failure answers 500 so ClickBank's Test URL shows it. cb202.php then posts to that account's own Slack webhook, after the answer (ClickBank waits 3 seconds). - Every other type is acknowledged as before (851K handles them). Shared code: InboundRequest carries the raw body; the Processor's test and ignored outcomes name the account when one verified; the Receiver's refusal no longer says "hash" (ClickBank's is a cipher). The page's ClickBank panel says to put tid=[[subid]] on the HopLink and asks for OpenSSL alone (mcrypt was never read). Tests: ClickBankAdapterTest (a known-answer vector from the openssl command line, the old cipher refused, the padding-oracle case, the envelope, every field's reading) and ClickBankInsIntegrationTest (two accounts and a third sharing a key, a real server and the real writer). PostbackHandlerTest is removed: it re-implemented the AES-128 decryption in the test and exercised nothing in the tree. tests/live/clickbank-ins.sh is the two-account live pass, encrypting with the openssl command line; legacy-pixels.sh encrypts as ClickBank does now. Backlog: 7PJV Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01VRoS2SViuxaucFaKWsFnLr
… words The Processor's no_click answer names PayKickstart's checkout link and its subid, which is the wrong advice for a ClickBank HopLink. The adapter now answers no_click itself, as JVZoo's does, naming the type, the receipt and tid=[[subid]]. Backlog: 7PJV Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01VRoS2SViuxaucFaKWsFnLr
changelogs.json: 1.9.79's lines, then the PayKickstart line. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01VRoS2SViuxaucFaKWsFnLr
… take a sale back cb202.php acted on TEST and SALE only and answered every other INS notification 200 with nothing done: a BILL (a subscription's rebill) recorded no revenue, and an RFND or a CGBK left the sale it refunded counted. ClickBankAdapter now reads every transactionType ClickBank's INS article lists (TYPES), through #268's Processor, as PayKickstart and JVZoo do: - BILL is a rebill: another conversion on the click under its own receipt (ClickBank's reporting: the first order's receipt, a dash and four digits), at its totalAccountAmount. - RFND (a refund), CGBK (a chargeback) and INSF (an eCheck returned unpaid) reverse the conversion under their receipt, by what totalAccountAmount says the account lost, sign dropped; none or 0 takes back the whole payment. JVZoo reverses in full because JVZoo refunds whole; ClickBank refunds part of a payment too (the Orders API's product change refunds "a portion of the old product's last recurring payment"). A payment is reversed once: the Processor answers a repeat stating the same amount as a duplicate and another amount as a 409 naming what is still credited. - CANCEL-REBILL and UNCANCEL-REBILL (a subscription cancelled, a cancellation undone), SUBSCRIPTION-CHG, ABANDONED_ORDER and the customer notifications move no money: 200 ignored, logged with the account. - TEST_SALE, TEST_BILL, TEST_RFND, CANCEL-TEST-REBILL and UNCANCEL-TEST-REBILL are tests, as TEST is. - A type the article does not list is a 400 naming it, rather than acknowledged: one that moves money would otherwise be lost unseen. Tests: the type table, rebills and reversals in ClickBankAdapterTest, and in ClickBankInsIntegrationTest on a real server with the real writer: a rebill beside its sale, a refund and its replays, a partial refund and the full one refused after it, a chargeback and a returned eCheck in full, a refund of a rebill alone, a refund of nothing on file, another account's click, the cancellations and an unlisted type. tests/live/clickbank-ins.sh drives each over HTTP for both accounts. Backlog: 851K Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01VRoS2SViuxaucFaKWsFnLr
…ckBank branch The Receiver's 403 now names the provider's signature field (ProviderAdapter::signatureField()); ClickBank's is its encryption. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01VRoS2SViuxaucFaKWsFnLr
Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01VRoS2SViuxaucFaKWsFnLr # Conflicts: # 202-config/changelogs.json
…JVZoo branch #268 ported the reinstatement 409 from this branch, keyed a refund with no id of its own by its body (so a refund after a deleted reversal reverses again), named the signature field in the 403 through a new ProviderAdapter::signatureField(), and merged 1.9.79 (#262). changelogs.json conflicted: 1.9.79's lines are kept, the JVZoo line after them. The JVZoo adapter gains signatureField() ("cverify") in this commit, since without it the class no longer satisfies the interface. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01VRoS2SViuxaucFaKWsFnLr
…ClickBank branch Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01VRoS2SViuxaucFaKWsFnLr # Conflicts: # 202-config/changelogs.json
…ppened p202RecordLegacyConversion() dated every row time(), and the Processor dropped the time a notification states, so an IPN PayKickstart delivered days late (it retries a failed delivery up to 13 times) was filed on the day it arrived in every view by conversion time: the conversion list's time filter, attribution, the click-to-conversion time. The recorder now takes a conv_time, and the Processor passes the stated time, held between the earliest it can be (the click a sale credits; for a refund, the sale it takes back, so the sale keeps its own day) and its arrival (a clock ahead of this server's). The answer and the log line say when a time was held. With no time stated, the row is dated when it arrives, as before. transaction_time is signed (every non-empty field is); one that is not a Unix time is now a 400 naming it, where it was read as no time. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01VRoS2SViuxaucFaKWsFnLr
…start IPN branch
GET /users/{id}/preferences now serves every integration secret as its
stored state (#270), and the PayKickstart IPN keys are one more:
paykickstart_ipn_secret_keys is on ServedPreferences::SECRET, answered
{"stored": true}, and the count this branch served in its place
(paykickstart_ipn_keys_stored, PayKickstartKeys::withoutKeys()) is gone.
GET /system/integrations keys_stored still says how many. The
PreferenceSecretsAreNotServedIntegrationTest round trip stores the keys
with the other secrets.
Conflicts: changelogs (1.9.79's lines, then this branch's), the
preferences docs and the OpenAPI schemas (1.9.79's, with the keys added
as a StoredSecret), and the controller (1.9.79's).
Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01VRoS2SViuxaucFaKWsFnLr
…ending The page's Remove form named a key by its place and the four characters it shows (…AB12). Another tab could remove that key and add a different one ending in the same four characters at its place; the first tab's form then matched the new key and removed it (measured on the pushed class: removed(['campaign-new-AB12', …], '1', '…AB12') dropped campaign-new-AB12). The form now carries an HMAC-SHA256 of the whole key under a secret this session holds server-side (PayKickstartKeys::removalToken()): the page shows nothing a reader could match a guess against, and a form made for one key cannot match another. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01VRoS2SViuxaucFaKWsFnLr
…ersal reverses again With #268's body-keyed refunds, a JVZoo refund sent after the reversal was deleted reverses the sale again, and the first refund sent again unchanged is a retry. The integration test now walks that path (SALE, RFND, SALE 409, delete, the RFND again, a later RFND) and pins the 403's wording to cverify; the unit test pins signatureField(). The guide and the changelog say what a refund does after the reversal is deleted. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01VRoS2SViuxaucFaKWsFnLr
…JVZoo branch #268 now dates a notification's row by the provider's own time, held to the click for a sale and to the sale for a refund, and brings 1.9.79's #270 (ServedPreferences: GET preferences serves each secret as {"stored": ...}), #280, #281 and #282. Conflicts: both changelogs keep the base's lines with the JVZoo line after them; the OpenAPI preferences update takes #270's oneOf shape, with the JVZoo key's rule in its string branch; the eval cases keep #268's admin-005 notes and add admin-006 after it. jvzoo_ipn_secret_key is already on ServedPreferences::SECRET, and JVZoo adds no other column. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01VRoS2SViuxaucFaKWsFnLr
…79's #270, #280, #281, #282) into the ClickBank branch Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01VRoS2SViuxaucFaKWsFnLr # Conflicts: # 202-config/changelogs.json # changelogs.txt
#268's Processor now dates a row by the provider's own time and holds it to the click (a sale) or the sale (a refund) and to arrival. JVZoo's ctranstime is that time: it was read leniently and only logged, so an unreadable one became "no time" and the row was dated when it arrived. It is now a Unix time of 1 to 10 digits, at most Notification::LAST_TIME, or the notification is refused with a 400 naming ctranstime; absent or empty, the row is dated when it arrives. JVZooIpnIntegrationTest gains a late notification: a sale three days late and its refund two days late keep their own times, a chargeback stating a time before its sale is dated at the sale, a ctranstime changed after signing is a 403, and one that is not a Unix time is a 400. The guide and the changelog say so. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01VRoS2SViuxaucFaKWsFnLr
…JVZoo branch #268's stale-Remove fix for PayKickstart's keys (PayKickstartKeys and its panel); it merged cleanly beside the JVZoo panel. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01VRoS2SViuxaucFaKWsFnLr
#268's Processor now dates a conversion by the time its notification states (held between the click and the arrival). ClickBankAdapter reads transactionTime with the offset it carries: RFC 3339 as 6.0 and 8.0 send it (2023-10-05T13:47:51-06:00) and ISO 8601's basic form as 7.0's notes describe (20200819T144359-0700). A time that is neither, or names no real moment (a 13th month, hour 24, beyond a conv_time), is a 400 naming the field rather than the time it arrived; none at all is dated on arrival, as before. Backlog: 7PJV Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01VRoS2SViuxaucFaKWsFnLr
…32b44 and 1.9.79) into the rebills branch Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01VRoS2SViuxaucFaKWsFnLr # Conflicts: # 202-config/PaymentNotification/ClickBank/ClickBankAdapter.php # changelogs.txt # tests/live/clickbank-ins.sh
…al is dated #268's Processor now keys a refund that has no id of its own by a hash of its body, so the same notification sent again finds its reversal, live or deleted, and any other one reverses the sale again once a deleted reversal no longer stands. It hashes the form fields, and ClickBank's notification is a JSON body with no form fields: every ClickBank refund had the same key. A chargeback that arrived after an operator deleted a refund's reversal was answered 200 "this CGBK is that refund sent again" and the sale stayed counted (ClickBankInsIntegrationTest, before this). A provider whose notification is not its form fields now says what one notification is (FingerprintsNotifications, which the Processor asks when the adapter implements it). ClickBank's is the decrypted order with its keys in order and without attemptCount, which ClickBank raises on each retry of the same notification; its ciphertext and IV need not repeat either. Also on the merged base: a SALE sent again while its refund stands is a 409 naming the reversal to delete; a refund, chargeback or returned eCheck is dated by its transactionTime, held at its sale when earlier. Tests for both, and tests/live/clickbank-ins.sh deletes a reversal through DELETE /conversions/{id} and drives the retry and the chargeback after it. Backlog: 851K Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01VRoS2SViuxaucFaKWsFnLr
…ick-messages branch Both branches carry PayKickstart (#268). The conflicts were additive: each changelog keeps both entries, WebEventPathsTest's list of endpoints that build the Receiver names cb202.php, jvzoo.php and paykickstart.php, and StoredVisitorIpSourceTest's reason for the Processor's sender address names all three providers. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01VRoS2SViuxaucFaKWsFnLr
…der's field The shared payment-notification Processor (#268) worded its no_click answer for PayKickstart ("the checkout link carried no subid") and its unknown_click answer with no field at all, saying the secret key "signed" the notification, which ClickBank's encrypted INS is not. JVZoo (#272) and ClickBank (#276/#288) raised NO_CLICK from their own parse() to get around it, so there were three copies of one rule, and a fourth adapter would have shipped PayKickstart's words. ProviderAdapter now has clickField(): a ClickField naming the field the click comes back in and the variable on the buyer's link that a tracker fills (PayKickstart's custom_subid from the checkout link's subid=[[subid]]; JVZoo's caffitid from the affiliate link's ?tid=[[subid]], or the subid in cvendthru from the buy link's ?subid=[[subid]], by role; ClickBank's tracking code or extclid from the HopLink's tid=[[subid]] or extclid=[[subid]]). The Processor builds both answers from it, once. JVZoo and ClickBank now return a Notification with no click and let the Processor answer; JVZoo keeps one NO_CLICK of its own, for an affiliate's tid that is there and is not a click id, since that answer shows the value, which a Notification does not carry. The "present but not a click id" refusals of PayKickstart, JVZoo's vendor role and ClickBank stay in the adapters as 400s naming the value. Every answer keeps its status and outcome but one: a ClickBank notification naming no click with an unreadable transactionTime is now a 400 naming the time, as PayKickstart's and JVZoo's already were, rather than 200 no_click (the adapter used to refuse before reading the time). A JVZoo or ClickBank no_click is now logged with its type and receipt. ClickAnswersAreWordedByTheProviderTest holds every ProviderAdapter in the tree (found through implements, extends and aliased imports) to being listed with each way it is built, sends a no-click notification of each kind through the real Processor for each, and refuses a shared file whose string literals name a provider's field and any NO_CLICK or UNKNOWN_CLICK raised outside the Processor but the one listed. Each integration suite sends a no-click and an unknown-click notification through the Receiver and asserts the kind, the status and that the answer names only its own platform's field. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01VRoS2SViuxaucFaKWsFnLr
Codex Review SummaryThis comment shows the latest Codex review activity on this pull request.
ℹ️ About Codex in GitHubYour team has set up Codex to review pull requests in this repo. Reviews are triggered when you
Codex reacts with 👀 while any review is running, comments if it has suggestions, and reacts with 👍 once all reviews finish with no findings. |
Brings in 1.9.79's new commits. The conflicts were in 202-config/changelogs.json, 202-config/functions-upgrade.php, changelogs.txt, where both sides added lines: the incoming lines are kept as they are and this branch's follow. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01VRoS2SViuxaucFaKWsFnLr
Brings in paykickstart-ipn (this branch's stacked base), and through it 1.9.79's new commits. The conflicts were in 202-config/changelogs.json, where both sides added lines: the incoming lines are kept as they are and this branch's follow. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01VRoS2SViuxaucFaKWsFnLr
Brings in paykickstart-ipn (this branch's stacked base), and through it 1.9.79's new commits. The conflicts were in 202-config/changelogs.json, where both sides added lines: the incoming lines are kept as they are and this branch's follow. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01VRoS2SViuxaucFaKWsFnLr
Brings in clickbank-per-account (this branch's stacked base), and through it 1.9.79's new commits. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01VRoS2SViuxaucFaKWsFnLr
Brings in jvzoo-ipn (this branch's stacked base), and through it 1.9.79's new commits. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01VRoS2SViuxaucFaKWsFnLr
Brings in clickbank-rebills-refunds (this branch's stacked base), and through it 1.9.79's new commits. The conflicts were in 202-config/changelogs.json, where both sides added lines: the incoming lines are kept as they are and this branch's follow. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01VRoS2SViuxaucFaKWsFnLr
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: b264b9ab55
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
Comment on lines
+227
to
+230
| $result = $this->record($n->clickId, $owner, $provider, $request, [ | ||
| 'transaction_id' => $original, | ||
| 'reversal' => true, | ||
| 'reversal_ref' => $reference, |
There was a problem hiding this comment.
When the same click has conversions from different sources with the same transaction ID—for example, after a redirector re-click sends the visitor to another payment platform—this reversal can target the wrong conversion. The processor passes the provider's raw ID while source is only separate metadata; MysqlConversionRepository::recordLocked() selects reversal targets by click_id and transaction_id alone, and sale deduplication likewise uses only tx:<id>. Because ClickBank, JVZoo, and PayKickstart ID formats overlap, prefix the ledger transaction/reversal key with the provider or make both target lookup and deduplication source-aware.
Useful? React with 👍 / 👎.
Owner
Author
There was a problem hiding this comment.
I traced this and I'm not changing it in this PR. The Processor is #268's, and this PR only rewords its answers.
The ledger keys a sale by the network's transaction id on its click, whatever path reports it. That is Conversion\Ledger\DedupeKey's tx:<id>, used by the postback, pixel and API paths too. That shared key is what merges one sale reported twice, for example a ClickBank INS and ClickBank's own postback for the same receipt. Prefixing the key with the provider would split those into two conversions, which is a wider change than this collision is worth.
The case it guards against needs one click with sales on two different payment platforms whose transaction ids are identical. Even then it isn't silent: the second sale is compared with the first (#15, RevenueReplay) and refused with the key named if they differ. I'm putting it to the owner along with the related question of which account a shared secret key credits (backlog K4SH).
Generated by Claude Code
Owner
Author
There was a problem hiding this comment.
Correction: the comparison is the Processor's own, not RevenueReplay. When the ledger answers duplicate for a payment, the Processor compares the stored amount with the stated one. A different amount is refused, and the answer gives both amounts ("…; this notification states X. Nothing was changed."). So the cross-platform collision is still answered rather than dropped, unless the two sales are for the same amount.
Generated by Claude Code
…ed as the first ClickBank's retry of a notification is the same order (its attemptCount aside), and ClickBankAdapter::fingerprint() keys a refund by exactly that, so a refund whose fingerprint finds no reversal on file is another notification. When a reversal already stood against the payment, the Processor still compared amounts only and answered one stating the same amount 200 "is already reversed; nothing was changed": a second partial refund of 10.00 after one of 10.00 (ClickBank refunds part of a payment, which is why this branch reads totalAccountAmount at all) was dropped with a status ClickBank never retries, while the account lost it too (CLAUDE.md #4, #15: the request was compared by amount when the code knew it was another one). For a provider that says what one notification is (FingerprintsNotifications), only that notification sent again is a duplicate; any other reversal of a payment one already stands against is a 409 naming the reversal on file and what is still credited, whatever it states, since a sale is reversed once. A chargeback of a refunded payment for the same amount is now that 409 too, where it was 200 duplicate: two notifications that each say the account lost the money are shown in ClickBank's log rather than one of them dropped unseen. PayKickstart, which keys refunds by its form fields, keeps its rule. Tests: ClickBankInsIntegrationTest's new testAnotherRefundOfTheSameAmountIsRefusedNotAnsweredAsTheFirst fails without the Processor change ("is already reversed; nothing was changed"), and the refund test's "same refund again" is ClickBank's retry of it (the same order, attemptCount raised) rather than a notification with another time or spelling. tests/live/clickbank-ins.sh sends the retry as the same order and a second refund of the same amount: 87 passed, 0 failed here; 85 passed, 2 failed (those two) with the Processor change taken out. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01VRoS2SViuxaucFaKWsFnLr
…ages Brings in 1.9.79-clickbank-rebills-refunds (this branch's stacked base), and through it 1.9.79's new commits. The conflicts were in changelogs.txt, where both sides added lines: the incoming lines are kept as they are and this branch's follow. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01VRoS2SViuxaucFaKWsFnLr
|
You have reached your Codex usage limits for code reviews. You can see your limits in the Codex usage dashboard. |
PayKickstartAdapter::storedSecrets() read every 202_users_pref row, and a
deleted account keeps its row and the keys in it. Its key kept verifying
IPNs, so a sale on a deleted account's click was recorded for it, and a key
the deleted account shared with a live one verified both. ClickBank's and
JVZoo's adapters, on the branches above, read their keys the same way, and
ClickBank refuses a key two accounts store with a 409: a deleted account
holding a live account's ClickBank key refused every notification of the
live account, with nothing in the UI to clear the deleted copy.
AccountSecrets::stored() is now the one read of a provider's secret column
that decides the accounts a notification can belong to: it joins
202_users and leaves out user_deleted = 1. A user switched off in Account ›
Users (user_active = 0) still owns its keys: a sale PayKickstart reports
for that account is not that person acting. ProviderAdapter documents it.
StoredSecretsReadThroughAccountSecretsTest holds every ProviderAdapter's
storedSecrets() to a call of AccountSecrets::stored() with no statement of
its own naming 202_users_pref, and every other statement in
202-config/PaymentNotification and the payment endpoints that names
202_users_pref to a listed reason (the Processor's currency read, for the
account already decided).
Tests: PayKickstartIpnIntegrationTest's new
testADeletedAccountsStoredKeyVerifiesNothing seeds a deleted account that
stores a live account's key and one of its own: the live account's sale is
recorded for it, the deleted account's click is a 404 and its own key a
403, and a switched-off account's sale is recorded. Planted: the
user_deleted filter removed from AccountSecrets fails it ("Recorded
transaction PK-TX-D2 on click 99040131"), and so does a user_active = 1
filter added (the switched-off account's IPN unverified); storedSecrets()
reading 202_users_pref itself fails both structural tests.
Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01VRoS2SViuxaucFaKWsFnLr
Brings in #268's AccountSecrets: a deleted account's stored keys verify nothing. The conflict was in changelogs.txt, where both sides changed lines of 1.9.79: #268's PayKickstart line is kept as it is and this branch's ClickBank line follows. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01VRoS2SViuxaucFaKWsFnLr
ClickBankAdapter::storedSecrets() read cb_key from every 202_users_pref row, and a deleted account keeps its row and its key. A notification that the key decrypts names every account that stores it, and a key two accounts store is refused (409, ambiguous_account): so a deleted account that still held the key a live account stores now made every one of the live account's notifications a 409, and the deleted account's copy cannot be cleared from the UI. A key only a deleted account stored still recorded sales on its clicks. storedSecrets() reads the keys through #268's AccountSecrets::stored(), which leaves out deleted accounts; a switched-off account (user_active = 0) still owns its key. StoredSecretsReadThroughAccountSecretsTest now holds ClickBankAdapter too, and lists the two statements it makes on 202_users_pref for the account a TEST already decided (confirmTest(), slackWebhook()). Tests: ClickBankInsIntegrationTest's new testADeletedAccountsStoredKeyIsNoSecondAccount: with a deleted account storing the first account's key, the first account's sale is recorded and its TEST marks only it verified; a key only the deleted account stores is a 403; a switched-off account's sale is recorded. Planted: storedSecrets() reading 202_users_pref itself, and AccountSecrets without its user_deleted filter, each fail it with the 409 ("The notification decrypts with a ClickBank secret key that more than one Prosper202 account stores"); a user_active = 1 filter fails it with a 403. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01VRoS2SViuxaucFaKWsFnLr
Brings in #268's AccountSecrets: a deleted account's stored keys verify nothing. The conflict was in changelogs.txt, where both sides changed lines of 1.9.79: #268's PayKickstart line is kept as it is and this branch's JVZoo line follows. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01VRoS2SViuxaucFaKWsFnLr
JVZooAdapter::storedSecrets() read jvzoo_ipn_secret_key from every 202_users_pref row, and a deleted account keeps its row and its key. Its key kept verifying JVZIPN notifications: a sale on a deleted account's click was recorded for it, and a key it shared with a live account verified both. storedSecrets() reads the key through #268's AccountSecrets::stored(), which leaves out deleted accounts; a switched-off account (user_active = 0) still owns its key. StoredSecretsReadThroughAccountSecretsTest, merged from #268, now finds JVZooAdapter and holds it to that read (it failed on the merge until this change). Tests: JVZooIpnIntegrationTest's new testADeletedAccountsStoredKeyVerifiesNothing: with a deleted account storing the account's key, the account's sale is recorded for it; the deleted account's click is a 404, a key only it stores a 403; a switched-off account's sale is recorded. Planted: storedSecrets() reading 202_users_pref itself, and AccountSecrets without its user_deleted filter, each fail it ("Recorded transaction D2 on click 99050141"); a user_active = 1 filter fails it with a 403. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01VRoS2SViuxaucFaKWsFnLr
Brings in #276 with #268's AccountSecrets: a deleted account's stored ClickBank and PayKickstart keys verify nothing, so a deleted account no longer makes a live account's ClickBank key ambiguous. The conflict was in changelogs.txt, where both sides changed lines of 1.9.79: the incoming PayKickstart and ClickBank lines are kept as they are and this branch's rebills line follows. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01VRoS2SViuxaucFaKWsFnLr
…ages Brings in the deleted-account key fix (a deleted account's stored PayKickstart or ClickBank key verifies nothing; every adapter's storedSecrets() reads through AccountSecrets::stored()). Conflicts: - ProviderAdapter.php: this branch's clickField() is kept, with the updated storedSecrets() docblock. - ClickBankAdapter.php and PayKickstartAdapter.php: both imports are kept (AccountSecrets and ClickField). - changelogs.txt: union. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01VRoS2SViuxaucFaKWsFnLr
Brings in JVZoo's half of the deleted-account key fix. Conflicts: - JVZooAdapter.php: both imports are kept. - changelogs.txt: JVZoo's line is the one with the new clause. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01VRoS2SViuxaucFaKWsFnLr
|
You have reached your Codex usage limits for code reviews. You can see your limits in the Codex usage dashboard. |
Contributor
tracking202
added a commit
that referenced
this pull request
Oct 28, 2026
# Conflicts: # 202-config/PaymentNotification/ClickBank/ClickBankAdapter.php # 202-config/PaymentNotification/PayKickstart/PayKickstartAdapter.php # 202-config/PaymentNotification/ProviderAdapter.php
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.
Stacked on #272 and #288 (and through them #268 and #276); merge after all four. The base is
1.9.79, so until they merge this diff also shows theirs.Backlog: 05RX
What and why
The shared payment-notification
Processor(#268) worded the answers about the click inside itself, for PayKickstart:no_click(200): "Transaction X names no click (the checkout link carried no subid)", PayKickstart's checkout link andsubid;unknown_click(404, also what the writer'sforeign_clickbecomes): "There is no click N in the account whose secret key signed this notification", which names no field at all and says "signed" of ClickBank's notifications, which are encrypted.JVZoo (#272) and ClickBank (#276/#288) got around the
no_clickwording by raisingNO_CLICKfrom their ownparse()in words of their own, so there were three copies of one rule, and a fourth adapter would have answered in PayKickstart's words.Now:
ProviderAdapter::clickField(): ClickFieldsays where the provider's notification carries the click and how a seller gets it there: the field, the link the buyer follows, and the variable on it a tracker fills (ClickFieldrefuses a variable without[[subid]]).custom_subid, from the checkout link'ssubid=[[subid]].JVZooRole::clickField()):caffitidfrom the JVZoo affiliate link's?tid=[[subid]]; thesubid in cvendthrufrom the buy link's?subid=[[subid]].tracking code or extclid, from the HopLink'stid=[[subid]] or extclid=[[subid]](the adapter readstrackingCodes[0], then the 8.0 affiliate'sextclid; the old hand-raised answer named onlytid).Processorbuilds both answers from it, once (noClick(),unknownClick()), for sales and reversals.Notificationwith no click and let theProcessoranswer. Their hand-raised copies are gone.Before and after, from the live pass below (one of each):
custom_subidVerified. Transaction PK-N-… names no click (the checkout link carried no subid), so nothing was recorded.Verified. The sales notification of transaction PK-N-… carries no custom_subid (the checkout link had no subid=[[subid]]), so it names no click and nothing was recorded.Verified. The SALE of receipt JVV… carries no subid in cvendthru (the buy link had no ?subid=[[subid]]), so it names no click; nothing was recorded.Verified. The SALE notification of transaction JVV… carries no subid in cvendthru (the buy link had no ?subid=[[subid]]), so it names no click and nothing was recorded.There is no click 999999901 in the account whose secret key signed this notification; nothing was recorded.There is no click 999999901 in the account whose secret key verified this notification; nothing was recorded. The click is the notification's tracking code or extclid, which the HopLink's tid=[[subid]] or extclid=[[subid]] carries when the buyer comes through a tracker of that account.The unknown-click answer reads the same for a click that is not there and for another account's click (both measured), so it still tells no key holder which click ids another account has.
Decisions for a reviewer
Notificationcannot):tidthat is there and is not a click id stays a200 no_clickraised by the adapter, showing the value: the tid is the affiliate's to use for other trackers, the guide documents it as200 no_click, andJVZooAdapterTest's moved-signature case expects it. It is the one listed exception in the new test.custom_subid, JVZoo's vendorsubidand ClickBank's tracking code /extclidthat are present and are not click ids stay400s naming the value.transactionTimeis now400namingtransactionTimerather than200 no_click: the adapter used to refuse for the missing click before it read the time, and now the order is read whole and the Processor finds no click. PayKickstart and JVZoo already answered that case400. Pinned byClickBankAdapterTest::testAnUnreadableTimeIsRefusedWhetherOrNotTheOrderNamesAClick. Every other answer keeps its kind and status (measured, below).parse()was logged as… notification from <address>: no_click (200): …with no type or receipt; through the Processor it is… notification: no_click (200), SALE (sale), transaction <receipt>: …, as PayKickstart's was. The tests that read the log were updated.The check
tests/PaymentNotification/ClickAnswersAreWordedByTheProviderTest(unit, no database):ProviderAdapterin the tree (byimplements, throughextends, through aliased and group imports; an interface extending it or an anonymous one fails by name) and fails unless each is listed inadapters(), each way it is built;Processorfor each adapter and assertsno_click,200, and that the answer names that adapter's field, link and variable and none of another provider's names it does not share;202-config/PaymentNotification/*.php) names any provider's field or link;NO_CLICK/UNKNOWN_CLICK(by case name, by value in a string, or anOutcomeKind::from/tryFrom/caseslookup) in anyPaymentNotificationfile butOutcomeKind,ProcessorandResponse, or in an adapter anywhere, except the listed JVZoo site (and fails if that site goes away);Planted defects, each run against the named test. All 15 were caught by that test failing, not by a compile error, and md5sum showed the tree unchanged after the run: the Processor's no-click in PayKickstart's words; the unknown-click naming
custom_subid; ClickBank raising its ownNO_CLICKagain, by name and byOutcomeKind::from(strtolower('NO' . '_CLICK')); a second JVZooNO_CLICK; the listed JVZoo site removed; a fourth adapter (plain, viause … as, via a group import alias, extending a listed one); an interface extendingProviderAdapter; ClickBank naming JVZoo's field; ClickBank naming a field it does not read; PayKickstart's variable naming ClickBank'sextclid(caught by the PayKickstart integration test's exact message, not the unit test, which takes an adapter's own declaration as given); and the ClickBank adapter restored to raising its own no-click as on the base (caught by three tests).Per adapter,
PayKickstartIpnIntegrationTest,JVZooIpnIntegrationTest(both roles) andClickBankInsIntegrationTest(including an unknownextclid) each send no-click and unknown-click notifications (sale and refund/chargeback, a missing click and another account's click) through the realReceiverandProcessoron a scratch database and assert the kind, the status, the exact answer and that it names only its own platform's field.Live end-to-end pass
On a local instance (MariaDB 10.11, PHP 8.3.6,
php -Sthroughrouter.php) installed without the licence step: the licence stub is down, andinstall.phpchecks the key in its own POST, so the schema and account were installed by runninginstall.php'sINSTALL::install_databases(),InstallAccount::create()and the partitions directly, and the API key inserted. Everything after that went over HTTP: preferences (PUT /users/1/preferenceswith the PayKickstart, JVZoo and ClickBank keys), a second account and its API key, a campaign, traffic source and tracker in each, a real click in each through the tracker's redirect, then signed/encrypted posts totracking202/static/paykickstart.php,jvzoo.php?role=affiliate,jvzoo.php?role=vendorandcb202.php. The signers were written in the script, not taken from the code under test: PayKickstart's HMAC-SHA1 over the trimmed values in name order, JVZoo's SHA-1 cverify, ClickBank's AES-256-CBC with theopensslcommand line.22 posts before (on the merged base, this branch's first commit) and after: per provider a sale and a refund naming no click, a sale and a refund naming a click that is not there, a sale naming the other account's click, and a sale on the account's own click (recorded, as a control); for JVZoo also an affiliate tid that is not a click id and the vendor role's no-subid/unknown cases. Every status and outcome is the same before and after (22 of 22, compared by script); only the
msgand the log line changed, as in the table above. No row was written on the missing click or the other account's click.tests/live/clickbank-ins.shwas also run against the instance after the change: 85 passed, 0 failed.Not run on the instance: the Go CLI, which was not built because this item is PHP only, and
tests/live/legacy-pixels.sh. That script grepsThere is no click, and the new unknown-click answer still starts with those words.Verification
.claude/skills/p202-verify/scripts/verify.sh --since 65f44e86 --changed(65f44e8 is this branch's merge of #272 and #288, so the ref selects this item's diff; see below for why notorigin/1.9.79), run on PHP 8.3.6 with a fullvendor/:--changedon the working tree before the commit printed the same five PASS lines. The unit tier ran 6241 tests with 5 skipped: the 4 documented "User class requires DB singleton" skips andCronEntryPointsFailLoudlyTest's no-install case, which skips because the live instance's202-config.phpsat in the checkout.Also run, by hand, on MariaDB 10.11 scratch databases (
x05rx_test):phpunit --group integration tests/PaymentNotification(61 tests, including the new per-adapter click tests) andphpunit tests/PaymentNotification(139 tests).Not run:
origin/1.9.79. That ref is two-dot, and this branch does not contain 1.9.79's latest commits (see below), so it would have selected tiers from their files, the Go tier among them, and this item is PHP only with no Go build on this host. Each stacked PR runs its own ladder; this one ran the tiers for its own diff.Not merged: origin/1.9.79 at d890d16
The brief was to merge
origin/1.9.79,origin/1.9.79-jvzoo-ipnandorigin/1.9.79-clickbank-rebills-refundsagain before opening this PR. The two stacked branches had not moved since this branch took them (5654a76, 801e5fc).origin/1.9.79had moved (#263, #274, #261, #264). Merging it conflicts outside the changelogs, in the 1.9.78 -> 1.9.79 rung of202-config/functions-upgrade.php, between #268'sUserTables::usersPref()reconcile (forpaykickstart_ipn_secret_keys) and 1.9.79's new list (TrackingTables::google(),usersPref()with['202_users_pref' => ['user_pref_privacy']]). That conflict is #268's: its branch lacks the same commits. So it was not resolved here. This branch picks the resolution up from #272 and #288 once #268 takes 1.9.79.Generated by Claude Code