Security and correctness hardening: throttling, cart authorization, settlement races, stale reservations, server-side catalogue

src/WidgetWorks.Application/Abstractions/IOrderRepository.cs

@@ -7,13 +7,24 @@ public interface IOrderRepository
     /// <summary>Atomically inserts the order and reserves stock. Returns false (rolled back) if any line is short.</summary>
     Task<bool> TryPlaceAsync(Order order, CancellationToken ct);
 
-    /// <summary>Records the provider + reference and parks the order in AwaitingPayment (async settlement).</summary>
-    Task MarkAwaitingPaymentAsync(Guid orderId, string provider, string reference, DateTimeOffset now, CancellationToken ct);
+    /// <summary>
+    /// Records the provider + reference and parks the order in AwaitingPayment (async settlement).
+    /// Returns false when the order had already moved on, so the write was declined.
+    /// </summary>
+    Task<bool> MarkAwaitingPaymentAsync(Guid orderId, string provider, string reference, DateTimeOffset now, CancellationToken ct);
 
-    Task MarkPaidAsync(Guid orderId, string provider, string reference, DateTimeOffset now, CancellationToken ct);
+    /// <summary>
+    /// Settles the order. Returns false when it was no longer awaiting settlement — a duplicate or
+    /// out-of-order provider event, which must not overwrite a decided order.
+    /// </summary>
+    Task<bool> MarkPaidAsync(Guid orderId, string provider, string reference, DateTimeOffset now, CancellationToken ct);
 
-    /// <summary>Marks the order failed and releases its inventory reservations.</summary>
-    Task MarkPaymentFailedAsync(Order order, string reason, DateTimeOffset now, CancellationToken ct);
+    /// <summary>
+    /// Marks the order failed and releases its inventory reservations, atomically. Returns false
+    /// when the order had already moved on; the caller must treat that as "someone else handled it"
+    /// rather than retrying, because the stock has already been dealt with.
+    /// </summary>
+    Task<bool> MarkPaymentFailedAsync(Order order, string reason, DateTimeOffset now, CancellationToken ct);
 
     /// <summary>
     /// Persists a fulfilment transition together with the inventory movement it implies, in one
@@ -23,6 +34,15 @@ public interface IOrderRepository
     /// </summary>
     Task UpdateStatusAsync(Order order, DateTimeOffset now, CancellationToken ct);
 
+    /// <summary>
+    /// Orders still parked in AwaitingPayment since before <paramref name="cutoff"/>, items loaded
+    /// so their reservations can be released. A settlement webhook that never arrives would
+    /// otherwise hold that stock forever.
+    /// </summary>
+    /// <param name="limit">Caps one sweep, so a large backlog is worked through over several passes
+    /// rather than in one long transaction.</param>
+    Task<IReadOnlyList<Order>> GetStaleAwaitingPaymentAsync(DateTimeOffset cutoff, int limit, CancellationToken ct);
+
     Task<Order?> GetByIdAsync(Guid id, CancellationToken ct);
 
     /// <summary>Finds an order by the payment provider + reference stored at authorization time (webhook correlation).</summary>

src/WidgetWorks.Application/Abstractions/IWidgetRepository.cs

@@ -3,11 +3,35 @@
 namespace WidgetWorks.Application.Abstractions;
 
 /// <summary>Filter/paging criteria for catalog listing and search.</summary>
-public sealed record WidgetQuery(string? Search, bool ActiveOnly, int Page, int PageSize)
+/// <summary>
+/// A catalogue listing request. Search and Category are independent narrowings combined with AND,
+/// so "turbine" within Mega means both, not either.
+/// </summary>
+/// <param name="Sort">
+/// One of <see cref="WidgetSort"/>. Never interpolated into SQL - the repository maps it through a
+/// fixed set of order-by clauses, so an unrecognised value falls back to the default rather than
+/// reaching the database.
+/// </param>
+public sealed record WidgetQuery(
+    string? Search,
+    bool ActiveOnly,
+    int Page,
+    int PageSize,
+    string? Category = null,
+    string? Sort = null)
 {
     public int Offset => (Math.Max(1, Page) - 1) * PageSize;
 }
 
