Skip to content

fix: shop-setup Phase 6 — webhook signature is sha1(body + secret), not HMAC-SHA1 - #27

Merged
JavadManashti merged 1 commit into
mainfrom
fix/shop-setup-webhook-signature-and-fulfillment
Aug 17, 2026
Merged

fix: shop-setup Phase 6 — webhook signature is sha1(body + secret), not HMAC-SHA1#27
JavadManashti merged 1 commit into
mainfrom
fix/shop-setup-webhook-signature-and-fulfillment

Conversation

@JavadManashti

Copy link
Copy Markdown
Collaborator

Corrects Phase 6 of shop-setup, whose webhook contract contradicted webhooks-impl and produced a handler that rejects every genuine Xsolla webhook.

shop-setup is the orchestrator loaded first for any "build me a shop" request, so Phase 6 is where an agent gets its webhook contract. Three of its five bullets were wrong.

The defect

- Verify the webhook signature (HMAC-SHA1)
- Handle `payment` notification type: extract `user.id` and purchased items
- Respond `HTTP 200` to acknowledge receipt
  • The signature is not an HMAC. Xsolla appends the secret to the raw body and hashes the result: lowercase(sha1(rawBody + secret)). webhooks-impl/references/signature-verification.md says so outright — "not an HMAC key. This is plain sha1(body + key)".
  • payment is the wrong fulfillment event for our default flow. Phase 5 of this same skill defaults to Method 1 (Store payment token), and Store orders deliver order_paid. Granting on payment also double-grants under separate delivery.
  • user.id is absent from order_paid. The player is user.external_id there; user.id is the Merchant-API (payment) shape.

Verification

Against our own fixture webhooks-impl/fixtures/order_paid.raw.txt:

Xsolla header sig   : bc951da308048cbc1e241e7469d9d9ac03917561
OLD (HMAC-SHA1)     : 720c4486087581277a95f36c834bc350413bc521 -> 400 INVALID_SIGNATURE
NEW (sha1 body+key) : bc951da308048cbc1e241e7469d9d9ac03917561 -> 2xx ACCEPT

event               : order_paid
OLD field user.id   : undefined
NEW user.external_id: "a1b2c3d4-0000-4000-8000-019fb6caa51e"
NEW items[]         : artifact_centaurs_axe x1
idempotency txid    : 2105129134

Every element of the corrected instruction resolves against the real payload; every element of the old one fails. The consequence on main today is not cosmetic — the player pays and receives nothing, which webhooks-impl calls the number-one cause of INVALID_SIGNATURE at go-live.

The change

Five bullets in, five bullets out, now agreeing with webhooks-impl:

  • raw-body SHA-1 with the secret appended and a constant-time compare, with an explicit "not HMAC-SHA1" since that is the trap
  • fulfillment event selected by the token method in use — order_paid (items[], user.external_id) for Methods 1–2, payment (custom_parameters, user.id) for Method 3, granting on order_paid only when both arrive. This mapping is already stated in webhooks-impl/references/testing.md.
  • idempotency by transaction id
  • 2xx on success, 400 + INVALID_SIGNATURE on mismatch — matching handler-and-reliability.md, which notes Xsolla's setup test explicitly checks for the 400

No other section touched. .cursor/skills/shop-setup/SKILL.md is updated in the same commit and is byte-identical to the source (same git blob hash); running sync-providers.yml locally against this branch produces no further changes.

Provenance

The defect was reported by an external contributor in #26. The finding was verified independently and landed here through internal review rather than merging that PR; #26 will be closed once this merges.

Phase 6 told the agent to verify with HMAC-SHA1 and to fulfil on `payment`
using `user.id`. Both are wrong for the flow this skill defaults to:

- Xsolla signs the raw body with plain `lowercase(sha1(rawBody + secret))`.
  `webhooks-impl/references/signature-verification.md` states this explicitly
  ("not an HMAC key. This is plain sha1(body + key)").
- Phase 5 defaults to Method 1 (Store payment token), and Store orders deliver
  `order_paid`, whose player field is `user.external_id` — `user.id` is absent
  from that payload. Granting on `payment` also double-grants under separate
  delivery.

Verified against `webhooks-impl/fixtures/order_paid.raw.txt`: the header
signature is bc951da3…, HMAC-SHA1 over the same bytes is 720c4486…, so a
handler built from Phase 6 as written rejected every genuine webhook with
400 INVALID_SIGNATURE — payment succeeds and the player receives nothing.

Brings the block in line with `webhooks-impl` (signature algorithm, event and
field mapping per token method, idempotency, and the 2xx / 400 INVALID_SIGNATURE
response contract) and mirrors it into `.cursor/skills/` as the provider sync
would. Five bullets in, five bullets out; no other section touched.

Reported externally in #26; landed here through internal review.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Copilot AI lite review requested due to automatic review settings August 14, 2026 14:08

Copilot AI left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

This PR corrects Phase 6 (“Webhook / fulfillment”) in the shop-setup orchestrator skill so its webhook contract matches webhooks-impl and reflects Xsolla’s actual signature algorithm, event shapes, idempotency expectations, and response semantics.

Changes:

  • Replaces the incorrect HMAC-SHA1 guidance with raw-body sha1(body + secret) (secret appended) and constant-time comparison guidance.
  • Updates fulfillment-event guidance to distinguish order_paid (Methods 1–2) vs payment (Method 3) and to avoid double-granting in separate delivery.
  • Updates acknowledgement semantics to accept 2xx on success and return 400 INVALID_SIGNATURE on signature mismatch.

Reviewed changes

Copilot reviewed 2 out of 2 changed files in this pull request and generated 2 comments.

File Description
skills/shop-setup/SKILL.md Updates Phase 6 webhook bullets to match webhooks-impl’s contract (signature, event selection, idempotency, response codes).
.cursor/skills/shop-setup/SKILL.md Mirrors the same Phase 6 webhook contract changes in the Cursor-synced provider copy.

💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.

Comment on lines +203 to +205
- Verify the signature over the **raw** body: `lowercase(sha1(rawBody + secret))`, compared in constant time — plain SHA-1 with the secret **appended**, **not** HMAC-SHA1 → `webhooks-impl`
- Handle the fulfillment event for the token method in use: `order_paid` (Methods 1–2 — items in `items[]`, player in `user.external_id`) or `payment` (Method 3 — items usually in `custom_parameters`, player in `user.id`); when both arrive (separate delivery), grant on `order_paid` only
- Grant the item/currency/key in the partner's game system, idempotently by transaction id
Comment on lines +203 to +205
- Verify the signature over the **raw** body: `lowercase(sha1(rawBody + secret))`, compared in constant time — plain SHA-1 with the secret **appended**, **not** HMAC-SHA1 → `webhooks-impl`
- Handle the fulfillment event for the token method in use: `order_paid` (Methods 1–2 — items in `items[]`, player in `user.external_id`) or `payment` (Method 3 — items usually in `custom_parameters`, player in `user.id`); when both arrive (separate delivery), grant on `order_paid` only
- Grant the item/currency/key in the partner's game system, idempotently by transaction id
@JavadManashti
JavadManashti merged commit 86fd309 into main Aug 17, 2026
2 of 3 checks passed
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.

3 participants