From 7e7bf6e9fdd5e9b03c9675542eeb5b867e6bb00c Mon Sep 17 00:00:00 2001 From: Chad Bentz <1760475+felickz@users.noreply.github.com> Date: Mon, 3 Aug 2026 13:41:14 -0400 Subject: [PATCH 1/3] docs: add guide for adding a CodeQL data extension Add a new "Adding a data extension" section to CONTRIBUTING.md covering how to build a data-extension (MaD) pack: an explicit rule of thumb for choosing between `ext` and `ext-library-sources` (sink/summary models vs. remote-source models, and which languages have each), the MaD row shape, the `manual/` vs `generated/` convention, the `extensionTargets` version-pinning gotcha that causes silent zero-result failures on a mismatched local CLI, and how data extensions get picked up automatically by `codeql test run`. Also tightens the pack-directory bullet in "Shipping a change to a query/library pack" to correctly scope `ext-library-sources` to csharp/java only (previously bundled with `ext`'s wider csharp/go/java/ python list), and links it to the new section. Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com> Copilot-Session: f02ef23b-e35d-4536-bd58-f2fbc2976ba9 --- CONTRIBUTING.md | 95 +++++++++++++++++++++++++++++++++++++++++++++++-- 1 file changed, 93 insertions(+), 2 deletions(-) diff --git a/CONTRIBUTING.md b/CONTRIBUTING.md index 5e76f963..4afd8197 100644 --- a/CONTRIBUTING.md +++ b/CONTRIBUTING.md @@ -60,6 +60,95 @@ Queries and libraries may not be actively maintained as the supported libraries After the query is merged, we welcome pull requests to improve it. +## Adding a data extension + +Sometimes a false negative or false positive isn't a gap in a query's own logic, but a third-party +library API that the standard `codeql/-all` libraries simply don't know about — a +framework method that's really a SQL/command/path-injection sink, or a getter that hands back +untrusted remote data. For those, add a **data extension** (a "MaD" — Model-as-Data — model) +instead of, or alongside, a query change. + +1. **Pick the right pack: `ext` vs `ext-library-sources`** + + Every language with data-extension support has a `/ext` pack (`csharp`/`go`/`java`/ + `python`). Two of those languages additionally have a second, narrower pack, + `/ext-library-sources` (`csharp`/`java` only). They hold different *kinds* of models, + and picking the right one matters: + + - **`/ext`** (published as `githubsecuritylab/codeql--extensions`) is what + you want for almost every contribution: **sink models** (this library method is a dangerous + operation), **summary models** (taint flows from one argument/return value to another through + a library method), and, where appropriate, hand-curated **source models** for a specific, + well-known framework API (e.g. a web framework's `getParameter()`-style methods). This is + where our own [Spring R2DBC `DatabaseClient` SQL injection sink models][pr-204] live + (`java/ext/manual/org.springframework.r2dbc.model.yml`) — use it as a concrete reference for + both the file layout and the row shape. + - **`/ext-library-sources`** (published as + `githubsecuritylab/codeql--library-sources`) exists only for `csharp`/`java` and + holds a narrower category: `sourceModel` rows (almost always `kind: "remote"`) that flag a + third-party library API as a place where untrusted/attacker-controlled data enters an + application. Because a `RemoteFlowSource` feeds *every* standard security query, not just one + vulnerability class, these models have a much larger blast radius than a typical sink model, + so they're kept in a separate, more conservatively-reviewed pack. Most of the content here is + bulk-generated from real-world library usage (see + `/src/library_sources/ExternalAPIsUsedWithUntrustedData.ql`) rather than + hand-written — `manual/` in this pack is essentially unused today. + + **Rule of thumb: if you're modeling a dangerous sink, or how taint flows through a library + method, use `ext`. Only reach for `ext-library-sources` if you're contributing a genuinely new + remote-data-entry-point (source) model, and only for csharp/java.** + +2. **Row shape** + + Each data extension is a `.yml` file with an `extensions:` list. Every entry has an + `addsTo.pack` (always `codeql/-all`), an `addsTo.extensible` + (`sinkModel`/`sourceModel`/`summaryModel`/`neutralModel`), and a `data:` list of rows. See the + [CodeQL model pack + documentation](https://docs.github.com/en/code-security/tutorials/customize-code-scanning/create-and-work-with-codeql-packs#creating-a-codeql-model-pack) + for the full column reference (`namespace, type, subtypes, name, signature, ext, input/output, + kind, provenance`). `java/ext/manual/org.springframework.r2dbc.model.yml` shows a real + `sinkModel`/`summaryModel` pair side by side, with comments explaining *why* each row is shaped + the way it is — comment your own rows the same way whenever the mapping from source code to a + MaD row isn't obvious at a glance. + +3. **`manual/` vs `generated/`** + + Both `ext` and `ext-library-sources` load `manual/*.yml` and `generated/*.yml` (see each pack's + `qlpack.yml`, `dataExtensions:`). By convention, `manual/` is for hand-authored, human-reviewed + models — this is where a new library's sink/summary models should go — while `generated/` is + bulk output from an automated model-generation tool/query and isn't meant to be hand-edited. + +4. **The `extensionTargets` version gotcha** + + Your pack's `qlpack.yml` pins `extensionTargets: codeql/-all: ''` (see + `/ext/qlpack.yml`). This must be satisfied by the `codeql/-all` version + bundled with whatever CodeQL CLI is running the analysis — **if it isn't, your new models + silently attach zero results, with no error**, instead of failing loudly. This bit us during + development of the Spring R2DBC models above: a local CLI older than this repo's pinned + [`.codeqlversion`][codeqlversion] bundled a `codeql/java-all` below the pinned + `extensionTargets` range, so `codeql test run` produced 0 results locally for a genuinely-working + model — while CI (on the correctly pinned CLI) passed. If a local test run for a new data + extension doesn't find what you expect, check `codeql version` against + [`.codeqlversion`][codeqlversion] before assuming your model is wrong. + + Also see the [note below](#ext-packs-no-install) on why these packs are never `codeql pack + install`ed/`upgrade`d. + +5. **Testing** + + Add a test under `/test/security///`: minimal source stubs that + reproduce the vulnerable call shape, a `.ql` query that imports the *real* standard query (e.g. + `SqlInjectionQuery`/`QueryInjectionFlow`, not a reimplementation of it), and a `.expected` file. + `/test/qlpack.yml` already depends on `githubsecuritylab/codeql--extensions` + (and `-library-sources` where applicable) — that dependency is what makes `codeql test run`'s + normal dependency resolution load your new data extensions automatically, unlike ad-hoc `codeql + database analyze`/`query run`, which need an explicit `--model-packs` flag. Confirm the test + actually exercises your model (not just that it compiles) by checking the CI job log for a + `PASSED` line naming your `.ql` file. + +Once merged, publish your change the same way as any other pack — see [Shipping a change to a +query/library pack](#shipping-a-change-to-a-querylibrary-pack) below. + ## Supported CodeQL versions Every query pack in this repository is compiled and tested against a specific, pinned version of the upstream CodeQL standard libraries (e.g. `codeql/java-all`). These queries are **only guaranteed to compile** against those exact library versions (see the latest release for the current versions): newer or older CodeQL CLI/library versions may rename or remove APIs the queries depend on (see [#151](https://github.com/GitHubSecurityLab/CodeQL-Community-Packs/pull/151) for an example, and [#145](https://github.com/GitHubSecurityLab/CodeQL-Community-Packs/issues/145) for the ongoing effort to refresh these pins). @@ -197,8 +286,9 @@ merge. Bumping your pack's own `version:` in a regular PR just *stages* the chan To ship a change: - [ ] Make your change in the pack directory you intend to publish: `/src` (queries), - `/lib` (library), or `/ext`/`/ext-library-sources` (extensions, - `csharp`/`go`/`java`/`python` only). + `/lib` (library), `/ext` (extensions, `csharp`/`go`/`java`/`python` only), + or `/ext-library-sources` (extensions, `csharp`/`java` only). See [Adding a data + extension](#adding-a-data-extension) above if you're not sure which extensions pack to use. - [ ] Bump `version:` in that pack's `qlpack.yml`, following [semver](https://semver.org/). Only bump the specific pack(s) you changed; other languages/pack types are unaffected and don't need touching. @@ -463,3 +553,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 +[pr-204]: https://github.com/GitHubSecurityLab/CodeQL-Community-Packs/pull/204 From b3a0e3b6a9bb3abe4b2a37644596819de84a64ea Mon Sep 17 00:00:00 2001 From: Chad Bentz <1760475+felickz@users.noreply.github.com> Date: Mon, 3 Aug 2026 13:53:36 -0400 Subject: [PATCH 2/3] docs: de-narrativize comment guidance, clarify extensionTargets affects pack consumers Two follow-ups on the "Adding a data extension" section from the initial commit: - Drop the "this bit us during development of the Spring R2DBC models" framing and the "our own file shows a real pair... comment your own rows the same way" phrasing in the row-shape bullet. Both read as insider narrative rather than generic guidance. State the comment-your-rows recommendation directly instead. - Clarify that the extensionTargets version gotcha is not a local-dev-only quirk: per CodeQL's model pack docs, a model pack's extensions are only applied if the resolved codeql/-all version satisfies the pinned extensionTargets range, silently and with no warning otherwise. This affects anyone consuming the published pack with a CLI outside that range, not only contributors testing locally. Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com> Copilot-Session: f02ef23b-e35d-4536-bd58-f2fbc2976ba9 --- CONTRIBUTING.md | 30 ++++++++++++++++++------------ 1 file changed, 18 insertions(+), 12 deletions(-) diff --git a/CONTRIBUTING.md b/CONTRIBUTING.md index 4afd8197..485ec58a 100644 --- a/CONTRIBUTING.md +++ b/CONTRIBUTING.md @@ -106,10 +106,10 @@ instead of, or alongside, a query change. [CodeQL model pack documentation](https://docs.github.com/en/code-security/tutorials/customize-code-scanning/create-and-work-with-codeql-packs#creating-a-codeql-model-pack) for the full column reference (`namespace, type, subtypes, name, signature, ext, input/output, - kind, provenance`). `java/ext/manual/org.springframework.r2dbc.model.yml` shows a real - `sinkModel`/`summaryModel` pair side by side, with comments explaining *why* each row is shaped - the way it is — comment your own rows the same way whenever the mapping from source code to a - MaD row isn't obvious at a glance. + kind, provenance`), and any existing file under `manual/` in this repo for a real example of the + YAML shape. Add a short comment above any row whose reasoning isn't obvious from the method name + alone — e.g. explain what data reaches the argument/return value you picked and why that makes + it dangerous (for a sink) or untrusted (for a source). 3. **`manual/` vs `generated/`** @@ -121,14 +121,20 @@ instead of, or alongside, a query change. 4. **The `extensionTargets` version gotcha** Your pack's `qlpack.yml` pins `extensionTargets: codeql/-all: ''` (see - `/ext/qlpack.yml`). This must be satisfied by the `codeql/-all` version - bundled with whatever CodeQL CLI is running the analysis — **if it isn't, your new models - silently attach zero results, with no error**, instead of failing loudly. This bit us during - development of the Spring R2DBC models above: a local CLI older than this repo's pinned - [`.codeqlversion`][codeqlversion] bundled a `codeql/java-all` below the pinned - `extensionTargets` range, so `codeql test run` produced 0 results locally for a genuinely-working - model — while CI (on the correctly pinned CLI) passed. If a local test run for a new data - extension doesn't find what you expect, check `codeql version` against + `/ext/qlpack.yml`). A model pack's data extensions are only applied if the + `codeql/-all` version actually being used for the analysis satisfies that range — + **if it doesn't, the extensions are silently skipped, with no error or warning**. This is a + general property of how CodeQL model packs work, not just a local-dev quirk: it affects + **anyone consuming the published pack**, not only contributors. Someone running an older (or + much newer) CodeQL CLI than this repo currently targets will get the pack installed + successfully but silently see none of its models take effect. This is part of why + `.codeqlversion` and every pack's version get bumped together whenever the pinned CLI changes + (see [Updating the pinned CodeQL CLI/library version](#updating-the-pinned-codeql-clilibrary-version)) + — letting `extensionTargets` drift out of sync with the CLI versions people are actually + running quietly reduces coverage for them. + + Practically, for your own local testing: if a new data extension doesn't seem to fire, check + `codeql version`'s bundled `codeql/-all` against this repo's pinned [`.codeqlversion`][codeqlversion] before assuming your model is wrong. Also see the [note below](#ext-packs-no-install) on why these packs are never `codeql pack From 3c383c82fddf577106be902ca470116d2be623be Mon Sep 17 00:00:00 2001 From: Chad Bentz <1760475+felickz@users.noreply.github.com> Date: Mon, 3 Aug 2026 22:30:39 -0400 Subject: [PATCH 3/3] docs: fix testing-section wording flagged by Copilot review Don't assert /test/qlpack.yml uniformly depends on the extensions pack - it's not true for python/test/qlpack.yml today. Reword to tell contributors to check/add the dependency instead of claiming it's already there. Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com> Copilot-Session: f02ef23b-e35d-4536-bd58-f2fbc2976ba9 --- CONTRIBUTING.md | 10 +++++----- 1 file changed, 5 insertions(+), 5 deletions(-) diff --git a/CONTRIBUTING.md b/CONTRIBUTING.md index 2bb6e499..eee0fa4b 100644 --- a/CONTRIBUTING.md +++ b/CONTRIBUTING.md @@ -135,12 +135,12 @@ instead of, or alongside, a query change. Add a test under `/test/security///`: minimal source stubs that reproduce the vulnerable call shape, a `.ql` query that imports the *real* standard query (e.g. `SqlInjectionQuery`/`QueryInjectionFlow`, not a reimplementation of it), and a `.expected` file. - `/test/qlpack.yml` already depends on `githubsecuritylab/codeql--extensions` + Check that `/test/qlpack.yml` depends on `githubsecuritylab/codeql--extensions` (and `-library-sources` where applicable) — that dependency is what makes `codeql test run`'s - normal dependency resolution load your new data extensions automatically, unlike ad-hoc `codeql - database analyze`/`query run`, which need an explicit `--model-packs` flag. Confirm the test - actually exercises your model (not just that it compiles) by checking the CI job log for a - `PASSED` line naming your `.ql` file. + normal dependency resolution load your data extensions automatically, unlike ad-hoc `codeql + database analyze`/`query run`, which need an explicit `--model-packs` flag. Add the dependency if + it's missing. Confirm the test actually exercises your model (not just that it compiles) by + checking the CI job log for a `PASSED` line naming your `.ql` file. Once merged, publish your change the same way as any other pack — see [Shipping a change to a query/library pack](#shipping-a-change-to-a-querylibrary-pack) below.