Suppress "Please specify an on.push hook" warning

[yhrn]

Hi,

I'm looking to using codeql-action for some shared workflows that are intended to be used across our organization. However, we have separate workflows for PRs and pushes to main so in the PR workflow we get the warning Please specify an on.push hook so that Code Scanning can compare pull requests against the state of the base branch. In this case the warning is a false positive since Code Scanning will run on push, only from a different workflow. And since this warning would be seen by devs across our organization that typically shouldn't need to care too much about the contents of the reusable workflow we see it as a significant problem.

What is your take on adding an action input parameter for suppressing this warning?

[adityasharad]

Are you also specifying a category for the analysis from those workflows, with the same category for each workflow? If a category is not specified, the CodeQL Action will generate a category for you, which by default includes the workflow name. This means the alerts from the two separate workflows would be considered distinct and wouldn't be correctly matched against each other.

For this reason, we generally recommend having the same workflow with both pull_request and push triggers, and the warning is intended to prevent such a mismatch. However I appreciate your configuration is slightly more advanced -- could you please share with us a rough outline of your workflows, or the motivation for having separate pull request and push workflows? With that information, we can think more about how to support this use case, either by allowing you to suppress the warning or by some other more intuitive method.

[yhrn]

Yes, I'm setting the category to the same value in both. Would it maybe make sense to suppress the warning if category is set?

The shared workflows here are intended for k8s deployed services. We use Skaffold, and typically Jib, to build images and render manifests.

Both workflows builds an image, runs unit tests and renders manifests. The PR workflow will additionally publish test reports and optionally diff the rendered manifests with the currently deployed ones and display the result. The push (main) workflow will start a deployment flow by pushing the rendered manifests to a "deployment" branch and then start a job that is waiting for a Argo CD to report back the result via a GH check.

It's obviously possible to put all of this in a single workflow but that would end up being a bunch of if-else spagetti. We could also keep the CodeQL stuff in a separate workflow that only does that, but that means there is more boilerplate needed in each repo, which is something we're trying to avoid with the shared workflows.

[yhrn]

@adityasharad Any more thoughts on this?

[adityasharad]

Thanks for explaining your use case, and for your patience while I discussed this with my colleagues. Your solution sounds sensible for that purpose.

A missing push trigger is unfortunately quite a common mistake from regular usage, and it's challenging to know from a single workflow whether the analysis has been configured correctly across multiple workflows, even if categories are in use. So we'd like to be careful about giving all users the option to suppress the warning, until we encounter more use cases similar to your own. Should we see more demand, we'll revisit your (sensible) suggestion of adding a suppression option.

However, I appreciate your desire to avoid the warning confusing the devs using your shared workflows. Here is a workaround that we think will help you avoid the warning, without meaningfully impacting your devs or changing the behaviour of your workflows. Will this work for you?

• In the PR workflow, add a push event trigger:

   on:
     pull_request:
       # same as before
     push:
       branches: [main]

• In the PR workflow, add an if condition on every job so that it doesn't actually run on the push events:

  jobs:
    job1:
      name: ...
      if: github.event_name == 'pull_request'
      # rest as before

• You will now see additional runs of the PR workflow in the Actions runs UI, corresponding to push events to main, but all jobs in those runs will be skipped, and no compute will be used for those jobs. This should not affect the status of any PRs.

[yhrn]

Thanks for the suggested workaround but the problem with it is that it needs to be added in every caller workflow which is something we'd like to avoid. Having this unexpected trigger/if condition everywhere and seeing the skipped jobs would potentially be even more confusing for all devs using the shared workflow than seeing the warning.

I'd kindly ask you to reconsider providing an explicit option to suppress the warning - as opposed to my suggestion about always suppressing it when category is provided this can't really be done by mistake. I would also be willing to create a PR if you were to reconsider.

[sgammon]

Second the suggestion for this; we do have an on.push hook, but the CodeQL workflow can't find it, because it is buried in a reusable workflow (which is a best practice!).

Please, either fix the logic to properly detect that on.push is hooked (for instance, by observing actual signals instead of guessing) or give us a way to suppress the warning.

[aeisenberg]

I moved this issue back to our triage column so we will discuss it again.

A proposal that I have is that we introduce an environment variable (eg- CODEQL_ACTION_SUPPRESS_ON_PUSH_WARNING) that, if set, will suppress the warning. We can also change the warning message so that adding the env var is a recommended way to avoid printing the message, but should only be used if you know what you're doing.

[aeisenberg]

@sgammon, I'd forgotten that we already tried to address this problem in #2274. Can you please make sure you are using action version 3.25.4 or later? If you are still seeing this warning, I will try the idea above.

[sgammon]

@aeisenberg we are pinned to v3, with no hash, so i would guess that we are already on that version or greater:

      - name: "Analysis: CodeQL (Language: ${{ inputs.language }})"
        uses: github/codeql-action/analyze@v3
        with:
          category: "/language:${{inputs.language}}"

This is confirmed by logs:

...
CODEQL_ACTION_VERSION: 3.25.14
...
/opt/hostedtoolcache/CodeQL/2.18.0/x64/codeql/codeql version --format=json

So, I guess, 3.25.14 for the action, and 2.18.0 for CodeQL itself. We are still seeing the warning, yeah.

[aeisenberg]

That is interesting. Does your workflow have a workflow_call trigger? And does it have a pull_request trigger?

[sgammon]

@aeisenberg It has a workflow_call trigger only, because we always handle pull_request and push hooks in an "outer" entrypoint job, and then dispatch actual jobs with reusable workflows.

[aeisenberg]

Do you have an example of this happening in a public repo? I can't find any.

I was just poking around in some of the public repositories that you seem to be a contributor to. Many of them use the pattern you are describing and none of them are emitting this warning as far as I can tell.

For example, this workflow: https://github.com/elide-dev/uuid/blob/main/.github/workflows/codeql.ci.yml has had a recent success here: https://github.com/elide-dev/uuid/actions/runs/10286210862 and the warning is not emitted.

[sgammon]

@aeisenberg I'm really not sure either, because you're right, I would expect the warning from that run. The repos where we are still seeing the warning are all private, and we rolled back CodeQL for now anyway. I will try to find a workflow run and isolate the conditions that produce the warning, or verify that we were on the wrong version.

[aeisenberg]

Please keep me posted. Thanks for checking up on this.

[aeisenberg]

I'll close this one out since I haven't heard back from you. Feel free to comment if you see this again.