Summary

An unguarded double submit into the asset save API endpoint caused the active logo to be marked for deletion upon the second submit.

Root Cause(s)

When a new logo is saved via the API the previously saved logo is marked for deletion. The API did not gate the previous logo from being marked as deleted if it was the same as the incoming logo. The API ran into these cases where the previous logo was the exact same as the new logo in cases of a double submit. The first submission marks the logo as the desired logo, and the second submission marks the logo for deletion. The most expected form of a double submit is a user hitting the `save` button at least two times before it is grayed out.

Impact and Analysis

The impact of this bug was that over a period of time the majority of logos except for 2 users were eventually deleted.

The double submit results in a unique call scenario to the API endpoint in that it will send the `asset_path` arg for both requests. This is in contrast to the standard save flow where the `asset_path` arg is not sent to the API endpoint if it remains unchanged. Another minor detail in the mix was that the logo uploader would always generate a unique asset path for each uploaded asset, this further biased the endpoint's design and put a blindspot on handling duplicate `asset_path` args. Due to the same `asset_path` being sent multiple times, and the endpoint not checking that the incoming asset_path was not the same as the existing asset_path, the second submit would mark the active logo for archival.

Furthermore, the assets bucket that was used to host the uploaded logos did not keep backups of the logos once they were archived, making recovery of the deleted logos impossible once we noticed and patched up the issue.

Lessons Learned

• Where there is data deletion, keep backups. This reigns even more true in the cases where automated systems are applying rule-sets for deletion as is the case here. Variable volume of usage means that we may not see the side-effects of bugs until much later down the road.
• Where possible build endpoints to be flow/client agnostic, and to always check it's own assumptions. There was undue influence on the endpoint's design from other parts of the flow as well as the client. These things amalgamated into the endpoint having an expectancy for the incoming `asset_path` to be unique each time.

Action Items

• [COMPLETED] Update bucket policy to version stored items, and to permanently delete after a prolonged period.