-
Notifications
You must be signed in to change notification settings - Fork 1.6k
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
Java: Threat model implementation with priorities. #14582
Conversation
This looks really nice @dbartol! |
Co-authored-by: Michael Nebel <[email protected]>
Co-authored-by: Michael Nebel <[email protected]>
Co-authored-by: Michael Nebel <[email protected]>
Oh, cool, I was going to ask if we had a way for QL tests to specify data extensions, but I guess the answer is "yes". |
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.
Really good! 👍
Lets also
(1) Get a review from someone on the java team.
(2) Run DCA (I will trigger an execution).
Some minor comments, otherwise LGTM! |
Co-authored-by: Anders Schack-Mulligen <[email protected]>
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.
LGTM 👍
Will re-run DCA.
DCA also looks good @dbartol : This PR can be merged now. |
This is an alternate implementation of threat models that depends on a new CLI option (
--threat-model
) that generates a temporary extension pack to configure the supported threat models. Compare to #14548, which does not require a new CLI option.This implementation has a few advantages over the other:
local
sources exceptenvironment
).all
threat model works as originally intended.The downside is that we have to add a new configuration setting to half a dozen places.
Implementation details
codeql database analyze
will accept a new--threat-model
option that can be specified multiple times. Each instance of the option accepts a string argument with the name of a threat model, optionally preceded by!
. Without a!
, the option enables the specified threat model, or, if the specified model is a group, all of its descendants. With a!
, the option disables the specified threat model or its descendants. The options are processed in order.Before running the analysis, the CLI will generate a temporary extension pack that extends the
threatModelConfiguration
predicate with one row for each instance of the--threat-model
option. The row will contain the following columns:kind
- The name of the threat model, without any!
prefix.enabled
- Aboolean
value set tofalse
if the argument had a!
prefix, andtrue
, otherwise.priority
- Anint
value specifying the order in which the option was processed. The first instance gets priority0
, the next gets priority1
, and so on.At evaluation time, the
codeql/threat-models
library processes thethreatModelConfiguration
table to determine which threat models are actually enabled.