-
Notifications
You must be signed in to change notification settings - Fork 15
Prefer Atomic Test UUID over calculated hash for Ability ID #45
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: master
Are you sure you want to change the base?
Conversation
|
We will need to marinate on this change. Not against it logically, but not sure of the fallout from the breaking change. |
|
I thought about that for a minute yesterday evening and came up with an idea that would on the one hand not be breaking but on the other hand require refactoring in the main Caldera repository. Roughly, the idea is as follows:
Maybe that opens another way forward with this MR. |
|
Here's an idea that wouldn't require refactoring:
Thoughts? |
|
Alright I have an alternative idea based on @leba-atr 's suggestion that will require some adjustments to core code and ability format, but will actually provide users with more flexibility down the line Proposal: add an For the atomic plugin in particular, the plugin service would grab the atomic test ID and use that as the primary ability ID, and use the hash as an alternate ID. This way, we don't break any user's custom adversary profiles that rely on the hash-based IDs, and users can use the more stable atomic test ID going forward or switch at their own pace Rather than looking up the primary ID first, CALDERA would simply look up whatever ID is referenced in the adversary profile. When abilities get imported, we'll have all of the associated IDs point to the same underlying ability object, but we'll still maintain the notion of a primary ID since that ID will be used for the yaml file name |
|
Personally, I'm in favor of the second approach. And especially so for the reason that it opens up the possibility to use custom identifiers in addition to the ones used by Caldera internally. Also, this allows to postpone the breaking change for users until the next major release where the deprecated approach can literally just be deleted from the code without the need to re-write lots of functionality. |
|
As much as I'd like to introduce a secondary ID(s) field, doing so would require quite significant updates to the core code in order to accommodate looking up and referencing abilities (and potentially other objects) with more than 1 ID. Similar amounts of effort would also be required to store and manage both atomic ability IDs as "official" ability object fields. However, I do have a middle approach in the works that looks like the following:
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Pull Request Overview
This PR changes how ability IDs are generated for Atomic tests by preferring the auto-generated UUID over the calculated MD5 hash. This addresses issues where hash-based IDs break references when test definitions are updated.
- Replaces MD5 hash-based ability ID generation with UUID-based approach for stability
- Maintains backward compatibility by falling back to hash when UUID is unavailable
- Introduces breaking changes for existing ability references
Tip: Customize your code reviews with copilot-instructions.md. Create the file or learn how to get started.
| Return True if an ability was saved. | ||
| """ | ||
| ability_id = hashlib.md5(json.dumps(test).encode()).hexdigest() | ||
| ability_id = test.get('auto_generated_guid') or hashlib.md5(json.dumps(test).encode()).hexdigest() |
Copilot
AI
Oct 6, 2025
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
[nitpick] The fallback to MD5 hash maintains the old behavior but could lead to inconsistent ID types. Consider documenting the expected format of 'auto_generated_guid' and whether it should be validated before use.
|
Can you address the above and resubmit for Review |
Description
Currently, the plugin calculates a md5 hash over the JSON serialized test definition for a given atomic test and then uses the hash value as the id for the plugin. While this is fine for static datasets, Atomic Tests every now and then receive updates. These updates cause the md5 hash to change an any reference in Caldera to break (e.g. abilities referenced from the stockpile plugin, check the debug output of Caldera for current examples).
This PR introduces a breaking change in the way how this plugin generates ability ids. Instead of calculating a hash over the test data, this plugin now prefers to use the auto-generated unique UUID that each test is assigned. This also affects how one references Atomic abilities in Plugins like stockpile but also custom in Adversaries created manually via API calls.
Context: when creating adversaries via manual API calls, one cannot just use the Atomic test uuid to link Abilities with a given Adversary but instead the custom hash value must be retrieved from the API beforehand. This makes it more tedious than I expected to reference Atomic tests in custom Adversaries.
Type of change
How Has This Been Tested?
Checklist: