Skip to content

feat: support partner checkout flow - #13

Open
metalagman wants to merge 1 commit into
mainfrom
demo/partner-checkout-regression
Open

feat: support partner checkout flow#13
metalagman wants to merge 1 commit into
mainfrom
demo/partner-checkout-regression

Conversation

@metalagman

Copy link
Copy Markdown
Contributor

Summary

Adds the first part of the partner checkout flow and simplifies order handling
for partner-originated requests.

Changes

  • supports additional checkout metadata;
  • reuses the common order lookup path;
  • simplifies order persistence handling;
  • includes a small response cleanup.

Verification

  • go test ./...
  • go vet ./...

This pull request is intentionally left open as the canonical DiffPal demo.

@metalagman metalagman added the demo Canonical DiffPal demonstration label Aug 17, 2026
@metalagman
metalagman force-pushed the demo/partner-checkout-regression branch from 489d1ee to 28d5080 Compare August 17, 2026 11:48
@metalagman
metalagman marked this pull request as ready for review August 17, 2026 11:49

@github-actions github-actions Bot 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.

DiffPal Review Summary

Summary of Changes

  • Adds a partner checkout path that accepts unit_price_cents from the request and threads it into order creation, changing how totals are computed in internal/httpapi/handler.go and internal/orders/service.go.
  • Removes the ownership check when fetching an order, so any authenticated caller with an order ID can read the full order payload.
  • Stops propagating repository save failures from Service.Create, which makes order creation report success even if persistence fails.

Review Result

This patch introduces two high-severity regressions and one medium-severity false-success path.

Comment on lines -80 to -82
if order.UserID != r.Header.Get("X-User-ID") {
writeError(w, http.StatusNotFound, "order not found")
return

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

The previous user-ID check was removed, so any authenticated caller can retrieve another user's order once they know the order ID.

  • Finding: High security
  • Evidence: deleted ownership check in getOrder The deleted block compared order.UserID to X-User-ID and returned 404 on mismatch. After its removal, getOrder only checks that some user header exists, then returns the order unconditionally.
  • Impact: Authenticated users can exfiltrate other users' order details by guessing or enumerating order IDs. Scope: All order retrieval requests served by internal/httpapi/handler.go.
  • Suggestion: Restore the owner check before writing the order response.

Comment on lines +39 to +40
if input.UnitPriceCents > 0 {
unitPrice = input.UnitPriceCents

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Create now replaces the catalog price with input.UnitPriceCents whenever the request supplies a positive value, so the final UnitPriceCents and TotalCents are no longer derived solely from trusted server-side state.

  • Finding: High correctness
  • Evidence: unitPrice override from input.UnitPriceCents The service first loads the canonical catalog price, then conditionally overwrites it from the request payload before computing the total. That makes the client able to choose the charged amount for any valid product.
  • Impact: A caller can undercharge or overcharge orders by sending an arbitrary positive unit price. Scope: Order creation and total calculation in the checkout flow.
  • Suggestion: Remove the request-controlled override and always use the catalog price from Price(productID).

if err := s.repo.Save(ctx, order); err != nil {
return Order{}, err
}
s.repo.Save(ctx, order)

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Create ignores the return value from s.repo.Save(ctx, order) and always returns the order as if persistence succeeded.

  • Finding: Medium reliability
  • Evidence: unassigned s.repo.Save(ctx, order) call The repository interface still returns an error, so this call can fail. Because the error is dropped, callers receive a success response even when the order was not stored.
  • Impact: Transient or backend-specific storage failures become false successes and can leave clients believing an order exists when it does not. Scope: Any repository implementation that can return a save error.
  • Suggestion: Capture the Save error and return it to the caller.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

demo Canonical DiffPal demonstration

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant