Skip to content

Fix: Unvalidated menu item prices on POST /restaurants allow negative-priced menu items and negative order totals - #330

Open
WesternConcrete wants to merge 2 commits into
masterfrom
devin/1788678964-validate-menu-item-prices
Open

WesternConcrete wants to merge 2 commits into
masterfrom
devin/1788678964-validate-menu-item-prices

Conversation

@WesternConcrete

@WesternConcrete WesternConcrete commented Sep 6, 2026

Copy link
Copy Markdown

Summary

Finding: Unvalidated menu item prices on POST /restaurants allow negative-priced menu items and negative order totals
Repo: COG-GTM/ftgo-monolith

Fix approach: enforce the menu-item invariant in the domain (MenuItem rejects blank id/name and null or non-positive price) so RestaurantService.create fails with IllegalArgumentException → 400 via GlobalExceptionHandler, instead of persisting prices that OrderService.createOrder later copies into OrderLineItem to produce negative order totals (or an NPE on a null price).

// MenuItem
public MenuItem(String id, String name, Money price) {
  // id/name must be non-blank
  requirePositivePrice(id, price);   // price != null && price.isPositive()
  ...
}
public void setPrice(Money price) { requirePositivePrice(id, price); ... }

// Money
public boolean isPositive() { return amount.signum() > 0; }

// RestaurantService.makeRestaurantMenu: reject null/empty menu

MenuItemPriceTest covers positive/negative/zero/null prices, blank id, and setPrice. Verified by compiling Money/MenuItem and running the test with JUnit directly (Maven Central is currently returning 429 to Gradle in this environment, so the full Gradle build could not be run here; CI should confirm).

Link to Devin session: https://app.devin.ai/sessions/1a5f132f26a742e6b54bb5c9749c9bcb
Open in Devin Desktop: https://app.devin.ai/desktop/session/1a5f132f26a742e6b54bb5c9749c9bcb?variant=devin
Requested by: @WesternConcrete


Devin Review

Co-Authored-By: Wes Convery <2wconvery@gmail.com>
@devin-ai-integration

Copy link
Copy Markdown

🤖 Devin AI Engineer

I'll be helping with this pull request! Here's what you should know:

✅ I will automatically:

  • Address comments on this PR. Add '(aside)' to your comment to have me ignore it.
  • Look at CI failures and help fix them

Note: I can only respond to comments from users who have write access to this repository.

⚙️ Control Options:

  • Disable automatic comment, CI, and merge conflict monitoring

devin-ai-integration[bot]

This comment was marked as resolved.

Co-Authored-By: Wes Convery <2wconvery@gmail.com>
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