feat(merchant): production-readiness sweep for the portal, checkout cancel and API hardening - #101
Conversation
…udit trail and key lifecycle to the portal API
…activity log and key rotation screens
…closure keyboard access
…ion serving with a smoke check
…al migration to 0047 and scope Payments to the active cluster
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. |
![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: 838d83ceff
ℹ️ 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".
| const issued = await this.keys.rotateKey(id, merchant.id); | ||
| await this.audit.record({ |
There was a problem hiding this comment.
If audit.record() fails after rotateKey() commits, the request returns an error after the old key has already been revoked and the successor stored, but the successor's one-time raw secret never reaches the merchant. This can leave an integration without a usable credential and an unlogged security-sensitive change; execute the rotation and audit insert in one transaction so an audit failure preserves the old key.
Useful? React with 👍 / 👎.
| ) : page === "account" ? ( | ||
| <section className="account-workspace" id="setup"> |
There was a problem hiding this comment.
When an owner edits the business profile and clicks another navigation item or uses browser history, this route branch unmounts BusinessProfileForm; its local draft disappears and the beforeunload warning does not run for History API navigation. The previous hidden-panel implementation kept the form mounted, so routed navigation should either preserve that behavior or call canLeave() before leaving the account page.
Useful? React with 👍 / 👎.
| const [existing] = await this.db.client | ||
| .select() | ||
| .from(apiKeys) | ||
| .where(and(eq(apiKeys.id, keyId), eq(apiKeys.merchantId, merchantId))) | ||
| .limit(1); | ||
| if (!existing || existing.revokedAt) | ||
| throw new ApiKeyNotFoundError(`api key ${keyId} not found`); |
There was a problem hiding this comment.
If two rotation requests read the same active key before either revokes it, both pass the revokedAt check, each issues a successor, and the idempotent revocation then lets both requests succeed. The merchant is left with two active replacement credentials instead of one, so selecting/claiming the old key, issuing its successor, and revoking it must be serialized or guarded by an atomic conditional update.
Useful? React with 👍 / 👎.
| const paymentDetail = path.match(/^\/payments\/(.+)$/); | ||
| if (paymentDetail?.[1]) | ||
| return { page: "payment", paymentId: decodeURIComponent(paymentDetail[1]) }; |
There was a problem hiding this comment.
A malformed deep link such as /payments/% causes decodeURIComponent to throw URIError while the workspace renders, crashing the entire portal instead of resolving to a safe page. Since pathnames are user-controlled and deep links are now supported, catch decoding failures and fall back to the overview, payments list, or a not-found route.
Useful? React with 👍 / 👎.
| .where(eq(merchants.id, merchant.id)) | ||
| .returning(); |
There was a problem hiding this comment.
If an operator verifies the merchant after this request reads the pending/rejected row but before the update executes, the unconditional WHERE id = ... update changes the newly verified merchant back to pending. Because ApiKeyGuard rechecks KYB on every live request, this race immediately disables the merchant's live keys; include the observed non-verified status in the update predicate and return a conflict when it no longer matches.
Useful? React with 👍 / 👎.
| export function formatDisplayAmount(currency: string, minor: string): string { | ||
| const amount = Number(minor) / 100; | ||
| if (!Number.isFinite(amount)) return `${currency} ${minor}`; |
There was a problem hiding this comment.
Payment-intent creation accepts an unbounded decimal digit string, so a valid amount above JavaScript's safe-integer range loses precision when converted with Number; sufficiently large values become Infinity and are then displayed as the unscaled minor-unit string. Payment history and detail can therefore show a different price from the stored intent, so format the amount with BigInt and the currency exponent as the existing checkout formatter does.
Useful? React with 👍 / 👎.
| // 3. An owner-authenticated portal route rejects an unauthenticated request | ||
| // rather than leaking data (401, not 200 and not a network/404 error). | ||
| try { | ||
| const me = await fetch(`${API_URL}/merchant-portal/me`); | ||
| check( | ||
| "Portal /me rejects an unauthenticated request", | ||
| me.status === 401, |
There was a problem hiding this comment.
In the preferred same-origin topology, a missing /merchant-portal/* reverse-proxy rule leaves the portal unusable, but this check still passes because it calls API_URL directly rather than the route the browser reaches at MERCHANT_URL. The remaining checks only verify the SPA shell, so a deployment with a working API and static portal but broken integration reports success; test the merchant-origin API path for same-origin deployments, and send an Origin header/check CORS for split-origin deployments.
Useful? React with 👍 / 👎.
| .where(eq(webhookDeliveries.endpointId, id)) | ||
| .orderBy(desc(webhookDeliveries.createdAt)) | ||
| .limit(take); |
There was a problem hiding this comment.
Once an endpoint has more than take delivery attempts, this route permanently hides every older attempt because it accepts no cursor and returns no nextCursor; ordering also omits the documented id tie-breaker. That prevents merchants from reaching historical failures during webhook troubleshooting, so implement the stated (createdAt, id) keyset pagination or stop truncating the diagnostic history.
Useful? React with 👍 / 👎.
…umber portal migration to 0050, keep least-privilege dashboard, adopt inline checkout disclosure and devnet live-key guard
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 990c142c34
ℹ️ 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".
| url: body.url, | ||
| eventTypes: body.eventTypes ?? null, | ||
| }); | ||
| await this.audit.record({ |
There was a problem hiding this comment.
If endpoint registration commits but audit.record() fails, this request returns an error before exposing the one-time signing secret, while the active endpoint remains stored and can immediately receive deliveries signed with a secret the merchant never obtained. Reads intentionally omit the secret, so retrying only creates another endpoint; wrap the endpoint insert and audit insert in the existing database transaction boundary so both roll back together.
Useful? React with 👍 / 👎.
| const data = await client.get<{ deliveries: Delivery[] }>( | ||
| `webhooks/${encodeURIComponent(endpoint.id)}/deliveries`, | ||
| ); | ||
| setDeliveries(data.deliveries); | ||
| } finally { |
There was a problem hiding this comment.
When the delivery request fails because of a network error, expired token, or backend response, the rejection escapes this async handler invoked with void; the finally block only stops the spinner, so the portal displays neither an error nor a retry indication and also produces an unhandled promise rejection. Catch the failure and expose it in the card or panel error state.
Useful? React with 👍 / 👎.
| } catch (error) { | ||
| if (error instanceof ApiKeyNotFoundError) | ||
| throw new NotFoundException('API key not found'); | ||
| throw error; |
There was a problem hiding this comment.
When a live key is no longer eligible for rotation—for example, the merchant has since been rejected, its settlement destination is incomplete, or the key belongs to an execution cluster now disabled—rotateKey() throws a plain KybNotVerifiedError, SettlementDestinationMissingError, or ExecutionClusterDisabledError. This catch maps only missing keys and rethrows the rest, so Nest returns a 500 for an expected merchant-actionable conflict; map these eligibility errors to the same 409 responses used by key issuance.
Useful? React with 👍 / 👎.
| // The reviewer's note stamped on a rejection, shown to the owner so a | ||
| // resubmission can fix the named problem. Null while pending or verified. | ||
| kybReviewNote: text('kyb_review_note'), |
There was a problem hiding this comment.
The new portal depends on kybStatus === 'rejected' and this note to show a rejection reason and enable the resubmission flow, but a repo-wide search finds this field only being serialized or cleared: no controller, service, or ops script can atomically set the rejected status and review note. Consequently the advertised workflow cannot be exercised without direct SQL and owners will never receive an actionable rejection reason through supported operations; add an authenticated operator rejection transition that persists both values.
Useful? React with 👍 / 👎.
| .from(webhookDeliveries) | ||
| .where(eq(webhookDeliveries.correlationId, intent.id)) | ||
| .orderBy(desc(webhookDeliveries.createdAt)); |
There was a problem hiding this comment.
Every payment-detail request filters the global webhook_deliveries table by correlation_id, but the schema only indexes endpoint, event, and retry-status columns. As delivery history grows, opening any payment detail therefore requires a sequential scan of the entire delivery table and can become slow or time out; add a migration with an index such as (correlation_id, created_at) to support this lookup and ordering.
Useful? React with 👍 / 👎.
…mit races, paginate webhook deliveries, harden money formatting, path decoding, draft-guarded nav and the smoke check
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 66b6ace8b1
ℹ️ 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".
| const endpoint = await this.endpoints.find(id, { | ||
| merchantId: merchant.id, | ||
| }); | ||
| await this.audit.record({ |
There was a problem hiding this comment.
If audit.record() fails after rotateSecret() persists the new primary, this request returns an error without revealing that one-time secret. The merchant's known secret remains usable only during the grace window and then webhook verification starts failing; retries can evict the known secondary even sooner. Run the rotation and audit insert in one transaction so an audit failure preserves the existing secret.
Useful? React with 👍 / 👎.
| const sameOrigin = MERCHANT_URL !== API_URL; | ||
| if (sameOrigin) { |
There was a problem hiding this comment.
The new comparison reverses the documented cases: in a true same-origin deployment API_URL === MERCHANT_URL, yet the script runs a CORS preflight even though docs/merchant-portal-production.md says no CORS entry is needed; with split origins the URLs differ, yet it requires the static merchant host to proxy /merchant-portal/me, which that topology intentionally does not provide. Consequently this smoke test rejects valid deployments instead of checking the correct browser path; select the topology explicitly or invert/rework this branching.
Useful? React with 👍 / 👎.
| */ | ||
|
|
||
| function subscribe(callback: () => void) { | ||
| window.addEventListener("popstate", callback); |
There was a problem hiding this comment.
When the account form is dirty and the owner presses Back or Forward, this popstate callback immediately updates the route and unmounts the form without invoking canLeave(). The newly added go() guard only covers explicit navigation clicks, so the browser-history case from the earlier finding remains; route history transitions through the blocker or keep the form mounted/persist its draft.
Useful? React with 👍 / 👎.
| onClick={() => | ||
| void run(async () => { | ||
| await request("destination", {}); | ||
| await refresh(); | ||
| setData(await client.post<Dashboard>("kyb/submit")); | ||
| }) |
There was a problem hiding this comment.
If an owner edits the business-profile form and clicks “Submit business details for verification” before pressing Save, this handler posts only kyb/submit; the draft lives inside BusinessProfileForm and is never included. The backend therefore marks the old persisted profile as submitted while the UI keeps showing the unsaved values alongside “Verification in review,” so reviewers receive different details from those the owner believes were submitted. Save the draft first or block submission while the profile is dirty.
Useful? React with 👍 / 👎.
| lt(paymentIntents.createdAt, after.createdAt), | ||
| and( | ||
| eq(paymentIntents.createdAt, after.createdAt), | ||
| lt(paymentIntents.id, after.id), |
There was a problem hiding this comment.
PostgreSQL stores created_at with microsecond precision, but cursorRow() materializes it as a JavaScript Date, which retains only milliseconds. If multiple intents are created within one millisecond and the cursor has nonzero microseconds, the next query compares against the truncated instant: older rows from that same millisecond are greater than the truncated cutoff and not equal to it, so they are skipped permanently. Carry the exact database timestamp in the cursor or order and compare at consistently truncated precision; the audit and webhook-delivery pagination use the same vulnerable pattern.
Useful? React with 👍 / 👎.
| setRows((current) => | ||
| reset ? page.payments : [...current, ...page.payments], | ||
| ); | ||
| setCursor(page.nextCursor); |
There was a problem hiding this comment.
If the owner applies a status or search filter while the initial request or a “Load more” request is still in flight, both requests remain active and whichever resolves last executes these state updates. A slower response for the previous filters can therefore replace or append rows and its cursor under the newly selected filters, producing a mixed or entirely unfiltered result set. Abort superseded requests or associate each response with the active filter/request generation before updating state.
Useful? React with 👍 / 👎.
| // leave a revocation that no trail records. | ||
| const revoked = await this.db.client.transaction(async (tx) => { | ||
| const result = await this.keys.revokeKey(id, tx); | ||
| await this.audit.record( |
There was a problem hiding this comment.
If the first revocation commits but its response is lost, retrying the request calls revokeKey(), which deliberately returns the already-revoked row, and then unconditionally writes another api_key.revoke entry here. No key state changed on the retry, yet the owner-visible security trail shows a second revocation at a new time, making the append-only audit inaccurate. Have the service report whether it claimed the active row and record the action only for the actual transition while retaining idempotent HTTP success.
Useful? React with 👍 / 👎.
… KYB-rejection transition, map rotation eligibility to 409, index webhook deliveries by correlation id, and handle delivery-load errors
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 4954d361c0
ℹ️ 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".
| throw new ConflictException( | ||
| 'This profile changed in another session. Reload the latest profile before saving.', | ||
| ); | ||
| await this.audit.record({ |
There was a problem hiding this comment.
If audit.record() fails after the preceding profile update commits, this endpoint returns an error even though the new profile and incremented version are already persisted, leaving the sensitive change absent from the promised audit trail; retrying with the form's old expectedVersion then produces a conflict. Run the profile update and audit insert in the same database transaction, as the key and webhook writes already do.
Useful? React with 👍 / 👎.
| : []), | ||
| ), | ||
| ) | ||
| .orderBy(desc(paymentIntents.createdAt), desc(paymentIntents.id)) |
There was a problem hiding this comment.
For a merchant with a large payment history, this common unfiltered query must scan the merchant/cluster rows and sort all of them by (created_at, id) before applying the small page limit. The schema has only the idempotency index (merchant_id, execution_cluster, idempotency_key), which cannot provide this ordering, so every page becomes progressively expensive; add a matching index beginning with (merchant_id, execution_cluster, created_at, id).
Useful? React with 👍 / 👎.
…ile drafts, keyset paginate at full timestamp precision, drop stale payment responses, skip idempotent revoke audit, correct smoke topology
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 023a4c1202
ℹ️ 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".
| db: DbExecutor = this.db.client, | ||
| ): Promise<{ secret: string; secondaryExpiresAt: Date }> { | ||
| const endpoint = await this.find(id, scope); | ||
| const endpoint = await this.find(id, scope, db); |
There was a problem hiding this comment.
When two rotation requests for the same endpoint run concurrently, both plain reads can capture the same primary secret before either update locks the row. The second update then overwrites the first request's new primary while restoring only the original secret as secondary, so the first caller receives a one-time secret that is immediately invalid. Lock the endpoint row before deriving the replacement or make the update conditional on the primary value/version read.
Useful? React with 👍 / 👎.
| throw new ConflictException( | ||
| 'Your verification status changed. Reload your account before resubmitting.', | ||
| ); | ||
| await this.audit.record({ |
There was a problem hiding this comment.
If this audit insert fails after the preceding merchant update commits, the request returns an error even though a rejection was already cleared and the merchant was moved to pending with a new submission timestamp. The promised append-only trail then omits the actual KYB transition, and retrying records only a later submission time; run the update and audit insert in one database transaction.
Useful? React with 👍 / 👎.
| const [url, setUrl] = useState(""); | ||
| const [mode, setMode] = useState<"test" | "live">("test"); | ||
| const [busy, setBusy] = useState(false); | ||
| const [secret, setSecret] = useState(""); |
There was a problem hiding this comment.
After endpoint creation or secret rotation succeeds, the only copy returned to the merchant is stored in this panel-local state. Navigating to any other workspace route unmounts WebhooksPanel and permanently discards that value even if it has not been copied; reads intentionally omit it while the endpoint remains active. Keep the reveal state above the routed branch or block navigation until the merchant explicitly dismisses it.
Useful? React with 👍 / 👎.
| .orderBy(desc(webhookDeliveries.createdAt), desc(webhookDeliveries.id)) | ||
| .limit(take + 1); |
There was a problem hiding this comment.
For an endpoint with substantial delivery history, this query must fetch and sort all rows matching endpoint_id by (created_at, id) before applying the small page limit. The schema's existing endpoint index contains only endpoint_id, so pagination becomes increasingly expensive as deliveries accumulate; add a composite index beginning with (endpoint_id, created_at, id) to support this ordering.
Useful? React with 👍 / 👎.
…dit entries and index the payment-history and delivery pagination orders
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: c7cca1b601
ℹ️ 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".
| await this.audit.record({ | ||
| merchantId: merchant.id, | ||
| actor: merchant.ownerProviderId ?? 'unknown', | ||
| action: 'destination.provision', | ||
| target: merchant.id, | ||
| }); |
There was a problem hiding this comment.
When a provisioning response is lost and the owner retries, provisionOrLink() intentionally returns the existing destination with provisioned: false, but this unconditional call still appends another destination.provision entry. The owner-visible append-only trail then reports a fresh sensitive change even though the retry changed nothing; use the returned provisioned flag to record only the actual transition.
Useful? React with 👍 / 👎.
| .from(webhookDeliveries) | ||
| .where(eq(webhookDeliveries.correlationId, intent.id)) | ||
| .orderBy(desc(webhookDeliveries.createdAt)); |
There was a problem hiding this comment.
For a payment sent to many endpoints or subjected to repeated manual redeliveries, this query loads every matching delivery into one response because it has no limit or cursor. Since manual redeliveries are explicitly unbounded, opening the payment detail can eventually produce an arbitrarily large database result, JSON payload, and rendered table; paginate or cap this collection as the per-endpoint delivery route does.
Useful? React with 👍 / 👎.
| mode, | ||
| name: keyName.trim() || undefined, | ||
| }); | ||
| setRawKey(key.raw); |
There was a problem hiding this comment.
After issuing or rotating a key, the creation and rotation controls become enabled again while the one-time secret is still displayed. If the owner performs another issuance or rotation before copying the first secret, this assignment replaces the only copy of the first raw key even though that credential remains active and reads cannot recover it; block further key mutations until dismissal or retain each outstanding reveal separately.
Useful? React with 👍 / 👎.
| const data = await client.get<{ endpoints: WebhookEndpoint[] }>( | ||
| "webhooks", | ||
| ); | ||
| setEndpoints(data.endpoints); | ||
| setStatus("ready"); |
There was a problem hiding this comment.
The endpoint creation form is usable while the initial load() is still in flight, and every load writes its response without a request generation check. If that initial GET captured the pre-creation list but arrives after the post-create refresh, it overwrites the newer list and makes the successfully created endpoint disappear from the UI until another reload; discard superseded responses or abort the prior request.
Useful? React with 👍 / 👎.
| @Post('kyb/submit') | ||
| async submitKyb(@Headers('authorization') authorization?: string) { | ||
| const merchant = await this.owner.owned(authorization); | ||
| if (merchant.kybStatus === 'verified') | ||
| throw new ConflictException('Your business is already verified.'); |
There was a problem hiding this comment.
A newly registered merchant has an empty businessProfile, and the profile schema also permits every field to remain an empty string, but this endpoint moves either case into the review queue without validating any required legal, contact, or address details. The untouched registration page therefore lets an owner submit immediately and shows “Verification in review” even though reviewers received no business information; reject submission until the minimum review fields are populated.
Useful? React with 👍 / 👎.
… one-time secrets in the workspace shell so navigation cannot discard them
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 586b421349
ℹ️ 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".
| const [updated] = await db | ||
| .update(webhookEndpoints) | ||
| .set({ enabled: false, updatedAt: new Date() }) | ||
| .where(eq(webhookEndpoints.id, endpoint.id)) |
There was a problem hiding this comment.
When two portal deletion requests reach disable() before either transaction commits, both can pass find() while the endpoint is enabled, and this unconditional update lets both requests succeed. MerchantPortalWebhooksController.remove() then appends a webhook.delete audit entry in each transaction even though only one state transition occurred, leaving the owner-visible activity log inaccurate. Make the update conditional on enabled = true (or lock the row) and report whether this request claimed the transition before auditing it.
Useful? React with 👍 / 👎.
| .update(merchants) | ||
| .set({ | ||
| kybStatus: 'pending', | ||
| kybSubmittedAt: now, |
There was a problem hiding this comment.
After this timestamp marks a profile as submitted, POST /merchant-portal/profile still permits pending merchants to change every KYB field without clearing the submission or requiring another review, and markKybVerified() does not check which profile version the operator reviewed. An owner can therefore change the legal name or address after the reviewer reads the submission but before verification is stamped, leaving the altered profile verified and enabling live-key issuance without those details having been reviewed. Persist the submitted profile version or snapshot and make verification conditional on it, or invalidate the submission whenever review-relevant fields change.
Useful? React with 👍 / 👎.
| // never obtained (reads omit it, so a retry would only orphan another). | ||
| const { endpoint, secret } = await this.db.client.transaction( | ||
| async (tx) => { | ||
| const created = await this.endpoints.register( |
There was a problem hiding this comment.
For every portal endpoint creation, this transaction calls WebhookEndpointService.register(), which performs an unbounded dns.lookup() before executing its insert. A slow or deliberately non-responsive hostname therefore holds a database connection and an open transaction for the resolver timeout; with the default pool of ten connections, one authenticated merchant can submit enough such requests to starve unrelated API traffic. Run the URL/DNS validation before acquiring the transaction and keep only the endpoint and audit inserts inside it.
Useful? React with 👍 / 👎.
| try { | ||
| const created = await client.post<WebhookEndpoint & { secret: string }>( | ||
| "webhooks", | ||
| { url, mode }, |
There was a problem hiding this comment.
When the deployment uses devnet, execution payments have database mode live but paymentDeliveryMode() intentionally routes them to test endpoints so they cannot be represented as production traffic. This portal form nevertheless offers and submits a raw live endpoint mode without considering the dashboard cluster; a merchant configuring a “Live” endpoint for their devnet execution key creates an enabled endpoint that the dispatcher will never select, so no payment webhooks arrive. Map devnet endpoint creation to the test delivery mode, or disable/relabel the live choice using the active cluster.
Useful? React with 👍 / 👎.
…y real destination provisioning, cap payment-detail deliveries, guard against overwriting an unshown secret and stale webhook list responses
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 0caffd92b8
ℹ️ 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".
| if (result.provisioned) | ||
| await this.audit.record({ | ||
| merchantId: merchant.id, | ||
| actor: merchant.ownerProviderId ?? 'unknown', | ||
| action: 'destination.provision', | ||
| target: merchant.id, | ||
| }); |
There was a problem hiding this comment.
When provisioning persists the settlement_accounts row but this audit insert subsequently fails, the endpoint returns an error even though the destination is active. A retry then makes SettlementProvisioningService.provisionOrLink() return provisioned: false, so this branch is skipped and the owner-visible trail permanently omits the actual provisioning. Persist the settlement row and audit entry in one database transaction after external provisioning, or use a durable outbox so the audit cannot be lost.
Useful? React with 👍 / 👎.
| function describe(entry: AuditEntry): string { | ||
| const parts: string[] = []; | ||
| if (entry.metadata) | ||
| for (const [key, value] of Object.entries(entry.metadata)) | ||
| parts.push(`${key}: ${value}`); | ||
| return parts.join(" · "); |
There was a problem hiding this comment.
When a merchant has multiple API keys or webhook endpoints, actions such as api_key.revoke, webhook.rotate_secret, and webhook.delete have no metadata, and this formatter ignores the target field already returned by the API. Their activity rows therefore contain only an action label and timestamp, so the owner cannot determine which credential or endpoint changed. Render the target or record and display safe identifying metadata such as the key fingerprint or endpoint URL.
Useful? React with 👍 / 👎.
| .where(eq(paymentAttempts.intentId, intent.id)) | ||
| .orderBy(desc(paymentAttempts.createdAt)) | ||
| .limit(1); |
There was a problem hiding this comment.
For a terminal Payment, this lookup cannot use payment_attempts_live_idx because that index contains only authorized and settling rows, while the query has no status predicate; the only other attempt index is on status. Opening a payment detail therefore requires scanning the global attempts table and sorting the matching rows as history grows. Add an index beginning with (intent_id, created_at) (and the ordering tie-breaker if one is added to the query).
Useful? React with 👍 / 👎.
… claim webhook deletion before auditing, resolve webhook URLs outside the transaction, and route devnet webhooks to test mode
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 188b5a97fc
ℹ️ 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".
| // read and this update. Only move to pending from the exact status | ||
| // observed, so a submission never reverts a merchant that just | ||
| // became verified (which would disable their live keys). | ||
| eq(merchants.kybStatus, merchant.kybStatus), |
There was a problem hiding this comment.
When another session saves the profile after owned() returns but before this update executes, the status can remain pending, so this predicate still succeeds and stores the stale merchant.profileVersion as the submitted version alongside the newly saved profile. The portal then reports “Verification in review,” but the operator verification path refuses it because the submitted and current versions differ; include profileVersion in this conditional update so the request returns a conflict instead.
Useful? React with 👍 / 👎.
| : current, | ||
| ); | ||
| setRawKey(""); | ||
| setSecretReveal(null); |
There was a problem hiding this comment.
The workspace-level reveal can now contain a webhook signing secret as well as an API key, and it deliberately survives navigation. If an owner creates or rotates a webhook, navigates to Developers, and revokes any API key before copying the webhook secret, this unconditional clear discards the only copy while the endpoint remains active. Leave the reveal intact unless the revoked credential is itself the revealed secret, or disable revocation while any reveal is pending.
Useful? React with 👍 / 👎.
| throw new Error('--mark-kyb-rejected requires --reason "<note>"'); | ||
| } | ||
| const res = await client.query( | ||
| `UPDATE merchants SET kyb_status = 'rejected', kyb_review_note = $2, kyb_verified_at = NULL, updated_at = now() WHERE id = $1 RETURNING id`, |
There was a problem hiding this comment.
The new rejection command updates any existing merchant without requiring an active submission whose version still matches the reviewed profile. A delayed review result can therefore reject a profile the owner has since edited, and accidentally targeting an already verified merchant clears verification and causes all of its live keys to start returning 403. Apply the same submitted-version/current-version guard used by the verification command, and restrict the transition to the expected review state.
Useful? React with 👍 / 👎.
| <p> | ||
| Xend posts a signed event to your endpoint when a Payment is | ||
| confirmed. Verify the signature before fulfilling an order; a browser | ||
| redirect alone is not proof of payment. |
There was a problem hiding this comment.
Endpoints created by this form omit eventTypes, so the dispatcher sends payment.failed and payment.expired as well as payment.succeeded. This copy instead says the webhook arrives when a Payment is confirmed and only instructs the merchant to verify its signature, but failed and expired events are also validly signed; an integration following that guidance can fulfill an unpaid order. Require consumers to check for payment.succeeded explicitly, or default the portal endpoint to that event type.
Useful? React with 👍 / 👎.
| check( | ||
| "API is not cacheable", | ||
| (health.headers.get("cache-control") ?? "").includes("no-store"), | ||
| ); |
There was a problem hiding this comment.
The production guide says this smoke test verifies the API's baseline security headers, which it defines to include X-Frame-Options, Cross-Origin-Opener-Policy, and production HSTS, but the script finishes its header checks after nosniff and Referrer-Policy and proceeds directly to cacheability. A proxy stripping those headers—or a backend accidentally running without production HSTS—therefore still passes the deployment check; add assertions for the remaining advertised headers.
Useful? React with 👍 / 👎.
… payment attempt, and identify every audit row by its object
…, keep a pending secret through revocation, gate fulfillment copy on payment.succeeded, and assert all advertised security headers
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 907ad05f85
ℹ️ 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".
| `UPDATE merchants SET kyb_status = 'verified', kyb_verified_at = now(), updated_at = now() | ||
| WHERE id = $1 AND kyb_submitted_version IS NOT NULL AND kyb_submitted_version = profile_version RETURNING id`, |
There was a problem hiding this comment.
If a profile is rejected and a delayed or accidentally repeated --mark-kyb-verified command runs afterward, this predicate still matches because rejection leaves kyb_submitted_version equal to profile_version. The command then overwrites the rejection with verified, re-enabling live-key access despite there being no active pending review; restrict the update to kyb_status = 'pending' so only the currently reviewed submission can be approved.
Useful? React with 👍 / 👎.
| "$schema": "https://openapi.vercel.sh/vercel.json", | ||
| "buildCommand": "npm run build", | ||
| "outputDirectory": "dist", | ||
| "rewrites": [{ "source": "/(.*)", "destination": "/index.html" }] |
There was a problem hiding this comment.
When the portal is deployed using this new Vercel configuration, the static app receives no X-Frame-Options or CSP frame-ancestors policy; the middleware added in this change protects only backend responses, not the document containing the authenticated key, webhook, and KYB controls. A malicious origin can therefore frame a signed-in portal and overlay UI to induce destructive actions such as key revocation or webhook deletion, so configure an anti-framing response header for the portal routes as well.
Useful? React with 👍 / 👎.
| const allowPrivate = | ||
| this.config.get<boolean>('WEBHOOK_ALLOW_PRIVATE_URLS') ?? false; | ||
| await assertPublicHttpsUrl(url, { allowPrivate }); |
There was a problem hiding this comment.
When a production deployment accidentally carries WEBHOOK_ALLOW_PRIVATE_URLS=true, this code passes it through unchanged and skips all resolved-address checks, even though the new production guide states that production keeps the SSRF guard enabled regardless. The configuration schema also permits true for every NODE_ENV, so an authenticated merchant can register an HTTPS endpoint resolving to an internal or loopback service and make the delivery worker request it; force this option off or fail startup outside development/test.
Useful? React with 👍 / 👎.
…ed so a repeated approval cannot overwrite a rejection
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: c09b7d1f73
ℹ️ 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".
| const [claimed] = await db | ||
| .update(apiKeys) | ||
| .set({ revokedAt: new Date() }) | ||
| .where( | ||
| and( | ||
| eq(apiKeys.id, keyId), | ||
| eq(apiKeys.merchantId, merchantId), | ||
| isNull(apiKeys.revokedAt), |
There was a problem hiding this comment.
If the rotation transaction commits but its HTTP response is lost, the old key is already revoked and the successor's raw secret is unrecoverable. Retrying with the same key ID then misses the revokedAt IS NULL predicate and returns not found, leaving a live integration without its old credential or the replacement credential. Add an idempotency mechanism or a rotation protocol that preserves access across ambiguous responses.
Useful? React with 👍 / 👎.
| title: 'Checkout canceled', | ||
| detail: | ||
| 'This attempt was canceled. Head back to the store to try again.', |
There was a problem hiding this comment.
When Checkout opens an intent whose backend status is already canceled, App treats that status as non-payable and renders this same branch. In that case the merchant permanently canceled the intent, so telling the consumer that only this attempt was canceled and to try the same order again is false; pass the cancellation source to Result or use copy that does not promise the intent remains payable.
Useful? React with 👍 / 👎.
| {payment.webhookDeliveries.length === 0 ? ( | ||
| <p> | ||
| No webhook deliveries recorded. A webhook fires only when a | ||
| Payment is confirmed on-chain. |
There was a problem hiding this comment.
For a failed or expired Payment with no recorded delivery, this empty state says webhooks fire only after on-chain confirmation, but the dispatcher also emits payment.failed and payment.expired. This can lead merchants diagnosing a missing failure notification to believe no webhook was expected; describe all configured outcome events here instead.
Useful? React with 👍 / 👎.
| | 'webhook.create' | ||
| | 'webhook.rotate_secret' | ||
| | 'webhook.delete' |
There was a problem hiding this comment.
The new audit action set covers webhook creation, rotation, and deletion, but the existing API-key-authenticated /v1/webhook_endpoints controller still calls WebhookEndpointService directly without recording any of them. A merchant—or a compromised integration key—can therefore change delivery destinations or signing secrets while the owner-visible Activity log remains empty; wire those mutations into the same transactional audit path or narrow the audit trail's stated coverage.
Useful? React with 👍 / 👎.
| case '--mark-kyb-verified': | ||
| args.markKybVerified = next(); | ||
| break; | ||
| case '--mark-kyb-rejected': | ||
| args.markKybRejected = next(); | ||
| break; |
There was a problem hiding this comment.
If an operator or automation invocation supplies both --mark-kyb-verified and --mark-kyb-rejected, the parser accepts both and main() silently executes the verification branch first. A malformed review command can therefore approve a merchant when rejection was also requested, enabling live-key eligibility; validate these decision flags as mutually exclusive before connecting to the database.
Useful? React with 👍 / 👎.
| if (!updated) | ||
| throw new WebhookSecretRotationConflictError( | ||
| `endpoint ${id} secret rotated concurrently; retry`, | ||
| ); |
There was a problem hiding this comment.
When two internal webhook-secret rotation requests race, the losing update now throws this conflict error, but WebhookAdminController.mapServiceError() only handles unsafe URLs and not-found errors. Nest therefore returns a 500 to the internal caller instead of the conflict response used by both merchant-facing controllers, which can cause generic server-error retries to rotate the winner's newly issued secret again; map this error to HTTP 409 on the internal route as well.
Useful? React with 👍 / 👎.
| profileVersion: body.expectedVersion + 1, | ||
| // Editing the profile withdraws any pending submission: the reviewer | ||
| // must see the changed details, so verification cannot be stamped | ||
| // against the version they already read. | ||
| kybSubmittedAt: null, | ||
| kybSubmittedVersion: null, |
There was a problem hiding this comment.
While a merchant is under review, opening the business-profile editor and pressing Save without changing any value still reaches this update, increments the profile version, and clears both submission fields. The UI permits such an unchanged save, so the merchant silently falls out of the reviewable state even though no reviewed detail changed; treat identical payloads as a no-op or clear the submission only when review-relevant data actually differs.
Useful? React with 👍 / 👎.
…inguish a merchant-canceled intent from a dismissed attempt, and describe all outcome events in the delivery empty state
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 975580f145
ℹ️ 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".
| .where( | ||
| and( | ||
| eq(webhookEndpoints.id, id), | ||
| eq(webhookEndpoints.secretPrimary, endpoint.secretPrimary), | ||
| ), |
There was a problem hiding this comment.
When secret rotation races with deletion and the delete commits after find() but before this update, the predicate still matches the disabled row because it checks only the ID and old secret. The rotation then returns a new one-time secret and records a webhook.rotate_secret audit entry for an endpoint that has already been deleted and can no longer receive deliveries. Include enabled = true in the conditional update so the losing rotation reports a conflict instead.
Useful? React with 👍 / 👎.
| entries: page.map((row) => ({ | ||
| id: row.id, | ||
| action: row.action, | ||
| target: row.target, | ||
| metadata: row.metadata, | ||
| at: row.at.toISOString(), |
There was a problem hiding this comment.
The new API-key webhook mutation path now stores actor: api_key:<id> specifically so the owner-visible Activity log can show who made the change, but this response mapper drops row.actor. Consequently an endpoint redirected or secret rotated through an integration key is indistinguishable from the owner performing it in the portal, undermining incident investigation. Return a safe representation such as the key ID/fingerprint or “You,” without exposing the raw provider identity.
Useful? React with 👍 / 👎.
| const onPopState = () => { | ||
| const wasAccount = currentPathRef.current === pagePath("account"); | ||
| const now = window.location.pathname; | ||
| if ( |
There was a problem hiding this comment.
When the account page is opened as /account/, which routeFor() explicitly accepts by stripping trailing slashes, currentPathRef retains the raw /account/ value and this equality check reports that the user was not on the account page. Pressing Back with a dirty business-profile draft therefore bypasses canLeave() and unmounts the form without confirmation; determine the previous page through routeFor() or normalize the stored pathname before comparing it.
Useful? React with 👍 / 👎.
| "$schema": "https://openapi.vercel.sh/vercel.json", | ||
| "buildCommand": "npm run build", | ||
| "outputDirectory": "dist", | ||
| "rewrites": [{ "source": "/(.*)", "destination": "/index.html" }] |
There was a problem hiding this comment.
When this Vercel configuration hosts the portal with VITE_API_BASE left empty—the documented default for the preferred same-origin topology—the catch-all also rewrites /merchant-portal/* requests to index.html, because no preceding rule forwards them to the backend. Dashboard GETs and all mutations then receive HTML with status 200 instead of API JSON, leaving the deployed portal unusable; add ordered API proxy rewrites before the SPA fallback or require a split-origin API base for this deployment.
Useful? React with 👍 / 👎.
…point, show a safe actor label in the audit feed, and normalize the account path in the history guard
…tion response never revokes a live key
|
@codex review |
1 participant
Closes the code side of the merchant production-readiness backlog in
docs/specs/merchant-production-readiness.md. The two exclusions in that spec, mainnet execution and above-limit physical-device acceptance, stay deferred and are not simulated here.Base is
codex/pay-with-xend-merchant-v1, which has not merged tomainyet, so this stacks on it and merges the latest review fixes from that branch in.What changed
Backend (owner-authenticated portal surface)
merchant_audit_log(separate from the operator log) records profile, key, webhook and KYB writes, read back through an owner-scoped/merchant-portal/audit./merchant-portal/webhooks): create, list, rotate secret, delete, and delivery diagnostics, reusing the existing endpoint service so SSRF validation and the secret lifecycle are identical to the API-key surface./merchant-portal/payments) with cursor pagination, status and reference filters, scoped to the active cluster, plus a detail route with the USDC quote, exchange-rate fields when present, the confirmation reference (settlement signature) and the webhook delivery status for the intent.Merchant portal (frontend)
VITE_API_BASE) for split-origin serving; it defaults to same-origin.Checkout
<summary>, so it is keyboard and screen-reader operable.Docs
Testing
Deferred
Mainnet execution and above-limit physical-device acceptance remain the two open gates and are out of scope here.