Skip to content

fix: Harden lambda-durable-saga-python-sam — security & correctness fixes - #3270

Open
awsTest1992 wants to merge 1 commit into
aws-samples:mainfrom
awsTest1992:lambda-durable-saga-python-sam
Open

fix: Harden lambda-durable-saga-python-sam — security & correctness fixes#3270
awsTest1992 wants to merge 1 commit into
aws-samples:mainfrom
awsTest1992:lambda-durable-saga-python-sam

Conversation

@awsTest1992

Copy link
Copy Markdown
Contributor

Summary

Follow-up to #3187. Addresses security and correctness findings from code review of the merged saga pattern.

High severity

  • Add input validation at the trust boundary (_validate_event) — rejects missing fields, non-positive quantities, NaN/negative amounts, and unbounded item lists
  • Replace two non-atomic writes in confirm_order with a single TransactWriteItems to prevent order=CONFIRMED / payment=RESERVED split-brain
  • Add ConditionExpression to compensate_payment so it can only refund a RESERVED payment (not an already-CAPTURED one)

Medium severity

  • Move order_id generation into a @durable_step so it's checkpointed and stable across replays
  • Move failed-order orders_table.put_item into a @durable_step (record_failed_order) — was a raw write in the orchestrator body, re-executed on every replay
  • Track successful_compensations vs failed_compensations separately; publish to SQS DLQ on compensation failure instead of silently returning success

Low severity

  • Replace arn:aws: with arn:${AWS::Partition}: for GovCloud portability
  • Add SagaCompensationDLQ (SQS) for compensation failure alerting, wired via COMPENSATION_DLQ_URL env var
  • Add RetryPresets.none() on reserve_inventory and process_payment steps — deterministic business failures (InsufficientInventoryError, PaymentDeclinedError) were retrying 6× by default, adding ~60s unnecessary delay
  • Move custom exception classes above step definitions (were defined after use)

README

  • Replace hardcoded lambda-durable-saga-* table/function names with ${STACK_NAME}-* variable pattern
  • Add Production considerations section (PITR, CMK, least-privilege IAM, DLQ monitoring)

All fixes verified via live deployment to us-east-1 — happy path and compensation path both tested end-to-end.

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.

3 participants