Skip to content

demo(15-non-idempotent-retry): 15 — Retry endpoint POSTed without an idempotency key#164

Open
EricCogen wants to merge 1 commit intomainfrom
demo/15-non-idempotent-retry
Open

demo(15-non-idempotent-retry): 15 — Retry endpoint POSTed without an idempotency key#164
EricCogen wants to merge 1 commit intomainfrom
demo/15-non-idempotent-retry

Conversation

@EricCogen
Copy link
Copy Markdown
Owner

15 — Retry endpoint POSTed without an idempotency key

Expected verdict: ❌ Fails — GauntletCI should fire GCI0022 (idempotency & retry safety).

What changed

PaymentsController gains a new Retry action exposed as
POST /payments/{id}/retry, with no idempotency-key parameter, header,
or dedup check anywhere in the handler:

[HttpPost("{id:guid}/retry")]
public async Task<ActionResult<PaymentResult>> Retry(
    Guid id,
    [FromBody] ChargeRequest request,
    CancellationToken ct)
{
    var result = await _payments.ChargeAsync(
        new PaymentRequest(request.OrderId, new Money(request.Amount, request.Currency), request.CustomerEmail),
        ct);
    return result.Success ? Ok(result) : UnprocessableEntity(result);
}

A retry endpoint is the single most likely thing to be invoked twice —
and "twice" on a charge endpoint means a duplicate charge.

Why this is risky

  • Network retries, queue redelivery, and angry users hammering the
    retry button all funnel here. Without an idempotency key, every one
    of them attempts a fresh charge.
  • The Stripe API takes an Idempotency-Key header for exactly this
    reason; not propagating one through this layer disables it.
  • The fix is two lines: accept an Idempotency-Key header, hash the
    request, and short-circuit on a cache hit.

What GauntletCI catches

GCI0022 Idempotency & Retry Safety — a [HttpPost(...)] endpoint
added to the diff with no IdempotencyKey / Idempotency-Key /
RequestId / MessageId / dedup signal anywhere in the surrounding
window.

…idempotency key

See scenarios/15-non-idempotent-retry/README.md for the expected verdict.
Copy link
Copy Markdown

@github-actions github-actions Bot left a comment

Choose a reason for hiding this comment

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

GauntletCI found issues in this PR. See inline comments for details.

return result.Success ? Ok(result) : UnprocessableEntity(result);
}

[HttpPost("{id:guid}/retry")]
Copy link
Copy Markdown

Choose a reason for hiding this comment

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

GCI0022 — Idempotency & Retry Safety

[HttpPost] endpoint in src/OrderService/Controllers/PaymentsController.cs has no idempotency key handling.

Evidence:

Line 27: [HttpPost("{id:guid}/retry")]

⚠️ Why it matters: Non-idempotent POST endpoints executed multiple times (retries, duplicate submissions) can create duplicate records or double-charge customers.

💡 Suggested action: Add an idempotency key header (e.g. Idempotency-Key), validate it server-side, and cache the response for duplicate requests.

Confidence: Medium | Severity: Warn

Copy link
Copy Markdown

@chatgpt-codex-connector chatgpt-codex-connector Bot left a comment

Choose a reason for hiding this comment

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

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: f345139e94

ℹ️ 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 +27 to +31
[HttpPost("{id:guid}/retry")]
public async Task<ActionResult<PaymentResult>> Retry(
Guid id,
[FromBody] ChargeRequest request,
CancellationToken ct)
Copy link
Copy Markdown

Choose a reason for hiding this comment

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

P1 Badge Add idempotency protection to retry POST

This retry action issues a fresh ChargeAsync call but accepts no idempotency token/header and performs no dedup lookup, so any client/network retry of POST /payments/{id}/retry can create duplicate charges. Retry endpoints are the highest-risk path for accidental replays, so this should propagate an idempotency key (or equivalent request identifier) before invoking the payment client.

Useful? React with 👍 / 👎.


[HttpPost("{id:guid}/retry")]
public async Task<ActionResult<PaymentResult>> Retry(
Guid id,
Copy link
Copy Markdown

Choose a reason for hiding this comment

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

P2 Badge Use the route payment ID in retry logic

The route requires {id:guid} but id is never used when building the retry request, so callers can pass any path ID and still trigger a charge based only on the body. That breaks the contract of a resource-specific retry endpoint and prevents validating/retrying the intended prior payment record.

Useful? React with 👍 / 👎.

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.

1 participant