From dc76ee9284f65752e1ed1531106743afbdefa642 Mon Sep 17 00:00:00 2001 From: Jeff Martin Date: Thu, 9 Jul 2026 19:17:09 -0500 Subject: [PATCH 01/11] actions: stop unpinned-tag flagging $/ self-references A $/ reference (e.g. "uses: $/path/to/action") is a same-repo self-reference that resolves to the commit the workflow is running at. It is inherently pinned, exactly like a "./" local reference, so it must never be reported by actions/unpinned-tag. Adds an isSelfReference(nwo) guard to the query plus a test fixture covering the bare "$/actions/foo" form and the "$/actions/foo@v1" form (the latter is rejected by the $/ rule but writable by a user; the guard suppresses it either way). Part of github/actions-dispatch#755. --- .../ql/src/Security/CWE-829/UnpinnedActionsTag.ql | 7 +++++++ .../2026-07-09-unpinned-tag-self-reference.md | 4 ++++ .../CWE-829/.github/workflows/self_ref_dollar.yml | 15 +++++++++++++++ 3 files changed, 26 insertions(+) create mode 100644 actions/ql/src/change-notes/2026-07-09-unpinned-tag-self-reference.md create mode 100644 actions/ql/test/query-tests/Security/CWE-829/.github/workflows/self_ref_dollar.yml diff --git a/actions/ql/src/Security/CWE-829/UnpinnedActionsTag.ql b/actions/ql/src/Security/CWE-829/UnpinnedActionsTag.ql index 530b8e48e0f5..9f994c59a643 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-repo self-reference (e.g. `$/path/to/action`), resolved at the +// commit the calling workflow is running. Like `./` local references, it is inherently pinned +// and can never be an unpinned-tag finding, so we never flag it. +bindingset[nwo] +private predicate isSelfReference(string nwo) { nwo.matches("$/%") } + private predicate getStepContainerName(UsesStep uses, string name) { exists(Workflow workflow | uses.getEnclosingWorkflow() = workflow and @@ -55,6 +61,7 @@ where getStepContainerName(uses, name) and uses.getVersion() = version and not isTrustedOwner(nwo) and + not isSelfReference(nwo) and not (if isContainerImage(nwo) then isPinnedContainer(version) else isPinnedCommit(version)) and not isImmutableAction(uses, nwo) select uses.getCalleeNode(), diff --git a/actions/ql/src/change-notes/2026-07-09-unpinned-tag-self-reference.md b/actions/ql/src/change-notes/2026-07-09-unpinned-tag-self-reference.md new file mode 100644 index 000000000000..c972741f1e7b --- /dev/null +++ b/actions/ql/src/change-notes/2026-07-09-unpinned-tag-self-reference.md @@ -0,0 +1,4 @@ +--- +category: minorAnalysis +--- +* The `actions/unpinned-tag` query no longer reports `$/` same-repo self-references (e.g. `uses: $/path/to/action`), which are inherently pinned to the running commit, just like `./` local references. 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..0c1275d35764 --- /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-repo self-reference resolved at the running commit. It is inherently + # pinned (like `./` local 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 isSelfReference(nwo)` suppression, since + # without it `$/actions/foo@v1` would otherwise be reported as an unpinned tag. + - uses: $/actions/foo@v1 From 5088f9f3ad164c7812aed8920f706ff4477cc80f Mon Sep 17 00:00:00 2001 From: Jeff Martin Date: Thu, 9 Jul 2026 19:17:42 -0500 Subject: [PATCH 02/11] actions: scaffold lockfile-aware pinning for unpinned-tag Adds the seam for making actions/unpinned-tag aware of a repository's Actions lockfile (.github/workflows/actions.lock), so that a tag ref bound to a verified commit in the lockfile is not reported as unpinned (Option A from the #755 spike). Introduces the extensible predicate pinnedByLockfileDataModel(workflow_path, nwo, ref) in ConfigExtensions.qll, re-exported through Config.qll, with a data-extension stub in ext/config/pinned_by_lockfile.yml documenting the intended row shape. The query gains a "not pinnedByLockfile(...)" clause keyed on the workflow file's relative path. The predicate is meant to be populated by the CodeQL Actions extractor, which must parse actions.lock at database-creation time using the canonical parser github.com/github/actions-lockfile/go. That extractor work is a separate change and is not implemented here; until it ships the predicate is empty and the new clause is a no-op. A test-scoped data extension exercises the clause end to end. Part of github/actions-dispatch#755. --- .../2026-07-09-pinned-by-lockfile.md | 4 ++++ .../ql/lib/codeql/actions/config/Config.qll | 13 ++++++++++++ .../actions/config/ConfigExtensions.qll | 20 +++++++++++++++++++ .../ql/lib/ext/config/pinned_by_lockfile.yml | 20 +++++++++++++++++++ .../Security/CWE-829/UnpinnedActionsTag.ql | 10 ++++++++++ .../2026-07-09-unpinned-tag-lockfile-aware.md | 4 ++++ actions/ql/test/qlpack.yml | 2 ++ .../.github/workflows/lockfile_pinned.yml | 17 ++++++++++++++++ .../CWE-829/pinned_by_lockfile.model.yml | 10 ++++++++++ 9 files changed, 100 insertions(+) create mode 100644 actions/ql/lib/change-notes/2026-07-09-pinned-by-lockfile.md create mode 100644 actions/ql/lib/ext/config/pinned_by_lockfile.yml create mode 100644 actions/ql/src/change-notes/2026-07-09-unpinned-tag-lockfile-aware.md create mode 100644 actions/ql/test/query-tests/Security/CWE-829/.github/workflows/lockfile_pinned.yml create mode 100644 actions/ql/test/query-tests/Security/CWE-829/pinned_by_lockfile.model.yml diff --git a/actions/ql/lib/change-notes/2026-07-09-pinned-by-lockfile.md b/actions/ql/lib/change-notes/2026-07-09-pinned-by-lockfile.md new file mode 100644 index 000000000000..d9b09e27ab81 --- /dev/null +++ b/actions/ql/lib/change-notes/2026-07-09-pinned-by-lockfile.md @@ -0,0 +1,4 @@ +--- +category: minorAnalysis +--- +* 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`). It is intended to be populated by the CodeQL Actions extractor and is currently unpopulated, so it has no effect until that support ships. diff --git a/actions/ql/lib/codeql/actions/config/Config.qll b/actions/ql/lib/codeql/actions/config/Config.qll index e6359c142582..adfee4c6aa8f 100644 --- a/actions/ql/lib/codeql/actions/config/Config.qll +++ b/actions/ql/lib/codeql/actions/config/Config.qll @@ -135,6 +135,19 @@ predicate trustedActionsOwnerDataModel(string owner) { Extensions::trustedActionsOwnerDataModel(owner) } +/** + * MaD models for `uses` references pinned by the repository's Actions lockfile + * (`.github/workflows/actions.lock`). Populated by the CodeQL Actions extractor; see + * `pinnedByLockfileDataModel` in `ConfigExtensions.qll`. + * Fields: + * - workflow_path: repo-relative path of the file containing the `uses:` reference + * - nwo: owner and name of the referenced action (e.g. `actions/checkout`) + * - ref: the ref as written in `uses:` (e.g. `v4`) + */ +predicate pinnedByLockfileDataModel(string workflow_path, string nwo, string ref) { + Extensions::pinnedByLockfileDataModel(workflow_path, nwo, ref) +} + /** * MaD models for untrusted git commands * Fields: diff --git a/actions/ql/lib/codeql/actions/config/ConfigExtensions.qll b/actions/ql/lib/codeql/actions/config/ConfigExtensions.qll index 87a919359404..102e14166769 100644 --- a/actions/ql/lib/codeql/actions/config/ConfigExtensions.qll +++ b/actions/ql/lib/codeql/actions/config/ConfigExtensions.qll @@ -68,6 +68,26 @@ extensible predicate immutableActionsDataModel(string action); */ extensible predicate trustedActionsOwnerDataModel(string owner); +/** + * Holds if the `uses` reference `nwo`@`ref` in the workflow or composite action file at + * `workflow_path` is pinned by an entry in the repository's Actions lockfile + * (`.github/workflows/actions.lock`). + * + * This predicate is intended to be populated by the CodeQL Actions extractor, which parses + * `actions.lock` at database-creation time using the canonical lockfile parser at + * `github.com/github/actions-lockfile/go`. Each lockfile entry binds an `nwo`@`ref` to a + * verified commit SHA, which is exactly the pinning evidence the `actions/unpinned-tag` query + * otherwise lacks. Until the extractor populates this predicate it is empty, so any clause that + * consumes it is a clean no-op and behaviour is unchanged for repositories without a lockfile. + * + * Fields: + * - `workflow_path`: repo-relative path of the file containing the `uses:` reference, + * e.g. `.github/workflows/ci.yml`. + * - `nwo`: owner and name of the referenced action, e.g. `actions/checkout`. + * - `ref`: the ref (tag or branch) as written in `uses:`, e.g. `v4`. + */ +extensible predicate pinnedByLockfileDataModel(string workflow_path, string nwo, string ref); + /** * Holds for git commands that may introduce untrusted data when called on an attacker controlled branch. */ diff --git a/actions/ql/lib/ext/config/pinned_by_lockfile.yml b/actions/ql/lib/ext/config/pinned_by_lockfile.yml new file mode 100644 index 000000000000..d2b3ad2f3977 --- /dev/null +++ b/actions/ql/lib/ext/config/pinned_by_lockfile.yml @@ -0,0 +1,20 @@ +extensions: + - addsTo: + pack: codeql/actions-all + extensible: pinnedByLockfileDataModel + # `pinnedByLockfileDataModel` records `uses:` references that are pinned by the repository's + # Actions lockfile (`.github/workflows/actions.lock`). It is intended to be populated by the + # CodeQL Actions extractor, which parses the lockfile at database-creation time using the + # canonical Go parser at `github.com/github/actions-lockfile/go`. Each row is + # `[workflow_path, nwo, ref]`: + # - workflow_path: repo-relative path of the file containing the `uses:` reference + # - nwo: owner/name of the referenced action (e.g. `actions/checkout`) + # - ref: the ref as written in `uses:` (e.g. `v4`) + # + # Example of the intended shape (commented out; real data is supplied by the extractor): + # - [".github/workflows/ci.yml", "actions/checkout", "v4"] + # + # Until the extractor populates this predicate it stays empty, so the + # `not pinnedByLockfile(...)` clause in `actions/unpinned-tag` is a no-op and behaviour is + # unchanged for repositories without a lockfile. + data: [] diff --git a/actions/ql/src/Security/CWE-829/UnpinnedActionsTag.ql b/actions/ql/src/Security/CWE-829/UnpinnedActionsTag.ql index 9f994c59a643..55f2ce6ecf0b 100644 --- a/actions/ql/src/Security/CWE-829/UnpinnedActionsTag.ql +++ b/actions/ql/src/Security/CWE-829/UnpinnedActionsTag.ql @@ -39,6 +39,15 @@ private predicate isContainerImage(string nwo) { nwo.regexpMatch("^docker://.+") bindingset[nwo] private predicate isSelfReference(string nwo) { nwo.matches("$/%") } +// Holds if `uses` (calling action `nwo` at `version`) is pinned by an entry in the repository's +// Actions lockfile (`.github/workflows/actions.lock`). The underlying `pinnedByLockfileDataModel` +// predicate is populated by the CodeQL Actions extractor when it parses the lockfile at +// database-creation time; until then this is a clean no-op and no lockfile-pinned refs are +// suppressed. See `pinnedByLockfileDataModel` in `ConfigExtensions.qll` for the intended shape. +private predicate pinnedByLockfile(UsesStep uses, string nwo, string version) { + pinnedByLockfileDataModel(uses.getLocation().getFile().getRelativePath(), nwo, version) +} + private predicate getStepContainerName(UsesStep uses, string name) { exists(Workflow workflow | uses.getEnclosingWorkflow() = workflow and @@ -62,6 +71,7 @@ where uses.getVersion() = version and not isTrustedOwner(nwo) and not isSelfReference(nwo) and + not pinnedByLockfile(uses, nwo, version) and not (if isContainerImage(nwo) then isPinnedContainer(version) else isPinnedCommit(version)) and not isImmutableAction(uses, nwo) select uses.getCalleeNode(), 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..beae154ff21d --- /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 `uses:` references that are recorded as pinned by a repository's Actions lockfile (`.github/workflows/actions.lock`) via the new `pinnedByLockfileDataModel` extensible predicate. This predicate is populated by the CodeQL Actions extractor and has no effect until that support ships. diff --git a/actions/ql/test/qlpack.yml b/actions/ql/test/qlpack.yml index 139e8e57c62e..ee46cee4c9af 100644 --- a/actions/ql/test/qlpack.yml +++ b/actions/ql/test/qlpack.yml @@ -9,4 +9,6 @@ dependencies: codeql/immutable-actions-list: ${workspace} extractor: actions tests: . +dataExtensions: + - query-tests/Security/CWE-829/*.model.yml warnOnImplicitThis: true diff --git a/actions/ql/test/query-tests/Security/CWE-829/.github/workflows/lockfile_pinned.yml b/actions/ql/test/query-tests/Security/CWE-829/.github/workflows/lockfile_pinned.yml new file mode 100644 index 000000000000..efc5c986a3b9 --- /dev/null +++ b/actions/ql/test/query-tests/Security/CWE-829/.github/workflows/lockfile_pinned.yml @@ -0,0 +1,17 @@ +on: + pull_request + +jobs: + build: + name: Build and test + runs-on: ubuntu-latest + steps: + # `some-owner/pinned-action@v1` is a tag ref that would normally be reported as an unpinned + # tag. The test data extension `pinned_by_lockfile.model.yml` records it as pinned by the + # repository's Actions lockfile, so the `not pinnedByLockfile(...)` clause suppresses it (this + # fixture is expected to produce no findings). This mirrors what the CodeQL Actions extractor + # will do by parsing `.github/workflows/actions.lock`. + # + # Negative control is provided for free by the many other fixtures in this directory whose tag + # refs are NOT recorded in the data extension and therefore remain reported. + - uses: some-owner/pinned-action@v1 diff --git a/actions/ql/test/query-tests/Security/CWE-829/pinned_by_lockfile.model.yml b/actions/ql/test/query-tests/Security/CWE-829/pinned_by_lockfile.model.yml new file mode 100644 index 000000000000..4853fc5a265e --- /dev/null +++ b/actions/ql/test/query-tests/Security/CWE-829/pinned_by_lockfile.model.yml @@ -0,0 +1,10 @@ +extensions: + - addsTo: + pack: codeql/actions-all + extensible: pinnedByLockfileDataModel + # Test data standing in for what the CodeQL Actions extractor will emit from + # `.github/workflows/actions.lock`. Records `some-owner/pinned-action@v1` in + # `lockfile_pinned.yml` as pinned by the lockfile so the `actions/unpinned-tag` + # query suppresses it. + data: + - [".github/workflows/lockfile_pinned.yml", "some-owner/pinned-action", "v1"] From 88c36ab6cfab1ae865c9cd7ef284b628d8fef23b Mon Sep 17 00:00:00 2001 From: Jeff Martin Date: Thu, 9 Jul 2026 19:55:44 -0500 Subject: [PATCH 03/11] actions: regenerate unpinned-tag test expected output Adding the self_ref_dollar.yml fixture introduces one new step-adjacency edge in the shared CWE-829 test database, which the UntrustedCheckoutCritical query dumps under its edges relation. Regenerate the expected output to include it. No new security findings: the $/ self-references and the lockfile-pinned ref correctly produce no unpinned-tag results. --- .../Security/CWE-829/UntrustedCheckoutCritical.expected | 1 + 1 file changed, 1 insertion(+) 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 b6c349bd64fe..998159dba7ed 100644 --- a/actions/ql/test/query-tests/Security/CWE-829/UntrustedCheckoutCritical.expected +++ b/actions/ql/test/query-tests/Security/CWE-829/UntrustedCheckoutCritical.expected @@ -196,6 +196,7 @@ edges | .github/workflows/resolve-args.yml:20:9:22:6 | Uses Step | .github/actions/download-artifact/action.yaml:6:7:25:4 | Uses Step | | .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/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 | From 75734f779843a2c66357f5d24210fa66c5f62045 Mon Sep 17 00:00:00 2001 From: Jeff Martin Date: Thu, 9 Jul 2026 20:52:36 -0500 Subject: [PATCH 04/11] actions: add lockfile-to-data-extension generator for unpinned-tag Add a Go tool that parses a repository's Actions lockfile (.github/workflows/actions.lock) with the canonical parser at github.com/github/actions-lockfile/go and emits a CodeQL data extension populating pinnedByLockfileDataModel, the predicate the actions/unpinned-tag query already consumes to suppress lockfile-pinned refs. The generator is transport-agnostic: it produces the same [workflow_path, nwo, ref] rows whether they ship as a model pack applied via --model-packs (as today, mirroring codeql/immutable-actions-list) or later feed an extractor-native relation, so the parsing core is reusable without touching the query. Lockfiles record the resolved ref (e.g. v4.3.1) while workflows usually write a shorter mutable tag (v4). Since the query matches the ref as written, the generator expands every full-semver resolved ref into its major.minor and major-only forms, so uses: owner/action@v4 is recognized as pinned by a v4.3.1 lockfile entry. Verified end to end against a synthetic repo: the lockfile-pinned short-tag ref is suppressed while unlocked refs still report. actions-lockfile is not yet public, so go.mod carries a local replace directive for building and testing; remove it once the module is published. --- .../lockfile-extension-generator/.gitignore | 1 + .../lockfile-extension-generator/README.md | 75 +++++++++++ .../lockfile-extension-generator/generator.go | 117 ++++++++++++++++++ .../generator_test.go | 91 ++++++++++++++ .../tools/lockfile-extension-generator/go.mod | 16 +++ .../tools/lockfile-extension-generator/go.sum | 10 ++ .../lockfile-extension-generator/main.go | 57 +++++++++ .../testdata/actions.lock | 19 +++ .../testdata/expected.yml | 16 +++ 9 files changed, 402 insertions(+) create mode 100644 actions/extractor/tools/lockfile-extension-generator/.gitignore create mode 100644 actions/extractor/tools/lockfile-extension-generator/README.md create mode 100644 actions/extractor/tools/lockfile-extension-generator/generator.go create mode 100644 actions/extractor/tools/lockfile-extension-generator/generator_test.go create mode 100644 actions/extractor/tools/lockfile-extension-generator/go.mod create mode 100644 actions/extractor/tools/lockfile-extension-generator/go.sum create mode 100644 actions/extractor/tools/lockfile-extension-generator/main.go create mode 100644 actions/extractor/tools/lockfile-extension-generator/testdata/actions.lock create mode 100644 actions/extractor/tools/lockfile-extension-generator/testdata/expected.yml diff --git a/actions/extractor/tools/lockfile-extension-generator/.gitignore b/actions/extractor/tools/lockfile-extension-generator/.gitignore new file mode 100644 index 000000000000..554d32ee98ca --- /dev/null +++ b/actions/extractor/tools/lockfile-extension-generator/.gitignore @@ -0,0 +1 @@ +/lockfile-extension-generator diff --git a/actions/extractor/tools/lockfile-extension-generator/README.md b/actions/extractor/tools/lockfile-extension-generator/README.md new file mode 100644 index 000000000000..99ce1cf07b91 --- /dev/null +++ b/actions/extractor/tools/lockfile-extension-generator/README.md @@ -0,0 +1,75 @@ +# lockfile-extension-generator + +Turns a repository's GitHub Actions lockfile (`.github/workflows/actions.lock`) +into a CodeQL data extension that populates the `pinnedByLockfileDataModel` +extensible predicate consumed by the `actions/unpinned-tag` +(`js/actions/actions-workflow-unpinned-tag`) query. + +## Why + +`actions/unpinned-tag` flags `uses:` references pinned to a mutable tag (e.g. +`actions/checkout@v4`) rather than an immutable commit SHA. When a repository +maintains an Actions lockfile, every such tag is already bound to a verified +commit SHA at run time by the lockfile — which is exactly the pinning evidence +the query otherwise lacks. Feeding the lockfile's bindings into +`pinnedByLockfileDataModel` lets the query suppress those references while still +flagging genuinely unpinned ones. + +The generator is deliberately **transport-agnostic**. It parses the lockfile +with the canonical parser at `github.com/github/actions-lockfile/go` and emits +the `[workflow_path, nwo, ref]` rows the query already matches on. Today those +rows ship as a data-extension model pack applied at analysis time via +`--model-packs` (the same mechanism as `codeql/immutable-actions-list`). The +same parsing core can later feed an extractor-native relation without changing +the query. + +## Ref normalization + +A lockfile records the *resolved* ref for each dependency — the CLI prefers a +full semver tag such as `v4.3.1`. A workflow author, however, usually writes a +shorter, mutable tag such as `v4` or `v4.3` in `uses:`, and the query matches on +the ref exactly as written. For every full-semver resolved ref the generator +therefore also emits its major.minor (`v4.3`) and major-only (`v4`) forms, so a +`uses: owner/action@v4` is recognized as pinned by a lockfile entry that +resolved to `v4.3.1`. Partial tags, pre-release tags, branches, and SHAs pass +through unchanged. + +## Usage + +``` +lockfile-extension-generator [output-file] +``` + +- ``: repository root to scan; the lockfile is read from + `/.github/workflows/actions.lock`. +- `[output-file]`: destination for the extension YAML; defaults to stdout. + +If the repository has no lockfile the generator exits successfully without +writing anything, so it is safe to run unconditionally. + +## Building and testing locally + +`github.com/github/actions-lockfile` is not yet public, so the module cannot be +fetched by the module proxy. Point the `replace` directive in `go.mod` at a +local clone before building or testing: + +``` +go mod edit -replace github.com/github/actions-lockfile/go=/path/to/actions-lockfile/go +go test ./... +``` + +Remove the `replace` directive once actions-lockfile is published. + +## End-to-end check + +``` +# A repo whose workflow writes `uses: owner/action@v4` while the lockfile +# resolves it to v4.3.1: +lockfile-extension-generator /path/to/repo /tmp/pack/ext/pinned.yml +codeql database analyze \ + codeql/actions-queries:Security/CWE-829/UnpinnedActionsTag.ql \ + --additional-packs=/tmp/pack --model-packs= +``` + +The lockfile-pinned reference is suppressed; references not covered by the +lockfile are still reported. diff --git a/actions/extractor/tools/lockfile-extension-generator/generator.go b/actions/extractor/tools/lockfile-extension-generator/generator.go new file mode 100644 index 000000000000..f7b0a43495f8 --- /dev/null +++ b/actions/extractor/tools/lockfile-extension-generator/generator.go @@ -0,0 +1,117 @@ +// Package main implements a generator that turns a repository's Actions lockfile +// (`.github/workflows/actions.lock`) into a CodeQL data extension populating the +// `pinnedByLockfileDataModel` extensible predicate consumed by the +// `actions/unpinned-tag` query. +// +// The generator is deliberately transport-agnostic: it parses the lockfile with +// the canonical parser at `github.com/github/actions-lockfile/go` and emits the +// same `[workflow_path, nwo, ref]` rows the query already matches on. Today those +// rows are shipped as a data-extension model pack (applied at analysis time via +// `--model-packs`, exactly like `codeql/immutable-actions-list`). The same parsing +// core can later feed an extractor-native TRAP relation without changing the query. +package main + +import ( + "fmt" + "sort" + "strings" + + "github.com/github/actions-lockfile/go/pkg/lockfile" +) + +// row is a single `pinnedByLockfileDataModel` tuple: the reference +// `nwo`@`ref` in the workflow or composite action file at `workflowPath` is +// pinned by the repository's Actions lockfile. +type row struct { + workflowPath string + nwo string + ref string +} + +// rowsFromLockfile parses lockfile `contents` and returns the deduplicated, +// sorted set of `pinnedByLockfileDataModel` rows it implies. +// +// A lockfile records the *resolved* ref for each dependency (the CLI prefers a +// full semver tag such as `v4.3.1`), but a workflow author usually writes a +// shorter, mutable tag such as `v4` or `v4.3` in `uses:`. The query matches on +// the ref exactly as written, so for every full-semver resolved ref we also +// emit its major-only (`v4`) and major.minor (`v4.3`) forms. This lets a +// `uses: owner/action@v4` be recognised as pinned by a lockfile entry that +// resolved to `v4.3.1`, which is the common real-world case. +func rowsFromLockfile(contents []byte) ([]row, error) { + f, err := lockfile.Parse(contents) + if err != nil { + return nil, fmt.Errorf("parsing lockfile: %w", err) + } + + seen := make(map[row]struct{}) + for workflowPath, pinKeys := range f.Workflows { + for _, key := range pinKeys { + pin, ok := lockfile.ParsePin(key) + if !ok { + continue + } + // Prefer the resolved ref recorded in the dependency metadata; fall + // back to the ref embedded in the pin key. + ref := pin.Ref + if dep, ok := f.Dependencies[key]; ok && dep.Ref != "" { + ref = dep.Ref + } + for _, r := range refVariants(ref) { + seen[row{workflowPath: workflowPath, nwo: pin.NWO, ref: r}] = struct{}{} + } + } + } + + rows := make([]row, 0, len(seen)) + for r := range seen { + rows = append(rows, r) + } + sort.Slice(rows, func(i, j int) bool { + if rows[i].workflowPath != rows[j].workflowPath { + return rows[i].workflowPath < rows[j].workflowPath + } + if rows[i].nwo != rows[j].nwo { + return rows[i].nwo < rows[j].nwo + } + return rows[i].ref < rows[j].ref + }) + return rows, nil +} + +// refVariants returns every ref string a workflow author might have written in +// `uses:` that the resolved `ref` covers. For a full semver tag (e.g. `v4.3.1`) +// this is the tag itself plus its major.minor (`v4.3`) and major-only (`v4`) +// forms; for anything else (a branch, a partial tag, a SHA) it is just the ref. +func refVariants(ref string) []string { + sv, ok := lockfile.ParseSemVer(ref) + if !ok || !sv.IsFull() { + return []string{ref} + } + variants := []string{ref} + if minor := sv.MinorTag(); minor != ref { + variants = append(variants, minor) + } + if major := sv.MajorTag(); major != ref { + variants = append(variants, major) + } + return variants +} + +// renderExtension serialises `rows` as a CodeQL data-extension YAML document that +// adds to the `pinnedByLockfileDataModel` extensible predicate in +// `codeql/actions-all`. +func renderExtension(rows []row) string { + var b strings.Builder + b.WriteString("# Generated by lockfile-extension-generator from .github/workflows/actions.lock.\n") + b.WriteString("# Do not edit by hand; regenerate from the repository's Actions lockfile.\n") + b.WriteString("extensions:\n") + b.WriteString(" - addsTo:\n") + b.WriteString(" pack: codeql/actions-all\n") + b.WriteString(" extensible: pinnedByLockfileDataModel\n") + b.WriteString(" data:\n") + for _, r := range rows { + fmt.Fprintf(&b, " - [%q, %q, %q]\n", r.workflowPath, r.nwo, r.ref) + } + return b.String() +} diff --git a/actions/extractor/tools/lockfile-extension-generator/generator_test.go b/actions/extractor/tools/lockfile-extension-generator/generator_test.go new file mode 100644 index 000000000000..512a6679dc47 --- /dev/null +++ b/actions/extractor/tools/lockfile-extension-generator/generator_test.go @@ -0,0 +1,91 @@ +package main + +import ( + "os" + "testing" +) + +func TestRowsFromLockfile(t *testing.T) { + contents, err := os.ReadFile("testdata/actions.lock") + if err != nil { + t.Fatalf("reading fixture: %v", err) + } + + rows, err := rowsFromLockfile(contents) + if err != nil { + t.Fatalf("rowsFromLockfile: %v", err) + } + + // Every full-semver resolved ref expands to {raw, minor, major} so a + // workflow that writes a shorter tag in `uses:` still matches. Rows are + // sorted by (workflowPath, nwo, ref). + want := []row{ + {".github/workflows/ci.yml", "actions/checkout", "v4"}, + {".github/workflows/ci.yml", "actions/checkout", "v4.3"}, + {".github/workflows/ci.yml", "actions/checkout", "v4.3.1"}, + {".github/workflows/ci.yml", "some-owner/pinned-action", "v1"}, + {".github/workflows/ci.yml", "some-owner/pinned-action", "v1.2"}, + {".github/workflows/ci.yml", "some-owner/pinned-action", "v1.2.0"}, + {".github/workflows/release.yml", "actions/checkout", "v4"}, + {".github/workflows/release.yml", "actions/checkout", "v4.3"}, + {".github/workflows/release.yml", "actions/checkout", "v4.3.1"}, + } + + if len(rows) != len(want) { + t.Fatalf("got %d rows, want %d:\n%#v", len(rows), len(want), rows) + } + for i := range want { + if rows[i] != want[i] { + t.Errorf("row %d: got %+v, want %+v", i, rows[i], want[i]) + } + } +} + +func TestRefVariants(t *testing.T) { + cases := []struct { + ref string + want []string + }{ + {"v4.3.1", []string{"v4.3.1", "v4.3", "v4"}}, + {"v1.0.0", []string{"v1.0.0", "v1.0", "v1"}}, + // Partial tags are already their own broadest form. + {"v4", []string{"v4"}}, + {"v4.3", []string{"v4.3"}}, + // Pre-release and non-semver refs pass through untouched. + {"v2.0.0-beta.1", []string{"v2.0.0-beta.1"}}, + {"main", []string{"main"}}, + } + for _, c := range cases { + got := refVariants(c.ref) + if len(got) != len(c.want) { + t.Errorf("refVariants(%q) = %v, want %v", c.ref, got, c.want) + continue + } + for i := range c.want { + if got[i] != c.want[i] { + t.Errorf("refVariants(%q) = %v, want %v", c.ref, got, c.want) + break + } + } + } +} + +func TestRenderExtensionMatchesGolden(t *testing.T) { + contents, err := os.ReadFile("testdata/actions.lock") + if err != nil { + t.Fatalf("reading fixture: %v", err) + } + rows, err := rowsFromLockfile(contents) + if err != nil { + t.Fatalf("rowsFromLockfile: %v", err) + } + got := renderExtension(rows) + + want, err := os.ReadFile("testdata/expected.yml") + if err != nil { + t.Fatalf("reading golden: %v", err) + } + if got != string(want) { + t.Errorf("rendered extension does not match testdata/expected.yml:\n--- got ---\n%s", got) + } +} diff --git a/actions/extractor/tools/lockfile-extension-generator/go.mod b/actions/extractor/tools/lockfile-extension-generator/go.mod new file mode 100644 index 000000000000..538ae9ae6922 --- /dev/null +++ b/actions/extractor/tools/lockfile-extension-generator/go.mod @@ -0,0 +1,16 @@ +module github.com/github/codeql/actions/extractor/tools/lockfile-extension-generator + +go 1.23 + +require github.com/github/actions-lockfile/go v0.0.0-00010101000000-000000000000 + +require gopkg.in/yaml.v3 v3.0.1 // indirect + +// LOCAL TESTING ONLY: github.com/github/actions-lockfile is not yet public, so it +// cannot be fetched by the module proxy. Point this at a local clone of the +// repository to build and test the generator: +// +// go mod edit -replace github.com/github/actions-lockfile/go=/path/to/actions-lockfile/go +// +// Remove this replace directive once actions-lockfile is published. +replace github.com/github/actions-lockfile/go => /Users/nodeselector/ghq/github.com/github/actions-lockfile/go diff --git a/actions/extractor/tools/lockfile-extension-generator/go.sum b/actions/extractor/tools/lockfile-extension-generator/go.sum new file mode 100644 index 000000000000..c4c1710c475c --- /dev/null +++ b/actions/extractor/tools/lockfile-extension-generator/go.sum @@ -0,0 +1,10 @@ +github.com/davecgh/go-spew v1.1.1 h1:vj9j/u1bqnvCEfJOwUhtlOARqs3+rkHYY13jYWTU97c= +github.com/davecgh/go-spew v1.1.1/go.mod h1:J7Y8YcW2NihsgmVo/mv3lAwl/skON4iLHjSsI+c5H38= +github.com/pmezard/go-difflib v1.0.0 h1:4DBwDE0NGyQoBHbLQYPwSUPoCMWR5BEzIk/f1lZbAQM= +github.com/pmezard/go-difflib v1.0.0/go.mod h1:iKH77koFhYxTK1pcRnkKkqfTogsbg7gZNVY4sRDYZ/4= +github.com/stretchr/testify v1.11.1 h1:7s2iGBzp5EwR7/aIZr8ao5+dra3wiQyKjjFuvgVKu7U= +github.com/stretchr/testify v1.11.1/go.mod h1:wZwfW3scLgRK+23gO65QZefKpKQRnfz6sD981Nm4B6U= +gopkg.in/check.v1 v0.0.0-20161208181325-20d25e280405 h1:yhCVgyC4o1eVCa2tZl7eS0r+SDo693bJlVdllGtEeKM= +gopkg.in/check.v1 v0.0.0-20161208181325-20d25e280405/go.mod h1:Co6ibVJAznAaIkqp8huTwlJQCZ016jof/cbN4VW5Yz0= +gopkg.in/yaml.v3 v3.0.1 h1:fxVm/GzAzEWqLHuvctI91KS9hhNmmWOoWu0XTYJS7CA= +gopkg.in/yaml.v3 v3.0.1/go.mod h1:K4uyk7z7BCEPqu6E+C64Yfv1cQ7kz7rIZviUmN+EgEM= diff --git a/actions/extractor/tools/lockfile-extension-generator/main.go b/actions/extractor/tools/lockfile-extension-generator/main.go new file mode 100644 index 000000000000..417c2a848e69 --- /dev/null +++ b/actions/extractor/tools/lockfile-extension-generator/main.go @@ -0,0 +1,57 @@ +package main + +import ( + "fmt" + "os" + "path/filepath" + + "github.com/github/actions-lockfile/go/pkg/lockfile" +) + +// main reads a repository's Actions lockfile and writes the corresponding +// `pinnedByLockfileDataModel` data extension. +// +// Usage: +// +// lockfile-extension-generator [output-file] +// +// `` is the root of the repository to scan; the lockfile is read +// from `/.github/workflows/actions.lock`. When `[output-file]` is +// omitted the extension is written to stdout. If the repository has no lockfile +// the generator exits successfully without writing anything, so it is safe to +// run unconditionally. +func main() { + if len(os.Args) < 2 || len(os.Args) > 3 { + fmt.Fprintf(os.Stderr, "usage: %s [output-file]\n", filepath.Base(os.Args[0])) + os.Exit(2) + } + sourceRoot := os.Args[1] + + lockPath := filepath.Join(sourceRoot, filepath.FromSlash(lockfile.Path)) + contents, err := os.ReadFile(lockPath) + if err != nil { + if os.IsNotExist(err) { + // No lockfile: nothing to pin. A clean no-op keeps this safe to run + // against every repository. + return + } + fmt.Fprintf(os.Stderr, "error: reading %s: %v\n", lockPath, err) + os.Exit(1) + } + + rows, err := rowsFromLockfile(contents) + if err != nil { + fmt.Fprintf(os.Stderr, "error: %v\n", err) + os.Exit(1) + } + + out := renderExtension(rows) + if len(os.Args) == 3 { + if err := os.WriteFile(os.Args[2], []byte(out), 0o644); err != nil { + fmt.Fprintf(os.Stderr, "error: writing %s: %v\n", os.Args[2], err) + os.Exit(1) + } + return + } + fmt.Print(out) +} diff --git a/actions/extractor/tools/lockfile-extension-generator/testdata/actions.lock b/actions/extractor/tools/lockfile-extension-generator/testdata/actions.lock new file mode 100644 index 000000000000..012688ac0993 --- /dev/null +++ b/actions/extractor/tools/lockfile-extension-generator/testdata/actions.lock @@ -0,0 +1,19 @@ +# Machine-generated by `gh actions-lock`; used here as a test fixture. +version: 'v0.0.2' +workflows: + '.github/workflows/ci.yml': + - 'actions/checkout@v4.3.1' + - 'some-owner/pinned-action@v1.2.0' + '.github/workflows/release.yml': + - 'actions/checkout@v4.3.1' +dependencies: + 'actions/checkout@v4.3.1': + ref: 'v4.3.1' + commit: 'sha1-34e114876b0b11c390a56381ad16ebd13914f8d5' + owner_id: 44036562 + repo_id: 197814629 + 'some-owner/pinned-action@v1.2.0': + ref: 'v1.2.0' + commit: 'sha1-1111111111111111111111111111111111111111' + owner_id: 12345 + repo_id: 67890 diff --git a/actions/extractor/tools/lockfile-extension-generator/testdata/expected.yml b/actions/extractor/tools/lockfile-extension-generator/testdata/expected.yml new file mode 100644 index 000000000000..4dba2d6f997c --- /dev/null +++ b/actions/extractor/tools/lockfile-extension-generator/testdata/expected.yml @@ -0,0 +1,16 @@ +# Generated by lockfile-extension-generator from .github/workflows/actions.lock. +# Do not edit by hand; regenerate from the repository's Actions lockfile. +extensions: + - addsTo: + pack: codeql/actions-all + extensible: pinnedByLockfileDataModel + data: + - [".github/workflows/ci.yml", "actions/checkout", "v4"] + - [".github/workflows/ci.yml", "actions/checkout", "v4.3"] + - [".github/workflows/ci.yml", "actions/checkout", "v4.3.1"] + - [".github/workflows/ci.yml", "some-owner/pinned-action", "v1"] + - [".github/workflows/ci.yml", "some-owner/pinned-action", "v1.2"] + - [".github/workflows/ci.yml", "some-owner/pinned-action", "v1.2.0"] + - [".github/workflows/release.yml", "actions/checkout", "v4"] + - [".github/workflows/release.yml", "actions/checkout", "v4.3"] + - [".github/workflows/release.yml", "actions/checkout", "v4.3.1"] From f0cfed0244fde086488558d2d3bad416cf44a40d Mon Sep 17 00:00:00 2001 From: Jeff Martin Date: Thu, 9 Jul 2026 21:10:04 -0500 Subject: [PATCH 05/11] actions: generate lockfile-pinned data extension during extraction Wire the lockfile-extension-generator into the Actions extractor autobuild so that codeql database create automatically emits the pinnedByLockfileDataModel data extension from a repository's .github/workflows/actions.lock. The extension is written into the database as a self-contained model pack under /lockfile-extension (codeql/actions-lockfile-pins). A new generate-lockfile-extension.sh runs after JS extraction: it locates the lockfile relative to the captured source root, resolves the generator (prebuilt binary if shipped, else builds from source when a Go toolchain is present), and writes the pack. It is a clean no-op when the repository has no lockfile, so it is safe to run against every database. CodeQL does not auto-apply extensions carried inside a database, so analysis still adds the pack explicitly via --model-packs codeql/actions-lockfile-pins (--additional-packs /lockfile-extension). Wiring that into the analysis harness is the remaining step and lives outside this repo. Verified end to end locally by overlaying the modified extractor into the CLI bundle: database create emits the extension, and analyze suppresses a lockfile-pinned short-tag ref (uses: owner/action@v4 resolved to v4.3.1) while still reporting refs not covered by the lockfile. --- actions/extractor/tools/autobuild.sh | 14 ++++ .../tools/generate-lockfile-extension.sh | 80 +++++++++++++++++++ .../lockfile-extension-generator/README.md | 28 +++++++ 3 files changed, 122 insertions(+) create mode 100755 actions/extractor/tools/generate-lockfile-extension.sh diff --git a/actions/extractor/tools/autobuild.sh b/actions/extractor/tools/autobuild.sh index f2cbb7ddfa7e..310da02eb67f 100755 --- a/actions/extractor/tools/autobuild.sh +++ b/actions/extractor/tools/autobuild.sh @@ -37,6 +37,11 @@ else export LGTM_INDEX_FILTERS fi +# Capture the source root before handing off to the JavaScript autobuilder, +# which may change the working directory. The Actions extractor runs from the +# source root, so `pwd` here is the root of the repository being analysed. +ACTIONS_SRC_ROOT="$(pwd)" + # Find the JavaScript extractor directory via `codeql resolve extractor`. CODEQL_EXTRACTOR_JAVASCRIPT_ROOT="$("${CODEQL_DIST}/codeql" resolve extractor --language javascript)" export CODEQL_EXTRACTOR_JAVASCRIPT_ROOT @@ -55,3 +60,12 @@ env CODEQL_EXTRACTOR_JAVASCRIPT_DIAGNOSTIC_DIR="${CODEQL_EXTRACTOR_ACTIONS_DIAGN CODEQL_EXTRACTOR_JAVASCRIPT_TRAP_DIR="${CODEQL_EXTRACTOR_ACTIONS_TRAP_DIR}" \ CODEQL_EXTRACTOR_JAVASCRIPT_WIP_DATABASE="${CODEQL_EXTRACTOR_ACTIONS_WIP_DATABASE}" \ "${JAVASCRIPT_AUTO_BUILD}" + +# Generate the lockfile-pinned data extension from the repository's Actions +# lockfile, if present. This is a no-op for repositories without a lockfile. +GENERATE_LOCKFILE_EXTENSION="$(CDPATH= cd "$(dirname "$0")" && pwd)/generate-lockfile-extension.sh" +if [ -x "${GENERATE_LOCKFILE_EXTENSION}" ]; then + echo "Generating lockfile-pinned data extension." + "${GENERATE_LOCKFILE_EXTENSION}" "${ACTIONS_SRC_ROOT}" || \ + echo "Lockfile-pinned data extension generation failed; continuing without it." >&2 +fi diff --git a/actions/extractor/tools/generate-lockfile-extension.sh b/actions/extractor/tools/generate-lockfile-extension.sh new file mode 100755 index 000000000000..7a3e89ec313e --- /dev/null +++ b/actions/extractor/tools/generate-lockfile-extension.sh @@ -0,0 +1,80 @@ +#!/bin/sh + +# Generates a CodeQL data extension that records the `uses:` references pinned by +# the repository's GitHub Actions lockfile (`.github/workflows/actions.lock`), +# populating the `pinnedByLockfileDataModel` extensible predicate consumed by the +# `actions/unpinned-tag` query. +# +# It is invoked by the Actions extractor autobuild during `codeql database +# create`. The generated extension is written into the database as a +# self-contained model pack under: +# +# /lockfile-extension/ +# qlpack.yml +# ext/pinned_by_lockfile.model.yml +# +# CodeQL does not auto-apply extensions carried inside a database, so analysis +# must add this pack explicitly, e.g.: +# +# codeql database analyze ... \ +# --additional-packs /lockfile-extension \ +# --model-packs codeql/actions-lockfile-pins +# +# The step is a clean no-op when the repository has no lockfile, so it is safe to +# run against every database. + +set -eu + +SRC_ROOT="${1:?usage: generate-lockfile-extension.sh }" +LOCKFILE="${SRC_ROOT}/.github/workflows/actions.lock" + +if [ ! -f "${LOCKFILE}" ]; then + echo "No Actions lockfile at '${LOCKFILE}'; skipping lockfile-pinned extension." + exit 0 +fi + +if [ -z "${CODEQL_EXTRACTOR_ACTIONS_WIP_DATABASE:-}" ]; then + echo "CODEQL_EXTRACTOR_ACTIONS_WIP_DATABASE is not set; cannot write lockfile extension." >&2 + exit 0 +fi + +SCRIPT_DIR="$(CDPATH= cd "$(dirname "$0")" && pwd)" +GEN_DIR="${SCRIPT_DIR}/lockfile-extension-generator" + +PACK_DIR="${CODEQL_EXTRACTOR_ACTIONS_WIP_DATABASE}/lockfile-extension" +EXT_DIR="${PACK_DIR}/ext" +mkdir -p "${EXT_DIR}" + +# Resolve the generator: prefer a prebuilt binary shipped with the extractor; +# otherwise build from source with the Go toolchain if it is available. +GEN_BIN="${GEN_DIR}/bin/lockfile-extension-generator" +RUN_GENERATOR="" +if [ -x "${GEN_BIN}" ]; then + RUN_GENERATOR="${GEN_BIN}" +elif command -v go >/dev/null 2>&1; then + BUILT_BIN="${CODEQL_EXTRACTOR_ACTIONS_SCRATCH_DIR:-${PACK_DIR}}/lockfile-extension-generator" + echo "Building lockfile-extension-generator from source with 'go build'." + ( cd "${GEN_DIR}" && go build -o "${BUILT_BIN}" . ) + RUN_GENERATOR="${BUILT_BIN}" +else + echo "No lockfile-extension-generator binary and no Go toolchain; skipping lockfile-pinned extension." >&2 + exit 0 +fi + +"${RUN_GENERATOR}" "${SRC_ROOT}" "${EXT_DIR}/pinned_by_lockfile.model.yml" + +cat > "${PACK_DIR}/qlpack.yml" <<'EOF' +name: codeql/actions-lockfile-pins +version: 0.0.1 +library: true +warnOnImplicitThis: true +# Generated per-database from the repository's Actions lockfile by the Actions +# extractor. Apply it at analysis time with `--model-packs +# codeql/actions-lockfile-pins` (and `--additional-packs /lockfile-extension`). +extensionTargets: + codeql/actions-all: "*" +dataExtensions: + - ext/*.model.yml +EOF + +echo "Wrote lockfile-pinned data extension to '${PACK_DIR}'." diff --git a/actions/extractor/tools/lockfile-extension-generator/README.md b/actions/extractor/tools/lockfile-extension-generator/README.md index 99ce1cf07b91..4d39191746d1 100644 --- a/actions/extractor/tools/lockfile-extension-generator/README.md +++ b/actions/extractor/tools/lockfile-extension-generator/README.md @@ -23,6 +23,34 @@ rows ship as a data-extension model pack applied at analysis time via same parsing core can later feed an extractor-native relation without changing the query. +## Extractor integration + +The Actions extractor runs this generator automatically during `codeql database +create` (see `../generate-lockfile-extension.sh`, invoked from +`../autobuild.sh`). When the repository has a lockfile, the extractor writes a +self-contained model pack into the database: + +``` +/lockfile-extension/ + qlpack.yml # name: codeql/actions-lockfile-pins + ext/pinned_by_lockfile.model.yml # generated pinnedByLockfileDataModel rows +``` + +CodeQL does not auto-apply extensions carried inside a database, so analysis +must add this pack explicitly: + +``` +codeql database analyze \ + codeql/actions-queries:Security/CWE-829/UnpinnedActionsTag.ql \ + --additional-packs /lockfile-extension \ + --model-packs codeql/actions-lockfile-pins +``` + +Wiring that flag into the analysis harness (e.g. the CodeQL Action) is the +remaining integration step and lives outside this repository. Generation is a +clean no-op for repositories without a lockfile, so the extractor step is safe +to run unconditionally. + ## Ref normalization A lockfile records the *resolved* ref for each dependency — the CLI prefers a From 5085314d752069c215ad6382822ef075e55b36a4 Mon Sep 17 00:00:00 2001 From: Jeff Martin Date: Thu, 9 Jul 2026 21:50:24 -0500 Subject: [PATCH 06/11] actions: drop machine-specific replace, document lockfile generator limits The committed go.mod for the lockfile-extension generator carried an absolute-path replace directive pointing at a local clone of the private actions-lockfile repo, which would leak a developer path and break builds for anyone else. Keep the how-to-test comment but drop the replace line; local testing uses `go mod edit -replace`. Also update the two change notes to state that the extractor now generates the pinnedByLockfileDataModel data into a database-local model pack (applied via --model-packs), and document the composite-action completeness gap in the generator README. --- .../tools/lockfile-extension-generator/README.md | 13 +++++++++++++ .../tools/lockfile-extension-generator/go.mod | 1 + .../change-notes/2026-07-09-pinned-by-lockfile.md | 2 +- .../2026-07-09-unpinned-tag-lockfile-aware.md | 2 +- 4 files changed, 16 insertions(+), 2 deletions(-) diff --git a/actions/extractor/tools/lockfile-extension-generator/README.md b/actions/extractor/tools/lockfile-extension-generator/README.md index 4d39191746d1..45eff56ac63e 100644 --- a/actions/extractor/tools/lockfile-extension-generator/README.md +++ b/actions/extractor/tools/lockfile-extension-generator/README.md @@ -101,3 +101,16 @@ codeql database analyze \ The lockfile-pinned reference is suppressed; references not covered by the lockfile are still reported. + +## Limitations + +- **Composite actions.** A lockfile records each workflow's *transitive* pin + list keyed by the workflow path. A `uses:` that appears only inside a + composite action file (`.github/actions/*/action.yml`) is therefore not + emitted against that action file's own path, so the query does not suppress + it. This is a completeness gap, not a correctness one: it can only leave a + reference reported, never wrongly suppress one, because a row is only ever + emitted for a `(path, nwo, ref)` the lockfile actually pins. +- **Ref forms the lockfile can't cover.** Only the resolved ref and its + major/major.minor forms are emitted. A `uses:` written with an unrelated tag + or a moving branch that the lockfile did not resolve from is not matched. diff --git a/actions/extractor/tools/lockfile-extension-generator/go.mod b/actions/extractor/tools/lockfile-extension-generator/go.mod index 538ae9ae6922..50cd43b16fb4 100644 --- a/actions/extractor/tools/lockfile-extension-generator/go.mod +++ b/actions/extractor/tools/lockfile-extension-generator/go.mod @@ -13,4 +13,5 @@ require gopkg.in/yaml.v3 v3.0.1 // indirect // go mod edit -replace github.com/github/actions-lockfile/go=/path/to/actions-lockfile/go // // Remove this replace directive once actions-lockfile is published. + replace github.com/github/actions-lockfile/go => /Users/nodeselector/ghq/github.com/github/actions-lockfile/go diff --git a/actions/ql/lib/change-notes/2026-07-09-pinned-by-lockfile.md b/actions/ql/lib/change-notes/2026-07-09-pinned-by-lockfile.md index d9b09e27ab81..7020d4418631 100644 --- a/actions/ql/lib/change-notes/2026-07-09-pinned-by-lockfile.md +++ b/actions/ql/lib/change-notes/2026-07-09-pinned-by-lockfile.md @@ -1,4 +1,4 @@ --- category: minorAnalysis --- -* 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`). It is intended to be populated by the CodeQL Actions extractor and is currently unpopulated, so it has no effect until that support ships. +* 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 behaviour is unchanged for repositories without a lockfile or analyses that do not opt in. 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 index beae154ff21d..35059072fcf1 100644 --- 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 @@ -1,4 +1,4 @@ --- category: minorAnalysis --- -* 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. This predicate is populated by the CodeQL Actions extractor and has no effect until that support ships. +* 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 behaviour is unchanged for repositories without a lockfile or analyses that do not opt in. From 75dc4324f6bd52f4bc9fa358a9010851fe21e70c Mon Sep 17 00:00:00 2001 From: Jeff Martin Date: Thu, 9 Jul 2026 21:52:46 -0500 Subject: [PATCH 07/11] actions: keep lockfile generator go.mod free of local replace The previous commit re-staged a dirty working-tree go.mod, so the machine-specific replace directive pointing at a local actions-lockfile clone leaked back into the tree. Drop it for real and move the local replace into a gitignored go.work so committed module metadata stays portable while local builds still resolve the not-yet-public dependency. --- .../extractor/tools/lockfile-extension-generator/.gitignore | 4 ++++ actions/extractor/tools/lockfile-extension-generator/go.mod | 2 -- 2 files changed, 4 insertions(+), 2 deletions(-) diff --git a/actions/extractor/tools/lockfile-extension-generator/.gitignore b/actions/extractor/tools/lockfile-extension-generator/.gitignore index 554d32ee98ca..b6ebbfc11b8c 100644 --- a/actions/extractor/tools/lockfile-extension-generator/.gitignore +++ b/actions/extractor/tools/lockfile-extension-generator/.gitignore @@ -1 +1,5 @@ /lockfile-extension-generator + +# local-only module workspace carrying the actions-lockfile replace +go.work +go.work.sum diff --git a/actions/extractor/tools/lockfile-extension-generator/go.mod b/actions/extractor/tools/lockfile-extension-generator/go.mod index 50cd43b16fb4..69d91a03f69b 100644 --- a/actions/extractor/tools/lockfile-extension-generator/go.mod +++ b/actions/extractor/tools/lockfile-extension-generator/go.mod @@ -13,5 +13,3 @@ require gopkg.in/yaml.v3 v3.0.1 // indirect // go mod edit -replace github.com/github/actions-lockfile/go=/path/to/actions-lockfile/go // // Remove this replace directive once actions-lockfile is published. - -replace github.com/github/actions-lockfile/go => /Users/nodeselector/ghq/github.com/github/actions-lockfile/go From b1165b5fe32aa6c69fd8dfb4579139066e0bb199 Mon Sep 17 00:00:00 2001 From: Jeff Martin Date: Thu, 9 Jul 2026 21:55:14 -0500 Subject: [PATCH 08/11] actions: rename self-reference to self repository in unpinned-tag Canonical terminology flip: `$/` resolves to the same REPOSITORY at the running SHA ("self repository"), while `./` is "self workspace". Rename the isSelfReference predicate to isSelfRepository, reword the code comment, and update the change note (renamed to ...-self-repository.md) and test fixture comments to match. No change to query results, the finding message, or any .expected output. --- actions/ql/src/Security/CWE-829/UnpinnedActionsTag.ql | 10 +++++----- .../2026-07-09-unpinned-tag-self-reference.md | 4 ---- .../2026-07-09-unpinned-tag-self-repository.md | 4 ++++ .../CWE-829/.github/workflows/self_ref_dollar.yml | 6 +++--- 4 files changed, 12 insertions(+), 12 deletions(-) delete mode 100644 actions/ql/src/change-notes/2026-07-09-unpinned-tag-self-reference.md create mode 100644 actions/ql/src/change-notes/2026-07-09-unpinned-tag-self-repository.md diff --git a/actions/ql/src/Security/CWE-829/UnpinnedActionsTag.ql b/actions/ql/src/Security/CWE-829/UnpinnedActionsTag.ql index 55f2ce6ecf0b..ceaf6e72e06a 100644 --- a/actions/ql/src/Security/CWE-829/UnpinnedActionsTag.ql +++ b/actions/ql/src/Security/CWE-829/UnpinnedActionsTag.ql @@ -33,11 +33,11 @@ private predicate isPinnedContainer(string version) { bindingset[nwo] private predicate isContainerImage(string nwo) { nwo.regexpMatch("^docker://.+") } -// A `$/` reference is a same-repo self-reference (e.g. `$/path/to/action`), resolved at the -// commit the calling workflow is running. Like `./` local references, it is inherently pinned -// and can never be an unpinned-tag finding, so we never flag it. +// 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 isSelfReference(string nwo) { nwo.matches("$/%") } +private predicate isSelfRepository(string nwo) { nwo.matches("$/%") } // Holds if `uses` (calling action `nwo` at `version`) is pinned by an entry in the repository's // Actions lockfile (`.github/workflows/actions.lock`). The underlying `pinnedByLockfileDataModel` @@ -70,7 +70,7 @@ where getStepContainerName(uses, name) and uses.getVersion() = version and not isTrustedOwner(nwo) and - not isSelfReference(nwo) and + not isSelfRepository(nwo) and not pinnedByLockfile(uses, nwo, version) and not (if isContainerImage(nwo) then isPinnedContainer(version) else isPinnedCommit(version)) and not isImmutableAction(uses, nwo) diff --git a/actions/ql/src/change-notes/2026-07-09-unpinned-tag-self-reference.md b/actions/ql/src/change-notes/2026-07-09-unpinned-tag-self-reference.md deleted file mode 100644 index c972741f1e7b..000000000000 --- a/actions/ql/src/change-notes/2026-07-09-unpinned-tag-self-reference.md +++ /dev/null @@ -1,4 +0,0 @@ ---- -category: minorAnalysis ---- -* The `actions/unpinned-tag` query no longer reports `$/` same-repo self-references (e.g. `uses: $/path/to/action`), which are inherently pinned to the running commit, just like `./` local references. 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/.github/workflows/self_ref_dollar.yml b/actions/ql/test/query-tests/Security/CWE-829/.github/workflows/self_ref_dollar.yml index 0c1275d35764..0d651b0bb0f6 100644 --- 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 @@ -6,10 +6,10 @@ jobs: name: Build and test runs-on: ubuntu-latest steps: - # `$/` is a same-repo self-reference resolved at the running commit. It is inherently - # pinned (like `./` local refs) and must never be reported as an unpinned tag. + # `$/` 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 isSelfReference(nwo)` suppression, since + # 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 From 7879f406b5b574849c8575dfa29609a821fed01f Mon Sep 17 00:00:00 2001 From: Jeff Martin Date: Thu, 9 Jul 2026 22:01:30 -0500 Subject: [PATCH 09/11] actions: make lockfile-extension generation atomic Stage the generated model pack in a temp dir inside the WIP database and publish it with a single rename only after it is fully written, with an EXIT trap that cleans up on any failure. Previously a failed 'go build' (expected until the private actions-lockfile dependency is public) left a half-written pack dir behind (an ext/ with no qlpack.yml) that could break analyses run with --additional-packs. Verified across three cases: repo with a lockfile (atomic publish), repo without one (clean no-op), and no Go toolchain available (graceful skip, no partial pack). --- .../tools/generate-lockfile-extension.sh | 23 +++++++++++++++---- 1 file changed, 18 insertions(+), 5 deletions(-) diff --git a/actions/extractor/tools/generate-lockfile-extension.sh b/actions/extractor/tools/generate-lockfile-extension.sh index 7a3e89ec313e..82b3ba2ecc15 100755 --- a/actions/extractor/tools/generate-lockfile-extension.sh +++ b/actions/extractor/tools/generate-lockfile-extension.sh @@ -42,8 +42,15 @@ SCRIPT_DIR="$(CDPATH= cd "$(dirname "$0")" && pwd)" GEN_DIR="${SCRIPT_DIR}/lockfile-extension-generator" PACK_DIR="${CODEQL_EXTRACTOR_ACTIONS_WIP_DATABASE}/lockfile-extension" -EXT_DIR="${PACK_DIR}/ext" -mkdir -p "${EXT_DIR}" + +# Stage the pack in a sibling temp dir inside the database directory (same +# filesystem as the final location) so the final `mv` is an atomic rename, and +# only publish a complete pack on success -- a failure part-way through never +# leaves a half-written pack dir (e.g. an `ext/` with no `qlpack.yml`) that would +# break `--additional-packs`. +STAGE_DIR="$(mktemp -d "${CODEQL_EXTRACTOR_ACTIONS_WIP_DATABASE}/lockfile-extension.XXXXXX")" +trap 'rm -rf "${STAGE_DIR}"' EXIT +mkdir -p "${STAGE_DIR}/ext" # Resolve the generator: prefer a prebuilt binary shipped with the extractor; # otherwise build from source with the Go toolchain if it is available. @@ -52,7 +59,7 @@ RUN_GENERATOR="" if [ -x "${GEN_BIN}" ]; then RUN_GENERATOR="${GEN_BIN}" elif command -v go >/dev/null 2>&1; then - BUILT_BIN="${CODEQL_EXTRACTOR_ACTIONS_SCRATCH_DIR:-${PACK_DIR}}/lockfile-extension-generator" + BUILT_BIN="${STAGE_DIR}/lockfile-extension-generator" echo "Building lockfile-extension-generator from source with 'go build'." ( cd "${GEN_DIR}" && go build -o "${BUILT_BIN}" . ) RUN_GENERATOR="${BUILT_BIN}" @@ -61,9 +68,9 @@ else exit 0 fi -"${RUN_GENERATOR}" "${SRC_ROOT}" "${EXT_DIR}/pinned_by_lockfile.model.yml" +"${RUN_GENERATOR}" "${SRC_ROOT}" "${STAGE_DIR}/ext/pinned_by_lockfile.model.yml" -cat > "${PACK_DIR}/qlpack.yml" <<'EOF' +cat > "${STAGE_DIR}/qlpack.yml" <<'EOF' name: codeql/actions-lockfile-pins version: 0.0.1 library: true @@ -77,4 +84,10 @@ dataExtensions: - ext/*.model.yml EOF +# Publish atomically: the pack only appears in the database once fully written. +rm -f "${STAGE_DIR}/lockfile-extension-generator" +rm -rf "${PACK_DIR}" +mv "${STAGE_DIR}" "${PACK_DIR}" +trap - EXIT + echo "Wrote lockfile-pinned data extension to '${PACK_DIR}'." From f64f1124d71f2cae0cb03e95144fc161b7d87600 Mon Sep 17 00:00:00 2001 From: Jeff Martin Date: Thu, 9 Jul 2026 22:12:33 -0500 Subject: [PATCH 10/11] actions: drop private actions-lockfile dependency from generator The generator pulled in github.com/github/actions-lockfile/go purely to parse a small, stable YAML file, which meant it could not build without a local clone of that (currently private) module -- forcing a gitignored go.work with a machine-specific replace and breaking any CI/bazel build. Parse the minimal core of the lockfile format directly instead (new lockfile.go: YAML unmarshal, pin-key parsing, and the semver major/minor/full logic), faithfully mirroring the canonical parser's semantics. The golden fixture (testdata/expected.yml) is unchanged, byte for byte, which proves the reimplementation matches. Added unit tests for parsePin, parseSemVer/isFull, and parseLockfile. The generator now depends only on gopkg.in/yaml.v3 and builds anywhere the Go toolchain is available, with no replace directive and no go.work. Verified end-to-end through the real extractor: on-demand 'go build' during database create succeeds with a stock toolchain, and the lockfile-pinned ref is still suppressed while an unlocked ref still fires. --- .../lockfile-extension-generator/.gitignore | 4 - .../lockfile-extension-generator/README.md | 28 ++-- .../lockfile-extension-generator/generator.go | 41 ++--- .../tools/lockfile-extension-generator/go.mod | 12 +- .../tools/lockfile-extension-generator/go.sum | 6 - .../lockfile-extension-generator/lockfile.go | 140 ++++++++++++++++++ .../lockfile_test.go | 92 ++++++++++++ .../lockfile-extension-generator/main.go | 4 +- 8 files changed, 272 insertions(+), 55 deletions(-) create mode 100644 actions/extractor/tools/lockfile-extension-generator/lockfile.go create mode 100644 actions/extractor/tools/lockfile-extension-generator/lockfile_test.go diff --git a/actions/extractor/tools/lockfile-extension-generator/.gitignore b/actions/extractor/tools/lockfile-extension-generator/.gitignore index b6ebbfc11b8c..554d32ee98ca 100644 --- a/actions/extractor/tools/lockfile-extension-generator/.gitignore +++ b/actions/extractor/tools/lockfile-extension-generator/.gitignore @@ -1,5 +1 @@ /lockfile-extension-generator - -# local-only module workspace carrying the actions-lockfile replace -go.work -go.work.sum diff --git a/actions/extractor/tools/lockfile-extension-generator/README.md b/actions/extractor/tools/lockfile-extension-generator/README.md index 45eff56ac63e..ed18259fc1ed 100644 --- a/actions/extractor/tools/lockfile-extension-generator/README.md +++ b/actions/extractor/tools/lockfile-extension-generator/README.md @@ -15,13 +15,14 @@ the query otherwise lacks. Feeding the lockfile's bindings into `pinnedByLockfileDataModel` lets the query suppress those references while still flagging genuinely unpinned ones. -The generator is deliberately **transport-agnostic**. It parses the lockfile -with the canonical parser at `github.com/github/actions-lockfile/go` and emits -the `[workflow_path, nwo, ref]` rows the query already matches on. Today those -rows ship as a data-extension model pack applied at analysis time via -`--model-packs` (the same mechanism as `codeql/immutable-actions-list`). The -same parsing core can later feed an extractor-native relation without changing -the query. +The generator is deliberately **transport-agnostic**. It parses the minimal, +stable core of the lockfile format directly (see `lockfile.go`) and emits the +`[workflow_path, nwo, ref]` rows the query already matches on. It has no +dependency on the `github.com/github/actions-lockfile` module, so it builds +anywhere the Go toolchain is available. Today those rows ship as a +data-extension model pack applied at analysis time via `--model-packs` (the same +mechanism as `codeql/immutable-actions-list`). The same parsing core can later +feed an extractor-native relation without changing the query. ## Extractor integration @@ -77,16 +78,19 @@ writing anything, so it is safe to run unconditionally. ## Building and testing locally -`github.com/github/actions-lockfile` is not yet public, so the module cannot be -fetched by the module proxy. Point the `replace` directive in `go.mod` at a -local clone before building or testing: +The generator depends only on `gopkg.in/yaml.v3`, so it builds and tests with a +stock Go toolchain and no special setup: ``` -go mod edit -replace github.com/github/actions-lockfile/go=/path/to/actions-lockfile/go +go build ./... go test ./... ``` -Remove the `replace` directive once actions-lockfile is published. +The golden test (`testdata/expected.yml`) pins the exact generated output for a +representative lockfile, so any change to parsing or ref normalization must be +reflected there deliberately. The lockfile parsing in `lockfile.go` mirrors the +canonical semantics of `github.com/github/actions-lockfile`; if that format +evolves, update `lockfile.go` and the golden fixture together. ## End-to-end check diff --git a/actions/extractor/tools/lockfile-extension-generator/generator.go b/actions/extractor/tools/lockfile-extension-generator/generator.go index f7b0a43495f8..1c00b24238a5 100644 --- a/actions/extractor/tools/lockfile-extension-generator/generator.go +++ b/actions/extractor/tools/lockfile-extension-generator/generator.go @@ -3,20 +3,23 @@ // `pinnedByLockfileDataModel` extensible predicate consumed by the // `actions/unpinned-tag` query. // -// The generator is deliberately transport-agnostic: it parses the lockfile with -// the canonical parser at `github.com/github/actions-lockfile/go` and emits the -// same `[workflow_path, nwo, ref]` rows the query already matches on. Today those -// rows are shipped as a data-extension model pack (applied at analysis time via -// `--model-packs`, exactly like `codeql/immutable-actions-list`). The same parsing -// core can later feed an extractor-native TRAP relation without changing the query. +// The generator is deliberately transport-agnostic: it parses the lockfile and +// emits the same `[workflow_path, nwo, ref]` rows the query already matches on. +// Today those rows are shipped as a data-extension model pack (applied at +// analysis time via `--model-packs`, exactly like `codeql/immutable-actions-list`). +// The same parsing core can later feed an extractor-native TRAP relation without +// changing the query. +// +// The lockfile format is owned by `github.com/github/actions-lockfile`; this +// generator parses the minimal, stable core it needs directly (see lockfile.go) +// so it has no dependency on that (currently private) module and builds anywhere +// the Go toolchain is available. package main import ( "fmt" "sort" "strings" - - "github.com/github/actions-lockfile/go/pkg/lockfile" ) // row is a single `pinnedByLockfileDataModel` tuple: the reference @@ -39,26 +42,26 @@ type row struct { // `uses: owner/action@v4` be recognised as pinned by a lockfile entry that // resolved to `v4.3.1`, which is the common real-world case. func rowsFromLockfile(contents []byte) ([]row, error) { - f, err := lockfile.Parse(contents) + doc, err := parseLockfile(contents) if err != nil { - return nil, fmt.Errorf("parsing lockfile: %w", err) + return nil, err } seen := make(map[row]struct{}) - for workflowPath, pinKeys := range f.Workflows { + for workflowPath, pinKeys := range doc.Workflows { for _, key := range pinKeys { - pin, ok := lockfile.ParsePin(key) + nwo, keyRef, ok := parsePin(key) if !ok { continue } // Prefer the resolved ref recorded in the dependency metadata; fall // back to the ref embedded in the pin key. - ref := pin.Ref - if dep, ok := f.Dependencies[key]; ok && dep.Ref != "" { + ref := keyRef + if dep, ok := doc.Dependencies[key]; ok && dep.Ref != "" { ref = dep.Ref } for _, r := range refVariants(ref) { - seen[row{workflowPath: workflowPath, nwo: pin.NWO, ref: r}] = struct{}{} + seen[row{workflowPath: workflowPath, nwo: nwo, ref: r}] = struct{}{} } } } @@ -84,15 +87,15 @@ func rowsFromLockfile(contents []byte) ([]row, error) { // this is the tag itself plus its major.minor (`v4.3`) and major-only (`v4`) // forms; for anything else (a branch, a partial tag, a SHA) it is just the ref. func refVariants(ref string) []string { - sv, ok := lockfile.ParseSemVer(ref) - if !ok || !sv.IsFull() { + sv, ok := parseSemVer(ref) + if !ok || !sv.isFull() { return []string{ref} } variants := []string{ref} - if minor := sv.MinorTag(); minor != ref { + if minor := sv.minorTag(); minor != ref { variants = append(variants, minor) } - if major := sv.MajorTag(); major != ref { + if major := sv.majorTag(); major != ref { variants = append(variants, major) } return variants diff --git a/actions/extractor/tools/lockfile-extension-generator/go.mod b/actions/extractor/tools/lockfile-extension-generator/go.mod index 69d91a03f69b..5fe4ede0532e 100644 --- a/actions/extractor/tools/lockfile-extension-generator/go.mod +++ b/actions/extractor/tools/lockfile-extension-generator/go.mod @@ -2,14 +2,4 @@ module github.com/github/codeql/actions/extractor/tools/lockfile-extension-gener go 1.23 -require github.com/github/actions-lockfile/go v0.0.0-00010101000000-000000000000 - -require gopkg.in/yaml.v3 v3.0.1 // indirect - -// LOCAL TESTING ONLY: github.com/github/actions-lockfile is not yet public, so it -// cannot be fetched by the module proxy. Point this at a local clone of the -// repository to build and test the generator: -// -// go mod edit -replace github.com/github/actions-lockfile/go=/path/to/actions-lockfile/go -// -// Remove this replace directive once actions-lockfile is published. +require gopkg.in/yaml.v3 v3.0.1 diff --git a/actions/extractor/tools/lockfile-extension-generator/go.sum b/actions/extractor/tools/lockfile-extension-generator/go.sum index c4c1710c475c..a62c313c5b0c 100644 --- a/actions/extractor/tools/lockfile-extension-generator/go.sum +++ b/actions/extractor/tools/lockfile-extension-generator/go.sum @@ -1,9 +1,3 @@ -github.com/davecgh/go-spew v1.1.1 h1:vj9j/u1bqnvCEfJOwUhtlOARqs3+rkHYY13jYWTU97c= -github.com/davecgh/go-spew v1.1.1/go.mod h1:J7Y8YcW2NihsgmVo/mv3lAwl/skON4iLHjSsI+c5H38= -github.com/pmezard/go-difflib v1.0.0 h1:4DBwDE0NGyQoBHbLQYPwSUPoCMWR5BEzIk/f1lZbAQM= -github.com/pmezard/go-difflib v1.0.0/go.mod h1:iKH77koFhYxTK1pcRnkKkqfTogsbg7gZNVY4sRDYZ/4= -github.com/stretchr/testify v1.11.1 h1:7s2iGBzp5EwR7/aIZr8ao5+dra3wiQyKjjFuvgVKu7U= -github.com/stretchr/testify v1.11.1/go.mod h1:wZwfW3scLgRK+23gO65QZefKpKQRnfz6sD981Nm4B6U= gopkg.in/check.v1 v0.0.0-20161208181325-20d25e280405 h1:yhCVgyC4o1eVCa2tZl7eS0r+SDo693bJlVdllGtEeKM= gopkg.in/check.v1 v0.0.0-20161208181325-20d25e280405/go.mod h1:Co6ibVJAznAaIkqp8huTwlJQCZ016jof/cbN4VW5Yz0= gopkg.in/yaml.v3 v3.0.1 h1:fxVm/GzAzEWqLHuvctI91KS9hhNmmWOoWu0XTYJS7CA= diff --git a/actions/extractor/tools/lockfile-extension-generator/lockfile.go b/actions/extractor/tools/lockfile-extension-generator/lockfile.go new file mode 100644 index 000000000000..a8fd87a913fa --- /dev/null +++ b/actions/extractor/tools/lockfile-extension-generator/lockfile.go @@ -0,0 +1,140 @@ +package main + +import ( + "fmt" + "regexp" + "strconv" + "strings" + + "gopkg.in/yaml.v3" +) + +// lockfilePath is the repo-relative location of the GitHub Actions lockfile. +const lockfilePath = ".github/workflows/actions.lock" + +// lockfileDoc is the subset of the Actions lockfile (schema v0.0.2) that the +// generator needs: the per-workflow flat list of canonical pin keys, and the +// resolved ref recorded for each dependency. +// +// The full format is owned by `github.com/github/actions-lockfile`. We parse it +// directly here rather than depending on that (currently private) module so the +// generator builds anywhere the Go toolchain is available, with no module-proxy +// access to a private repository. The fields we read are a stable, minimal core +// of the format; see the schema in that repository for the authoritative shape. +type lockfileDoc struct { + Version string `yaml:"version"` + Workflows map[string][]string `yaml:"workflows"` + Dependencies map[string]lockfileDependency `yaml:"dependencies"` +} + +type lockfileDependency struct { + Ref string `yaml:"ref"` + Commit string `yaml:"commit"` +} + +func parseLockfile(contents []byte) (*lockfileDoc, error) { + var doc lockfileDoc + if err := yaml.Unmarshal(contents, &doc); err != nil { + return nil, fmt.Errorf("parsing lockfile YAML: %w", err) + } + return &doc, nil +} + +// parsePin parses a canonical lockfile pin key of the form "OWNER/REPO@REF". +// +// It mirrors `lockfile.ParsePin`: the "@" separates repo from ref, the repo +// portion must contain exactly one "/", owner and repo are lower-cased (git +// forges treat them case-insensitively) while the ref preserves its casing, and +// a ref may not be empty or contain a colon. Sub-action paths +// ("owner/repo/sub@ref") are rejected because the lockfile pins at repo+ref +// granularity. Returns ok=false for anything that does not match. +func parsePin(s string) (nwo, ref string, ok bool) { + atIdx := strings.IndexByte(s, '@') + if atIdx <= 0 || atIdx == len(s)-1 { + return "", "", false + } + repoPath := s[:atIdx] + ref = s[atIdx+1:] + + if strings.Count(repoPath, "/") != 1 { + return "", "", false + } + slashIdx := strings.IndexByte(repoPath, '/') + owner := repoPath[:slashIdx] + repo := repoPath[slashIdx+1:] + if owner == "" || repo == "" { + return "", "", false + } + if ref == "" || strings.Contains(ref, ":") { + return "", "", false + } + nwo = strings.ToLower(owner) + "/" + strings.ToLower(repo) + return nwo, ref, true +} + +// semVer holds the parsed components of a semver-ish Actions tag. The scheme is +// deliberately lax (bare "2.0.0", partial "v4"/"v4.2", arbitrary suffixes all +// appear in the wild), matching `lockfile.SemVer`. +type semVer struct { + prefix string + major int + minor int + patch int + rest string + raw string +} + +var versionRE = regexp.MustCompile(`^(v?)(\d+)(?:\.(\d+))?(?:\.(\d+))?(.*)$`) + +func parseSemVer(tag string) (semVer, bool) { + if isFullSha(tag) { + return semVer{}, false + } + m := versionRE.FindStringSubmatch(tag) + if m == nil { + return semVer{}, false + } + major, err := strconv.Atoi(m[2]) + if err != nil { + return semVer{}, false + } + minor := 0 + if m[3] != "" { + if minor, err = strconv.Atoi(m[3]); err != nil { + return semVer{}, false + } + } + patch := 0 + if m[4] != "" { + if patch, err = strconv.Atoi(m[4]); err != nil { + return semVer{}, false + } + } + return semVer{prefix: m[1], major: major, minor: minor, patch: patch, rest: m[5], raw: tag}, true +} + +func (s semVer) majorTag() string { return fmt.Sprintf("%s%d", s.prefix, s.major) } + +func (s semVer) minorTag() string { return fmt.Sprintf("%s%d.%d", s.prefix, s.major, s.minor) } + +// isFull reports whether the tag has all three components (major.minor.patch) +// and no pre-release suffix. Tags like "v4" or "v4.2" return false. Only a full +// version uniquely identifies a release, so only full versions get expanded +// into their shorter mutable forms. +func (s semVer) isFull() bool { + return s.rest == "" && s.raw != s.majorTag() && s.raw != s.minorTag() +} + +// isFullSha reports whether s looks like a full commit hash (SHA-1 or SHA-256), +// which must not be treated as a version tag. +func isFullSha(s string) bool { + if len(s) != 40 && len(s) != 64 { + return false + } + for _, c := range s { + if !((c >= '0' && c <= '9') || (c >= 'a' && c <= 'f') || (c >= 'A' && c <= 'F')) { + return false + } + } + return true +} diff --git a/actions/extractor/tools/lockfile-extension-generator/lockfile_test.go b/actions/extractor/tools/lockfile-extension-generator/lockfile_test.go new file mode 100644 index 000000000000..f3f64359cac0 --- /dev/null +++ b/actions/extractor/tools/lockfile-extension-generator/lockfile_test.go @@ -0,0 +1,92 @@ +package main + +import "testing" + +func TestParsePin(t *testing.T) { + cases := []struct { + in string + wantNWO string + wantRef string + wantOK bool + }{ + {"actions/checkout@v4.3.1", "actions/checkout", "v4.3.1", true}, + {"Some-Owner/Pinned-Action@v1.2.0", "some-owner/pinned-action", "v1.2.0", true}, // owner/repo lower-cased, ref preserved + {"owner/repo@V1.2.0", "owner/repo", "V1.2.0", true}, // ref casing preserved + {"owner/repo@sha1-deadbeef", "owner/repo", "sha1-deadbeef", true}, // colon-free ref accepted + {"owner/repo", "", "", false}, // no @ + {"owner/repo@", "", "", false}, // empty ref + {"@v1", "", "", false}, // no repo path + {"owner/repo/sub@v1", "", "", false}, // sub-action path rejected + {"owneronly@v1", "", "", false}, // no slash + {"owner/repo@ref:with:colon", "", "", false}, // colon rejected + } + for _, c := range cases { + nwo, ref, ok := parsePin(c.in) + if ok != c.wantOK || nwo != c.wantNWO || ref != c.wantRef { + t.Errorf("parsePin(%q) = (%q, %q, %v), want (%q, %q, %v)", + c.in, nwo, ref, ok, c.wantNWO, c.wantRef, c.wantOK) + } + } +} + +func TestParseSemVerAndIsFull(t *testing.T) { + cases := []struct { + tag string + wantOK bool + wantFull bool + wantMajor string + wantMinor string + }{ + {"v4.3.1", true, true, "v4", "v4.3"}, + {"4.3.1", true, true, "4", "4.3"}, // bare, no "v" prefix + {"v4", true, false, "v4", "v4.0"}, + {"v4.3", true, false, "v4", "v4.3"}, + {"v2.0.0-beta.1", true, false, "v2", "v2.0"}, // pre-release: not full + {"main", false, false, "", ""}, + {"0123456789abcdef0123456789abcdef01234567", false, false, "", ""}, // 40-char SHA + } + for _, c := range cases { + sv, ok := parseSemVer(c.tag) + if ok != c.wantOK { + t.Errorf("parseSemVer(%q) ok = %v, want %v", c.tag, ok, c.wantOK) + continue + } + if !ok { + continue + } + if got := sv.isFull(); got != c.wantFull { + t.Errorf("parseSemVer(%q).isFull() = %v, want %v", c.tag, got, c.wantFull) + } + if got := sv.majorTag(); got != c.wantMajor { + t.Errorf("parseSemVer(%q).majorTag() = %q, want %q", c.tag, got, c.wantMajor) + } + if got := sv.minorTag(); got != c.wantMinor { + t.Errorf("parseSemVer(%q).minorTag() = %q, want %q", c.tag, got, c.wantMinor) + } + } +} + +func TestParseLockfile(t *testing.T) { + doc, err := parseLockfile([]byte(`version: 'v0.0.2' +workflows: + '.github/workflows/ci.yml': + - 'actions/checkout@v4.3.1' +dependencies: + 'actions/checkout@v4.3.1': + ref: 'v4.3.1' + commit: 'sha1-abc' +`)) + if err != nil { + t.Fatalf("parseLockfile: %v", err) + } + if doc.Version != "v0.0.2" { + t.Errorf("version = %q, want v0.0.2", doc.Version) + } + keys := doc.Workflows[".github/workflows/ci.yml"] + if len(keys) != 1 || keys[0] != "actions/checkout@v4.3.1" { + t.Errorf("workflows entry = %v", keys) + } + if dep := doc.Dependencies["actions/checkout@v4.3.1"]; dep.Ref != "v4.3.1" { + t.Errorf("dependency ref = %q, want v4.3.1", dep.Ref) + } +} diff --git a/actions/extractor/tools/lockfile-extension-generator/main.go b/actions/extractor/tools/lockfile-extension-generator/main.go index 417c2a848e69..4c7811a591c1 100644 --- a/actions/extractor/tools/lockfile-extension-generator/main.go +++ b/actions/extractor/tools/lockfile-extension-generator/main.go @@ -4,8 +4,6 @@ import ( "fmt" "os" "path/filepath" - - "github.com/github/actions-lockfile/go/pkg/lockfile" ) // main reads a repository's Actions lockfile and writes the corresponding @@ -27,7 +25,7 @@ func main() { } sourceRoot := os.Args[1] - lockPath := filepath.Join(sourceRoot, filepath.FromSlash(lockfile.Path)) + lockPath := filepath.Join(sourceRoot, filepath.FromSlash(lockfilePath)) contents, err := os.ReadFile(lockPath) if err != nil { if os.IsNotExist(err) { From fbe00dea732f48846fb0d48236eab2808ab9eac7 Mon Sep 17 00:00:00 2001 From: Jeff Martin Date: Thu, 9 Jul 2026 22:31:27 -0500 Subject: [PATCH 11/11] actions: emit empty data list for zero-row lockfile extension A lockfile that pins no repo-level actions (e.g. only sub-path actions like github/codeql-action/init@v3, which parsePin skips) produced a bare `data:` (YAML null) extension, which CodeQL's `resolve extensions-by-pack` rejects and aborts the analysis. Emit `data: []` for the zero-row case, matching the repo convention, and note the narrow transitive-per-path over-suppression edge in the change note. --- .../lockfile-extension-generator/generator.go | 10 +++++++++ .../generator_test.go | 22 +++++++++++++++++++ .../2026-07-09-unpinned-tag-lockfile-aware.md | 2 +- 3 files changed, 33 insertions(+), 1 deletion(-) diff --git a/actions/extractor/tools/lockfile-extension-generator/generator.go b/actions/extractor/tools/lockfile-extension-generator/generator.go index 1c00b24238a5..9bd0ee23ff6d 100644 --- a/actions/extractor/tools/lockfile-extension-generator/generator.go +++ b/actions/extractor/tools/lockfile-extension-generator/generator.go @@ -112,6 +112,16 @@ func renderExtension(rows []row) string { b.WriteString(" - addsTo:\n") b.WriteString(" pack: codeql/actions-all\n") b.WriteString(" extensible: pinnedByLockfileDataModel\n") + // `data` must be a YAML sequence. When there are no rows we must emit an + // explicit empty list (`data: []`); a bare `data:` parses as null, which + // CodeQL's extension loader rejects (`resolve extensions-by-pack` fails and + // aborts the whole analysis). Zero rows is reachable whenever a lockfile is + // present but pins no repo-level actions (e.g. only sub-path actions such as + // `github/codeql-action/init@v3`, which `parsePin` intentionally skips). + if len(rows) == 0 { + b.WriteString(" data: []\n") + return b.String() + } b.WriteString(" data:\n") for _, r := range rows { fmt.Fprintf(&b, " - [%q, %q, %q]\n", r.workflowPath, r.nwo, r.ref) diff --git a/actions/extractor/tools/lockfile-extension-generator/generator_test.go b/actions/extractor/tools/lockfile-extension-generator/generator_test.go index 512a6679dc47..cfa71e0acb9b 100644 --- a/actions/extractor/tools/lockfile-extension-generator/generator_test.go +++ b/actions/extractor/tools/lockfile-extension-generator/generator_test.go @@ -2,6 +2,7 @@ package main import ( "os" + "strings" "testing" ) @@ -89,3 +90,24 @@ func TestRenderExtensionMatchesGolden(t *testing.T) { t.Errorf("rendered extension does not match testdata/expected.yml:\n--- got ---\n%s", got) } } + +func TestRenderExtensionZeroRowsEmitsEmptyList(t *testing.T) { + // A lockfile that pins only sub-path actions (skipped by parsePin) or an + // empty `workflows` map yields zero rows while a lockfile is present. The + // rendered extension must use `data: []`, not a bare `data:` (YAML null), + // which CodeQL's extension loader rejects and which aborts the analysis. + rows, err := rowsFromLockfile([]byte("version: \"v0.0.2\"\nworkflows:\n \".github/workflows/ci.yml\":\n - \"github/codeql-action/init@v3\"\ndependencies: {}\n")) + if err != nil { + t.Fatalf("rowsFromLockfile: %v", err) + } + if len(rows) != 0 { + t.Fatalf("expected zero rows for a sub-path-only lockfile, got %d: %v", len(rows), rows) + } + got := renderExtension(rows) + if !strings.Contains(got, "data: []") { + t.Errorf("zero-row extension must emit `data: []`, got:\n%s", got) + } + if strings.Contains(got, "data:\n") { + t.Errorf("zero-row extension must not emit a null `data:` line, got:\n%s", got) + } +} 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 index 35059072fcf1..a2f3332b4e4f 100644 --- 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 @@ -1,4 +1,4 @@ --- category: minorAnalysis --- -* 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 behaviour is unchanged for repositories without a lockfile or analyses that do not opt in. +* 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 behaviour 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.