Skip to content

Conversation

@mrkickling
Copy link
Contributor

@mrkickling mrkickling commented Jan 24, 2025

  • Adapt the current (old) mal simulator to the new mal-toolbox 0.2.0 with changes to the language graph.
  • Convert all test models to new model format.
  • Make sure tests pass

The intention is to merge this to main and create a release before we are done with #87, just to support maltoolbox 0.2.0 as soon as possible in the simulator.

@mrkickling
Copy link
Contributor Author

The test that failed is because the order of choices for the BFS attacker are not the same anymore. Why this happens is worth looking into, but probably it is just because the observations are built up differently. I will compare two attack graphs (one from mal-toolbox 0.1.0 and one from 0.2.0) and see if that gives any clues.

If nothing seems fishy I will just rewrite the test to match the new order.

@mrkickling
Copy link
Contributor Author

mrkickling commented Jan 24, 2025

The main thing I noticed is that the IDs are not the same in the new 0.2.0 attack graph compared to the old 0.1.0 attack graph (created from the same model). Does this seem normal @andrewbwm ? Other than that the two attack graphs seem identical (I have not checked thoroughly) so I will just rewrite the test on monday and then put this branch in review.

@andrewbwm
Copy link
Contributor

The main thing I noticed is that the IDs are not the same in the new 0.2.0 attack graph compared to the old 0.1.0 attack graph (created from the same model). Does this seem normal @andrewbwm ? Other than that the two attack graphs seem identical (I have not checked thoroughly) so I will just rewrite the test on monday and then put this test in review.

That is normal because attack steps are probably created in a slightly different order now, but it shouldn't really matter. Also, you don't need to adjust their ids between versions, there is no expectation about the ordering of ids. They just need to be unique.

It's also a good idea if we have the tests not really depend too much on ids, but rather on names. Not sure which test this is but we can have a look at it if it isn't obvious how to rewrite it to use names.

@mrkickling
Copy link
Contributor Author

The main thing I noticed is that the IDs are not the same in the new 0.2.0 attack graph compared to the old 0.1.0 attack graph (created from the same model). Does this seem normal @andrewbwm ? Other than that the two attack graphs seem identical (I have not checked thoroughly) so I will just rewrite the test on monday and then put this test in review.

That is normal because attack steps are probably created in a slightly different order now, but it shouldn't really matter. Also, you don't need to adjust their ids between versions, there is no expectation about the ordering of ids. They just need to be unique.

It's also a good idea if we have the tests not really depend too much on ids, but rather on names. Not sure which test this is but we can have a look at it if it isn't obvious how to rewrite it to use names.

Good, good!
I will use the name instead, and it is quite obvious how to convert it.

@mrkickling mrkickling marked this pull request as ready for review January 27, 2025 09:53
@mrkickling mrkickling requested a review from andrewbwm January 27, 2025 09:53
@mrkickling mrkickling closed this Jan 27, 2025
@mrkickling mrkickling deleted the adapt-to-maltoolbox-0.2.0 branch February 14, 2025 12:50
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