Skip to content

Commit f14d2b9

Browse files
committed
fix: use unconstrained extensionTargets ('*') for all languages, not a version floor
Supersedes the earlier Java-only, version-floor approach with the correct fix, based on further investigation: - Extends the fix to all 4 languages that have ext/ext-library-sources packs (java, csharp, go, python), not just java. - Uses fully unconstrained extensionTargets: '*' instead of a version floor ('>=X.Y.Z'). A floor only protects consumers on newer CLIs; consumers on older CLIs below the floor would still be silently dropped - the same bug, just shifted. - Git archaeology on java/ext/qlpack.yml shows '*' was the original, intentional design for ~2.9 years (Sept 2023-July 2026) until PR #166's CLI-bump automation blindly regex-replaced every codeql/<pkg>: version line - dependencies: and extensionTargets: alike - across all 4 languages, with no distinction between the two. This was an unintentional regression, not a deliberate compatibility fix. - extensionTargets never participates in codeql pack install/upgrade's registry dependency resolution (confirmed empirically: no lock-file entry is ever generated for it), so '*' here cannot 'fetch latest' the way an unconstrained dependencies: entry would - it only means 'apply regardless of which <lang>-all version is in play'. - Dedicated research across all 4 languages' ExternalFlowExtensions.qll history in github/codeql found exactly one genuine breaking change, ever: neutralModel gained a kind column in both Java and C# (codeql/java-all and codeql/csharp-all 0.6.2, CodeQL CLI v2.13.3, May 2023). Go and Python have never had a breaking change. Even that one break fails loudly (QL compile error), never silently - so this repo's own CI is a sufficient tripwire for any future break, without needing a preemptive floor. Also: - pin-codeql-library-versions.sh and update-codeql-version.yml comments updated to describe the '*' decision (was previously written to describe a version-floor approach that was never actually correct). - Added a short protective comment above extensionTargets: in all 6 qlpack.yml files explaining what happens if it's ever pinned/floored again, so a future manual edit doesn't reintroduce #206. Fixes #206
1 parent c349de9 commit f14d2b9

9 files changed

Lines changed: 139 additions & 75 deletions

File tree

‎.github/scripts/pin-codeql-library-versions.sh‎

