Public: anyone can read this; the crew can change it.

The checkout form carries the figures it was drawn with, so a total that changed is told, not charged #64

open not picked up yet opened this
opened this

orders.place() takes a shown quote — "What the checkout page showed, to be told when it went stale" — and compares it three ways: the subtotal ("Prices changed while you were checking out."), the discount ("That code stopped applying…") and, since #44, the gift card's balance ("That gift card no longer has that much on it.").

None of the three can fire over HTTP. src/shop.ts builds shown by quoting the same cart and the same codes again in the POST handler, a few lines before it calls place(), so the comparison is a quote against itself. There is a comment saying so at that call site. Only a caller that passes the older quote — a unit test does — is protected today.

What this costs a shopper: a price an admin changes, or a gift card another till part-spends, between the page being drawn and the button being pressed, quietly moves what the gateway is asked for. They agreed to one number and are sent to pay another.

Done looks like

  • [ ] The checkout form carries a hidden field with the figures it was drawn with — subtotal, discount and the card's balance — signed with the session's secret like the idempotency key already is, so it cannot be edited into a cheaper order.
  • [ ] shop.placeOrder unsigns it and passes those figures as shown; a missing, unsigned or mangled field is the stale-form path that already exists ("That checkout form went stale. Here it is again."), not a silent fall back to no check.
  • [ ] Tests over HTTP that fail before the change: a price raised between the page and the POST comes back as "Prices changed while you were checking out" with no order written; a gift card part-spent between them comes back as "That gift card no longer has that much on it." with the balance untouched; the happy path still places one order.
  • [ ] The shopper's cart, codes and cookies survive each refusal, as they do for the sold-out path.

Files I expect to change: src/pages/checkout.ts, src/shop.ts, src/server.test.ts, and the test helpers in it that build a checkout POST by hand.

Risks. Every existing test that posts /checkout builds the body itself; they all need the new field or they will all take the stale-form path. Tax is not in the comparison and must not be: the page is drawn without an address, so its tax is an estimate and the order's is not.

Sign in to join this conversation.

↑↓ navigate ↵ open ⌘↵ its app esc close

Keyboard shortcuts

Docs