Skip to content

fix(checkout): guard decrypt-failure sentinel + client-side INR QR (closes #44) - #45

Closed
gitchadd wants to merge 1 commit into
mainfrom
fix/decrypt-failure-rendering
Closed

fix(checkout): guard decrypt-failure sentinel + client-side INR QR (closes #44)#45
gitchadd wants to merge 1 commit into
mainfrom
fix/decrypt-failure-rendering

Conversation

@gitchadd

Copy link
Copy Markdown
Collaborator

What

Closes #44 (surfaced in the PR #43 adversarial review). Two coupled decrypt-failure warts on main:

  1. The sentinel "Session changed" was rendered as a copyable VPA (CopyRow) and encoded into the INR QR — on a decrypt failure the user saw/copied/scanned a bogus address.
  2. The INR QR was built by the third-party api.qrserver.com, leaking the counterparty VPA + amount + order id to a third party on every INR accepted screen.

Changes

  • Sentinel guard (any currency): decryptedUpi === "Session changed" → "Couldn't load payment details. Please contact support." instead of a payment address. Placed first so it covers compound currencies too, and never misfires on a valid value.
  • INR QR gated on a resolved VPA (not the sentinel) + a real fiatDisplay.
  • Client-side QR: the api.qrserver.com <img> is replaced by a lazy qrcode.react QRCodeSVG (split chunk, out of the main /checkout bundle). Same upi://pay?... content, generated locally → third-party leak closed.

Adds the qrcode.react dependency (small, lazy-loaded, already used in user-app-spa). Focused port of the fix that shipped on the archived feat/upi-intent branch (tag archive/feat-upi-intent).

Verification

  • npm run verify green: typecheck, build, 142 tests.
  • Leak closed, verified at the bundle level: no api.qrserver.com in the runtime dist/*.js|*.cjs (the only .map match is this PR's own code comment); QRCodeSVG present in the runtime bundle.
  • Honest gap (Directive feat(v1.1.1-bridge): chatEnabled flag + actionable filter — temporary fallback #7): Checkout has no render-test harness, so the sentinel→error branch is verified by read + typecheck, not a unit test. The client-side QR is the same QRCodeSVG approach e2e-verified earlier on the archived branch (renders, no api.qrserver.com request). A Checkout render-harness is a worthwhile follow-up.

Deploy gate: human review + explicit per-merge approval. Not for auto-merge.

On decrypt failure order-machine sets decryptedUpi="Session changed". main
rendered that sentinel as a copyable VPA (CopyRow) and encoded it into the INR
QR, and shipped the QR (VPA + amount + order id) to third-party api.qrserver.com
on every INR accepted screen.

- Show "Couldn't load payment details. Please contact support." for the sentinel
  (any currency) instead of rendering it as a payment address.
- Gate the INR QR on a resolved VPA (not the sentinel) + a real amount.
- Replace the api.qrserver.com <img> with a lazy client-side qrcode.react QR,
  closing the third-party data leak. Same upi:// QR content, generated locally.

Closes #44 (surfaced in the PR #43 review). verify green: typecheck + build +
142 tests; no api.qrserver.com in the runtime bundle.
@gitchadd

Copy link
Copy Markdown
Collaborator Author

Agentic adversarial review — PRs #43 + #45 reviewed together

Three independent read-only agents (bug-hunter; system-architect; compliance) over both branches, plus my own verification of the load-bearing claims.

Verdict: both clean — no production bugs, compliance PASS. Ship them as a PAIR. #43's test is genuine red/green (143 tests); #45's 142 is accurate and the api.qrserver.com leak removal is independently verified at the runtime-bundle level (the only .map hit is this PR's own code comment). Both compose correctly; no merge conflict (they touch different files).

Merge-ordering coupling — the one real interaction finding

Independent branches, but a coupled pair:

Follow-ups the review generated (being fixed, not filed-and-forgotten)

  1. Shared sentinel constant. "Session changed" is a magic-string contract across order-machine.ts (produces) and Checkout.tsx (branches on the exact literal) — unsynchronized copies in 2 files; a typo silently breaks the guard with no type error. → export a DECRYPT_FAILED_SENTINEL and use it everywhere.
  2. Restore the .includes("@") VPA guard the port dropped from the archived branch — closes a malformed-Result-Ok edge (a decrypted-but-non-VPA value would still render/QR). Low likelihood, but the archived fix closed it.
  3. Checkout.tsx render-harness test — every sibling widget has one; the sentinel→error branch (fix(checkout): guard decrypt-failure sentinel + client-side INR QR (closes #44) #45's whole user-facing payload) currently ships with zero render coverage (fix(checkout): guard decrypt-failure sentinel + client-side INR QR (closes #44) #45 discloses this honestly per Directive feat(v1.1.1-bridge): chatEnabled flag + actionable filter — temporary fallback #7).

Repo-completeness verified myself: decryptedUpi renders only in Checkout.tsx, so the guard is complete. The full typed decryptError state refactor is correct long-term but is tracked debt, not a merge blocker — sentinel reuse was the right scope for a hotfix.

Deploy gate: merge needs explicit per-merge approval. This review is the agentic step.

@gitchadd

Copy link
Copy Markdown
Collaborator Author

Superseded by #46 (consolidated decrypt-failure hardening). #44 is closed by #46.

@gitchadd gitchadd closed this Jun 18, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Decrypt-failure sentinel "Session changed" rendered as a VPA (copy-row + INR QR), no @-guard

1 participant