Lines changed: 22 additions & 20 deletions
Original file line numberDiff line numberDiff line change
@@ -10,12 +10,14 @@
1010
# - those are model/extension packs that declare `extensionTargets:`, not
1111
# `dependencies:`, and are excluded from this script's file list entirely. See
1212
# CONTRIBUTING.md's "Supported CodeQL versions" section ("Why `extensionTargets`
13-
# is a floor, not an exact pin") for why: unlike `dependencies:`, an unsatisfied
13+
# is always `'*'`, never pinned") for why: unlike `dependencies:`, an unsatisfied
1414
# `extensionTargets` constraint doesn't fail loudly, it silently drops the whole
15-
# extension pack, so re-pinning it to an exact version on every CLI bump (the
16-
# way this script handles `dependencies:`) turns routine consumer CLI drift into
17-
# silent, undetected loss of coverage (GitHubSecurityLab/CodeQL-Community-Packs#206).
18-
# `extensionTargets` floors are maintained by hand instead, as open-ended ranges.
15+
# extension pack, so re-pinning it - to an exact version OR a version floor -
16+
# on every CLI bump (the way this script handles `dependencies:`) turns routine
17+
# consumer CLI drift into silent, undetected loss of coverage
18+
# (GitHubSecurityLab/CodeQL-Community-Packs#206). `extensionTargets` is left as
19+
# the fully unconstrained `'*'` instead, and this script must never touch it -
20+
# see the comment above `extensionTargets:` in each `<language>/ext*/qlpack.yml`.
1921
#
2022
# Why this exists (for `dependencies:`): `codeql pack upgrade` resolves an
2123
# unconstrained `'*'` dependency to the *latest-ever-published* version in the
@@ -87,21 +89,21 @@ echo "Discovered $(wc -l < "$VERSIONS_FILE") bundled codeql/* packages for CLI $
8789
# codeql/<lang>/downgrades, that have nothing to do with this repo) - and
8890
# excluding every `<language>/ext` and `<language>/ext-library-sources`
8991
# model/extension pack. Those use `extensionTargets:` instead of
90-
# `dependencies:`, and deliberately do NOT get re-pinned to the exact bundle
91-
# version on every CLI bump the way `dependencies:` does here (see
92-
# CONTRIBUTING.md's "Supported CodeQL versions" section, "Why
93-
# `extensionTargets` is a floor, not an exact pin"). Unlike `dependencies:`,
94-
# an unsatisfied `extensionTargets` constraint doesn't fail loudly - the CLI
95-
# just silently drops the whole extension pack (zero data-extension rows
96-
# applied, only a low-visibility `WARNING: ... is unused`) - so constantly
97-
# re-pinning it to whatever this repo's own CI happens to test against turns
98-
# every routine consumer CLI/version mismatch into silent, undetected loss of
99-
# coverage (see GitHubSecurityLab/CodeQL-Community-Packs#206). Instead,
100-
# `extensionTargets` is set by hand as an open-ended floor
101-
# (`codeql/<lang>-all: '>=X.Y.Z'`) and only ever raised when a maintainer
102-
# confirms an actual breaking change to the models-as-data schema (e.g. an
103-
# extensible predicate's arity/column set changed) - our own test suite
104-
# failing is the trigger to look for that, not a routine CLI bump.
92+
# `dependencies:`, and deliberately do NOT get re-pinned on every CLI bump the
93+
# way `dependencies:` does here (see CONTRIBUTING.md's "Supported CodeQL
94+
# versions" section, "Why `extensionTargets` is always `'*'`, never pinned or
95+
# floored"). Unlike `dependencies:`, an unsatisfied `extensionTargets`
96+
# constraint doesn't fail loudly - the CLI just silently drops the whole
97+
# extension pack (zero data-extension rows applied, only a low-visibility
98+
# `WARNING: ... is unused`) - so re-pinning it to whatever this repo's own CI
99+
# happens to test against, on every routine CLI bump, turns every consumer
100+
# CLI/version mismatch into silent, undetected loss of coverage (see
101+
# GitHubSecurityLab/CodeQL-Community-Packs#206). Instead, `extensionTargets`
102+
# is left as the fully unconstrained `codeql/<lang>-all: '*'` and only ever
103+
# changed by hand if a maintainer confirms an actual breaking change to the
104+
# models-as-data schema (e.g. an extensible predicate's arity/column set
105+
# changed) - our own test suite failing is the trigger to look for that, not
106+
# a routine CLI bump.
105107
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/*")
106108

107109
declare -A PINNED_COUNT=()

‎.github/workflows/update-codeql-version.yml‎

Lines changed: 22 additions & 17 deletions
Original file line numberDiff line numberDiff line change
@@ -87,7 +87,8 @@ jobs:
8787
# NOTE: this only re-pins `dependencies:` in query/library packs
8888
# (<language>/src, <language>/lib, etc.) - it deliberately skips
8989
# <language>/ext and <language>/ext-library-sources, whose
90-
# `extensionTargets:` floors are maintained by hand (see
90+
# `extensionTargets:` is intentionally left as an unconstrained
91+
# `'*'` and must never be auto-rewritten (see
9192
# pin-codeql-library-versions.sh's header comment and
9293
# CONTRIBUTING.md's "Supported CodeQL versions" section).
9394
.github/scripts/pin-codeql-library-versions.sh "${{ steps.version.outputs.version }}"
@@ -97,10 +98,10 @@ jobs:
9798
GITHUB_TOKEN: ${{ github.token }}
9899
run: |
99100
set -euo pipefail
100-
# Exclusions (the first three below are also excluded in
101-
# .github/scripts/pin-codeql-library-versions.sh; the last one is
102-
# NOT - ext/ext-library-sources still need their extensionTargets
103-
# versions pinned, but must be skipped here to avoid lock-file churn):
101+
# Exclusions (all four are also excluded in
102+
# .github/scripts/pin-codeql-library-versions.sh, though ext/
103+
# ext-library-sources are excluded there for a different reason -
104+
# see that script's header comment):
104105
# - ./ql/hotspots is a standalone local dev tool (see
105106
# ql/hotspots/README.md and .github/workflows/hotspots.yml), not
106107
# one of the per-language src/lib/ext/ext-library-sources packs
@@ -115,14 +116,16 @@ jobs:
115116
# checkout (used by ql/hotspots) and CodeQL's own per-pack build
116117
# caches, respectively - neither is a repo pack either.
117118
# - <lang>/ext and <lang>/ext-library-sources are CodeQL model/extension packs
118-
# (`extensionTargets`, no `dependencies`) - NOT excluded from
119-
# pin-codeql-library-versions.sh (their extensionTargets versions still need
120-
# pinning), but excluded here: `codeql pack upgrade` (re)writes their
121-
# codeql-pack.lock.yml with an empty `dependencies: {}` map, and a checked-in
122-
# lock file in that state makes a later `codeql pack create`/`publish` emit a
123-
# bogus `addsTo.pack '...' is not an extension target of '...'` warning for
124-
# every data extension in the pack (a known CodeQL CLI bug, see
125-
# https://github.com/github/codeql/issues/20211). See CONTRIBUTING.md.
119+
# (`extensionTargets`, no `dependencies`) - excluded here for TWO independent
120+
# reasons, either of which alone would be enough: (1) `codeql pack upgrade`
121+
# (re)writes their codeql-pack.lock.yml with an empty `dependencies: {}` map,
122+
# and a checked-in lock file in that state makes a later `codeql pack
123+
# create`/`publish` emit a bogus `addsTo.pack '...' is not an extension target
124+
# of '...'` warning for every data extension in the pack (a known CodeQL CLI
125+
# bug, see https://github.com/github/codeql/issues/20211); (2) `extensionTargets`
126+
# is intentionally left as an unconstrained `'*'` (see qlpack.yml's own comment
127+
# and CONTRIBUTING.md) and must never be auto-rewritten by tooling - see
128+
# GitHubSecurityLab/CodeQL-Community-Packs#206.
126129
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
127130
echo "::group::codeql pack upgrade $dir"
128131
codeql pack upgrade "$dir"
@@ -242,12 +245,14 @@ jobs:
242245
echo '- Updates `.codeqlversion` to `'"${{ steps.version.outputs.version }}"'`.'
243246
echo '- Pins every `codeql/<lang>-all` / `codeql/<lang>-queries` `dependencies:`'
244247
echo ' entry across query/library `qlpack.yml` files (not `*/ext` or'
245-
echo ' `*/ext-library-sources`, whose `extensionTargets` floors are maintained by'
246-
echo ' hand) to the exact version shipped in the official CodeQL Bundle for this CLI'
247-
echo ' release (see `.github/scripts/pin-codeql-library-versions.sh`) - this keeps'
248+
echo ' `*/ext-library-sources`, whose `extensionTargets` is intentionally left'
249+
echo ' unconstrained and must never be auto-rewritten) to the exact version'
250+
echo ' shipped in the official CodeQL Bundle for this CLI release (see'
251+
echo ' `.github/scripts/pin-codeql-library-versions.sh`) - this keeps'
248252
echo ' `codeql pack upgrade` from jumping those libraries to registry-latest instead'
249253
echo ' of the version this CLI actually ships/tests against.'
250-
echo '- Runs `codeql pack upgrade <dir>` for every pack directory to refresh its'
254+
echo '- Runs `codeql pack upgrade <dir>` for every query/library pack directory (again'
255+
echo ' excluding `*/ext` and `*/ext-library-sources`) to refresh its'
251256
echo ' `codeql-pack.lock.yml` against the new CLI and pinned library versions.'
252257
echo
253258
if [[ -n "${{ steps.release_bump.outputs.bump }}" ]]; then

‎CONTRIBUTING.md‎

Lines changed: 53 additions & 32 deletions
Original file line numberDiff line numberDiff line change
@@ -124,40 +124,61 @@ The pinning is codified per language across:
124124
> `extensionTargets:` instead, and are handled completely differently; see the next warning box.
125125
126126
> [!WARNING]
127-
> <a id="extensiontargets-floor"></a>**Why `extensionTargets` is an open-ended floor, not an exact
128-
> pin (unlike `dependencies:` above):** `extensionTargets` and `dependencies:` look similar but fail
129-
> very differently when a consumer's CodeQL CLI doesn't bundle the exact version this repo pinned
130-
> against. An unsatisfiable `dependencies:` constraint fails **loudly** - `codeql pack
131-
> install`/`resolve-dependencies` refuses with `ERROR: No valid pack solution found...` - and even a
132-
> stale/incompatible pin on the CLI-bundled `codeql/<lang>-all` itself is harmless in practice,
133-
> because the CLI always resolves that specific dependency to whatever `-all` version it actually
134-
> bundles regardless of the declared range, and any genuine incompatibility then surfaces as an
135-
> ordinary QL compile error. An unsatisfiable `extensionTargets` constraint does neither: the CLI
136-
> just **silently drops the entire extension pack** - zero data-extension rows applied, no compile
137-
> error, no SARIF-level signal, job stays green - and surfaces only a low-visibility
138-
> `WARNING: Extension pack '<name>' is unused.` that's easy to miss in CI logs. Exact-pinning
139-
> `extensionTargets` to whatever this repo's own CI happens to test against on every routine CLI bump
140-
> (the way [`pin-codeql-library-versions.sh`][pin-codeql-library-versions-script] handles
141-
> `dependencies:`) therefore turns *any* drift between this repo's tested CLI version and a
127+
> <a id="extensiontargets-floor"></a>**Why `extensionTargets` is always `'*'` (fully unconstrained),
128+
> never pinned or floored (unlike `dependencies:` above):** `extensionTargets` and `dependencies:`
129+
> look similar but fail very differently when a consumer's CodeQL CLI doesn't bundle the exact
130+
> version this repo pinned against. An unsatisfiable `dependencies:` constraint fails **loudly** -
131+
> `codeql pack install`/`resolve-dependencies` refuses with `ERROR: No valid pack solution
132+
> found...` - and even a stale/incompatible pin on the CLI-bundled `codeql/<lang>-all` itself is
133+
> harmless in practice, because the CLI always resolves that specific dependency to whatever `-all`
134+
> version it actually bundles regardless of the declared range, and any genuine incompatibility then
135+
> surfaces as an ordinary QL compile error. An unsatisfiable `extensionTargets` constraint does
136+
> neither: the CLI just **silently drops the entire extension pack** - zero data-extension rows
137+
> applied, no compile error, no SARIF-level signal, job stays green - and surfaces only a
138+
> low-visibility `WARNING: Extension pack '<name>' is unused.` that's easy to miss in CI logs.
139+
> Pinning or flooring `extensionTargets` to whatever this repo's own CI happens to test against on
140+
> every routine CLI bump (the way [`pin-codeql-library-versions.sh`][pin-codeql-library-versions-script]
141+
> handles `dependencies:`) therefore turns *any* drift between this repo's tested CLI version and a
142142
> consumer's actual CLI version into silent, undetected loss of coverage - this is exactly what
143143
> happened in [#206][issue-206], reproduced and confirmed there across a local CLI, a genuine GHCR
144144
> registry install, and a real hosted GitHub Actions run.
145145
>
146-
> The fix: `extensionTargets` is set **by hand** as an open-ended floor
147-
> (`codeql/<lang>-all: '>=X.Y.Z'`, no upper bound) instead of an exact pin, and
148-
> [`pin-codeql-library-versions.sh`][pin-codeql-library-versions-script] explicitly excludes
149-
> `*/ext` and `*/ext-library-sources` from its rewrite loop so a routine CLI bump can never silently
150-
> re-tighten it back to an exact version. An open floor works because the models-as-data extensible
151-
> predicates (`sinkModel`, `sourceModel`, `summaryModel`, etc.) these packs contribute rows to are a
152-
> stable schema that only changes on a genuine breaking change (an extensible predicate's
153-
> arity/column set changing) - which is rare and would show up as real compilation/test failures in
154-
> this repo's own CI, not something that needs to be guarded against on every release. Only raise an
155-
> `extensionTargets` floor by hand when there's evidence of an actual such breaking change (upstream
156-
> `github/codeql` CHANGELOG "Breaking Changes" entries are a good place to check, though in practice
157-
> most of those are QL library API changes that affect query/library packs, not the data-extension
158-
> row format) - not just because a newer CLI happens to be available. As of this writing, Java's
159-
> `<language>/ext`/`ext-library-sources` packs pilot this open-floor approach; other languages still
160-
> use an exact `extensionTargets` pin pending the same conversion.
146+
> The fix: `extensionTargets` is set to the fully unconstrained `codeql/<lang>-all: '*'` for every
147+
> language, and [`pin-codeql-library-versions.sh`][pin-codeql-library-versions-script] explicitly
148+
> excludes `*/ext` and `*/ext-library-sources` from its rewrite loop so a routine CLI bump can never
149+
> silently tighten it to an exact version or floor again. This is not a new/unproven approach: `'*'`
150+
> was the **original design** for these packs - `java/ext/qlpack.yml`, for example, used
151+
> `extensionTargets: codeql/java-all: '*'` continuously for roughly two and a half years (September
152+
> 2023 to July 2026, spanning many `codeql/java-all` major version bumps) with no reported issues,
153+
> until an unrelated CLI-bump automation PR
154+
> ([#166](https://github.com/GitHubSecurityLab/CodeQL-Community-Packs/pull/166)) blindly regex-replaced
155+
> every `codeql/<pkg>:` version line - `dependencies:` and `extensionTargets:` alike, with no
156+
> distinction between the two - across all four languages, unintentionally introducing the exact-pin
157+
> that later caused #206. Reverting to `'*'` restores the long-standing, empirically-proven-safe
158+
> status quo rather than adopting something new.
159+
>
160+
> `'*'` works safely here because (a) `extensionTargets` never participates in `codeql pack
161+
> install`/`upgrade`'s registry dependency resolution at all - it's purely a compatibility check
162+
> against whatever `codeql/<lang>-all` the *query* pack itself already resolved via its own
163+
> `dependencies:`, so `'*'` cannot "fetch latest" the way an unconstrained `dependencies:` entry
164+
> would - and (b) the models-as-data extensible predicates (`sinkModel`, `sourceModel`,
165+
> `summaryModel`, `neutralModel`, etc.) these packs contribute rows to are a very stable schema.
166+
> A dedicated investigation across all four supported languages, tracing every commit to each
167+
> language's `ExternalFlowExtensions.qll` in [github/codeql](https://github.com/github/codeql) from
168+
> the predicates' original introduction (late 2022) to today, found exactly **one** genuine
169+
> breaking change, ever: `neutralModel` gained a `kind` column (5 → 6 positional values) in both
170+
> Java and C#, shipped in `codeql/java-all`/`codeql/csharp-all` **0.6.2** (CodeQL CLI **v2.13.3**,
171+
> May 2023) - documented in each language's own `CHANGELOG.md`. Go and Python have never had a
172+
> breaking change to any of these predicates. Critically, even that one historical break is not a
173+
> silent-drop scenario: an old 5-column `neutralModel` row loaded against the newer 6-column schema
174+
> fails with an ordinary QL compile error (wrong arity) - loud, and caught immediately by this
175+
> repo's own CI - exactly the kind of failure `dependencies:` constraints are designed to guard
176+
> against, and exactly why an `extensionTargets` floor/pin isn't needed to protect against it either.
177+
> **No evidence exists of a floor ever having been required historically; a genuine future breaking
178+
> change would fail this repo's own CI loudly, which is the correct and sufficient signal** - only
179+
> raise an `extensionTargets` floor by hand if that ever actually happens (upstream `github/codeql`
180+
> CHANGELOG entries for the relevant `ExternalFlowExtensions.qll`/`ModelsAsData.qll` predicates are
181+
> the place to check), never preemptively.
161182
162183

163184
**This section no longer hand-maintains a version table** - it used to, but that table went stale
@@ -356,8 +377,8 @@ coding agent) in the loop for the hard part — fixing whatever the new CLI brea
356377
> `codeql-pack.lock.yml` before committing.
357378
>
358379
> This is a separate concern from how `extensionTargets`' *version value* itself is chosen and
359-
> maintained — see the ["Why `extensionTargets` is an open-ended floor, not an exact
360-
> pin"](#extensiontargets-floor) warning box above.
380+
> maintained — see the ["Why `extensionTargets` is always `'*'`, never pinned or
381+
> floored"](#extensiontargets-floor) warning box above.
361382
362383
> [!WARNING]
363384
> The `.codeqlversion` bump and the pack version bumps don't have to land in the same PR, but

‎csharp/ext-library-sources/qlpack.yml‎

Lines changed: 7 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1,8 +1,14 @@
11
library: true
22
name: githubsecuritylab/codeql-csharp-library-sources
33
version: 0.7.3
4+
# Keep this '*' (unconstrained) - never pin/floor it to a specific
5+
# codeql/csharp-all version. Unlike `dependencies:`, an unsatisfied
6+
# extensionTargets constraint doesn't fail loudly: it silently drops this
7+
# whole extension pack (zero models applied) with only a low-visibility
8+
# "is unused" warning, no CI signal for downstream consumers. See
9+
# GitHubSecurityLab/CodeQL-Community-Packs#206 and CONTRIBUTING.md.
410
extensionTargets:
5-
codeql/csharp-all: '7.1.1'
11+
codeql/csharp-all: '*'
612
dataExtensions:
713
- 'manual/*.yml'
814
- 'manual/**/*.yml'

‎csharp/ext/qlpack.yml‎

Lines changed: 7 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1,8 +1,14 @@
11
library: true
22
name: githubsecuritylab/codeql-csharp-extensions
33
version: 0.7.3
4+
# Keep this '*' (unconstrained) - never pin/floor it to a specific
5+
# codeql/csharp-all version. Unlike `dependencies:`, an unsatisfied
6+
# extensionTargets constraint doesn't fail loudly: it silently drops this
7+
# whole extension pack (zero models applied) with only a low-visibility
8+
# "is unused" warning, no CI signal for downstream consumers. See
9+
# GitHubSecurityLab/CodeQL-Community-Packs#206 and CONTRIBUTING.md.
410
extensionTargets:
5-
codeql/csharp-all: '7.1.1'
11+
codeql/csharp-all: '*'
612
dataExtensions:
713
- 'manual/*.yml'
814
- 'manual/**/*.yml'

‎go/ext/qlpack.yml‎

Lines changed: 7 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1,8 +1,14 @@
11
library: true
22
name: githubsecuritylab/codeql-go-extensions
33
version: 0.7.3
4+
# Keep this '*' (unconstrained) - never pin/floor it to a specific
5+
# codeql/go-all version. Unlike `dependencies:`, an unsatisfied
6+
# extensionTargets constraint doesn't fail loudly: it silently drops this
7+
# whole extension pack (zero models applied) with only a low-visibility
8+
# "is unused" warning, no CI signal for downstream consumers. See
9+
# GitHubSecurityLab/CodeQL-Community-Packs#206 and CONTRIBUTING.md.
410
extensionTargets:
5-
codeql/go-all: '7.2.2'
11+
codeql/go-all: '*'
612
dataExtensions:
713
- 'manual/*.yml'
814
- 'manual/**/*.yml'

0 commit comments

Comments
 (0)