Skip to content

Commit 5ae6d72

Browse files
committed
actions/unpinned-tag: match lockfile pins case-insensitively on owner/repo
The lockfile generator lower-cases owner/repo (GitHub treats them case-insensitively) while preserving the ref, so a mixed-case ref such as `uses: Azure/login@v1` never matched the lowercase pinnedByLockfileDataModel row and was still reported as unpinned. Lower-case nwo only for the data-model lookup, keeping source casing in the alert message. Adds a mixed-case suppression test plus a mixed-case control that must still fire.
1 parent fbe00de commit 5ae6d72

5 files changed

Lines changed: 21 additions & 1 deletion

File tree

actions/ql/src/Security/CWE-829/UnpinnedActionsTag.ql

Lines changed: 7 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -44,8 +44,14 @@ private predicate isSelfRepository(string nwo) { nwo.matches("$/%") }
4444
// predicate is populated by the CodeQL Actions extractor when it parses the lockfile at
4545
// database-creation time; until then this is a clean no-op and no lockfile-pinned refs are
4646
// suppressed. See `pinnedByLockfileDataModel` in `ConfigExtensions.qll` for the intended shape.
47+
bindingset[nwo]
4748
private predicate pinnedByLockfile(UsesStep uses, string nwo, string version) {
48-
pinnedByLockfileDataModel(uses.getLocation().getFile().getRelativePath(), nwo, version)
49+
// The lockfile generator lower-cases owner/repo (GitHub treats them
50+
// case-insensitively) but preserves the ref, so match `nwo` case-insensitively
51+
// and `version` exactly. `nwo` keeps its source casing everywhere else (e.g. the
52+
// alert message) so authors still see the ref as written.
53+
pinnedByLockfileDataModel(uses.getLocation().getFile().getRelativePath(), nwo.toLowerCase(),
54+
version)
4955
}
5056

5157
private predicate getStepContainerName(UsesStep uses, string name) {

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

Lines changed: 8 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -15,3 +15,11 @@ jobs:
1515
# Negative control is provided for free by the many other fixtures in this directory whose tag
1616
# refs are NOT recorded in the data extension and therefore remain reported.
1717
- uses: some-owner/pinned-action@v1
18+
# `Mixed-Owner/Pinned-Action@v1` is pinned by the lockfile too, but written with the mixed-case
19+
# owner/repo that authors commonly use (Azure, GoogleCloudPlatform, ...). The generator emits
20+
# the lockfile nwo lower-cased, so the query must match owner/repo case-insensitively; this ref
21+
# is therefore expected to be suppressed (no finding).
22+
- uses: Mixed-Owner/Pinned-Action@v1
23+
# Negative control: a mixed-case ref that is NOT recorded in the lockfile data must still be
24+
# reported, guarding against over-suppression of every mixed-case ref.
25+
- uses: Mixed-Owner/Unpinned-Action@v2

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

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -17,6 +17,7 @@
1717
| .github/workflows/label_trusted_checkout2.yml:21:13:21:36 | completely/fakeaction@v2 | Unpinned 3rd party Action 'label_trusted_checkout2.yml' step $@ uses 'completely/fakeaction' with ref 'v2', not a pinned commit hash | .github/workflows/label_trusted_checkout2.yml:21:7:25:4 | Uses Step | Uses Step |
1818
| .github/workflows/label_trusted_checkout2.yml:25:13:25:37 | fakerepo/comment-on-pr@v1 | Unpinned 3rd party Action 'label_trusted_checkout2.yml' step $@ uses 'fakerepo/comment-on-pr' with ref 'v1', not a pinned commit hash | .github/workflows/label_trusted_checkout2.yml:25:7:28:21 | Uses Step | Uses Step |
1919
| .github/workflows/level0.yml:36:15:36:47 | rlespinasse/github-slug-action@v4 | Unpinned 3rd party Action 'Poutine Level 0' step $@ uses 'rlespinasse/github-slug-action' with ref 'v4', not a pinned commit hash | .github/workflows/level0.yml:36:9:39:6 | Uses Step | Uses Step |
20+
| .github/workflows/lockfile_pinned.yml:25:13:25:42 | Mixed-Owner/Unpinned-Action@v2 | Unpinned 3rd party Action 'lockfile_pinned.yml' step $@ uses 'Mixed-Owner/Unpinned-Action' with ref 'v2', not a pinned commit hash | .github/workflows/lockfile_pinned.yml:25:7:25:43 | Uses Step | Uses Step |
2021
| .github/workflows/mend.yml:31:15:31:34 | ruby/setup-ruby@v1 | Unpinned 3rd party Action 'Test' step $@ uses 'ruby/setup-ruby' with ref 'v1', not a pinned commit hash | .github/workflows/mend.yml:29:9:33:28 | Uses Step | Uses Step |
2122
| .github/workflows/pr-workflow.yml:60:15:60:52 | amannn/action-semantic-pull-request@v5 | Unpinned 3rd party Action 'pr-workflow' step $@ uses 'amannn/action-semantic-pull-request' with ref 'v5', not a pinned commit hash | .github/workflows/pr-workflow.yml:60:9:70:6 | Uses Step | Uses Step |
2223
| .github/workflows/pr-workflow.yml:109:15:109:42 | actionsdesk/lfs-warning@v3.2 | Unpinned 3rd party Action 'pr-workflow' step $@ uses 'actionsdesk/lfs-warning' with ref 'v3.2', not a pinned commit hash | .github/workflows/pr-workflow.yml:109:9:124:6 | Uses Step | Uses Step |

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

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -138,6 +138,8 @@ edges
138138
| .github/workflows/level0.yml:122:9:125:6 | Uses Step | .github/workflows/level0.yml:125:9:129:6 | Uses Step |
139139
| .github/workflows/level0.yml:125:9:129:6 | Uses Step | .github/workflows/level0.yml:129:9:133:6 | Uses Step |
140140
| .github/workflows/level0.yml:129:9:133:6 | Uses Step | .github/workflows/level0.yml:133:9:135:23 | Run Step |
141+
| .github/workflows/lockfile_pinned.yml:17:7:22:4 | Uses Step | .github/workflows/lockfile_pinned.yml:22:7:25:4 | Uses Step |
142+
| .github/workflows/lockfile_pinned.yml:22:7:25:4 | Uses Step | .github/workflows/lockfile_pinned.yml:25:7:25:43 | Uses Step |
141143
| .github/workflows/mend.yml:13:9:22:6 | Run Step: set_ref | .github/workflows/mend.yml:22:9:29:6 | Uses Step |
142144
| .github/workflows/mend.yml:22:9:29:6 | Uses Step | .github/workflows/mend.yml:29:9:33:28 | Uses Step |
143145
| .github/workflows/poc2.yml:28:9:37:6 | Uses Step: branch-deploy | .github/workflows/poc2.yml:37:9:42:6 | Uses Step |

actions/ql/test/query-tests/Security/CWE-829/pinned_by_lockfile.model.yml

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -8,3 +8,6 @@ extensions:
88
# query suppresses it.
99
data:
1010
- [".github/workflows/lockfile_pinned.yml", "some-owner/pinned-action", "v1"]
11+
# Lower-cased form of `Mixed-Owner/Pinned-Action@v1`, as the generator emits it. Exercises the
12+
# case-insensitive owner/repo match in the `actions/unpinned-tag` query.
13+
- [".github/workflows/lockfile_pinned.yml", "mixed-owner/pinned-action", "v1"]

0 commit comments

Comments
 (0)