diff --git a/actions/ql/lib/change-notes/2026-09-01-actions-lock-yaml.md b/actions/ql/lib/change-notes/2026-09-01-actions-lock-yaml.md index ed0a25515da0..0290350eae73 100644 --- a/actions/ql/lib/change-notes/2026-09-01-actions-lock-yaml.md +++ b/actions/ql/lib/change-notes/2026-09-01-actions-lock-yaml.md @@ -2,4 +2,4 @@ category: feature --- * GitHub Actions databases now extract `actions.lock` files. The new `ActionsLock` class - provides access to their YAML abstract syntax trees. + provides access to their YAML abstract syntax trees and structurally valid workflow pins. diff --git a/actions/ql/lib/codeql/actions/Lock.qll b/actions/ql/lib/codeql/actions/Lock.qll index 8fb8a8a8b359..020b097300db 100644 --- a/actions/ql/lib/codeql/actions/Lock.qll +++ b/actions/ql/lib/codeql/actions/Lock.qll @@ -2,9 +2,43 @@ * Provides classes for working with GitHub Actions lockfiles. */ +private import actions private import codeql.actions.ast.internal.Yaml -/** An `actions.lock` file. */ -class ActionsLock extends YamlDocument { - ActionsLock() { this.getFile().getBaseName() = "actions.lock" } +/** A `.github/workflows/actions.lock` file. */ +class ActionsLock extends YamlDocument, YamlMapping { + ActionsLock() { this.getFile().getRelativePath() = ".github/workflows/actions.lock" } + + private predicate pins0(string workflowPath, string pinnedNwo, string ref) { + exists(YamlSequence workflowPins, YamlScalar pinNode, YamlMapping dependency, string pin | + this.lookup("workflows").(YamlMapping).lookup(workflowPath) = workflowPins and + workflowPins.getElement(_) = pinNode and + pin = pinNode.getValue() and + pinnedNwo = pin.regexpCapture("^([^/@:]+/[^/@:]+)@([^:]+)$", 1) and + ref = pin.regexpCapture("^([^/@:]+/[^/@:]+)@([^:]+)$", 2) and + this.lookup("dependencies").(YamlMapping).lookup(pin) = dependency and + dependency.lookup("ref").(YamlScalar).getValue() = ref and + dependency + .lookup("commit") + .(YamlScalar) + .getValue() + .regexpMatch("^(sha1-[A-Fa-f0-9]{40}|sha256-[A-Fa-f0-9]{64})$") + ) + } + + /** + * Holds if this lockfile pins the use at `uses` to `ref` with a full commit digest. + * Repository pins also cover sub-actions such as `actions/cache/save`. + */ + predicate pins(UsesStep uses, string ref) { + exists(string workflowPath, string pinnedNwo, string nwo | + this.pins0(workflowPath, pinnedNwo, ref) and + workflowPath = uses.getLocation().getFile().getRelativePath() and + nwo = uses.getCallee() + | + nwo.toLowerCase() = pinnedNwo.toLowerCase() + or + nwo.toLowerCase().prefix(pinnedNwo.length() + 1) = pinnedNwo.toLowerCase() + "/" + ) + } } diff --git a/actions/ql/src/Security/CWE-829/UnpinnedActionsTag.ql b/actions/ql/src/Security/CWE-829/UnpinnedActionsTag.ql index f98439063712..8a1f821d073a 100644 --- a/actions/ql/src/Security/CWE-829/UnpinnedActionsTag.ql +++ b/actions/ql/src/Security/CWE-829/UnpinnedActionsTag.ql @@ -33,6 +33,12 @@ private predicate isPinnedContainer(string version) { bindingset[nwo] private predicate isContainerImage(string nwo) { nwo.regexpMatch("^docker://.+") } +// A `$/` reference is a same-repository (self repository) reference (e.g. `$/path/to/action`), +// resolved at the commit the calling workflow is running. Like `./` local (self workspace) +// references, it is inherently pinned and can never be an unpinned-tag finding, so we never flag it. +bindingset[nwo] +private predicate isSelfRepository(string nwo) { nwo.matches("$/%") } + private predicate hasUsesContainerName(Uses uses, string name) { exists(Workflow workflow | uses.getEnclosingWorkflow() = workflow and @@ -55,6 +61,8 @@ where hasUsesContainerName(uses, name) and uses.getVersion() = version and not isTrustedOwner(nwo) and + not isSelfRepository(nwo) and + not any(ActionsLock lock).pins(uses, version) and not ( if uses instanceof UsesStep and isContainerImage(nwo) then isPinnedContainer(version) diff --git a/actions/ql/src/change-notes/2026-07-09-unpinned-tag-lockfile-aware.md b/actions/ql/src/change-notes/2026-07-09-unpinned-tag-lockfile-aware.md new file mode 100644 index 000000000000..258850e3c52a --- /dev/null +++ b/actions/ql/src/change-notes/2026-07-09-unpinned-tag-lockfile-aware.md @@ -0,0 +1,4 @@ +--- +category: minorAnalysis +--- +* The `actions/unpinned-tag` query no longer reports action references pinned by a structurally valid `.github/workflows/actions.lock` entry for the enclosing workflow. diff --git a/actions/ql/src/change-notes/2026-07-09-unpinned-tag-self-repository.md b/actions/ql/src/change-notes/2026-07-09-unpinned-tag-self-repository.md new file mode 100644 index 000000000000..02e1eac704b2 --- /dev/null +++ b/actions/ql/src/change-notes/2026-07-09-unpinned-tag-self-repository.md @@ -0,0 +1,4 @@ +--- +category: minorAnalysis +--- +* 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. diff --git a/actions/ql/test/query-tests/Security/CWE-829-Lockfile/.github/workflows/actions.lock b/actions/ql/test/query-tests/Security/CWE-829-Lockfile/.github/workflows/actions.lock new file mode 100644 index 000000000000..8f42f6f8181f --- /dev/null +++ b/actions/ql/test/query-tests/Security/CWE-829-Lockfile/.github/workflows/actions.lock @@ -0,0 +1,30 @@ +version: future-version +workflows: + .github/workflows/rust-ci.yml: + - DToLnAy/RuSt-ToOlChAiN@v1 + - mismatched/action@v1 + - malformed/action@v1 + - missing/action@v1 + .github/workflows/other.yml: + - other-workflow/action@v1 +dependencies: + DToLnAy/RuSt-ToOlChAiN@v1: + ref: v1 + commit: sha1-6c977a6ca4077a0ceb28ffbe03f59d46e9ac8772 + owner_id: 1940490 + repo_id: 260749683 + other-workflow/action@v1: + ref: v1 + commit: sha1-1111111111111111111111111111111111111111 + owner_id: 1 + repo_id: 2 + mismatched/action@v1: + ref: V1 + commit: sha1-2222222222222222222222222222222222222222 + owner_id: 3 + repo_id: 4 + malformed/action@v1: + ref: v1 + commit: 6c977a6ca4077a0ceb28ffbe03f59d46e9ac8772 + owner_id: 5 + repo_id: 6 diff --git a/actions/ql/test/query-tests/Security/CWE-829-Lockfile/.github/workflows/rust-ci.yml b/actions/ql/test/query-tests/Security/CWE-829-Lockfile/.github/workflows/rust-ci.yml new file mode 100644 index 000000000000..3838b3589dc5 --- /dev/null +++ b/actions/ql/test/query-tests/Security/CWE-829-Lockfile/.github/workflows/rust-ci.yml @@ -0,0 +1,16 @@ +on: + pull_request + +jobs: + build: + runs-on: ubuntu-latest + steps: + - uses: dtolnay/rust-toolchain@v1 + - uses: DToLnAy/RuSt-ToOlChAiN/save@v1 + - uses: dtolnay/rust-toolchain@V1 # $ Alert + - uses: other-workflow/action@v1 # $ Alert + - uses: mismatched/action@v1 # $ Alert + - uses: malformed/action@v1 # $ Alert + - uses: missing/action@v1 # $ Alert + reusable: + uses: dtolnay/rust-toolchain/.github/workflows/reusable.yml@v1 # $ Alert diff --git a/actions/ql/test/query-tests/Security/CWE-829-Lockfile/UnpinnedActionsTag.expected b/actions/ql/test/query-tests/Security/CWE-829-Lockfile/UnpinnedActionsTag.expected new file mode 100644 index 000000000000..98fd715f4a81 --- /dev/null +++ b/actions/ql/test/query-tests/Security/CWE-829-Lockfile/UnpinnedActionsTag.expected @@ -0,0 +1,6 @@ +| .github/workflows/rust-ci.yml:10:13:10:37 | dtolnay/rust-toolchain@V1 | Unpinned 3rd party Action 'rust-ci.yml' step $@ uses 'dtolnay/rust-toolchain' with ref 'V1', not a pinned commit hash | .github/workflows/rust-ci.yml:10:7:11:4 | Uses Step | Uses Step | +| .github/workflows/rust-ci.yml:11:13:11:36 | other-workflow/action@v1 | Unpinned 3rd party Action 'rust-ci.yml' step $@ uses 'other-workflow/action' with ref 'v1', not a pinned commit hash | .github/workflows/rust-ci.yml:11:7:12:4 | Uses Step | Uses Step | +| .github/workflows/rust-ci.yml:12:13:12:32 | mismatched/action@v1 | Unpinned 3rd party Action 'rust-ci.yml' step $@ uses 'mismatched/action' with ref 'v1', not a pinned commit hash | .github/workflows/rust-ci.yml:12:7:13:4 | Uses Step | Uses Step | +| .github/workflows/rust-ci.yml:13:13:13:31 | malformed/action@v1 | Unpinned 3rd party Action 'rust-ci.yml' step $@ uses 'malformed/action' with ref 'v1', not a pinned commit hash | .github/workflows/rust-ci.yml:13:7:14:4 | Uses Step | Uses Step | +| .github/workflows/rust-ci.yml:14:13:14:29 | missing/action@v1 | Unpinned 3rd party Action 'rust-ci.yml' step $@ uses 'missing/action' with ref 'v1', not a pinned commit hash | .github/workflows/rust-ci.yml:14:7:15:2 | Uses Step | Uses Step | +| .github/workflows/rust-ci.yml:16:11:16:66 | dtolnay/rust-toolchain/.github/workflows/reusable.yml@v1 | Job $@ in 'rust-ci.yml' uses reusable workflow 'dtolnay/rust-toolchain/.github/workflows/reusable.yml' with ref 'v1', not a pinned commit hash | .github/workflows/rust-ci.yml:16:5:16:77 | Job: reusable | Job: reusable | diff --git a/actions/ql/test/query-tests/Security/CWE-829-Lockfile/UnpinnedActionsTag.qlref b/actions/ql/test/query-tests/Security/CWE-829-Lockfile/UnpinnedActionsTag.qlref new file mode 100644 index 000000000000..d33a5b380859 --- /dev/null +++ b/actions/ql/test/query-tests/Security/CWE-829-Lockfile/UnpinnedActionsTag.qlref @@ -0,0 +1,2 @@ +query: Security/CWE-829/UnpinnedActionsTag.ql +postprocess: utils/ActionsInlineExpectationsTestQuery.ql diff --git a/actions/ql/test/query-tests/Security/CWE-829-Lockfile/options b/actions/ql/test/query-tests/Security/CWE-829-Lockfile/options new file mode 100644 index 000000000000..af54d6532407 --- /dev/null +++ b/actions/ql/test/query-tests/Security/CWE-829-Lockfile/options @@ -0,0 +1 @@ +semmle-extractor-options: --file-type YAML .github/workflows/actions.lock diff --git a/actions/ql/test/query-tests/Security/CWE-829/.github/workflows/self_ref_dollar.yml b/actions/ql/test/query-tests/Security/CWE-829/.github/workflows/self_ref_dollar.yml new file mode 100644 index 000000000000..0d651b0bb0f6 --- /dev/null +++ b/actions/ql/test/query-tests/Security/CWE-829/.github/workflows/self_ref_dollar.yml @@ -0,0 +1,15 @@ +on: + pull_request + +jobs: + build: + name: Build and test + runs-on: ubuntu-latest + steps: + # `$/` is a same-repository (self repository) reference resolved at the running commit. It is + # inherently pinned (like `./` self workspace refs) and must never be reported as an unpinned tag. + - uses: $/actions/foo + # `$/…@ref` is rejected by the `$/` rule, but a user could still write it. It must also + # never be flagged; this case exercises the `not isSelfRepository(nwo)` suppression, since + # without it `$/actions/foo@v1` would otherwise be reported as an unpinned tag. + - uses: $/actions/foo@v1 diff --git a/actions/ql/test/query-tests/Security/CWE-829/UntrustedCheckoutCritical.expected b/actions/ql/test/query-tests/Security/CWE-829/UntrustedCheckoutCritical.expected index 32999ec70e27..381ca41243f1 100644 --- a/actions/ql/test/query-tests/Security/CWE-829/UntrustedCheckoutCritical.expected +++ b/actions/ql/test/query-tests/Security/CWE-829/UntrustedCheckoutCritical.expected @@ -241,6 +241,7 @@ edges | .github/workflows/resolve-args.yml:20:9:22:6 | Uses Step | .github/workflows/resolve-args.yml:22:9:36:13 | Run Step: resolve-step | | .github/workflows/reusable_local.yml:23:9:26:6 | Uses Step | .github/workflows/reusable_local.yml:26:9:29:7 | Run Step | | .github/workflows/reusable_local.yml:25:17:25:36 | inputs.branch | .github/workflows/reusable_local.yml:23:9:26:6 | Uses Step | +| .github/workflows/self_ref_dollar.yml:11:7:15:4 | Uses Step | .github/workflows/self_ref_dollar.yml:15:7:15:29 | Uses Step | | .github/workflows/test1.yml:18:9:21:6 | Uses Step | .github/workflows/test1.yml:21:9:24:6 | Run Step | | .github/workflows/test1.yml:21:9:24:6 | Run Step | .github/workflows/test1.yml:24:9:25:39 | Run Step | | .github/workflows/test2.yml:13:9:16:6 | Uses Step | .github/workflows/test2.yml:16:9:20:52 | Uses Step | diff --git a/actions/ql/test/utils/ActionsInlineExpectationsTestQuery.ql b/actions/ql/test/utils/ActionsInlineExpectationsTestQuery.ql new file mode 100644 index 000000000000..a67c200cf117 --- /dev/null +++ b/actions/ql/test/utils/ActionsInlineExpectationsTestQuery.ql @@ -0,0 +1,30 @@ +/** + * @kind test-postprocess + */ + +private import codeql.Locations as Locations +private import codeql.actions.ast.internal.Yaml as Yaml +private import codeql.util.test.InlineExpectationsTest as T +import T::TestPostProcessing + +private module Impl implements T::InlineExpectationsTestSig { + class Location = Locations::Location; + + class ExpectationComment extends Yaml::YamlComment { + string getContents() { result = this.getText() } + } +} + +private module Input implements T::TestPostProcessing::InputSig { + string getRelativeUrl(Locations::Location location) { + exists(int startLine, int startColumn, int endLine, int endColumn | + location.hasLocationInfo(_, startLine, startColumn, endLine, endColumn) + | + result = + location.getFile().getRelativePath() + ":" + startLine + ":" + startColumn + ":" + endLine + + ":" + endColumn + ) + } +} + +import T::TestPostProcessing::Make