Skip to content

Add way to customize environmentCheck Actions QL#22173

Draft
knewbury01 wants to merge 3 commits into
github:mainfrom
knewbury01:knewbury01/customize-actions-sanitizers
Draft

Add way to customize environmentCheck Actions QL#22173
knewbury01 wants to merge 3 commits into
github:mainfrom
knewbury01:knewbury01/customize-actions-sanitizers

Conversation

@knewbury01

Copy link
Copy Markdown
Contributor

Add implementation for technique to customize EnvironmentCheck in actions queries

Currently 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.qll usage for a similar intended edit location is alright to do (probably the former would be more ideal).

@github-actions github-actions Bot added the Actions Analysis of GitHub Actions label Jul 10, 2026

@michaelnebel michaelnebel left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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.

EnvironmentCheck() {
exists(string selected |
selectDeploymentEnvironmentDataModel(selected) and
if selected = "EnvironmentCheckMaD"

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thank you for looking into this!
I am not as such familiar with the actions QL implementation, so bear with me 😄

As far as I understand the idea of the pull request is to make it configurable which Environment instances that are added as ControlChecks. The PR implements support for this in two ways

  • Either by supplying environment names as tuples in enabledDeploymentEnvironmentDataModel OR
  • By extending the abstract class CustomEnvEnable.
    On top of that there is an extensible predicate selectDeploymentEnvironmentDataModel used to decide, which of the above strategies to use (if any).

A couple of questions:

  • Would it suffice to only have the MaD variant? The Yaml files are loaded dynamically and only supporting a single way of doing it will make it easier to maintain.
  • What is the reason for also having the abstract class way of doing it?

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

the idea was in case there are more complicated cases for the scenarios where environment checks are only relevant in certain scenarios
but those cases are abstract currently so for simplicity in the modelling I will go with just MaD for now. we can keep in mind if a concrete case arises that custom QL is at least possible

thanks!

Comment thread actions/ql/lib/ext/config/customize_checks.yml Outdated
Comment thread actions/ql/lib/ext/config/deployment_environment.yml Outdated
Comment thread actions/ql/lib/actions.qll Outdated
@knewbury01

Copy link
Copy Markdown
Contributor Author

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

but I am having a bit of trouble envisioning how to test this other than add a new test that just checks the enabled control checks... I dont see such a lib test currently. the reason it is a bit tough is bc I think we want the default behaviour to still be "add all envs as sanitizers" and the specific behaviour to be "enable just specific ones" which means all the security queries would still have the same result regardless of whether the mechanism is active or not... toggling it on and off wont affect the test result and therefore in my mind its a bit of a bad unit test ideology

any better ideas very welcome!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Actions Analysis of GitHub Actions

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants