Actions: ActorIfCheck should not protect events that do not populate the checked field

A condition like 'github.event.pull_request.user.login != ...' on a
workflow triggered by issues events is always true since
github.event.pull_request is not populated for issues events, but
ActorIfCheck still treated it as a protective check, suppressing
alerts such as actions/code-injection/critical.

Override protectsCategoryAndEvent in ActorIfCheck so that checks on
event payload fields only protect events whose payload contains the
corresponding context, using contextTriggerDataModel as the mapping.

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Copilot-Session: 4084febb-f9c7-44df-baf3-c8be8e9932a7

Author: computersarebad

actions/ql/lib/change-notes/2026-08-17-actor-if-check-event-validity.md

@@ -0,0 +1,4 @@
+---
+category: minorAnalysis
+---
+* Altered the logic of `ActorIfCheck` so that checks on actor fields read from the event payload (e.g. `github.event.pull_request.user.login`) only count as protection for events whose payload actually populates that field. Previously, a condition such as `github.event.pull_request.user.login != 'name'` on a workflow triggered by `issues` events was treated as a protective check even though `github.event.pull_request` is not populated for `issues` events, which makes the condition vacuous. This change will result in more results being found by the queries that rely on control checks, such as `actions/code-injection/critical`.

actions/ql/lib/codeql/actions/security/ControlChecks.qll

@@ -312,17 +312,28 @@ class LabelIfCheck extends LabelCheck instanceof If {
   }
 }
 
