Skip to content

Commit b6adb1a

Browse files
authored
Merge pull request #22155 from nodeselector/nodeselector-actions-lockfile-aware-pinning
Make actions/unpinned-tag lockfile- and $/-aware
2 parents b9cb90c + 85d5925 commit b6adb1a

16 files changed

Lines changed: 157 additions & 5 deletions

File tree

actions/ql/integration-tests/actions-lock/src/actions.lock renamed to actions/ql/integration-tests/actions-lock/src/.github/workflows/actions.lock

File renamed without changes.

actions/ql/lib/codeql/actions/Lock.qll

Lines changed: 38 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -2,9 +2,44 @@
22
* Provides classes for working with GitHub Actions lockfiles.
33
*/
44

5+
private import actions
56
private import codeql.actions.ast.internal.Yaml
67

7-
/** An `actions.lock` file. */
8-
class ActionsLock extends YamlDocument {
9-
ActionsLock() { this.getFile().getBaseName() = "actions.lock" }
8+
/** A `.github/workflows/actions.lock` file. */
9+
class ActionsLock extends YamlDocument, YamlMapping {
10+
ActionsLock() { this.getFile().getRelativePath() = ".github/workflows/actions.lock" }
11+
12+
pragma[nomagic]
13+
private predicate pins0(string workflowPath, string pinnedNwo, string ref) {
14+
exists(YamlSequence workflowPins, YamlScalar pinNode, YamlMapping dependency, string pin |
15+
this.lookup("workflows").(YamlMapping).lookup(workflowPath) = workflowPins and
16+
workflowPins.getElement(_) = pinNode and
17+
pin = pinNode.getValue() and
18+
pinnedNwo = pin.regexpCapture("^([^/@:]+/[^/@:]+)@([^:]+)$", 1) and
19+
ref = pin.regexpCapture("^([^/@:]+/[^/@:]+)@([^:]+)$", 2) and
20+
this.lookup("dependencies").(YamlMapping).lookup(pin) = dependency and
21+
dependency.lookup("ref").(YamlScalar).getValue() = ref and
22+
dependency
23+
.lookup("commit")
24+
.(YamlScalar)
25+
.getValue()
26+
.regexpMatch("^(sha1-[A-Fa-f0-9]{40}|sha256-[A-Fa-f0-9]{64})$")
27+
)
28+
}
29+
30+
/**
31+
* Holds if this lockfile pins the use at `uses` to `ref` with a full commit digest.
32+
* Repository pins also cover sub-actions such as `actions/cache/save`.
33+
*/
34+
predicate pins(UsesStep uses, string ref) {
35+
exists(string workflowPath, string pinnedNwo, string nwo |
36+
this.pins0(workflowPath, pinnedNwo, ref) and
37+
workflowPath = uses.getLocation().getFile().getRelativePath() and
38+
nwo = uses.getCallee()
39+
|
40+
nwo.toLowerCase() = pinnedNwo.toLowerCase()
41+
or
42+
nwo.toLowerCase().prefix(pinnedNwo.length() + 1) = pinnedNwo.toLowerCase() + "/"
43+
)
44+
}
1045
}

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

Lines changed: 8 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -33,6 +33,12 @@ private predicate isPinnedContainer(string version) {
3333
bindingset[nwo]
3434
private predicate isContainerImage(string nwo) { nwo.regexpMatch("^docker://.+") }
3535

36+
// A `$/` reference is a same-repository (self repository) reference (e.g. `$/path/to/action`),
37+
// resolved at the commit the calling workflow is running. Like `./` local (self workspace)
38+
// references, it is inherently pinned and can never be an unpinned-tag finding, so we never flag it.
39+
bindingset[nwo]
40+
private predicate isSelfRepository(string nwo) { nwo.matches("$/%") }
41+
3642
private predicate hasUsesContainerName(Uses uses, string name) {
3743
exists(Workflow workflow |
3844
uses.getEnclosingWorkflow() = workflow and
@@ -55,6 +61,8 @@ where
5561
hasUsesContainerName(uses, name) and
5662
uses.getVersion() = version and
5763
not isTrustedOwner(nwo) and
64+
not isSelfRepository(nwo) and
65+
not any(ActionsLock lock).pins(uses, version) and
5866
not (
5967
if uses instanceof UsesStep and isContainerImage(nwo)
6068
then isPinnedContainer(version)
Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,4 @@
1+
---
2+
category: minorAnalysis
3+
---
4+
* The `actions/unpinned-tag` query no longer reports action references pinned by a structurally valid `.github/workflows/actions.lock` entry for the enclosing workflow.
Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,4 @@
1+
---
2+
category: minorAnalysis
3+
---
4+
* The `actions/unpinned-tag` query no longer reports `$/` self repository references (e.g. `uses: $/path/to/action`), which resolve to the same repository at the running commit and are therefore inherently pinned, just like `./` self workspace (local) references.

actions/ql/test/library-tests/actions-lock/actions.lock renamed to actions/ql/test/library-tests/actions-lock/.github/workflows/actions.lock

File renamed without changes.
Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1 +1 @@
1-
semmle-extractor-options: actions.lock
1+
semmle-extractor-options: --file-type YAML .github/workflows/actions.lock
Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1 +1 @@
1-
| actions.lock:0:0:0:0 | actions.lock |
1+
| .github/workflows/actions.lock:0:0:0:0 | .github/workflows/actions.lock |
Lines changed: 30 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,30 @@
1+
version: future-version
2+
workflows:
3+
.github/workflows/rust-ci.yml:
4+
- DToLnAy/RuSt-ToOlChAiN@v1
5+
- mismatched/action@v1
6+
- malformed/action@v1
7+
- missing/action@v1
8+
.github/workflows/other.yml:
9+
- other-workflow/action@v1
10+
dependencies:
11+
DToLnAy/RuSt-ToOlChAiN@v1:
12+
ref: v1
13+
commit: sha1-6c977a6ca4077a0ceb28ffbe03f59d46e9ac8772
14+
owner_id: 1940490
15+
repo_id: 260749683
16+
other-workflow/action@v1:
17+
ref: v1
18+
commit: sha1-1111111111111111111111111111111111111111
19+
owner_id: 1
20+
repo_id: 2
21+
mismatched/action@v1:
22+
ref: V1
23+
commit: sha1-2222222222222222222222222222222222222222
24+
owner_id: 3
25+
repo_id: 4
26+
malformed/action@v1:
27+
ref: v1
28+
commit: 6c977a6ca4077a0ceb28ffbe03f59d46e9ac8772
29+
owner_id: 5
30+
repo_id: 6
Lines changed: 16 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,16 @@
1+
on:
2+
pull_request
3+
4+
jobs:
5+
build:
6+
runs-on: ubuntu-latest
7+
steps:
8+
- uses: dtolnay/rust-toolchain@v1
9+
- uses: DToLnAy/RuSt-ToOlChAiN/save@v1
10+
- uses: dtolnay/rust-toolchain@V1 # $ Alert
11+
- uses: other-workflow/action@v1 # $ Alert
12+
- uses: mismatched/action@v1 # $ Alert
13+
- uses: malformed/action@v1 # $ Alert
14+
- uses: missing/action@v1 # $ Alert
15+
reusable:
16+
uses: dtolnay/rust-toolchain/.github/workflows/reusable.yml@v1 # $ Alert

0 commit comments

Comments
 (0)