Skip to content

Fix unsafe sale item deletion and amount mutation#61

Draft
cursor[bot] wants to merge 1 commit into
mainfrom
cursor/critical-bug-investigation-0d6b
Draft

Fix unsafe sale item deletion and amount mutation#61
cursor[bot] wants to merge 1 commit into
mainfrom
cursor/critical-bug-investigation-0d6b

Conversation

@cursor

@cursor cursor Bot commented Jun 13, 2026

Copy link
Copy Markdown

Bug and impact

  • Sale item deletion accepted GET requests and restored inventory outside a transaction. A logged-in user could be induced to visit the delete URL, or duplicate/concurrent requests could restore stock and write inventory transactions for the same sale item, inflating inventory and corrupting stock history.
  • The sale detail page recalculated and wrote sale amounts during a GET request. Viewing a historical completed sale whose header totals differed from line items could silently rewrite total_amount/final_amount without updating balance payment records or member statistics.
  • SaleItem.save() ignored inventory update failures after saving the line item and sale total, allowing scripts/tests/model-level callers to record a sale item without stock being deducted.

Root cause

  • sale_delete_item was a write action exposed as a link/GET and did not lock the sale item or inventory row.
  • sale_detail contained self-healing raw SQL writes in a read-only view.
  • The model-level sale item save path did not check the (success, ..., error) result from update_inventory().

Fix

  • Require POST for sale item deletion, render a CSRF-protected form, and perform sale/item/inventory updates inside a single locked transaction.
  • Remove the sale detail GET auto-mutation logic.
  • Make SaleItem.save() atomic and raise on inventory update failure so the line item and sale total roll back.
  • Added regression tests for POST-only deletion, non-mutating sale detail, and model-level rollback on inventory failure.

Validation

  • python3 manage.py test inventory.tests.test_sale_status inventory.tests.test_sales_balance_payment inventory.tests.test_services.MemberServiceTest
  • python3 manage.py test inventory.tests.test_models

Both passed; Django reported only the pre-existing staticfiles.W004 warning for missing /workspace/static.

Open in Web View Automation 

Co-authored-by: Xianist Lab <zhtyyx@users.noreply.github.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