+/**
+ * Gets a regular expression matching a condition on an actor field that is
+ * only populated for events whose payload contains the `context_prefix` context.
+ */
+private string eventPayloadActorFieldRegex(string context_prefix) {
+  context_prefix = "github.event.pull_request" and
+  result = "\\bgithub\\.event\\.pull_request\\.user\\.login\\b"
+  or
+  context_prefix = "github.event.head_commit" and
+  result = "\\bgithub\\.event\\.head_commit\\.author\\.name\\b"
+  or
+  context_prefix = "github.event.commits" and
+  result = "\\bgithub\\.event\\.commits.*\\.author\\.name\\b"
+}
+
 class ActorIfCheck extends ActorCheck instanceof If {
   ActorIfCheck() {
     // eg: github.event.pull_request.user.login == 'admin'
     exists(
       normalizeExpr(this.getCondition())
-          .regexpFind([
-              "\\bgithub\\.event\\.pull_request\\.user\\.login\\b",
-              "\\bgithub\\.event\\.head_commit\\.author\\.name\\b",
-              "\\bgithub\\.event\\.commits.*\\.author\\.name\\b",
-              "\\bgithub\\.event\\.sender\\.login\\b"
-            ], _, _)
+          .regexpFind([eventPayloadActorFieldRegex(_), "\\bgithub\\.event\\.sender\\.login\\b"], _,
+            _)
     )
     or
     // eg: github.actor == 'admin'
@@ -333,6 +344,37 @@ class ActorIfCheck extends ActorCheck instanceof If {
     ) and
     not normalizeExpr(this.getCondition()).matches("%[bot]%")
   }
+
+  override predicate protectsCategoryAndEvent(string category, string event) {
+    ActorCheck.super.protectsCategoryAndEvent(category, event) and
+    (
+      // `github.actor`, `github.triggering_actor` and `github.event.sender.login`
+      // are populated for every event
+      exists(
+        normalizeExpr(this.(If).getCondition())
+            .regexpFind("\\bgithub\\.event\\.sender\\.login\\b", _, _)
+      )
+      or
+      exists(
+        normalizeExpr(this.(If).getCondition())
+            .regexpFind(["\\bgithub\\.actor\\b", "\\bgithub\\.triggering_actor\\b",], _, _)
+      ) and
+      not normalizeExpr(this.(If).getCondition()).matches("%[bot]%")
+      or
+      // actor fields read from the event payload are only populated for events
+      // whose payload contains the corresponding context. eg: a check on
+      // `github.event.pull_request.user.login` cannot restrict the actor of an
+      // `issues` event since `github.event.pull_request` is not populated there,
+      // which makes the condition vacuous
+      exists(string context_prefix |
+        contextTriggerDataModel(event, context_prefix) and
+        exists(
+          normalizeExpr(this.(If).getCondition())
+              .regexpFind(eventPayloadActorFieldRegex(context_prefix), _, _)
+        )
+      )
+    )
+  }
 }
 
 class PullRequestTargetRepositoryIfCheck extends RepositoryCheck instanceof If {

actions/ql/test/query-tests/Security/CWE-094/.github/workflows/actor_check_valid_event.yml

@@ -0,0 +1,12 @@
+on:
+  pull_request_target:
+    types: [opened]
+
+jobs:
+  # The `if:` condition checks an actor field that is populated for
+  # `pull_request_target` events, so the injectable step is protected.
+  valid-actor-check:
+    runs-on: ubuntu-latest
+    if: github.event.pull_request.user.login == 'trusted-user'
+    steps:
+      - run: echo '${{ github.event.pull_request.title }}'

actions/ql/test/query-tests/Security/CWE-094/.github/workflows/actor_check_wrong_event.yml

@@ -0,0 +1,20 @@
+on:
+  issues:
+    types: [opened]
+
+jobs:
+  # The `if:` condition compares an actor field that is never populated for
+  # `issues` events, so it is always true and does not protect the injectable step.
+  vacuous-actor-check:
+    runs-on: ubuntu-latest
+    if: github.event.pull_request.user.login != 'some-bot[bot]'
+    steps:
+      - run: echo '${{ github.event.issue.title }}'
+
+  # The `if:` condition checks an actor context that is populated for every
+  # event, so the injectable step is protected.
+  valid-actor-check:
+    runs-on: ubuntu-latest
+    if: github.actor == 'trusted-user'
+    steps:
+      - run: echo '${{ github.event.issue.title }}'

actions/ql/test/query-tests/Security/CWE-094/CodeInjectionCritical.expected

@@ -270,6 +270,9 @@ nodes
 | .github/actions/external/ultralytics/actions/action.yaml:66:3:66:6 | input body | semmle.label | input body |
 | .github/actions/external/ultralytics/actions/action.yaml:96:16:96:33 | inputs.body | semmle.label | inputs.body |
 | .github/actions/external/ultralytics/actions/action.yaml:223:25:223:60 | github.head_ref \|\| github.ref | semmle.label | github.head_ref \|\| github.ref |
+| .github/workflows/actor_check_valid_event.yml:12:21:12:58 | github.event.pull_request.title | semmle.label | github.event.pull_request.title |
+| .github/workflows/actor_check_wrong_event.yml:12:21:12:51 | github.event.issue.title | semmle.label | github.event.issue.title |
+| .github/workflows/actor_check_wrong_event.yml:20:21:20:51 | github.event.issue.title | semmle.label | github.event.issue.title |
 | .github/workflows/argus_case_study.yml:15:9:24:6 | Uses Step: remove_quotations [replaced] | semmle.label | Uses Step: remove_quotations [replaced] |
 | .github/workflows/argus_case_study.yml:17:25:17:53 | github.event.issue.title | semmle.label | github.event.issue.title |
 | .github/workflows/argus_case_study.yml:22:20:22:39 | env.ISSUE_TITLE | semmle.label | env.ISSUE_TITLE |
@@ -710,6 +713,7 @@ subpaths
 | .github/actions/external/TestOrg/TestRepo/.github/actions/clone-repo/action.yaml:22:19:22:37 | inputs.title | .github/workflows/composite-action-caller-4.yml:14:19:14:56 | github.event.pull_request.title | .github/actions/external/TestOrg/TestRepo/.github/actions/clone-repo/action.yaml:22:19:22:37 | inputs.title | Potential code injection in $@, which may be controlled by an external user ($@). | .github/actions/external/TestOrg/TestRepo/.github/actions/clone-repo/action.yaml:22:19:22:37 | inputs.title | ${{ inputs.title }} | .github/workflows/composite-action-caller-4.yml:4:3:4:21 | pull_request_target | pull_request_target |
 | .github/actions/external/ultralytics/actions/action.yaml:96:16:96:33 | inputs.body | .github/workflows/test29.yml:35:18:35:54 | github.event.pull_request.body | .github/actions/external/ultralytics/actions/action.yaml:96:16:96:33 | inputs.body | Potential code injection in $@, which may be controlled by an external user ($@). | .github/actions/external/ultralytics/actions/action.yaml:96:16:96:33 | inputs.body | ${{ inputs.body }} | .github/workflows/test29.yml:12:3:12:21 | pull_request_target | pull_request_target |
 | .github/actions/external/ultralytics/actions/action.yaml:223:25:223:60 | github.head_ref \|\| github.ref | .github/actions/external/ultralytics/actions/action.yaml:223:25:223:60 | github.head_ref \|\| github.ref | .github/actions/external/ultralytics/actions/action.yaml:223:25:223:60 | github.head_ref \|\| github.ref | Potential code injection in $@, which may be controlled by an external user ($@). | .github/actions/external/ultralytics/actions/action.yaml:223:25:223:60 | github.head_ref \|\| github.ref | ${{ github.head_ref \|\| github.ref }} | .github/workflows/test29.yml:12:3:12:21 | pull_request_target | pull_request_target |
+| .github/workflows/actor_check_wrong_event.yml:12:21:12:51 | github.event.issue.title | .github/workflows/actor_check_wrong_event.yml:12:21:12:51 | github.event.issue.title | .github/workflows/actor_check_wrong_event.yml:12:21:12:51 | github.event.issue.title | Potential code injection in $@, which may be controlled by an external user ($@). | .github/workflows/actor_check_wrong_event.yml:12:21:12:51 | github.event.issue.title | ${{ github.event.issue.title }} | .github/workflows/actor_check_wrong_event.yml:2:3:2:8 | issues | issues |
 | .github/workflows/argus_case_study.yml:27:33:27:77 | steps.remove_quotations.outputs.replaced | .github/workflows/argus_case_study.yml:17:25:17:53 | github.event.issue.title | .github/workflows/argus_case_study.yml:27:33:27:77 | steps.remove_quotations.outputs.replaced | Potential code injection in $@, which may be controlled by an external user ($@). | .github/workflows/argus_case_study.yml:27:33:27:77 | steps.remove_quotations.outputs.replaced | ${{steps.remove_quotations.outputs.replaced}} | .github/workflows/argus_case_study.yml:4:3:4:8 | issues | issues |
 | .github/workflows/artifactpoisoning1.yml:27:67:27:92 | steps.pr.outputs.id | .github/workflows/artifactpoisoning1.yml:14:9:20:6 | Uses Step | .github/workflows/artifactpoisoning1.yml:27:67:27:92 | steps.pr.outputs.id | Potential code injection in $@, which may be controlled by an external user ($@). | .github/workflows/artifactpoisoning1.yml:27:67:27:92 | steps.pr.outputs.id | ${{ steps.pr.outputs.id }} | .github/workflows/artifactpoisoning1.yml:4:3:4:14 | workflow_run | workflow_run |
 | .github/workflows/artifactpoisoning2.yml:22:17:22:42 | steps.pr.outputs.id | .github/workflows/artifactpoisoning2.yml:13:9:19:6 | Uses Step: pr | .github/workflows/artifactpoisoning2.yml:22:17:22:42 | steps.pr.outputs.id | Potential code injection in $@, which may be controlled by an external user ($@). | .github/workflows/artifactpoisoning2.yml:22:17:22:42 | steps.pr.outputs.id | ${{ steps.pr.outputs.id }} | .github/workflows/artifactpoisoning2.yml:4:3:4:14 | workflow_run | workflow_run |

actions/ql/test/query-tests/Security/CWE-094/CodeInjectionMedium.expected

@@ -270,6 +270,9 @@ nodes
 | .github/actions/external/ultralytics/actions/action.yaml:66:3:66:6 | input body | semmle.label | input body |
 | .github/actions/external/ultralytics/actions/action.yaml:96:16:96:33 | inputs.body | semmle.label | inputs.body |
 | .github/actions/external/ultralytics/actions/action.yaml:223:25:223:60 | github.head_ref \|\| github.ref | semmle.label | github.head_ref \|\| github.ref |
+| .github/workflows/actor_check_valid_event.yml:12:21:12:58 | github.event.pull_request.title | semmle.label | github.event.pull_request.title |
+| .github/workflows/actor_check_wrong_event.yml:12:21:12:51 | github.event.issue.title | semmle.label | github.event.issue.title |
+| .github/workflows/actor_check_wrong_event.yml:20:21:20:51 | github.event.issue.title | semmle.label | github.event.issue.title |
 | .github/workflows/argus_case_study.yml:15:9:24:6 | Uses Step: remove_quotations [replaced] | semmle.label | Uses Step: remove_quotations [replaced] |
 | .github/workflows/argus_case_study.yml:17:25:17:53 | github.event.issue.title | semmle.label | github.event.issue.title |
 | .github/workflows/argus_case_study.yml:22:20:22:39 | env.ISSUE_TITLE | semmle.label | env.ISSUE_TITLE |
@@ -709,6 +712,8 @@ subpaths
 | .github/actions/action7/action.yml:214:41:214:69 | inputs.github_username | .github/actions/action7/action.yml:214:41:214:69 | inputs.github_username | .github/actions/action7/action.yml:214:41:214:69 | inputs.github_username | Potential code injection in $@, which may be controlled by an external user. | .github/actions/action7/action.yml:214:41:214:69 | inputs.github_username | ${{ inputs.github_username }} |
 | .github/actions/action7/action.yml:215:41:215:66 | inputs.github_email | .github/actions/action7/action.yml:215:41:215:66 | inputs.github_email | .github/actions/action7/action.yml:215:41:215:66 | inputs.github_email | Potential code injection in $@, which may be controlled by an external user. | .github/actions/action7/action.yml:215:41:215:66 | inputs.github_email | ${{ inputs.github_email }} |
 | .github/actions/action7/action.yml:217:25:217:60 | github.head_ref \|\| github.ref | .github/actions/action7/action.yml:217:25:217:60 | github.head_ref \|\| github.ref | .github/actions/action7/action.yml:217:25:217:60 | github.head_ref \|\| github.ref | Potential code injection in $@, which may be controlled by an external user. | .github/actions/action7/action.yml:217:25:217:60 | github.head_ref \|\| github.ref | ${{ github.head_ref \|\| github.ref }} |
+| .github/workflows/actor_check_valid_event.yml:12:21:12:58 | github.event.pull_request.title | .github/workflows/actor_check_valid_event.yml:12:21:12:58 | github.event.pull_request.title | .github/workflows/actor_check_valid_event.yml:12:21:12:58 | github.event.pull_request.title | Potential code injection in $@, which may be controlled by an external user. | .github/workflows/actor_check_valid_event.yml:12:21:12:58 | github.event.pull_request.title | ${{ github.event.pull_request.title }} |
+| .github/workflows/actor_check_wrong_event.yml:20:21:20:51 | github.event.issue.title | .github/workflows/actor_check_wrong_event.yml:20:21:20:51 | github.event.issue.title | .github/workflows/actor_check_wrong_event.yml:20:21:20:51 | github.event.issue.title | Potential code injection in $@, which may be controlled by an external user. | .github/workflows/actor_check_wrong_event.yml:20:21:20:51 | github.event.issue.title | ${{ github.event.issue.title }} |
 | .github/workflows/changed-files.yml:20:24:20:76 | steps.changed-files1.outputs.all_changed_files | .github/workflows/changed-files.yml:15:9:18:6 | Uses Step: changed-files1 | .github/workflows/changed-files.yml:20:24:20:76 | steps.changed-files1.outputs.all_changed_files | Potential code injection in $@, which may be controlled by an external user. | .github/workflows/changed-files.yml:20:24:20:76 | steps.changed-files1.outputs.all_changed_files | ${{ steps.changed-files1.outputs.all_changed_files }} |
 | .github/workflows/changed-files.yml:40:24:40:76 | steps.changed-files3.outputs.all_changed_files | .github/workflows/changed-files.yml:33:9:38:6 | Uses Step: changed-files3 | .github/workflows/changed-files.yml:40:24:40:76 | steps.changed-files3.outputs.all_changed_files | Potential code injection in $@, which may be controlled by an external user. | .github/workflows/changed-files.yml:40:24:40:76 | steps.changed-files3.outputs.all_changed_files | ${{ steps.changed-files3.outputs.all_changed_files }} |
 | .github/workflows/changed-files.yml:58:24:58:76 | steps.changed-files5.outputs.all_changed_files | .github/workflows/changed-files.yml:53:9:56:6 | Uses Step: changed-files5 | .github/workflows/changed-files.yml:58:24:58:76 | steps.changed-files5.outputs.all_changed_files | Potential code injection in $@, which may be controlled by an external user. | .github/workflows/changed-files.yml:58:24:58:76 | steps.changed-files5.outputs.all_changed_files | ${{ steps.changed-files5.outputs.all_changed_files }} |