Add way to customize environmentCheck Actions QL#22173
Conversation
michaelnebel
left a comment
michaelnebel
left a comment
There was a problem hiding this comment.
Thank you very much for doing this! It is highly appreciated!
Would it be possible to add some tests as well?
Note, that for testing the Yaml alternative it is possible to add an .ext.yml file next to the .ql test file - it just needs to have the same base name as the test file. That is, if there is a test named test.ql then adding test.ext.yml makes it possible to add tuples to the extensible predicates that only applies for test.ql.
@michaelnebel thank you so much for the review! addressed most, except the testing request, which I agree with
I have added a library test for this. I think that makes the most sense, thanks for letting me know about how to enable MaD for just one test! |
There was a problem hiding this comment.
Pull request overview
Adds model-driven customization for which GitHub Actions deployment environments qualify as control checks.
Changes:
- Defines a deployment-environment data extension.
- Filters
EnvironmentCheckusing supplied environment names. - Adds coverage for listed and unlisted environments.
Show a summary per file
| File | Description |
|---|---|
controlchecktest.ql |
Queries control checks. |
controlchecktest.ext.yml |
Supplies a test environment model. |
controlchecktest.expected |
Records the expected selected environment. |
controlcheck.yml |
Provides listed and unlisted environments. |
deployment_environment.yml |
Adds empty default extension data. |
ControlChecks.qll |
Applies environment filtering. |
ConfigExtensions.qll |
Declares the extensible predicate. |
Config.qll |
Exposes the configuration predicate. |
Review details
Comments suppressed due to low confidence (1)
actions/ql/lib/ext/config/deployment_environment.yml:5
- The customization selector described by the PR is missing. With only this allowlist predicate, “customization disabled” is indistinguishable from an enabled customization that supplies zero environments; the latter therefore falls back to treating every referenced environment as a control check. Add a separate mode/technique predicate and gate allowlist filtering on it so an explicitly empty allowlist can disable all environment checks.
- addsTo:
pack: codeql/actions-all
extensible: enabledDeploymentEnvironmentDataModel
data: []
- Files reviewed: 8/8 changed files
- Comments generated: 3
- Review effort level: Medium
Add implementation for technique to customize
EnvironmentCheckin actions queriesCurrently there is no way to customize Actions query "sanitizers" ie
ControlChecks. This PR adds a sample mechanism that uses 1) a specifier in MaD that designates which technique to use (the default is no customization) and 2) a mechanism that fills the definition of EnvironmentCheck with either i) a QL custom definition or ii) a MaD custom definition.The purpose of this is to allow for checks to be selectively turned off or narrowed based on real info related to deployment environments in repos (this would need to be externally supplied) .
This PR is similar to and borrows ideas from this PR.
The intention of this work would be to potentially apply this style to any check type or other sanitizer types in other languages, if the mechanisms used here are found to be suitable.
One important consideration of this work is that for Actions there is currently no Customizations.qll file. Either this PR should also add one , or it should be carefully considered if the current
actions.qllusage for a similar intended edit location is alright to do (probably the former would be more ideal).