Merge pull request #22243 from JarLob/improve/actions-untrusted-check… · github/codeql@ddb7e3f · GitHub
Skip to content

Commit ddb7e3f

Browse files
aschackmullaschackmull
authored
Merge pull request #22243 from JarLob/improve/actions-untrusted-checkout-source-link
Actions: Improve `actions/cache-poisoning/poisonable-step` and `actions/untrusted-checkout/critical` path UI
2 parents 0fb9774 + a44c25c commit ddb7e3f

8 files changed

Lines changed: 269 additions & 65 deletions

File tree

‎actions/ql/lib/codeql/actions/security/UntrustedCheckoutQuery.qll‎

Lines changed: 47 additions & 0 deletions

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

Lines changed: 19 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -18,20 +18,30 @@ import codeql.actions.security.CachePoisoningQuery
1818
import codeql.actions.security.PoisonableSteps
1919
import codeql.actions.security.ControlChecks
2020

21-
query predicate edges(Step a, Step b) { a.getNextStep() = b }
21+
query predicate edges(AstNode predecessor, AstNode successor) {
22+
predecessor.(Step).getNextStep() = successor
23+
or
24+
checkoutReferenceEdge(predecessor, successor)
25+
}
2226

23-
from LocalJob job, Event event, Step source, Step step, string message, string path
27+
from
28+
LocalJob job, Event event, Step source, Step step, string message, string path,
29+
AstNode untrustedInput, string untrustedInputText
2430
where
2531
// the job checkouts untrusted code from a pull request or downloads an untrusted artifact
2632
job.getAStep() = source and
2733
(
2834
source instanceof PRHeadCheckoutStep and
29-
message = "due to privilege checkout of untrusted code." and
30-
path = source.(PRHeadCheckoutStep).getPath()
35+
message = "due to privilege checkout of untrusted code from" and
36+
path = source.(PRHeadCheckoutStep).getPath() and
37+
untrustedInput = getCheckoutReference(source) and
38+
untrustedInputText = getCheckoutReferenceText(untrustedInput)
3139
or
3240
source instanceof UntrustedArtifactDownloadStep and
33-
message = "due to downloading an untrusted artifact." and
34-
path = source.(UntrustedArtifactDownloadStep).getPath()
41+
message = "due to downloading" and
42+
path = source.(UntrustedArtifactDownloadStep).getPath() and
43+
untrustedInput = source and
44+
untrustedInputText = "an untrusted artifact"
3545
) and
3646
// the checkout/download is not controlled by an access check
3747
not exists(ControlCheck check |
@@ -57,6 +67,6 @@ where
5767
step instanceof PoisonableStep and
5868
// excluding privileged workflows since they can be exploited in easier circumstances
5969
not job.isPrivileged()
60-
select step, source, step,
61-
"Potential cache poisoning in the context of the default branch " + message + " ($@).", event,
62-
event.getName()
70+
select step, untrustedInput, step,
71+
"Potential cache poisoning in the context of the default branch " + message + " $@. ($@).",
72+
untrustedInput, untrustedInputText, event, event.getName()

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

Lines changed: 13 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -18,10 +18,18 @@ import codeql.actions.security.UntrustedCheckoutQuery
1818
import codeql.actions.security.PoisonableSteps
1919
import codeql.actions.security.ControlChecks
2020

21-
query predicate edges(Step a, Step b) { a.getNextStep() = b }
21+
query predicate edges(AstNode predecessor, AstNode successor) {
22+
predecessor.(Step).getNextStep() = successor
23+
or
24+
checkoutReferenceEdge(predecessor, successor)
25+
}
2226

23-
from PRHeadCheckoutStep checkout, PoisonableStep poisonable, Event event
27+
from
28+
PRHeadCheckoutStep checkout, PoisonableStep poisonable, Event event, AstNode checkoutReference,
29+
string checkoutReferenceText
2430
where
31+
checkoutReference = getCheckoutReference(checkout) and
32+
checkoutReferenceText = getCheckoutReferenceText(checkoutReference) and
2533
// the checkout is followed by a known poisonable step
2634
checkout.getAFollowingStep() = poisonable and
2735
(
@@ -51,6 +59,6 @@ where
5159
event.getName() = checkoutTriggers() and
5260
not exists(ControlCheck check | check.protects(checkout, event, "untrusted-checkout")) and
5361
not exists(ControlCheck check | check.protects(poisonable, event, "untrusted-checkout"))
54-
select checkout, checkout, poisonable,
55-
"Checkout of untrusted code in a privileged workflow with later potential execution (event trigger: $@).",
56-
event, event.getName()
62+
select checkout, checkoutReference, poisonable,
63+
"Checkout of untrusted code from $@ in a privileged workflow with later potential execution (event trigger: $@).",
64+
checkoutReference, checkoutReferenceText, event, event.getName()

‎actions/ql/src/change-notes/2026-07-28-checkout-provenance.md‎

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,4 @@
1+
---
2+
category: fix
3+
---
4+
* The `actions/cache-poisoning/poisonable-step` and `actions/untrusted-checkout/critical` queries now start paths at the expressions that control untrusted checkouts and link their alert messages to those expressions.

‎actions/ql/test/query-tests/Security/CWE-349/.github/workflows/cache_write_capable_workflow_dispatch.yml‎

Lines changed: 23 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,23 @@
1+
on: workflow_dispatch
2+
3+
jobs:
4+
cache:
5+
permissions: {}
6+
runs-on: ubuntu-latest
7+
steps:
8+
- id: pr
9+
env:
10+
HEAD_SHA: ${{ github.event.inputs.head_sha }}
11+
run: |
12+
jq -cn --arg sha "$HEAD_SHA" '{head: {sha: $sha}}' |
13+
sed 's/^/json=/' >> "$GITHUB_OUTPUT"
14+
- env:
15+
HEAD_SHA: ${{ fromJSON(steps.pr.outputs.json).head.sha }}
16+
run: |
17+
git fetch origin "$HEAD_SHA"
18+
git checkout "$HEAD_SHA"
19+
- run: npm install
20+
- uses: actions/cache@v4
21+
with:
22+
path: .npm
23+
key: workflow-dispatch

‎actions/ql/test/query-tests/Security/CWE-349/CachePoisoningViaDirectCache.expected‎

Lines changed: 3 additions & 0 deletions

0 commit comments

Comments
 (0)