diff --git a/actions/ql/lib/change-notes/2026-07-20-matrix-declared-values.md b/actions/ql/lib/change-notes/2026-07-20-matrix-declared-values.md new file mode 100644 index 000000000000..69c92ceadb3c --- /dev/null +++ b/actions/ql/lib/change-notes/2026-07-20-matrix-declared-values.md @@ -0,0 +1,4 @@ +--- +category: feature +--- +* Added `MatrixExpression.getADeclaredValue()` for accessing scalar values declared for a matrix expression. \ No newline at end of file diff --git a/actions/ql/lib/codeql/actions/Ast.qll b/actions/ql/lib/codeql/actions/Ast.qll index 6e76e4cd665a..5a0fa2f86944 100644 --- a/actions/ql/lib/codeql/actions/Ast.qll +++ b/actions/ql/lib/codeql/actions/Ast.qll @@ -397,4 +397,7 @@ class InputsExpression extends SimpleReferenceExpression instanceof InputsExpres class EnvExpression extends SimpleReferenceExpression instanceof EnvExpressionImpl { } -class MatrixExpression extends SimpleReferenceExpression instanceof MatrixExpressionImpl { } +class MatrixExpression extends SimpleReferenceExpression instanceof MatrixExpressionImpl { + /** Gets a scalar value declared for this matrix expression. */ + string getADeclaredValue() { result = super.getLiteralValues() } +} diff --git a/actions/ql/lib/codeql/actions/ast/internal/Ast.qll b/actions/ql/lib/codeql/actions/ast/internal/Ast.qll index 82d4a1186708..37d3dc932fe3 100644 --- a/actions/ql/lib/codeql/actions/ast/internal/Ast.qll +++ b/actions/ql/lib/codeql/actions/ast/internal/Ast.qll @@ -1874,7 +1874,12 @@ class MatrixExpressionImpl extends SimpleReferenceExpressionImpl { exists(StrategyImpl s, MatrixAccessPathImpl p, ScalarValueImpl v | (s = this.getEnclosingJob().getStrategy() or s = this.getEnclosingWorkflow().getStrategy()) and p.toString() = fieldAccess and - resolveMatrixAccessPath(s.getMatrix(), p).getNode(_) = v.getNode() and + ( + resolveMatrixAccessPath(s.getMatrix(), p).getNode(_) = v.getNode() + or + resolveMatrixAccessPath(s.getMatrix().lookup("include").(YamlSequence).getElementNode(_), p) + .getNode(_) = v.getNode() + ) and // Exclude values containing matrix expressions to avoid recursion not exists(MatrixExpressionImpl e | e.getParentNode() = v) and result = v.getValue() @@ -1903,7 +1908,7 @@ class MatrixAccessPathImpl extends TMatrixAccessPathNode { } private YamlMappingLikeNode resolveMatrixAccessPath( - // TODO: support `include` and `exclude` keys + // TODO: support `exclude` keys // https://docs.github.com/en/actions/using-jobs/using-a-matrix-for-your-jobs#expanding-or-adding-matrix-configurations YamlMappingLikeNode root, MatrixAccessPathImpl accessPath ) { diff --git a/actions/ql/lib/codeql/actions/security/UntrustedCheckoutQuery.qll b/actions/ql/lib/codeql/actions/security/UntrustedCheckoutQuery.qll index bd8ec6f035e9..569516752a92 100644 --- a/actions/ql/lib/codeql/actions/security/UntrustedCheckoutQuery.qll +++ b/actions/ql/lib/codeql/actions/security/UntrustedCheckoutQuery.qll @@ -227,6 +227,14 @@ abstract class MutableRefCheckoutStep extends PRHeadCheckoutStep { } /** Checkout of a Pull Request HEAD ref */ abstract class SHACheckoutStep extends PRHeadCheckoutStep { } +private predicate hasOnlyStaticMatrixValues(Expression expr) { + expr instanceof MatrixExpression and + exists(expr.(MatrixExpression).getADeclaredValue()) and + not exists(string value | + value = expr.(MatrixExpression).getADeclaredValue() and value.matches("%${{%") + ) +} + /** Checkout of a Pull Request HEAD ref using actions/checkout action */ class ActionsMutableRefCheckout extends MutableRefCheckoutStep instanceof UsesStep { ActionsMutableRefCheckout() { @@ -246,7 +254,8 @@ class ActionsMutableRefCheckout extends MutableRefCheckoutStep instanceof UsesSt expr.(StepsExpression).getStepId() = value or expr.(SimpleReferenceExpression).getFieldName() = value and - not expr instanceof GitHubExpression + not expr instanceof GitHubExpression and + not hasOnlyStaticMatrixValues(expr) or expr.(NeedsExpression).getNeededJobId() = value or diff --git a/actions/ql/src/change-notes/2026-07-20-static-matrix-checkout-precision.md b/actions/ql/src/change-notes/2026-07-20-static-matrix-checkout-precision.md new file mode 100644 index 000000000000..bfd2107910d7 --- /dev/null +++ b/actions/ql/src/change-notes/2026-07-20-static-matrix-checkout-precision.md @@ -0,0 +1,4 @@ +--- +category: minorAnalysis +--- +* The untrusted checkout and cache poisoning queries no longer treat references to statically defined matrix values as attacker-controlled checkout references. Matrix values containing GitHub Actions expressions remain reportable. \ No newline at end of file diff --git a/actions/ql/test/query-tests/Security/CWE-349/.github/workflows/dynamic_matrix_include_ref.yml b/actions/ql/test/query-tests/Security/CWE-349/.github/workflows/dynamic_matrix_include_ref.yml new file mode 100644 index 000000000000..c1b88e14c906 --- /dev/null +++ b/actions/ql/test/query-tests/Security/CWE-349/.github/workflows/dynamic_matrix_include_ref.yml @@ -0,0 +1,17 @@ +on: issue_comment + +jobs: + test: + runs-on: ubuntu-latest + strategy: + matrix: + ref: ["main"] + include: + - ref: "${{ github.event.comment.body }}" + steps: + - uses: actions/checkout@v4 + with: + repository: example/example + ref: ${{ matrix.ref }} + # VULNERABLE: an included job checks out a ref controlled by the event payload. + - run: npm install \ No newline at end of file diff --git a/actions/ql/test/query-tests/Security/CWE-349/.github/workflows/dynamic_matrix_ref.yml b/actions/ql/test/query-tests/Security/CWE-349/.github/workflows/dynamic_matrix_ref.yml new file mode 100644 index 000000000000..a66b3518a841 --- /dev/null +++ b/actions/ql/test/query-tests/Security/CWE-349/.github/workflows/dynamic_matrix_ref.yml @@ -0,0 +1,15 @@ +on: issue_comment + +jobs: + test: + runs-on: ubuntu-latest + strategy: + matrix: + ref: ["${{ github.event.comment.body }}"] + steps: + - uses: actions/checkout@v4 + with: + repository: example/example + ref: ${{ matrix.ref }} + # VULNERABLE: matrix.ref is controlled by the event payload. + - run: npm install \ No newline at end of file diff --git a/actions/ql/test/query-tests/Security/CWE-349/.github/workflows/neg_static_matrix_ref.yml b/actions/ql/test/query-tests/Security/CWE-349/.github/workflows/neg_static_matrix_ref.yml new file mode 100644 index 000000000000..cdfa8ff65e2b --- /dev/null +++ b/actions/ql/test/query-tests/Security/CWE-349/.github/workflows/neg_static_matrix_ref.yml @@ -0,0 +1,15 @@ +on: issue_comment + +jobs: + test: + runs-on: ubuntu-latest + strategy: + matrix: + ref: ["main"] + steps: + - uses: actions/checkout@v4 + with: + repository: example/example + ref: ${{ matrix.ref }} + # NOT VULNERABLE: matrix.ref can only be the static value "main". + - run: npm install \ No newline at end of file diff --git a/actions/ql/test/query-tests/Security/CWE-349/CachePoisoningViaDirectCache.expected b/actions/ql/test/query-tests/Security/CWE-349/CachePoisoningViaDirectCache.expected index e89db92356f2..d86f66ba040a 100644 --- a/actions/ql/test/query-tests/Security/CWE-349/CachePoisoningViaDirectCache.expected +++ b/actions/ql/test/query-tests/Security/CWE-349/CachePoisoningViaDirectCache.expected @@ -17,6 +17,8 @@ edges | .github/workflows/direct_cache5.yml:17:9:21:6 | Uses Step | .github/workflows/direct_cache5.yml:21:9:22:21 | Run Step | | .github/workflows/direct_cache6.yml:13:9:16:6 | Uses Step | .github/workflows/direct_cache6.yml:16:9:20:6 | Uses Step | | .github/workflows/direct_cache6.yml:16:9:20:6 | Uses Step | .github/workflows/direct_cache6.yml:20:9:26:46 | Uses Step: cache-pip | +| .github/workflows/dynamic_matrix_include_ref.yml:12:9:17:6 | Uses Step | .github/workflows/dynamic_matrix_include_ref.yml:17:9:17:24 | Run Step | +| .github/workflows/dynamic_matrix_ref.yml:10:9:15:6 | Uses Step | .github/workflows/dynamic_matrix_ref.yml:15:9:15:24 | Run Step | | .github/workflows/neg_direct_cache1.yml:14:9:17:6 | Uses Step | .github/workflows/neg_direct_cache1.yml:17:9:21:6 | Uses Step | | .github/workflows/neg_direct_cache1.yml:17:9:21:6 | Uses Step | .github/workflows/neg_direct_cache1.yml:21:9:22:21 | Run Step | | .github/workflows/neg_direct_cache2.yml:14:9:17:6 | Uses Step | .github/workflows/neg_direct_cache2.yml:17:9:21:6 | Uses Step | @@ -34,6 +36,7 @@ edges | .github/workflows/neg_poisonable_step1.yml:11:9:14:6 | Uses Step: comment-branch | .github/workflows/neg_poisonable_step1.yml:14:9:19:6 | Uses Step | | .github/workflows/neg_poisonable_step1.yml:14:9:19:6 | Uses Step | .github/workflows/neg_poisonable_step1.yml:19:9:20:30 | Run Step | | .github/workflows/neg_poisonable_step2.yml:13:9:16:6 | Uses Step | .github/workflows/neg_poisonable_step2.yml:16:9:17:54 | Run Step | +| .github/workflows/neg_static_matrix_ref.yml:10:9:15:6 | Uses Step | .github/workflows/neg_static_matrix_ref.yml:15:9:15:24 | Run Step | | .github/workflows/poisonable_step1.yml:10:9:12:6 | Uses Step: comment-branch | .github/workflows/poisonable_step1.yml:12:9:15:6 | Uses Step | | .github/workflows/poisonable_step1.yml:12:9:15:6 | Uses Step | .github/workflows/poisonable_step1.yml:15:9:17:2 | Run Step | | .github/workflows/poisonable_step1.yml:21:9:23:6 | Uses Step: comment-branch | .github/workflows/poisonable_step1.yml:23:9:26:6 | Uses Step | diff --git a/actions/ql/test/query-tests/Security/CWE-349/CachePoisoningViaPoisonableStep.expected b/actions/ql/test/query-tests/Security/CWE-349/CachePoisoningViaPoisonableStep.expected index 9a5bbb48384e..9090ac7499ed 100644 --- a/actions/ql/test/query-tests/Security/CWE-349/CachePoisoningViaPoisonableStep.expected +++ b/actions/ql/test/query-tests/Security/CWE-349/CachePoisoningViaPoisonableStep.expected @@ -24,6 +24,10 @@ edges | .github/workflows/direct_cache6.yml:13:9:16:6 | Uses Step | .github/workflows/direct_cache6.yml:16:9:20:6 | Uses Step | | .github/workflows/direct_cache6.yml:15:17:15:57 | github.event.pull_request.head.sha | .github/workflows/direct_cache6.yml:13:9:16:6 | Uses Step | | .github/workflows/direct_cache6.yml:16:9:20:6 | Uses Step | .github/workflows/direct_cache6.yml:20:9:26:46 | Uses Step: cache-pip | +| .github/workflows/dynamic_matrix_include_ref.yml:12:9:17:6 | Uses Step | .github/workflows/dynamic_matrix_include_ref.yml:17:9:17:24 | Run Step | +| .github/workflows/dynamic_matrix_include_ref.yml:15:17:15:33 | matrix.ref | .github/workflows/dynamic_matrix_include_ref.yml:12:9:17:6 | Uses Step | +| .github/workflows/dynamic_matrix_ref.yml:10:9:15:6 | Uses Step | .github/workflows/dynamic_matrix_ref.yml:15:9:15:24 | Run Step | +| .github/workflows/dynamic_matrix_ref.yml:13:17:13:33 | matrix.ref | .github/workflows/dynamic_matrix_ref.yml:10:9:15:6 | Uses Step | | .github/workflows/neg_direct_cache1.yml:14:9:17:6 | Uses Step | .github/workflows/neg_direct_cache1.yml:17:9:21:6 | Uses Step | | .github/workflows/neg_direct_cache1.yml:16:17:16:57 | github.event.pull_request.head.sha | .github/workflows/neg_direct_cache1.yml:14:9:17:6 | Uses Step | | .github/workflows/neg_direct_cache1.yml:17:9:21:6 | Uses Step | .github/workflows/neg_direct_cache1.yml:21:9:22:21 | Run Step | @@ -46,6 +50,7 @@ edges | .github/workflows/neg_poisonable_step1.yml:14:9:19:6 | Uses Step | .github/workflows/neg_poisonable_step1.yml:19:9:20:30 | Run Step | | .github/workflows/neg_poisonable_step1.yml:17:17:17:60 | steps.comment-branch.outputs.head_sha | .github/workflows/neg_poisonable_step1.yml:14:9:19:6 | Uses Step | | .github/workflows/neg_poisonable_step2.yml:13:9:16:6 | Uses Step | .github/workflows/neg_poisonable_step2.yml:16:9:17:54 | Run Step | +| .github/workflows/neg_static_matrix_ref.yml:10:9:15:6 | Uses Step | .github/workflows/neg_static_matrix_ref.yml:15:9:15:24 | Run Step | | .github/workflows/poisonable_step1.yml:10:9:12:6 | Uses Step: comment-branch | .github/workflows/poisonable_step1.yml:12:9:15:6 | Uses Step | | .github/workflows/poisonable_step1.yml:12:9:15:6 | Uses Step | .github/workflows/poisonable_step1.yml:15:9:17:2 | Run Step | | .github/workflows/poisonable_step1.yml:14:17:14:60 | steps.comment-branch.outputs.head_sha | .github/workflows/poisonable_step1.yml:12:9:15:6 | Uses Step | @@ -66,6 +71,8 @@ edges | .github/workflows/poisonable_step5.yml:20:17:20:57 | github.event.pull_request.head.ref | .github/workflows/poisonable_step5.yml:17:9:22:6 | Uses Step | | .github/workflows/poisonable_step5.yml:22:9:24:6 | Uses Step | .github/workflows/poisonable_step5.yml:24:9:28:31 | Uses Step | #select +| .github/workflows/dynamic_matrix_include_ref.yml:17:9:17:24 | Run Step | .github/workflows/dynamic_matrix_include_ref.yml:15:17:15:33 | matrix.ref | .github/workflows/dynamic_matrix_include_ref.yml:17:9:17:24 | Run Step | Potential cache poisoning in the context of the default branch due to privilege checkout of untrusted code from $@. ($@). | .github/workflows/dynamic_matrix_include_ref.yml:15:17:15:33 | matrix.ref | matrix.ref | .github/workflows/dynamic_matrix_include_ref.yml:1:5:1:17 | issue_comment | issue_comment | +| .github/workflows/dynamic_matrix_ref.yml:15:9:15:24 | Run Step | .github/workflows/dynamic_matrix_ref.yml:13:17:13:33 | matrix.ref | .github/workflows/dynamic_matrix_ref.yml:15:9:15:24 | Run Step | Potential cache poisoning in the context of the default branch due to privilege checkout of untrusted code from $@. ($@). | .github/workflows/dynamic_matrix_ref.yml:13:17:13:33 | matrix.ref | matrix.ref | .github/workflows/dynamic_matrix_ref.yml:1:5:1:17 | issue_comment | issue_comment | | .github/workflows/poisonable_step1.yml:15:9:17:2 | Run Step | .github/workflows/poisonable_step1.yml:14:17:14:60 | steps.comment-branch.outputs.head_sha | .github/workflows/poisonable_step1.yml:15:9:17:2 | Run Step | Potential cache poisoning in the context of the default branch due to privilege checkout of untrusted code from $@. ($@). | .github/workflows/poisonable_step1.yml:14:17:14:60 | steps.comment-branch.outputs.head_sha | steps.comment-branch.outputs.head_sha | .github/workflows/poisonable_step1.yml:2:3:2:15 | issue_comment | issue_comment | | .github/workflows/poisonable_step1.yml:26:9:28:2 | Uses Step | .github/workflows/poisonable_step1.yml:25:17:25:60 | steps.comment-branch.outputs.head_sha | .github/workflows/poisonable_step1.yml:26:9:28:2 | Uses Step | Potential cache poisoning in the context of the default branch due to privilege checkout of untrusted code from $@. ($@). | .github/workflows/poisonable_step1.yml:25:17:25:60 | steps.comment-branch.outputs.head_sha | steps.comment-branch.outputs.head_sha | .github/workflows/poisonable_step1.yml:2:3:2:15 | issue_comment | issue_comment | | .github/workflows/poisonable_step1.yml:37:9:37:75 | Run Step | .github/workflows/poisonable_step1.yml:36:17:36:60 | steps.comment-branch.outputs.head_sha | .github/workflows/poisonable_step1.yml:37:9:37:75 | Run Step | Potential cache poisoning in the context of the default branch due to privilege checkout of untrusted code from $@. ($@). | .github/workflows/poisonable_step1.yml:36:17:36:60 | steps.comment-branch.outputs.head_sha | steps.comment-branch.outputs.head_sha | .github/workflows/poisonable_step1.yml:2:3:2:15 | issue_comment | issue_comment |