diff --git a/.github/scripts/pin-codeql-library-versions.sh b/.github/scripts/pin-codeql-library-versions.sh index 8c36d7b8..a7666e18 100755 --- a/.github/scripts/pin-codeql-library-versions.sh +++ b/.github/scripts/pin-codeql-library-versions.sh @@ -1,21 +1,40 @@ #!/usr/bin/env bash -# Pins every `codeql/` dependency in this repo's qlpack.yml files to the -# exact library version shipped in the official CodeQL Bundle for a given CLI +# Pins every `codeql/` `dependencies:` entry in this repo's query/library +# pack qlpack.yml files (`/src`, `/lib`, etc.) to the exact +# library version shipped in the official CodeQL Bundle for a given CLI # release - overwriting whatever value is currently there (an unconstrained # `'*'`, or an exact version pinned by a previous run of this script against # an older CLI). # -# Why this exists: `codeql pack upgrade` resolves an unconstrained `'*'` -# dependency to the *latest-ever-published* version in the configured -# registry (GHCR) - completely independent of whatever CodeQL CLI version is -# pinned in `.codeqlversion`. That mismatch can jump `codeql/-all` -# several major versions ahead of what the pinned CLI actually ships/tests -# against, which can silently break analyses (see CONTRIBUTING.md's -# "Updating the pinned CodeQL CLI/library version" section for a worked -# example). Pinning these `codeql/*` deps to the exact bundle-paired version -# before running `codeql pack upgrade` makes that resolution deterministic -# and keeps every pack's declared library dependency in lockstep with the -# CLI version this repo says it supports. +# Deliberately does NOT touch `/ext` or `/ext-library-sources` +# - those are model/extension packs that declare `extensionTargets:`, not +# `dependencies:`, and are excluded from this script's file list entirely. See +# CONTRIBUTING.md's "Supported CodeQL versions" section ("Why `extensionTargets` +# is always `'*'`, never pinned") for why: unlike `dependencies:`, an unsatisfied +# `extensionTargets` constraint doesn't fail loudly, it silently drops the whole +# extension pack, so re-pinning it - to an exact version OR a version floor - +# on every CLI bump (the way this script handles `dependencies:`) turns routine +# consumer CLI drift into silent, undetected loss of coverage +# (GitHubSecurityLab/CodeQL-Community-Packs#206). `extensionTargets` is left as +# the fully unconstrained `'*'` instead, and this script must never touch it - +# see the comment above `extensionTargets:` in each `/ext*/qlpack.yml`. +# +# Why this exists (for `dependencies:`): `codeql pack upgrade` resolves an +# unconstrained `'*'` dependency to the *latest-ever-published* version in the +# configured registry (GHCR) - completely independent of whatever CodeQL CLI +# version is pinned in `.codeqlversion`. That mismatch can jump +# `codeql/-all` several major versions ahead of what the pinned CLI +# actually ships/tests against, which can silently break analyses (see +# CONTRIBUTING.md's "Updating the pinned CodeQL CLI/library version" section +# for a worked example). Pinning these `codeql/*` deps to the exact +# bundle-paired version before running `codeql pack upgrade` makes that +# resolution deterministic and keeps every pack's declared library dependency +# in lockstep with the CLI version this repo says it supports. This is safe +# for `dependencies:` (unlike `extensionTargets:`) because a genuinely +# unsatisfiable `dependencies:` constraint fails loudly at `codeql pack +# install`/`resolve-dependencies` time (`ERROR: No valid pack solution +# found...`), and any incompatibility in the CLI-bundled `codeql/-all` +# itself surfaces as an ordinary QL compile error - never a silent drop. # # This script is idempotent and must be re-run (with a new target version) # on every subsequent CLI bump: once a dependency is pinned to an exact @@ -67,8 +86,25 @@ echo "Discovered $(wc -l < "$VERSIONS_FILE") bundled codeql/* packages for CLI $ # artifacts (.codeql/ pack caches, the /codeql cloned-repo checkout dir, and # /codeql_home, where .github/actions/install-codeql downloads/extracts the # CodeQL CLI - which ships its own small vendored qlpack.yml packs, e.g. -# codeql//downgrades, that have nothing to do with this repo). -mapfile -t QLPACK_FILES < <(find . -name qlpack.yml -not -path "*/.codeql/*" -not -path "./codeql/*" -not -path "./codeql_home/*") +# codeql//downgrades, that have nothing to do with this repo) - and +# excluding every `/ext` and `/ext-library-sources` +# model/extension pack. Those use `extensionTargets:` instead of +# `dependencies:`, and deliberately do NOT get re-pinned on every CLI bump the +# way `dependencies:` does here (see CONTRIBUTING.md's "Supported CodeQL +# versions" section, "Why `extensionTargets` is always `'*'`, never pinned or +# floored"). Unlike `dependencies:`, an unsatisfied `extensionTargets` +# constraint doesn't fail loudly - the CLI just silently drops the whole +# extension pack (zero data-extension rows applied, only a low-visibility +# `WARNING: ... is unused`) - so re-pinning it to whatever this repo's own CI +# happens to test against, on every routine CLI bump, turns every consumer +# CLI/version mismatch into silent, undetected loss of coverage (see +# GitHubSecurityLab/CodeQL-Community-Packs#206). Instead, `extensionTargets` +# is left as the fully unconstrained `codeql/-all: '*'` and only ever +# changed by hand if a maintainer confirms an actual breaking change to the +# models-as-data schema (e.g. an extensible predicate's arity/column set +# changed) - our own test suite failing is the trigger to look for that, not +# a routine CLI bump. +mapfile -t QLPACK_FILES < <(find . -name qlpack.yml -not -path "*/.codeql/*" -not -path "./codeql/*" -not -path "./codeql_home/*" -not -path "*/ext/*" -not -path "*/ext-library-sources/*") declare -A PINNED_COUNT=() while read -r pkg ver; do diff --git a/.github/workflows/update-codeql-version.yml b/.github/workflows/update-codeql-version.yml index 4dbc2c89..09a656d6 100644 --- a/.github/workflows/update-codeql-version.yml +++ b/.github/workflows/update-codeql-version.yml @@ -84,6 +84,13 @@ jobs: env: GH_TOKEN: ${{ github.token }} run: | + # NOTE: this only re-pins `dependencies:` in query/library packs + # (/src, /lib, etc.) - it deliberately skips + # /ext and /ext-library-sources, whose + # `extensionTargets:` is intentionally left as an unconstrained + # `'*'` and must never be auto-rewritten (see + # pin-codeql-library-versions.sh's header comment and + # CONTRIBUTING.md's "Supported CodeQL versions" section). .github/scripts/pin-codeql-library-versions.sh "${{ steps.version.outputs.version }}" - name: Upgrade every pack's dependencies @@ -91,10 +98,10 @@ jobs: GITHUB_TOKEN: ${{ github.token }} run: | set -euo pipefail - # Exclusions (the first three below are also excluded in - # .github/scripts/pin-codeql-library-versions.sh; the last one is - # NOT - ext/ext-library-sources still need their extensionTargets - # versions pinned, but must be skipped here to avoid lock-file churn): + # Exclusions (all four are also excluded in + # .github/scripts/pin-codeql-library-versions.sh, though ext/ + # ext-library-sources are excluded there for a different reason - + # see that script's header comment): # - ./ql/hotspots is a standalone local dev tool (see # ql/hotspots/README.md and .github/workflows/hotspots.yml), not # one of the per-language src/lib/ext/ext-library-sources packs @@ -109,14 +116,16 @@ jobs: # checkout (used by ql/hotspots) and CodeQL's own per-pack build # caches, respectively - neither is a repo pack either. # - /ext and /ext-library-sources are CodeQL model/extension packs - # (`extensionTargets`, no `dependencies`) - NOT excluded from - # pin-codeql-library-versions.sh (their extensionTargets versions still need - # pinning), but excluded here: `codeql pack upgrade` (re)writes their - # codeql-pack.lock.yml with an empty `dependencies: {}` map, and a checked-in - # lock file in that state makes a later `codeql pack create`/`publish` emit a - # bogus `addsTo.pack '...' is not an extension target of '...'` warning for - # every data extension in the pack (a known CodeQL CLI bug, see - # https://github.com/github/codeql/issues/20211). See CONTRIBUTING.md. + # (`extensionTargets`, no `dependencies`) - excluded here for TWO independent + # reasons, either of which alone would be enough: (1) `codeql pack upgrade` + # (re)writes their codeql-pack.lock.yml with an empty `dependencies: {}` map, + # and a checked-in lock file in that state makes a later `codeql pack + # create`/`publish` emit a bogus `addsTo.pack '...' is not an extension target + # of '...'` warning for every data extension in the pack (a known CodeQL CLI + # bug, see https://github.com/github/codeql/issues/20211); (2) `extensionTargets` + # is intentionally left as an unconstrained `'*'` (see qlpack.yml's own comment + # and CONTRIBUTING.md) and must never be auto-rewritten by tooling - see + # GitHubSecurityLab/CodeQL-Community-Packs#206. for dir in $(find . -name qlpack.yml -not -path "./ql/hotspots/*" -not -path "./codeql_home/*" -not -path "./codeql/*" -not -path "*/.codeql/*" -not -path "*/ext/*" -not -path "*/ext-library-sources/*" -exec dirname {} \;); do echo "::group::codeql pack upgrade $dir" codeql pack upgrade "$dir" @@ -234,12 +243,16 @@ jobs: echo echo "This PR:" echo '- Updates `.codeqlversion` to `'"${{ steps.version.outputs.version }}"'`.' - echo '- Pins every `codeql/-all` / `codeql/-queries` dependency across all' - echo ' `qlpack.yml` files to the exact version shipped in the official CodeQL Bundle' - echo ' for this CLI release (see `.github/scripts/pin-codeql-library-versions.sh`) -' - echo ' this keeps `codeql pack upgrade` from jumping those libraries to' - echo ' registry-latest instead of the version this CLI actually ships/tests against.' - echo '- Runs `codeql pack upgrade ` for every pack directory to refresh its' + echo '- Pins every `codeql/-all` / `codeql/-queries` `dependencies:`' + echo ' entry across query/library `qlpack.yml` files (not `*/ext` or' + echo ' `*/ext-library-sources`, whose `extensionTargets` is intentionally left' + echo ' unconstrained and must never be auto-rewritten) to the exact version' + echo ' shipped in the official CodeQL Bundle for this CLI release (see' + echo ' `.github/scripts/pin-codeql-library-versions.sh`) - this keeps' + echo ' `codeql pack upgrade` from jumping those libraries to registry-latest instead' + echo ' of the version this CLI actually ships/tests against.' + echo '- Runs `codeql pack upgrade ` for every query/library pack directory (again' + echo ' excluding `*/ext` and `*/ext-library-sources`) to refresh its' echo ' `codeql-pack.lock.yml` against the new CLI and pinned library versions.' echo if [[ -n "${{ steps.release_bump.outputs.bump }}" ]]; then diff --git a/CONTRIBUTING.md b/CONTRIBUTING.md index 5e76f963..ae4d8a6d 100644 --- a/CONTRIBUTING.md +++ b/CONTRIBUTING.md @@ -118,6 +118,68 @@ The pinning is codified per language across: > this shortly after this pinning behavior was introduced and a pack's `qlpack.yml` still shows > `codeql/: '*'`, that just means its dependencies haven't been re-resolved since, not that the > convention doesn't apply to it. +> +> **This exact-pin treatment applies to `dependencies:` only** (query/library packs: `src`, `lib`, +> etc.) - `/ext` and `/ext-library-sources` model/extension packs use +> `extensionTargets:` instead, and are handled completely differently; see the next warning box. + +> [!WARNING] +> **Why `extensionTargets` is always `'*'` (fully unconstrained), +> never pinned or floored (unlike `dependencies:` above):** `extensionTargets` and `dependencies:` +> look similar but fail very differently when a consumer's CodeQL CLI doesn't bundle the exact +> version this repo pinned against. An unsatisfiable `dependencies:` constraint fails **loudly** - +> `codeql pack install`/`resolve-dependencies` refuses with `ERROR: No valid pack solution +> found...` - and even a stale/incompatible pin on the CLI-bundled `codeql/-all` itself is +> harmless in practice, because the CLI always resolves that specific dependency to whatever `-all` +> version it actually bundles regardless of the declared range, and any genuine incompatibility then +> surfaces as an ordinary QL compile error. An unsatisfiable `extensionTargets` constraint does +> neither: the CLI just **silently drops the entire extension pack** - zero data-extension rows +> applied, no compile error, no SARIF-level signal, job stays green - and surfaces only a +> low-visibility `WARNING: Extension pack '' is unused.` that's easy to miss in CI logs. +> Pinning or flooring `extensionTargets` to whatever this repo's own CI happens to test against on +> every routine CLI bump (the way [`pin-codeql-library-versions.sh`][pin-codeql-library-versions-script] +> handles `dependencies:`) therefore turns *any* drift between this repo's tested CLI version and a +> consumer's actual CLI version into silent, undetected loss of coverage - this is exactly what +> happened in [#206][issue-206], reproduced and confirmed there across a local CLI, a genuine GHCR +> registry install, and a real hosted GitHub Actions run. +> +> The fix: `extensionTargets` is set to the fully unconstrained `codeql/-all: '*'` for every +> language, and [`pin-codeql-library-versions.sh`][pin-codeql-library-versions-script] explicitly +> excludes `*/ext` and `*/ext-library-sources` from its rewrite loop so a routine CLI bump can never +> silently tighten it to an exact version or floor again. This is not a new/unproven approach: `'*'` +> was the **original design** for these packs - `java/ext/qlpack.yml`, for example, used +> `extensionTargets: codeql/java-all: '*'` continuously for roughly two and a half years (September +> 2023 to July 2026, spanning many `codeql/java-all` major version bumps) with no reported issues, +> until an unrelated CLI-bump automation PR +> ([#166](https://github.com/GitHubSecurityLab/CodeQL-Community-Packs/pull/166)) blindly regex-replaced +> every `codeql/:` version line - `dependencies:` and `extensionTargets:` alike, with no +> distinction between the two - across all four languages, unintentionally introducing the exact-pin +> that later caused #206. Reverting to `'*'` restores the long-standing, empirically-proven-safe +> status quo rather than adopting something new. +> +> `'*'` works safely here because (a) `extensionTargets` never participates in `codeql pack +> install`/`upgrade`'s registry dependency resolution at all - it's purely a compatibility check +> against whatever `codeql/-all` the *query* pack itself already resolved via its own +> `dependencies:`, so `'*'` cannot "fetch latest" the way an unconstrained `dependencies:` entry +> would - and (b) the models-as-data extensible predicates (`sinkModel`, `sourceModel`, +> `summaryModel`, `neutralModel`, etc.) these packs contribute rows to are a very stable schema. +> A dedicated investigation across all four supported languages, tracing every commit to each +> language's `ExternalFlowExtensions.qll` in [github/codeql](https://github.com/github/codeql) from +> the predicates' original introduction (late 2022) to today, found exactly **one** genuine +> breaking change, ever: `neutralModel` gained a `kind` column (5 → 6 positional values) in both +> Java and C#, shipped in `codeql/java-all`/`codeql/csharp-all` **0.6.2** (CodeQL CLI **v2.13.3**, +> May 2023) - documented in each language's own `CHANGELOG.md`. Go and Python have never had a +> breaking change to any of these predicates. Critically, even that one historical break is not a +> silent-drop scenario: an old 5-column `neutralModel` row loaded against the newer 6-column schema +> fails with an ordinary QL compile error (wrong arity) - loud, and caught immediately by this +> repo's own CI - exactly the kind of failure `dependencies:` constraints are designed to guard +> against, and exactly why an `extensionTargets` floor/pin isn't needed to protect against it either. +> **No evidence exists of a floor ever having been required historically; a genuine future breaking +> change would fail this repo's own CI loudly, which is the correct and sufficient signal** - only +> raise an `extensionTargets` floor by hand if that ever actually happens (upstream `github/codeql` +> CHANGELOG entries for the relevant `ExternalFlowExtensions.qll`/`ModelsAsData.qll` predicates are +> the place to check), never preemptively. + **This section no longer hand-maintains a version table** - it used to, but that table went stale across multiple CLI bumps in a row (nobody remembered to update it, and there was no CI check @@ -313,6 +375,10 @@ coding agent) in the loop for the hard part — fixing whatever the new CLI brea > If you run `codeql pack install`/`upgrade` against one of these directories locally while > developing (e.g. to sanity-check a new data extension), delete the resulting > `codeql-pack.lock.yml` before committing. +> +> This is a separate concern from how `extensionTargets`' *version value* itself is chosen and +> maintained — see the ["Why `extensionTargets` is always `'*'`, never pinned or +> floored"](#extensiontargets-floor) warning box above. > [!WARNING] > The `.codeqlversion` bump and the pack version bumps don't have to land in the same PR, but @@ -463,3 +529,4 @@ Please do get in touch (privacy@github.com) if you have any questions about this [pr-155]: https://github.com/GitHubSecurityLab/CodeQL-Community-Packs/pull/155 [pr-158]: https://github.com/GitHubSecurityLab/CodeQL-Community-Packs/pull/158 [pr-159]: https://github.com/GitHubSecurityLab/CodeQL-Community-Packs/pull/159 +[issue-206]: https://github.com/GitHubSecurityLab/CodeQL-Community-Packs/issues/206 diff --git a/csharp/ext-library-sources/qlpack.yml b/csharp/ext-library-sources/qlpack.yml index 2dd7632a..52ede412 100644 --- a/csharp/ext-library-sources/qlpack.yml +++ b/csharp/ext-library-sources/qlpack.yml @@ -1,8 +1,14 @@ library: true name: githubsecuritylab/codeql-csharp-library-sources version: 0.7.3 +# Keep this '*' (unconstrained) - never pin/floor it to a specific +# codeql/csharp-all version. Unlike `dependencies:`, an unsatisfied +# extensionTargets constraint doesn't fail loudly: it silently drops this +# whole extension pack (zero models applied) with only a low-visibility +# "is unused" warning, no CI signal for downstream consumers. See +# GitHubSecurityLab/CodeQL-Community-Packs#206 and CONTRIBUTING.md. extensionTargets: - codeql/csharp-all: '7.1.1' + codeql/csharp-all: '*' dataExtensions: - 'manual/*.yml' - 'manual/**/*.yml' diff --git a/csharp/ext/qlpack.yml b/csharp/ext/qlpack.yml index e38847da..53693f08 100644 --- a/csharp/ext/qlpack.yml +++ b/csharp/ext/qlpack.yml @@ -1,8 +1,14 @@ library: true name: githubsecuritylab/codeql-csharp-extensions version: 0.7.3 +# Keep this '*' (unconstrained) - never pin/floor it to a specific +# codeql/csharp-all version. Unlike `dependencies:`, an unsatisfied +# extensionTargets constraint doesn't fail loudly: it silently drops this +# whole extension pack (zero models applied) with only a low-visibility +# "is unused" warning, no CI signal for downstream consumers. See +# GitHubSecurityLab/CodeQL-Community-Packs#206 and CONTRIBUTING.md. extensionTargets: - codeql/csharp-all: '7.1.1' + codeql/csharp-all: '*' dataExtensions: - 'manual/*.yml' - 'manual/**/*.yml' diff --git a/go/ext/qlpack.yml b/go/ext/qlpack.yml index 1546602b..f7abb2f3 100644 --- a/go/ext/qlpack.yml +++ b/go/ext/qlpack.yml @@ -1,8 +1,14 @@ library: true name: githubsecuritylab/codeql-go-extensions version: 0.7.3 +# Keep this '*' (unconstrained) - never pin/floor it to a specific +# codeql/go-all version. Unlike `dependencies:`, an unsatisfied +# extensionTargets constraint doesn't fail loudly: it silently drops this +# whole extension pack (zero models applied) with only a low-visibility +# "is unused" warning, no CI signal for downstream consumers. See +# GitHubSecurityLab/CodeQL-Community-Packs#206 and CONTRIBUTING.md. extensionTargets: - codeql/go-all: '7.2.2' + codeql/go-all: '*' dataExtensions: - 'manual/*.yml' - 'manual/**/*.yml' diff --git a/java/ext-library-sources/qlpack.yml b/java/ext-library-sources/qlpack.yml index 933df57f..1ce181a8 100644 --- a/java/ext-library-sources/qlpack.yml +++ b/java/ext-library-sources/qlpack.yml @@ -1,8 +1,14 @@ library: true name: githubsecuritylab/codeql-java-library-sources version: 0.7.3 +# Keep this '*' (unconstrained) - never pin/floor it to a specific +# codeql/java-all version. Unlike `dependencies:`, an unsatisfied +# extensionTargets constraint doesn't fail loudly: it silently drops this +# whole extension pack (zero models applied) with only a low-visibility +# "is unused" warning, no CI signal for downstream consumers. See +# GitHubSecurityLab/CodeQL-Community-Packs#206 and CONTRIBUTING.md. extensionTargets: - codeql/java-all: '9.2.2' + codeql/java-all: '*' dataExtensions: - 'manual/*.yml' - 'manual/**/*.yml' diff --git a/java/ext/qlpack.yml b/java/ext/qlpack.yml index 649ac4b4..368bd705 100644 --- a/java/ext/qlpack.yml +++ b/java/ext/qlpack.yml @@ -1,8 +1,14 @@ library: true name: githubsecuritylab/codeql-java-extensions version: 0.7.3 +# Keep this '*' (unconstrained) - never pin/floor it to a specific +# codeql/java-all version. Unlike `dependencies:`, an unsatisfied +# extensionTargets constraint doesn't fail loudly: it silently drops this +# whole extension pack (zero models applied) with only a low-visibility +# "is unused" warning, no CI signal for downstream consumers. See +# GitHubSecurityLab/CodeQL-Community-Packs#206 and CONTRIBUTING.md. extensionTargets: - codeql/java-all: '9.2.2' + codeql/java-all: '*' dataExtensions: - 'manual/*.yml' - 'manual/**/*.yml' diff --git a/python/ext/qlpack.yml b/python/ext/qlpack.yml index 5a29bf76..7c9397d1 100644 --- a/python/ext/qlpack.yml +++ b/python/ext/qlpack.yml @@ -1,8 +1,14 @@ library: true name: githubsecuritylab/codeql-python-extensions version: 0.7.3 +# Keep this '*' (unconstrained) - never pin/floor it to a specific +# codeql/python-all version. Unlike `dependencies:`, an unsatisfied +# extensionTargets constraint doesn't fail loudly: it silently drops this +# whole extension pack (zero models applied) with only a low-visibility +# "is unused" warning, no CI signal for downstream consumers. See +# GitHubSecurityLab/CodeQL-Community-Packs#206 and CONTRIBUTING.md. extensionTargets: - codeql/python-all: '7.2.2' + codeql/python-all: '*' dataExtensions: - 'manual/*.yml' - 'manual/**/*.yml'