Add threat-models as a property to config file and inputs - #1653
aeisenberg wants to merge 1 commit into
Conversation
Sorry, something went wrong.
There's a lot of changes here, but it's pretty formulaic. It follows the approach used by the `queries` input and config property. `threat-models` can appear as an input or in the config file. If it appears in the input, then we need to either merge it with the threat-models in the config (if prefixed with `+`) or overwrite it. There's no danger if someone uses `threat-models` with an older CLI since the CLI can handle configs with extra properties.
There was a problem hiding this comment.
Another approach here would be to merge the work on allowing users to pass config properties via a config input (#1590), then letting users pass threat models via something like:
uses: github/codeql-action/init@v2
with:
config: |
threat-models: <threat models>An advantage of this is consistency — we have a single way to pass in threat models. Another is less special-purpose code — it would be nice to be able to handle inputs like these in the CLI without having to add code to handle them in the Action too. A disadvantage is that users couldn't use a config file but then also override or add some threat models. I'm not sure how important that is.
Sorry, something went wrong.
|
Thanks good point. It keeps the code significantly simpler. My only concern about this is the inconsistency. We allow users to specify packs and queries via inputs. Would we not want to do the same with threat models? Perhaps the best approach would be to explicitly deprecate the packs and queries inputs (and any others that are duplicated in the config file). |
Sorry, something went wrong.
|
Short term, I will not be pursuing this since there is a way to use threat models with no changes to the action. If we decide later that we want to add a specific input for threat models, then I can start working on this again. |
Sorry, something went wrong.
There's a lot of changes here, but it's pretty formulaic. It follows the approach used by the queries input and config property. threat-models can appear as an input or in the config file. If it appears in the input, then we need to either merge it with the threat-models in the config (if prefixed with +) or overwrite it.
There's no danger if someone uses threat-models with an older CLI since the CLI can handle configs with extra properties.
Merge / deployment checklist