Merge branch 'master' of https://github.com/github/codeql-actions

ql/lib/codeql/actions/Helper.qll

@@ -248,8 +248,7 @@ predicate inPrivilegedCompositeAction(AstNode node) {
 predicate inPrivilegedExternallyTriggerableJob(AstNode node) {
   exists(Job j |
     j = node.getEnclosingJob() and
-    j.isPrivilegedExternallyTriggerable() and
-    not exists(ControlCheck check, Event e | j.getATriggerEvent() = e | check.protects(node, e))
+    j.isPrivilegedExternallyTriggerable()
   )
 }
 

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

@@ -1,17 +1,46 @@
 import actions
 
+string any_relevant_category() {
+  result =
+    [
+      "untrusted-checkout", "output-clobbering", "envpath-injection", "envvar-injection",
+      "command-injection", "argument-injection", "code-injection", "cache-poisoning",
+      "untrusted-checkout-toctou", "artifact-poisoning"
+    ]
+}
+
+string any_non_toctou_category() {
+  result = any_relevant_category() and not result = "untrusted-checkout-toctou"
+}
+
+string any_relevant_event() {
+  result =
+    [
+      "pull_request_target",
+      "issue_comment",
+      "pull_request_comment",
+      "workflow_run",
+      "issues",
+      "fork",
+      "watch",
+      "discussion_comment",
+      "discussion"
+    ]
+}
+
 /** An If node that contains an actor, user or label check */
 abstract class ControlCheck extends AstNode {
   ControlCheck() {
     this instanceof If or
     this instanceof Environment or
-    this instanceof UsesStep
+    this instanceof UsesStep or
+    this instanceof Run
   }
 
-  predicate protects(Step step, Event event) {
+  predicate protects(Step step, Event event, string category) {
     event.getEnclosingWorkflow() = step.getEnclosingWorkflow() and
-    this.getAProtectedEvent() = event.getName() and
-    this.dominates(step)
+    this.dominates(step) and
+    this.protectsCategoryAndEvent(category, event.getName())
   }
 
   predicate dominates(Step step) {
@@ -30,80 +59,71 @@ abstract class ControlCheck extends AstNode {
       step.getEnclosingJob().getANeededJob().getEnvironment() = this
     )
     or
-    this.(UsesStep).getAFollowingStep() = step
+    this.(Step).getAFollowingStep() = step
   }
 
-  abstract string getAProtectedEvent();
-
-  abstract boolean protectsAgainstRefMutationAttacks();
+  abstract predicate protectsCategoryAndEvent(string category, string event);
 }
 
 abstract class AssociationCheck extends ControlCheck {
-  // checks who you are (identity)
-  // association checks are effective against pull requests since they can control who is making the PR
-  // they are not effective against issue_comment since the author of the comment may not be the same as the author of the PR
-  // someone entitled to trigger the workflow with a comment, may no detect a malicious comment, or the comment may mutate after approval
-  override string getAProtectedEvent() { result = ["pull_request", "pull_request_target"] }
-
-  override boolean protectsAgainstRefMutationAttacks() { result = true }
+  // Checks if the actor is a MEMBER/OWNER the repo
+  // - they are effective against pull requests and workflow_run (since these are triggered by pull_requests) since they can control who is making the PR
+  // - they are not effective against issue_comment since the author of the comment may not be the same as the author of the PR
+  override predicate protectsCategoryAndEvent(string category, string event) {
+    event = ["pull_request_target", "workflow_run"] and category = any_relevant_category()
+  }
 }
 
 abstract class ActorCheck extends ControlCheck {
-  // checks who you are (identity)
-  // actor checks are effective against pull requests since they can control who is making the PR
-  // they are not effective against issue_comment since the author of the comment may not be the same as the author of the PR
-  // someone entitled to trigger the workflow with a comment, may no detect a malicious comment, or the comment may mutate after approval
-  override string getAProtectedEvent() { result = ["pull_request", "pull_request_target"] }
-
-  override boolean protectsAgainstRefMutationAttacks() { result = true }
+  // checks for a specific actor
+  // - they are effective against pull requests and workflow_run (since these are triggered by pull_requests) since they can control who is making the PR
+  // - they are not effective against issue_comment since the author of the comment may not be the same as the author of the PR
+  override predicate protectsCategoryAndEvent(string category, string event) {
+    event = ["pull_request_target", "workflow_run"] and category = any_relevant_category()
+  }
 }
 
 abstract class RepositoryCheck extends ControlCheck {
-  // repository checks are effective against pull requests since they can control where the code is coming from
-  // they are not effective against issue_comment since the repository will always be the same
-  // who you are (identity)
-  override string getAProtectedEvent() { result = ["pull_request", "pull_request_target"] }
-
-  override boolean protectsAgainstRefMutationAttacks() { result = true }
+  // checks that the origin of the code is the same as the repository.
+  // for pull_requests, that means that it triggers only on local branches or repos from the same org
+  // - they are effective against pull requests/workflow_run since they can control where the code is coming from
+  // - they are not effective against issue_comment since the repository will always be the same
+  override predicate protectsCategoryAndEvent(string category, string event) {
+    event = ["pull_request_target", "workflow_run"] and category = any_relevant_category()
+  }
 }
 
 abstract class PermissionCheck extends ControlCheck {
-  // permission checks are effective against pull requests since they can control who can make changes
-  // they are not effective against issue_comment since the author of the comment may not be the same as the author of the PR
-  // someone entitled to trigger the workflow with a comment, may no detect a malicious comment, or the comment may mutate after approval
-  // who you are (identity)
-  override string getAProtectedEvent() { result = ["pull_request", "pull_request_target"] }
-
-  override boolean protectsAgainstRefMutationAttacks() { result = true }
+  // checks that the actor has a specific permission level
+  // - they are effective against pull requests/workflow_run since they can control who can make changes
+  // - they are not effective against issue_comment since the author of the comment may not be the same as the author of the PR
+  override predicate protectsCategoryAndEvent(string category, string event) {
+    event = ["pull_request_target", "workflow_run", "issue_comment"] and
+    category = any_relevant_category()
+  }
 }
 
 abstract class LabelCheck extends ControlCheck {
-  // does it protect injection attacks but not pwn requests?
-  // pwn requests are susceptible to checkout of mutable code
-  // but injection attacks are not, although a branch name can be changed after approval and perhaps also some other things
-  // they do actually protext against untrusted code execution (sha)
-  // what you have (approval)
-  // TODO: A check should be a combination of:
-  // - event type (pull_request, issue_comment, etc)
-  // - category (untrusted mutable code, untrusted immutable code, code injection, etc)
-  //  - we dont know this unless we pass category to inPrivilegedContext and into ControlCheck.protects
-  //  - we can decide if a control check is effective based only on the ast node
-  override string getAProtectedEvent() { result = ["pull_request", "pull_request_target"] }
-
-  // ref can be mutated after approval
-  override boolean protectsAgainstRefMutationAttacks() { result = false }
+  // checks if the issue/pull_request is labeled, which implies that it could have been approved
+  // - they dont protect against mutation attacks
+  override predicate protectsCategoryAndEvent(string category, string event) {
+    event = ["pull_request_target", "workflow_run"] and category = any_non_toctou_category()
+  }
 }
 
 class EnvironmentCheck extends ControlCheck instanceof Environment {
   // Environment checks are not effective against any mutable attacks
-  // they do actually protext against untrusted code execution (sha)
-  // what you have (approval)
-  EnvironmentCheck() { any() }
+  // they do actually protect against untrusted code execution (sha)
+  override predicate protectsCategoryAndEvent(string category, string event) {
+    event = ["pull_request_target", "workflow_run"] and category = any_non_toctou_category()
+  }
+}
 
-  override string getAProtectedEvent() { result = ["pull_request", "pull_request_target"] }
-
-  // ref can be mutated after approval
-  override boolean protectsAgainstRefMutationAttacks() { result = false }
+abstract class CommentVsHeadDateCheck extends ControlCheck {
+  override predicate protectsCategoryAndEvent(string category, string event) {
+    // by itself, this check is not effective against any attacks
+    none()
+  }
 }
 
 /* Specific implementations of control checks */
@@ -162,28 +182,37 @@ class RepositoryIfCheck extends RepositoryCheck instanceof If {
 class AssociationIfCheck extends AssociationCheck instanceof If {
   AssociationIfCheck() {
     // eg: contains(fromJson('["MEMBER", "OWNER"]'), github.event.comment.author_association)
-    exists(
-      normalizeExpr(this.getCondition())
-          .regexpFind([
-              "\\bgithub\\.event\\.comment\\.author_association\\b",
-              "\\bgithub\\.event\\.issue\\.author_association\\b",
-              "\\bgithub\\.event\\.pull_request\\.author_association\\b",
-            ], _, _)
-    )
+    normalizeExpr(this.getCondition())
+        .splitAt("\n")
+        .regexpMatch([
+            ".*\\bgithub\\.event\\.comment\\.author_association\\b.*",
+            ".*\\bgithub\\.event\\.issue\\.author_association\\b.*",
+            ".*\\bgithub\\.event\\.pull_request\\.author_association\\b.*",
+          ]) and
+    normalizeExpr(this.getCondition()).splitAt("\n").regexpMatch(".*\\bMEMBER\\b.*") and
+    normalizeExpr(this.getCondition()).splitAt("\n").regexpMatch(".*\\bOWNER\\b.*")
   }
 }
 
 class AssociationActionCheck extends AssociationCheck instanceof UsesStep {
   AssociationActionCheck() {
     this.getCallee() = "TheModdingInquisition/actions-team-membership" and
-    not exists(this.getArgument("exit"))
-    or
-    this.getArgument("exit") = "true"
+    (
+      not exists(this.getArgument("exit"))
+      or
+      this.getArgument("exit") = "true"
+    )
   }
 }
 
 class PermissionActionCheck extends PermissionCheck instanceof UsesStep {
   PermissionActionCheck() {
+    this.getCallee() = "sushichop/action-repository-permission" and
+    this.getArgument("required-permission") = ["write", "admin"]
+    or
+    this.getCallee() = "prince-chrismc/check-actor-permissions-action" and
+    this.getArgument("permission") = ["write", "admin"]
+    or
     this.getCallee() = "lannonbr/repo-permission-check-action" and
     this.getArgument("permission") = ["write", "admin"]
     or
@@ -195,3 +224,13 @@ class PermissionActionCheck extends PermissionCheck instanceof UsesStep {
     )
   }
 }
+
+class BashCommentVsHeadDateCheck extends CommentVsHeadDateCheck, Run {
+  BashCommentVsHeadDateCheck() {
+    exists(string line |
+      line = this.getScript().splitAt("\n") and
+      line.toLowerCase()
+          .regexpMatch(".*date\\s+-d.*(commit_at|pushed_at|comment_at|commented_at).*date\\s+-d.*(commit_at|pushed_at|comment_at|commented_at).*")
+    )
+  }
+}

ql/lib/ext/config/externally_triggereable_events.yml

@@ -6,13 +6,14 @@ extensions:
       - ["discussion"]
       - ["discussion_comment"]
       - ["fork"]
+      - ["watch"]
       - ["issue_comment"]
       - ["issues"]
-      - ["pull_request"]
+      - ["pull_request"] # non-privileged
       - ["pull_request_comment"]
       - ["pull_request_review"]
       - ["pull_request_review_comment"]
       - ["pull_request_target"]
-      - ["workflow_run"] # depending on trigger workflow
+      - ["workflow_run"] # depending on branch filter
       - ["workflow_call"] # depending on caller
 

ql/src/Security/CWE-074/OutputClobberingHigh.ql

@@ -16,6 +16,7 @@ import actions
 import codeql.actions.security.OutputClobberingQuery
 import codeql.actions.dataflow.ExternalFlow
 import OutputClobberingFlow::PathGraph
+import codeql.actions.security.ControlChecks
 
 from OutputClobberingFlow::PathNode source, OutputClobberingFlow::PathNode sink
 where
@@ -23,9 +24,20 @@ where
   inPrivilegedContext(sink.getNode().asExpr()) and
   // exclude paths to file read sinks from non-artifact sources
   (
-    not source.getNode().(RemoteFlowSource).getSourceType() = "artifact"
+    not source.getNode().(RemoteFlowSource).getSourceType() = "artifact" and
+    not exists(ControlCheck check |
+      check
+          .protects(sink.getNode().asExpr(),
+            source.getNode().asExpr().getEnclosingJob().getATriggerEvent(), "code-injection")
+    )
     or
     source.getNode().(RemoteFlowSource).getSourceType() = "artifact" and
+    not exists(ControlCheck check |
+      check
+          .protects(sink.getNode().asExpr(),
+            source.getNode().asExpr().getEnclosingJob().getATriggerEvent(),
+            ["untrusted-checkout", "artifact-poisoning"])
+    ) and
     (
       sink.getNode() instanceof OutputClobberingFromFileReadSink or
       sink.getNode() instanceof WorkflowCommandClobberingFromFileReadSink or

ql/src/Security/CWE-077/EnvPathInjectionCritical.ql

@@ -15,15 +15,27 @@
 import actions
 import codeql.actions.security.EnvPathInjectionQuery
 import EnvPathInjectionFlow::PathGraph
+import codeql.actions.security.ControlChecks
 
 from EnvPathInjectionFlow::PathNode source, EnvPathInjectionFlow::PathNode sink
 where
   EnvPathInjectionFlow::flowPath(source, sink) and
   inPrivilegedContext(sink.getNode().asExpr()) and
   (
-    not source.getNode().(RemoteFlowSource).getSourceType() = "artifact"
+    not source.getNode().(RemoteFlowSource).getSourceType() = "artifact" and
+    not exists(ControlCheck check |
+      check
+          .protects(sink.getNode().asExpr(),
+            source.getNode().asExpr().getEnclosingJob().getATriggerEvent(), "code-injection")
+    )
     or
     source.getNode().(RemoteFlowSource).getSourceType() = "artifact" and
+    not exists(ControlCheck check |
+      check
+          .protects(sink.getNode().asExpr(),
+            source.getNode().asExpr().getEnclosingJob().getATriggerEvent(),
+            ["untrusted-checkout", "artifact-poisoning"])
+    ) and
     sink.getNode() instanceof EnvPathInjectionFromFileReadSink
   )
 select sink.getNode(), source, sink,

ql/src/Security/CWE-077/EnvVarInjectionCritical.ql

@@ -16,16 +16,33 @@ import actions
 import codeql.actions.security.EnvVarInjectionQuery
 import codeql.actions.dataflow.ExternalFlow
 import EnvVarInjectionFlow::PathGraph
+import codeql.actions.security.ControlChecks
 
 from EnvVarInjectionFlow::PathNode source, EnvVarInjectionFlow::PathNode sink
 where
   EnvVarInjectionFlow::flowPath(source, sink) and
   inPrivilegedContext(sink.getNode().asExpr()) and
+  not exists(ControlCheck check |
+    check
+        .protects(sink.getNode().asExpr(),
+          source.getNode().asExpr().getEnclosingJob().getATriggerEvent(), "envvar-injection")
+  ) and
   // exclude paths to file read sinks from non-artifact sources
   (
-    not source.getNode().(RemoteFlowSource).getSourceType() = "artifact"
+    not source.getNode().(RemoteFlowSource).getSourceType() = "artifact" and
+    not exists(ControlCheck check |
+      check
+          .protects(sink.getNode().asExpr(),
+            source.getNode().asExpr().getEnclosingJob().getATriggerEvent(), "code-injection")
+    )
     or
     source.getNode().(RemoteFlowSource).getSourceType() = "artifact" and
+    not exists(ControlCheck check |
+      check
+          .protects(sink.getNode().asExpr(),
+            source.getNode().asExpr().getEnclosingJob().getATriggerEvent(),
+            ["untrusted-checkout", "artifact-poisoning"])
+    ) and
     (
       sink.getNode() instanceof EnvVarInjectionFromFileReadSink or
       madSink(sink.getNode(), "envvar-injection")

ql/src/Security/CWE-088/ArgumentInjectionCritical.ql

@@ -14,11 +14,17 @@
 import actions
 import codeql.actions.security.ArgumentInjectionQuery
 import ArgumentInjectionFlow::PathGraph
+import codeql.actions.security.ControlChecks
 
 from ArgumentInjectionFlow::PathNode source, ArgumentInjectionFlow::PathNode sink
 where
   ArgumentInjectionFlow::flowPath(source, sink) and
-  inPrivilegedContext(sink.getNode().asExpr())
+  inPrivilegedContext(sink.getNode().asExpr()) and
+  not exists(ControlCheck check |
+    check
+        .protects(sink.getNode().asExpr(),
+          source.getNode().asExpr().getEnclosingJob().getATriggerEvent(), "argument-injection")
+  )
 select sink.getNode(), source, sink,
   "Potential argument injection in $@ command, which may be controlled by an external user.", sink,
   sink.getNode().(ArgumentInjectionSink).getCommand()

ql/src/Security/CWE-094/CodeInjectionCritical.ql

@@ -17,11 +17,17 @@
 import actions
 import codeql.actions.security.CodeInjectionQuery
 import CodeInjectionFlow::PathGraph
+import codeql.actions.security.ControlChecks
 
 from CodeInjectionFlow::PathNode source, CodeInjectionFlow::PathNode sink
 where
   CodeInjectionFlow::flowPath(source, sink) and
   inPrivilegedContext(sink.getNode().asExpr()) and
+  not exists(ControlCheck check |
+    check
+        .protects(sink.getNode().asExpr(),
+          source.getNode().asExpr().getEnclosingJob().getATriggerEvent(), "code-injection")
+  ) and
   // exclude cases where the sink is a JS script and the expression uses toJson
   not exists(UsesStep script |
     script.getCallee() = "actions/github-script" and

ql/src/Security/CWE-349/CachePoisoningViaCodeInjection.ql

@@ -26,7 +26,9 @@ where
   // job can be triggered by an external user
   e.isExternallyTriggerable() and
   // the checkout is not controlled by an access check
-  not exists(ControlCheck check | check.protects(source.getNode().asExpr(), j.getATriggerEvent())) and
+  not exists(ControlCheck check |
+    check.protects(source.getNode().asExpr(), j.getATriggerEvent(), "code-injection")
+  ) and
   // excluding privileged workflows since they can be exploited in easier circumstances
   not j.isPrivileged() and
   (

ql/src/Security/CWE-349/CachePoisoningViaDirectCache.ql

@@ -57,7 +57,9 @@ where
     path = source.(UntrustedArtifactDownloadStep).getPath()
   ) and
   // the checkout/download is not controlled by an access check
-  not exists(ControlCheck check | check.protects(source, j.getATriggerEvent())) and
+  not exists(ControlCheck check |
+    check.protects(source, j.getATriggerEvent(), ["untrusted-checkout", "artifact-poisoning"])
+  ) and
   j.getATriggerEvent() = e and
   // job can be triggered by an external user
   e.isExternallyTriggerable() and

ql/src/Security/CWE-349/CachePoisoningViaPoisonableStep.ql

@@ -34,7 +34,9 @@ where
     path = source.(UntrustedArtifactDownloadStep).getPath()
   ) and
   // the checkout/download is not controlled by an access check
-  not exists(ControlCheck check | check.protects(source, j.getATriggerEvent())) and
+  not exists(ControlCheck check |
+    check.protects(source, j.getATriggerEvent(), ["untrusted-checkout", "artifact-poisoning"])
+  ) and
   j.getATriggerEvent() = e and
   // job can be triggered by an external user
   e.isExternallyTriggerable() and

ql/src/Security/CWE-367/UntrustedCheckoutTOCTOUCritical.ql

@@ -24,11 +24,10 @@ where
   // the checked-out code may lead to arbitrary code execution
   checkout.getAFollowingStep() = s and
   // the checkout occurs in a privileged context
-  j.isPrivilegedExternallyTriggerable() and
+  inPrivilegedContext(checkout) and
   // the mutable checkout step is protected by an Insufficient access check
-  check.dominates(checkout) and
-  check.protects(checkout, j.getATriggerEvent()) and
-  check.protectsAgainstRefMutationAttacks() = false
+  check.protects(checkout, j.getATriggerEvent(), "untrusted-checkout") and
+  not check.protects(checkout, j.getATriggerEvent(), "untrusted-checkout-toctou")
 select s, checkout, s,
   "Insufficient protection against execution of untrusted code on a privileged workflow on check $@.",
   check, check.toString()

ql/src/Security/CWE-367/UntrustedCheckoutTOCTOUHigh.ql

@@ -22,11 +22,10 @@ where
   // there are no evidences that the checked-out gets executed
   not checkout.getAFollowingStep() instanceof PoisonableStep and
   // the checkout occurs in a privileged context
-  j.isPrivilegedExternallyTriggerable() and
+  inPrivilegedContext(checkout) and
   // the mutable checkout step is protected by an Insufficient access check
-  check.dominates(checkout) and
-  check.protects(checkout, j.getATriggerEvent()) and
-  check.protectsAgainstRefMutationAttacks() = false
+  check.protects(checkout, j.getATriggerEvent(), "untrusted-checkout") and
+  not check.protects(checkout, j.getATriggerEvent(), "untrusted-checkout-toctou")
 select checkout,
   "Insufficient protection against execution of untrusted code on a privileged workflow on step $@.",
   check, check.toString()

ql/src/Security/CWE-829/UntrustedCheckoutCritical.ql

@@ -20,10 +20,33 @@ import codeql.actions.security.ControlChecks
 
 query predicate edges(Step a, Step b) { a.getNextStep() = b }
 
-from PRHeadCheckoutStep checkout, PoisonableStep s
+from PRHeadCheckoutStep checkout, PoisonableStep step
 where
   // the checkout is followed by a known poisonable step
-  checkout.getAFollowingStep() = s and
+  checkout.getAFollowingStep() = step and
   // the checkout occurs in a privileged context
-  inPrivilegedContext(checkout)
-select s, checkout, s, "Execution of untrusted code on a privileged workflow."
+  inPrivilegedContext(checkout) and
+  (
+    // issue_comment: check for date comparison checks and actor/access control checks
+    exists(Event event |
+      event.getName() = "issue_comment" and
+      event = checkout.getEnclosingJob().getATriggerEvent() and
+      not exists(ControlCheck check, CommentVsHeadDateCheck date_check |
+        (
+          check instanceof ActorCheck or
+          check instanceof AssociationCheck or
+          check instanceof PermissionCheck
+        ) and
+        check.dominates(checkout) and
+        date_check.dominates(checkout)
+      )
+    )
+    or
+    // not issue_comment triggered workflows
+    exists(Event event |
+      not event.getName() = "issue_comment" and
+      event = checkout.getEnclosingJob().getATriggerEvent() and
+      not exists(ControlCheck check | check.protects(checkout, event, "untrusted-checkout"))
+    )
+  )
+select step, checkout, step, "Execution of untrusted code on a privileged workflow."

ql/src/Security/CWE-829/UntrustedCheckoutHigh.ql

@@ -23,5 +23,26 @@ where
   // the checkout is NOT followed by a known poisonable step
   not checkout.getAFollowingStep() instanceof PoisonableStep and
   // the checkout occurs in a privileged context
-  inPrivilegedContext(checkout)
+  inPrivilegedContext(checkout) and
+  (
+    // issue_comment: check for date comparison checks and actor/access control checks
+    exists(Event e |
+      e.getName() = "issue_comment" and
+      checkout.getEnclosingJob().getATriggerEvent() = e and
+      not exists(ControlCheck write_check, CommentVsHeadDateCheck data_check |
+        (write_check instanceof ActorCheck or write_check instanceof AssociationCheck) and
+        write_check.dominates(checkout) and
+        data_check.dominates(checkout)
+      )
+    )
+    or
+    // not issue_comment triggered workflows
+    exists(Event event |
+      not event.getName() = "issue_comment" and
+      not exists(ControlCheck check |
+        check
+            .protects(checkout, checkout.getEnclosingJob().getATriggerEvent(), "untrusted-checkout")
+      )
+    )
+  )
 select checkout, "Potential execution of untrusted code on a privileged workflow."

ql/test/query-tests/Security/CWE-829/.github/workflows/test11.yml

@@ -0,0 +1,94 @@
+name: Test
+
+on:
+
+  issue_comment:
+    types: [created]
+
+jobs:
+
+  deploy:
+    name: Update deployment
+    if:  >-
+         ${{
+            github.event.issue.pull_request &&
+            (contains(github.event.comment.body, '/deploy') || contains(github.event.comment.body, '/rollback')) &&
+            contains(github.event.issue.labels.*.name, 'Deployment Update') &&
+            github.event.comment.user.type != 'Bot'
+          }}
+
+    runs-on: [self-hosted, production]
+
+    permissions:
+      contents: write
+      issues: write
+      pull-requests: write
+      statuses: write
+
+    steps:
+
+    - name: Check comment keywords
+      shell: bash
+      env:
+        COMMENT_BODY: ${{ github.event.comment.body }}
+        PR_COMMENT_ALLOW_LIST: ${{ secrets.PR_COMMENT_ALLOW_LIST }}
+      run: |
+        function list_subset { local list1="$1"; local list2="$2"; result=0; for item in $list2; do if ! [[ $list1 =~ (^|[[:space:]])"$item"($|[[:space:]]) ]]; then result=1; fi; done; return $result; }
+
+        if `list_subset "echo $PR_COMMENT_ALLOW_LIST" "echo $COMMENT_BODY"` ; then
+          echo "Command keywords allowed. Proceeding!"
+        else
+          echo "Command keywords not allowed. Skipping!"
+          exit 1
+        fi
+
+    - name: Get environment from comment
+      id: environment
+      shell: bash
+      env:
+        COMMENT_BODY: ${{ github.event.comment.body }}
+        COMMENT_AT: ${{ github.event.comment.created_at }}
+        GH_REPO: ${{ github.repository }}
+        PR_NUMBER: ${{ github.event.issue.number }}
+        GH_TOKEN: ${{ secrets.GITHUB_TOKEN }}
+      run: |
+        pr="$(gh api /repos/${GH_REPO}/pulls/${PR_NUMBER})"
+        head_sha="$(echo "$pr" | jq -r .head.sha)"
+        pushed_at="$(echo "$pr" | jq -r .pushed_at)"
+
+        if [[ $(date -d "$pushed_at" +%s) -gt $(date -d "$COMMENT_AT" +%s) ]]; then
+          echo "Deployment not allowed because the PR was pushed to (at $pushed_at) after the triggering comment was issued (at $COMMENT_AT)"
+          exit 1
+        fi
+
+        target=$(echo "$COMMENT_BODY" | sed 's/.* //') && \
+        deploy_type=$(echo "$COMMENT_BODY" | sed 's/ .*//')
+
+        if [[ $target == "scorer" ]]; then
+          echo "env=async scorer" >> $GITHUB_OUTPUT
+        else
+          env=$(echo "$target")
+          echo "env=$env" >> $GITHUB_OUTPUT
+        fi
+
+        if [[ $deploy_type == "/deploy" ]]; then
+          echo "depl=deployment" >> $GITHUB_OUTPUT
+        elif [[ $deploy_type == "/rollback" ]]; then
+          echo "depl=rollback" >> $GITHUB_OUTPUT
+        else
+          echo "depl=unknown deployment type" >> $GITHUB_OUTPUT
+        fi
+
+        echo "head_sha=$head_sha" >> $GITHUB_OUTPUT
+
+    - name: Checkout PR branch
+      if: contains(github.event.comment.body, '/deploy')
+      uses: actions/checkout@692973e3d937129bcbf40652eb9f2f61becf3332  # v4
+      with:
+        ref: ${{ steps.environment.outputs.head_sha }}
+
+    - name: Environment setup
+      uses: ./.github/actions/setup-env
+      with:
+        azure_creds: ${{ secrets.AZURE_CREDENTIALS }}
+

ql/test/query-tests/Security/CWE-829/.github/workflows/test12.yml

@@ -0,0 +1,96 @@
+name: Test
+
+on:
+
+  issue_comment:
+    types: [created]
+
+jobs:
+
+  deploy:
+    name: Update deployment
+    if: >
+      github.event.issue.pull_request &&
+      (contains(github.event.comment.body, '/deploy') || contains(github.event.comment.body, '/rollback')) &&
+      contains(github.event.issue.labels.*.name, 'Deployment Update') &&
+      github.event.comment.user.type != 'Bot' &&
+      (
+        github.event.issue.author_association == 'OWNER' ||
+        github.event.issue.author_association == 'COLLABORATOR' ||
+        github.event.issue.author_association == 'MEMBER'
+      ) 
+    runs-on: [self-hosted, production]
+
+    permissions:
+      contents: write
+      issues: write
+      pull-requests: write
+      statuses: write
+
+    steps:
+
+    - name: Check comment keywords
+      shell: bash
+      env:
+        COMMENT_BODY: ${{ github.event.comment.body }}
+        PR_COMMENT_ALLOW_LIST: ${{ secrets.PR_COMMENT_ALLOW_LIST }}
+      run: |
+        function list_subset { local list1="$1"; local list2="$2"; result=0; for item in $list2; do if ! [[ $list1 =~ (^|[[:space:]])"$item"($|[[:space:]]) ]]; then result=1; fi; done; return $result; }
+
+        if `list_subset "echo $PR_COMMENT_ALLOW_LIST" "echo $COMMENT_BODY"` ; then
+          echo "Command keywords allowed. Proceeding!"
+        else
+          echo "Command keywords not allowed. Skipping!"
+          exit 1
+        fi
+
+    - name: Get environment from comment
+      id: environment
+      shell: bash
+      env:
+        COMMENT_BODY: ${{ github.event.comment.body }}
+        COMMENT_AT: ${{ github.event.comment.created_at }}
+        GH_REPO: ${{ github.repository }}
+        PR_NUMBER: ${{ github.event.issue.number }}
+        GH_TOKEN: ${{ secrets.GITHUB_TOKEN }}
+      run: |
+        pr="$(gh api /repos/${GH_REPO}/pulls/${PR_NUMBER})"
+        head_sha="$(echo "$pr" | jq -r .head.sha)"
+        pushed_at="$(echo "$pr" | jq -r .pushed_at)"
+
+        if [[ $(date -d "$pushed_at" +%s) -gt $(date -d "$COMMENT_AT" +%s) ]]; then
+          echo "Deployment not allowed because the PR was pushed to (at $pushed_at) after the triggering comment was issued (at $COMMENT_AT)"
+          exit 1
+        fi
+
+        target=$(echo "$COMMENT_BODY" | sed 's/.* //') && \
+        deploy_type=$(echo "$COMMENT_BODY" | sed 's/ .*//')
+
+        if [[ $target == "scorer" ]]; then
+          echo "env=async scorer" >> $GITHUB_OUTPUT
+        else
+          env=$(echo "$target")
+          echo "env=$env" >> $GITHUB_OUTPUT
+        fi
+
+        if [[ $deploy_type == "/deploy" ]]; then
+          echo "depl=deployment" >> $GITHUB_OUTPUT
+        elif [[ $deploy_type == "/rollback" ]]; then
+          echo "depl=rollback" >> $GITHUB_OUTPUT
+        else
+          echo "depl=unknown deployment type" >> $GITHUB_OUTPUT
+        fi
+
+        echo "head_sha=$head_sha" >> $GITHUB_OUTPUT
+
+    - name: Checkout PR branch
+      if: contains(github.event.comment.body, '/deploy')
+      uses: actions/checkout@692973e3d937129bcbf40652eb9f2f61becf3332  # v4
+      with:
+        ref: ${{ steps.environment.outputs.head_sha }}
+
+    - name: Environment setup
+      uses: ./.github/actions/setup-env
+      with:
+        azure_creds: ${{ secrets.AZURE_CREDENTIALS }}
+

ql/test/query-tests/Security/CWE-829/.github/workflows/test13.yml

@@ -0,0 +1,31 @@
+on:
+  issue_comment:
+    types:
+      - created
+jobs:
+  danger-for-external:
+    name: Danger for external - Node.js 16
+    if: |
+      github.event_name == 'issue_comment' && github.event.action == 'created'
+      && github.event.issue.pull_request != null
+      && startsWith(github.event.comment.body, '/danger')
+    runs-on: ubuntu-latest
+    steps:
+    - name: Check repository permission for user
+      uses: sushichop/action-repository-permission@v2
+      with:
+        required-permission: write
+        reaction-permitted: rocket
+        comment-not-permitted: Sorry, you don't have enough permission to execute `/danger`...
+    - name: Clone the PR source
+      uses: actions/checkout@v3
+      with:
+        ref: refs/pull/${{ github.event.issue.number }}/head
+        fetch-depth: 0
+    - uses: actions/setup-node@v3
+      with:
+        node-version: 16
+    - name: Danger JS
+      run: npx danger ci
+      env:
+        GITHUB_TOKEN: ${{ secrets.GITHUB_TOKEN }}

ql/test/query-tests/Security/CWE-829/UnpinnedActionsTag.expected

@@ -27,4 +27,5 @@
 | .github/workflows/pr-workflow.yml:452:9:453:6 | Uses Step | Unpinned 3rd party Action 'pr-workflow' step $@ uses 'determinatesystems/magic-nix-cache-action' with ref 'main', not a pinned commit hash | .github/workflows/pr-workflow.yml:452:9:453:6 | Uses Step | Uses Step |
 | .github/workflows/pr-workflow.yml:453:9:459:6 | Uses Step | Unpinned 3rd party Action 'pr-workflow' step $@ uses 'cachix/cachix-action' with ref 'master', not a pinned commit hash | .github/workflows/pr-workflow.yml:453:9:459:6 | Uses Step | Uses Step |
 | .github/workflows/test7.yml:24:9:27:6 | Uses Step | Unpinned 3rd party Action 'Benchmark' step $@ uses 'pnpm/action-setup' with ref 'v3', not a pinned commit hash | .github/workflows/test7.yml:24:9:27:6 | Uses Step | Uses Step |
+| .github/workflows/test13.yml:14:7:20:4 | Uses Step | Unpinned 3rd party Action 'test13.yml' step $@ uses 'sushichop/action-repository-permission' with ref 'v2', not a pinned commit hash | .github/workflows/test13.yml:14:7:20:4 | Uses Step | Uses Step |
 | .github/workflows/unpinned_tags.yml:10:7:11:4 | Uses Step | Unpinned 3rd party Action 'unpinned_tags.yml' step $@ uses 'foo/bar' with ref 'v1', not a pinned commit hash | .github/workflows/unpinned_tags.yml:10:7:11:4 | Uses Step | Uses Step |

ql/test/query-tests/Security/CWE-829/UntrustedCheckoutCritical.expected

@@ -175,6 +175,15 @@ edges
 | .github/workflows/test8.yml:20:9:26:6 | Uses Step | .github/workflows/test8.yml:26:9:29:2 | Run Step |
 | .github/workflows/test9.yml:11:9:16:6 | Uses Step | .github/workflows/test9.yml:16:9:17:48 | Run Step |
 | .github/workflows/test10.yml:20:9:25:6 | Uses Step | .github/workflows/test10.yml:25:9:30:2 | Run Step |
+| .github/workflows/test11.yml:30:7:45:4 | Run Step | .github/workflows/test11.yml:45:7:84:4 | Run Step: environment |
+| .github/workflows/test11.yml:45:7:84:4 | Run Step: environment | .github/workflows/test11.yml:84:7:90:4 | Uses Step |
+| .github/workflows/test11.yml:84:7:90:4 | Uses Step | .github/workflows/test11.yml:90:7:93:54 | Uses Step |
+| .github/workflows/test12.yml:32:7:47:4 | Run Step | .github/workflows/test12.yml:47:7:86:4 | Run Step: environment |
+| .github/workflows/test12.yml:47:7:86:4 | Run Step: environment | .github/workflows/test12.yml:86:7:92:4 | Uses Step |
+| .github/workflows/test12.yml:86:7:92:4 | Uses Step | .github/workflows/test12.yml:92:7:95:54 | Uses Step |
+| .github/workflows/test13.yml:14:7:20:4 | Uses Step | .github/workflows/test13.yml:20:7:25:4 | Uses Step |
+| .github/workflows/test13.yml:20:7:25:4 | Uses Step | .github/workflows/test13.yml:25:7:28:4 | Uses Step |
+| .github/workflows/test13.yml:25:7:28:4 | Uses Step | .github/workflows/test13.yml:28:7:31:50 | Run Step |
 | .github/workflows/test.yml:13:9:14:6 | Uses Step | .github/workflows/test.yml:14:9:25:6 | Run Step |
 | .github/workflows/test.yml:14:9:25:6 | Run Step | .github/workflows/test.yml:25:9:33:6 | Run Step |
 | .github/workflows/test.yml:25:9:33:6 | Run Step | .github/workflows/test.yml:33:9:37:34 | Run Step |
@@ -223,7 +232,7 @@ edges
 | .github/workflows/test7.yml:49:9:58:20 | Run Step: benchmark-pr | .github/workflows/test7.yml:19:9:24:6 | Uses Step | .github/workflows/test7.yml:49:9:58:20 | Run Step: benchmark-pr | Execution of untrusted code on a privileged workflow. |
 | .github/workflows/test9.yml:16:9:17:48 | Run Step | .github/workflows/test9.yml:11:9:16:6 | Uses Step | .github/workflows/test9.yml:16:9:17:48 | Run Step | Execution of untrusted code on a privileged workflow. |
 | .github/workflows/test10.yml:25:9:30:2 | Run Step | .github/workflows/test10.yml:20:9:25:6 | Uses Step | .github/workflows/test10.yml:25:9:30:2 | Run Step | Execution of untrusted code on a privileged workflow. |
-| .github/workflows/untrusted_checkout3.yml:13:9:13:23 | Run Step | .github/actions/dangerous-git-checkout/action.yml:6:7:11:4 | Uses Step | .github/workflows/untrusted_checkout3.yml:13:9:13:23 | Run Step | Execution of untrusted code on a privileged workflow. |
+| .github/workflows/test11.yml:90:7:93:54 | Uses Step | .github/workflows/test11.yml:84:7:90:4 | Uses Step | .github/workflows/test11.yml:90:7:93:54 | Uses Step | Execution of untrusted code on a privileged workflow. |
 | .github/workflows/untrusted_checkout4.yml:61:7:67:4 | Run Step | .github/workflows/untrusted_checkout4.yml:55:7:61:4 | Uses Step | .github/workflows/untrusted_checkout4.yml:61:7:67:4 | Run Step | Execution of untrusted code on a privileged workflow. |
 | .github/workflows/untrusted_checkout4.yml:67:7:73:4 | Run Step | .github/workflows/untrusted_checkout4.yml:55:7:61:4 | Uses Step | .github/workflows/untrusted_checkout4.yml:67:7:73:4 | Run Step | Execution of untrusted code on a privileged workflow. |
 | .github/workflows/untrusted_checkout4.yml:73:7:79:4 | Run Step | .github/workflows/untrusted_checkout4.yml:55:7:61:4 | Uses Step | .github/workflows/untrusted_checkout4.yml:73:7:79:4 | Run Step | Execution of untrusted code on a privileged workflow. |

ql/test/query-tests/Security/CWE-829/UntrustedCheckoutHigh.expected

@@ -18,6 +18,7 @@
 | .github/workflows/pr-workflow.yml:103:9:109:6 | Uses Step | Potential execution of untrusted code on a privileged workflow. |
 | .github/workflows/pr-workflow.yml:139:9:144:6 | Uses Step | Potential execution of untrusted code on a privileged workflow. |
 | .github/workflows/pr-workflow.yml:444:9:449:6 | Uses Step | Potential execution of untrusted code on a privileged workflow. |
+| .github/workflows/test13.yml:20:7:25:4 | Uses Step | Potential execution of untrusted code on a privileged workflow. |
 | .github/workflows/untrusted_checkout2.yml:14:9:19:72 | Run Step | Potential execution of untrusted code on a privileged workflow. |
 | .github/workflows/workflow_run_untrusted_checkout.yml:13:9:16:6 | Uses Step | Potential execution of untrusted code on a privileged workflow. |
 | .github/workflows/workflow_run_untrusted_checkout.yml:16:9:18:31 | Uses Step | Potential execution of untrusted code on a privileged workflow. |