Skip to content

Commit 059814e

Browse files
committed
actions/unpinned-tag: normalize lockfile action identity
1 parent 8c89f0e commit 059814e

10 files changed

Lines changed: 34 additions & 62 deletions

File tree

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1,4 +1,4 @@
11
---
22
category: minorAnalysis
33
---
4-
* Added a new extensible predicate `pinnedByLockfileDataModel(workflow_path, nwo, ref)`, which records `uses:` references that are pinned by a repository's Actions lockfile (`.github/workflows/actions.lock`). The CodeQL Actions extractor generates this data at database-creation time into a database-local model pack (`codeql/actions-lockfile-pins`); it has no effect unless that pack is supplied to analysis (for example via `--model-packs`), so behavior is unchanged for repositories without a lockfile or analyses that do not opt in.
4+
* Added a new extensible predicate `pinnedByLockfileDataModel(workflow_path, nwo, ref)`, which lets model packs record `uses:` references pinned by a repository's Actions lockfile (`.github/workflows/actions.lock`).

actions/ql/lib/codeql/actions/config/Config.qll

Lines changed: 11 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -137,15 +137,22 @@ predicate trustedActionsOwnerDataModel(string owner) {
137137

138138
/**
139139
* MaD models for `uses` references pinned by the repository's Actions lockfile
140-
* (`.github/workflows/actions.lock`). Populated by the CodeQL Actions extractor; see
141-
* `pinnedByLockfileDataModel` in `ConfigExtensions.qll`.
140+
* (`.github/workflows/actions.lock`).
142141
* Fields:
143142
* - workflow_path: repo-relative path of the file containing the `uses:` reference
144-
* - nwo: owner and name of the referenced action (e.g. `actions/checkout`)
143+
* - nwo: referenced action, optionally including a sub-action path (e.g. `actions/cache/save`)
145144
* - ref: the ref as written in `uses:` (e.g. `v4`)
146145
*/
146+
bindingset[nwo]
147147
predicate pinnedByLockfileDataModel(string workflow_path, string nwo, string ref) {
148-
Extensions::pinnedByLockfileDataModel(workflow_path, nwo, ref)
148+
exists(string pinnedNwo |
149+
Extensions::pinnedByLockfileDataModel(workflow_path, pinnedNwo, ref) and
150+
(
151+
nwo.toLowerCase() = pinnedNwo
152+
or
153+
nwo.toLowerCase().prefix(pinnedNwo.length() + 1) = pinnedNwo + "/"
154+
)
155+
)
149156
}
150157

151158
/**

actions/ql/lib/codeql/actions/config/ConfigExtensions.qll

Lines changed: 4 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -73,17 +73,14 @@ extensible predicate trustedActionsOwnerDataModel(string owner);
7373
* `workflow_path` is pinned by an entry in the repository's Actions lockfile
7474
* (`.github/workflows/actions.lock`).
7575
*
76-
* This predicate is intended to be populated by the CodeQL Actions extractor, which parses
77-
* `actions.lock` at database-creation time using the canonical lockfile parser at
78-
* `github.com/github/actions-lockfile/go`. Each lockfile entry binds an `nwo`@`ref` to a
79-
* verified commit SHA, which is exactly the pinning evidence the `actions/unpinned-tag` query
80-
* otherwise lacks. Until the extractor populates this predicate it is empty, so any clause that
81-
* consumes it is a clean no-op and behavior is unchanged for repositories without a lockfile.
76+
* Supply rows from a model pack generated with the canonical parser at
77+
* `github.com/github/actions-lockfile/go/pkg/lockfile`. Each lockfile entry binds an `nwo`@`ref`
78+
* to a verified commit SHA. Without such a model pack this predicate is empty.
8279
*
8380
* Fields:
8481
* - `workflow_path`: repo-relative path of the file containing the `uses:` reference,
8582
* e.g. `.github/workflows/ci.yml`.
86-
* - `nwo`: owner and name of the referenced action, e.g. `actions/checkout`.
83+
* - `nwo`: canonical owner and repository from the lockfile pin, e.g. `actions/cache`.
8784
* - `ref`: the ref (tag or branch) as written in `uses:`, e.g. `v4`.
8885
*/
8986
extensible predicate pinnedByLockfileDataModel(string workflow_path, string nwo, string ref);

actions/ql/lib/ext/config/pinned_by_lockfile.yml

Lines changed: 0 additions & 20 deletions
This file was deleted.

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

Lines changed: 1 addition & 16 deletions
Original file line numberDiff line numberDiff line change
@@ -39,21 +39,6 @@ private predicate isContainerImage(string nwo) { nwo.regexpMatch("^docker://.+")
3939
bindingset[nwo]
4040
private predicate isSelfRepository(string nwo) { nwo.matches("$/%") }
4141

42-
// Holds if `uses` (calling action `nwo` at `version`) is pinned by an entry in the repository's
43-
// Actions lockfile (`.github/workflows/actions.lock`). The underlying `pinnedByLockfileDataModel`
44-
// predicate is populated by the CodeQL Actions extractor when it parses the lockfile at
45-
// database-creation time; until then this is a clean no-op and no lockfile-pinned refs are
46-
// suppressed. See `pinnedByLockfileDataModel` in `ConfigExtensions.qll` for the intended shape.
47-
bindingset[nwo]
48-
private predicate pinnedByLockfile(UsesStep uses, string nwo, string version) {
49-
// The extractor populates this predicate with lower-cased owner/repo (GitHub treats
50-
// them 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)
55-
}
56-
5742
private predicate getStepContainerName(UsesStep uses, string name) {
5843
exists(Workflow workflow |
5944
uses.getEnclosingWorkflow() = workflow and
@@ -77,7 +62,7 @@ where
7762
uses.getVersion() = version and
7863
not isTrustedOwner(nwo) and
7964
not isSelfRepository(nwo) and
80-
not pinnedByLockfile(uses, nwo, version) and
65+
not pinnedByLockfileDataModel(uses.getLocation().getFile().getRelativePath(), nwo, version) and
8166
not (if isContainerImage(nwo) then isPinnedContainer(version) else isPinnedCommit(version)) and
8267
not isImmutableAction(uses, nwo)
8368
select uses.getCalleeNode(),
Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1,4 +1,4 @@
11
---
22
category: minorAnalysis
33
---
4-
* The `actions/unpinned-tag` query no longer reports `uses:` references that are recorded as pinned by a repository's Actions lockfile (`.github/workflows/actions.lock`) via the new `pinnedByLockfileDataModel` extensible predicate. The CodeQL Actions extractor generates this data at database-creation time into a database-local model pack (`codeql/actions-lockfile-pins`); references are suppressed only when that pack is supplied to analysis (for example via `--model-packs`), so behavior is unchanged for repositories without a lockfile or analyses that do not opt in. Because the lockfile keys its pins transitively by workflow path, a stale lockfile could in rare cases suppress a directly-written unpinned tag that shares an action and major version with a transitively-pinned dependency of the same workflow; keeping the lockfile current avoids this.
4+
* The `actions/unpinned-tag` query no longer reports `uses:` references recorded by the new `pinnedByLockfileDataModel` extensible predicate. Lockfile pins match repository sub-actions while preserving ref casing.

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

Lines changed: 9 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -9,17 +9,21 @@ jobs:
99
# `some-owner/pinned-action@v1` is a tag ref that would normally be reported as an unpinned
1010
# tag. The test data extension `pinned_by_lockfile.model.yml` records it as pinned by the
1111
# repository's Actions lockfile, so the `not pinnedByLockfile(...)` clause suppresses it (this
12-
# fixture is expected to produce no findings). This mirrors what the CodeQL Actions extractor
13-
# will do by parsing `.github/workflows/actions.lock`.
12+
# fixture is expected to produce no findings). This mirrors data supplied by an external
13+
# model pack generated from `.github/workflows/actions.lock`.
1414
#
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
1818
# `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 extractor 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).
19+
# owner/repo that authors commonly use (Azure, GoogleCloudPlatform, ...).
20+
# Lockfile pins canonicalize owner/repo, so this ref is also expected to be suppressed (no
21+
# finding).
2222
- uses: Mixed-Owner/Pinned-Action@v1
2323
# Negative control: a mixed-case ref that is NOT recorded in the lockfile data must still be
2424
# reported, guarding against over-suppression of every mixed-case ref.
2525
- uses: Mixed-Owner/Unpinned-Action@v2
26+
# A repository-scoped pin also covers its sub-actions.
27+
- uses: some-owner/pinned-action/save@v1
28+
# Ref casing must match the canonical lockfile pin exactly.
29+
- uses: some-owner/pinned-action@V1

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

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -17,7 +17,8 @@
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 |
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:27:4 | Uses Step | Uses Step |
21+
| .github/workflows/lockfile_pinned.yml:29:13:29:39 | some-owner/pinned-action@V1 | Unpinned 3rd party Action 'lockfile_pinned.yml' step $@ uses 'some-owner/pinned-action' with ref 'V1', not a pinned commit hash | .github/workflows/lockfile_pinned.yml:29:7:29:40 | Uses Step | Uses Step |
2122
| .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 |
2223
| .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 |
2324
| .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: 3 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -139,7 +139,9 @@ edges
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 |
141141
| .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 |
142+
| .github/workflows/lockfile_pinned.yml:22:7:25:4 | Uses Step | .github/workflows/lockfile_pinned.yml:25:7:27:4 | Uses Step |
143+
| .github/workflows/lockfile_pinned.yml:25:7:27:4 | Uses Step | .github/workflows/lockfile_pinned.yml:27:7:29:4 | Uses Step |
144+
| .github/workflows/lockfile_pinned.yml:27:7:29:4 | Uses Step | .github/workflows/lockfile_pinned.yml:29:7:29:40 | Uses Step |
143145
| .github/workflows/mend.yml:13:9:22:6 | Run Step: set_ref | .github/workflows/mend.yml:22:9:29:6 | Uses Step |
144146
| .github/workflows/mend.yml:22:9:29:6 | Uses Step | .github/workflows/mend.yml:29:9:33:28 | Uses Step |
145147
| .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: 2 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -2,12 +2,8 @@ extensions:
22
- addsTo:
33
pack: codeql/actions-all
44
extensible: pinnedByLockfileDataModel
5-
# Test data standing in for what the CodeQL Actions extractor will emit from
6-
# `.github/workflows/actions.lock`. Records `some-owner/pinned-action@v1` in
7-
# `lockfile_pinned.yml` as pinned by the lockfile so the `actions/unpinned-tag`
8-
# query suppresses it.
5+
# Canonical lockfile pin rows supplied by an external model pack.
96
data:
107
- [".github/workflows/lockfile_pinned.yml", "some-owner/pinned-action", "v1"]
11-
# Lower-cased form of `Mixed-Owner/Pinned-Action@v1`, as the extractor emits it. Exercises the
12-
# case-insensitive owner/repo match in the `actions/unpinned-tag` query.
8+
# Owner/repo is canonicalized; ref casing is preserved.
139
- [".github/workflows/lockfile_pinned.yml", "mixed-owner/pinned-action", "v1"]

0 commit comments

Comments
 (0)