+/// <summary>The orderings the catalogue offers. Values are part of the API contract.</summary>
+public static class WidgetSort
+{
+    public const string Featured = "featured";
+    public const string PriceAscending = "price-asc";
+    public const string PriceDescending = "price-desc";
+    public const string Name = "name";
+}
+
 public interface IWidgetRepository
 {
     Task<Widget?> GetByIdAsync(Guid id, CancellationToken ct);

src/WidgetWorks.Application/Carts/AddItem/AddCartItemHandler.cs

@@ -40,7 +40,10 @@ public async Task<Result<CartView>> Handle(AddCartItemCommand command, Cancellat
 
     private async Task<Cart> ResolveCartAsync(Guid? cartId, Guid? userId, CancellationToken ct)
     {
-        if (cartId is { } id && await carts.GetAsync(id, ct) is { } existing)
+        // A supplied id is only honoured when the caller may actually use that cart. A foreign one
+        // falls through to the caller's own rather than erroring, so an attacker learns nothing about
+        // whether the id existed and an honest client with a stale id simply carries on.
+        if (cartId is { } id && await carts.GetAsync(id, ct) is { } existing && CartAccess.IsPermitted(existing, userId))
         {
             return existing;
         }

src/WidgetWorks.Application/Carts/CartAccess.cs

@@ -0,0 +1,25 @@
+using WidgetWorks.Domain.Carts;
+
+namespace WidgetWorks.Application.Carts;
+
+/// <summary>
+/// The single rule for who may touch a cart.
+///
+/// A guest cart carries no owner and is reachable by anyone holding its id — that is what lets a
+/// visitor fill a basket before signing in, and it is a capability model: the id is the credential.
+/// The moment a cart belongs to a user, only that user may reach it, so signing in genuinely
+/// protects the basket instead of leaving it as exposed as a guest's.
+///
+/// Kept as one function rather than repeated per handler so a new cart operation cannot quietly ship
+/// without the check, and so the rule can be tested on its own.
+/// </summary>
+public static class CartAccess
+{
+    /// <param name="cart">The cart that was loaded by id.</param>
+    /// <param name="requestedBy">The signed-in user, or null for an anonymous caller.</param>
+    public static bool IsPermitted(Cart cart, Guid? requestedBy)
+    {
+        ArgumentNullException.ThrowIfNull(cart);
+        return cart.UserId is null || cart.UserId == requestedBy;
+    }
+}

src/WidgetWorks.Application/Carts/GetCart/GetCartHandler.cs

@@ -1,9 +1,10 @@
 using WidgetWorks.Application.Abstractions;
 using WidgetWorks.Domain.Common;
+using WidgetWorks.Application.Carts;
 
 namespace WidgetWorks.Application.Carts.GetCart;
 
-public sealed record GetCartQuery(Guid CartId);
+public sealed record GetCartQuery(Guid CartId, Guid? RequestedBy);
 
 public sealed class GetCartHandler(ICartRepository carts, IWidgetRepository widgets)
 {
@@ -15,6 +16,13 @@ public async Task<Result<CartView>> Handle(GetCartQuery query, CancellationToken
             return Result<CartView>.Fail("Cart not found.");
         }
 
+        if (!CartAccess.IsPermitted(cart, query.RequestedBy))
+        {
+            // Deliberately the same answer as a missing cart: telling an unauthorized caller that
+            // the cart exists would confirm a guess.
+            return Result<CartView>.Fail("Cart not found.");
+        }
+
         return Result<CartView>.Success(await CartAssembler.BuildAsync(cart, widgets, ct));
     }
 }

src/WidgetWorks.Application/Carts/RemoveItem/RemoveCartItemHandler.cs

@@ -1,9 +1,10 @@
 using WidgetWorks.Application.Abstractions;
 using WidgetWorks.Domain.Common;
+using WidgetWorks.Application.Carts;
 
 namespace WidgetWorks.Application.Carts.RemoveItem;
 
-public sealed record RemoveCartItemCommand(Guid CartId, Guid WidgetId);
+public sealed record RemoveCartItemCommand(Guid CartId, Guid WidgetId, Guid? RequestedBy);
 
 public sealed class RemoveCartItemHandler(ICartRepository carts, IWidgetRepository widgets, TimeProvider clock)
 {
@@ -15,6 +16,13 @@ public async Task<Result<CartView>> Handle(RemoveCartItemCommand command, Cancel
             return Result<CartView>.Fail("Cart not found.");
         }
 
+        if (!CartAccess.IsPermitted(cart, command.RequestedBy))
+        {
+            // Deliberately the same answer as a missing cart: telling an unauthorized caller that
+            // the cart exists would confirm a guess.
+            return Result<CartView>.Fail("Cart not found.");
+        }
+
         await carts.RemoveItemAsync(command.CartId, command.WidgetId, ct);
         await carts.TouchAsync(command.CartId, clock.GetUtcNow(), ct);
 

src/WidgetWorks.Application/Carts/UpdateItem/UpdateCartItemHandler.cs

@@ -1,10 +1,11 @@
 using WidgetWorks.Application.Abstractions;
 using WidgetWorks.Domain.Common;
+using WidgetWorks.Application.Carts;
 
 namespace WidgetWorks.Application.Carts.UpdateItem;
 
 /// <summary>Sets an absolute quantity for a line; a quantity of zero removes it.</summary>
-public sealed record UpdateCartItemCommand(Guid CartId, Guid WidgetId, int Quantity);
+public sealed record UpdateCartItemCommand(Guid CartId, Guid WidgetId, int Quantity, Guid? RequestedBy);
 
 public sealed class UpdateCartItemHandler(ICartRepository carts, IWidgetRepository widgets, TimeProvider clock)
 {
@@ -16,6 +17,13 @@ public async Task<Result<CartView>> Handle(UpdateCartItemCommand command, Cancel
             return Result<CartView>.Fail("Cart not found.");
         }
 
+        if (!CartAccess.IsPermitted(cart, command.RequestedBy))
+        {
+            // Deliberately the same answer as a missing cart: telling an unauthorized caller that
+            // the cart exists would confirm a guess.
+            return Result<CartView>.Fail("Cart not found.");
+        }
+
         var now = clock.GetUtcNow();
         if (command.Quantity <= 0)
         {

src/WidgetWorks.Application/Catalog/Browse/BrowseWidgetsHandler.cs

@@ -2,7 +2,13 @@
 
 namespace WidgetWorks.Application.Catalog.Browse;
 
-public sealed record BrowseWidgetsQuery(string? Search, bool IncludeInactive, int Page, int PageSize);
+public sealed record BrowseWidgetsQuery(
+    string? Search,
+    bool IncludeInactive,
+    int Page,
+    int PageSize,
+    string? Category = null,
+    string? Sort = null);
 
 public sealed class BrowseWidgetsHandler(IWidgetRepository widgets)
 {
@@ -18,7 +24,9 @@ public async Task<PagedResult<WidgetView>> Handle(BrowseWidgetsQuery query, Canc
             string.IsNullOrWhiteSpace(query.Search) ? null : query.Search.Trim(),
             ActiveOnly: !query.IncludeInactive,
             page,
-            size);
+            size,
+            Category: string.IsNullOrWhiteSpace(query.Category) ? null : query.Category.Trim(),
+            Sort: query.Sort);
 
         var items = await widgets.SearchAsync(repoQuery, ct);
         var total = await widgets.CountAsync(repoQuery, ct);

src/WidgetWorks.Application/Checkout/ConfirmPayment/ConfirmPaymentHandler.cs

@@ -38,7 +38,14 @@ public async Task<Result<string>> Handle(ConfirmPaymentCommand command, Cancella
 
         if (command.Type == PaymentEventType.Succeeded)
         {
-            await orders.MarkPaidAsync(order.Id, order.PaymentProvider ?? command.Provider, command.Reference, now, ct);
+            // The status check above is a courtesy; this is the decision. Two deliveries of the
+            // same event both pass that check, and the database picks exactly one winner. Only the
+            // winner sends a receipt, so a retried webhook cannot email the customer twice.
+            if (!await orders.MarkPaidAsync(order.Id, order.PaymentProvider ?? command.Provider, command.Reference, now, ct))
+            {
+                return Result<string>.Success(order.Status);
+            }
+
             order.Status = OrderStatus.Paid;
 
             try
@@ -58,7 +65,13 @@ public async Task<Result<string>> Handle(ConfirmPaymentCommand command, Cancella
             return Result<string>.Success(OrderStatus.Paid);
         }
 
-        await orders.MarkPaymentFailedAsync(order, "Payment failed.", now, ct);
+        // Same contract on the failure path, and it matters more here: the losing caller must not
+        // release the reservation a second time.
+        if (!await orders.MarkPaymentFailedAsync(order, "Payment failed.", now, ct))
+        {
+            return Result<string>.Success(order.Status);
+        }
+
         order.Status = OrderStatus.PaymentFailed;
         return Result<string>.Success(OrderStatus.PaymentFailed);
     }

src/WidgetWorks.Application/Checkout/PlaceOrder/CheckoutHandler.cs

@@ -65,8 +65,10 @@ public async Task<Result<CheckoutResult>> Handle(CheckoutCommand command, Cancel
         }
 
         var cart = await carts.GetAsync(command.CartId, ct);
-        if (cart is null)
+        if (cart is null || !CartAccess.IsPermitted(cart, command.UserId))
         {
+            // One answer for "no such cart" and "not yours": checking out someone else's basket
+            // would otherwise disclose its contents in the resulting order.
             return Fail("Cart not found.");
         }
 
@@ -93,7 +95,10 @@ public async Task<Result<CheckoutResult>> Handle(CheckoutCommand command, Cancel
 
         if (payment.Status == PaymentStatus.Declined)
         {
-            await orders.MarkPaymentFailedAsync(order, payment.Error ?? "Payment failed.", clock.GetUtcNow(), ct);
+            // Discarded deliberately: this order was created moments ago and is still Pending, so
+            // the compare-and-set cannot decline. A webhook arriving later is the contended path,
+            // and ConfirmPaymentHandler is where the answer is acted on.
+            _ = await orders.MarkPaymentFailedAsync(order, payment.Error ?? "Payment failed.", clock.GetUtcNow(), ct);
             return Fail(payment.Error ?? "Payment failed.");
         }
 
@@ -103,7 +108,7 @@ public async Task<Result<CheckoutResult>> Handle(CheckoutCommand command, Cancel
         {
             // Async settlement (redirect/BNPL): keep the reservation, park the order, and let the
             // provider webhook finalize it. The receipt email is sent on confirmation, not here.
-            await orders.MarkAwaitingPaymentAsync(order.Id, payment.Provider, reference, clock.GetUtcNow(), ct);
+            _ = await orders.MarkAwaitingPaymentAsync(order.Id, payment.Provider, reference, clock.GetUtcNow(), ct);
             order.Status = OrderStatus.AwaitingPayment;
             await carts.DeleteAsync(cart.Id, ct);
 
@@ -113,7 +118,7 @@ public async Task<Result<CheckoutResult>> Handle(CheckoutCommand command, Cancel
         }
 
         // Synchronous success.
-        await orders.MarkPaidAsync(order.Id, payment.Provider, reference, clock.GetUtcNow(), ct);
+        _ = await orders.MarkPaidAsync(order.Id, payment.Provider, reference, clock.GetUtcNow(), ct);
         order.Status = OrderStatus.Paid;
         await carts.DeleteAsync(cart.Id, ct);
 

src/WidgetWorks.Application/Checkout/ReleaseStale/ReleaseStaleReservationsHandler.cs

@@ -0,0 +1,99 @@
+using Microsoft.Extensions.Logging;
+using WidgetWorks.Application.Abstractions;
+
+namespace WidgetWorks.Application.Checkout.ReleaseStale;
+
+/// <summary>
+/// How long an unsettled order may hold stock, and how often to look. Bound from the
+/// <c>Reservations</c> configuration section.
+/// </summary>
+public sealed class ReservationOptions
+{
+    /// <summary>
+    /// How long an order may sit in AwaitingPayment before its stock is handed back.
+    ///
+    /// This is a trade, not a tuning knob: too short and a slow but honest bank redirect loses a
+    /// customer's basket; too long and abandoned or abusive orders hold the catalogue hostage.
+    /// Fifteen minutes is longer than any interactive redirect and short enough that a shopper who
+    /// returns to an out-of-stock item is rare.
+    /// </summary>
+    public int ExpireAfterMinutes { get; set; } = 15;
+
+    /// <summary>How often the sweep runs.</summary>
+    public int SweepIntervalMinutes { get; set; } = 5;
+
+    /// <summary>
+    /// Most orders released in one pass. A backlog is worked through over several sweeps rather
+    /// than in one long pass, so a bad day cannot turn into a slow transaction storm.
+    /// </summary>
+    public int BatchSize { get; set; } = 100;
+
+    /// <summary>Turns the sweep off — for a host that should not run background work.</summary>
+    public bool Enabled { get; set; } = true;
+}
+
+/// <summary>
+/// Hands back stock held by orders whose payment never settled.
+///
+/// Checkout reserves stock the moment an order is placed. When settlement is asynchronous the order
+/// parks in AwaitingPayment and waits for a provider webhook. If that webhook never arrives — a
+/// provider outage, a dropped delivery, a shopper who closed the tab at the bank's redirect — the
+/// reservation is held forever, and without this sweep the only route back is an administrator
+/// editing inventory counts by hand.
+///
+/// The release itself reuses <see cref="IOrderRepository.MarkPaymentFailedAsync"/>: it already sets
+/// the status and releases the reservation in one transaction, and its compare-and-set means a
+/// webhook landing at the same moment as a sweep cannot both act. PaymentFailed is also the honest
+/// description of a settlement that never came, so no new status is needed.
+///
+/// Scheduling deliberately lives elsewhere. This type is a plain handler so the policy can be tested
+/// without a timer, a host, or a clock that really waits.
+/// </summary>
+public sealed class ReleaseStaleReservationsHandler(
+    IOrderRepository orders,
+    TimeProvider clock,
+    ReservationOptions options,
+    ILogger<ReleaseStaleReservationsHandler> logger)
+{
+    /// <summary>Runs one sweep. Returns how many orders had their stock returned to sale.</summary>
+    public async Task<int> Handle(CancellationToken ct)
+    {
+        var now = clock.GetUtcNow();
+        var cutoff = now.AddMinutes(-Math.Max(1, options.ExpireAfterMinutes));
+        var stale = await orders.GetStaleAwaitingPaymentAsync(cutoff, Math.Max(1, options.BatchSize), ct);
+
+        var released = 0;
+        foreach (var order in stale)
+        {
+            // Checked between orders rather than only at the top: a shutdown midway through a large
+            // batch should stop cleanly, and each release is already committed on its own.
+            ct.ThrowIfCancellationRequested();
+
+            if (await orders.MarkPaymentFailedAsync(order, "Payment was not completed in time.", now, ct))
+            {
+                released++;
+            }
+            else
+            {
+                // The order moved on between the query and the write — almost always a webhook
+                // that landed first, which is the good outcome. Recorded at debug because it is
+                // expected, not a fault.
+                logger.LogDebug(
+                    "Order {OrderNumber} settled before the sweep reached it; nothing released.",
+                    order.OrderNumber);
+            }
+        }
+
+        if (released > 0)
+        {
+            // Worth a real log line: it means customers or scripts are abandoning payments, and a
+            // rising count is the signal that something upstream is wrong.
+            logger.LogInformation(
+                "Released stock held by {Released} order(s) unsettled since before {Cutoff:o}.",
+                released,
+                cutoff);
+        }
+
+        return released;
+    }
+}