fix(idempotency): pass by value on idem key to guard inadvertent mutations

[ojongerius]

This protects against functions that inadvertently change the incoming event data

Description of changes:

Switch to using a deep copy of data. Without this, we've seen two documents written for one function invocation that (inadvertently) mutated the event data:

• An initial document with a hashed idempotency key based on the original payload that will be stuck in INPROGRESS and has no return data
• A second document with a hashed idempotency key based on the mutated payload with status COMPLETED and the correct return value

Happy to add a test if there is an appetite for this change.

Checklist

• Meet tenets criteria
• Update tests
• Update docs
• PR title follows conventional commit semantics

[boring-cyborg]

Thanks a lot for your first contribution! Please check out our contributing guidelines and don't hesitate to ask whatever you need.

[ojongerius]

FWIW some code to reproduce, the assertion that only one document is written fails without my patch.

Results:

'Items': [{'id': 'test-func.handler#8339d7030a6b244b42e6bb381d2c947c',
                'expiration': Decimal('1649130578'), 'status': 'INPROGRESS'},
                {'id': 'test-func.handler#5cd779b2a9dc323203f55b7ef9a6053b',
                 'data': 'null',
                 'expiration': Decimal('1649130578'),
                 'status': 'COMPLETED'}]

Test code:

def test_event_change_changes_key(dynamodb, aws_credentials):
    table = dynamodb.create_table(
        TableName='idempotency',
        KeySchema=[{"AttributeName": "id", "KeyType": "HASH"}],
        AttributeDefinitions=[{"AttributeName": "id", "AttributeType": "S"}],
        BillingMode="PAY_PER_REQUEST",
    )
    table.wait_until_exists()
    assert table.scan()['Count'] == 0

    event = {"detail": {"items": ["item_one", "item_two"]}}

    @idempotent(persistence_store=DynamoDBPersistenceLayer(table_name="idempotency"))
    def handler(event, context):
        items = event.get("detail").get("items")
        items.pop() # 👈 💣

    handler(event, {})

    scan_results = table.scan()
    print(f"ddb items:{scan_results}")
    assert scan_results['Count'] == 1

[heitorlessa]

Hi @ojongerius! Could you please create a bug report and add a test for this?

I can see customers inadvertently missing the copy by reference nature leading to this issue - deep copy is the right call.

Once we have a bug report and a quick test, we should be able to merge it quickly.

Thank you 🙏

[codecov-commenter]

Codecov Report

Merging #1090 (2e9797a) into develop (f50ec86) will increase coverage by 0.00%.
The diff coverage is 100.00%.

@@           Coverage Diff            @@
##           develop    #1090   +/-   ##
========================================
  Coverage    99.96%   99.96%           
========================================
  Files          119      119           
  Lines         5377     5378    +1     
  Branches       613      613           
========================================
+ Hits          5375     5376    +1     
  Partials         2        2           

Impacted Files	Coverage Δ
...ws_lambda_powertools/utilities/idempotency/base.py	100.00% <100.00%> (ø)

---

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update f50ec86...2e9797a. Read the comment docs.

[ojongerius]

Cheers @heitorlessa! I've added a test and created #1093

[heitorlessa]

there's an issue with the test setup as hashed_idempotency_key is the same in both instances; meaning the test passes with or without the fix.

I'll push a fix today. We'll need to improve the build out of stub expected params (eventually moving to DynamoDB Local to reduce boilerplate for functional tests)

[heitorlessa]

@ojongerius tests fixed - could you please have one last look before we merge?

Thanks

[ojongerius]

Thanks for that 🙏

Had one last look and verified this in my own environment 👍

[boring-cyborg]

Awesome work, congrats on your first merged pull request and thank you for helping improve everyone's experience!