CVE detection engine — correctness fixes, spec, tests, audit #2
@@ -164,14 +164,21 @@ into the per-recipe log**:
|
||||
|
||||
```
|
||||
python3 /srv/cc-ci/cc-ci-plan/advisory-scan.py <recipe> --from <old-app-version> --to <new-app-version> \
|
||||
[--window <image-key>=<old>:<new> ...]
|
||||
[--image <name> <old> <new>]...
|
||||
```
|
||||
|
||||
**Pass a `--window` for EVERY sidecar you bumped** (redis, postgres, nginx …), not just the app —
|
||||
each image is judged by its own version range, and an image without a window is not counted at all.
|
||||
e.g. discourse bumping app 3.5.3→2026.7.1 *and* redis 7.4→8.10:
|
||||
`--from 3.5.3 --to 2026.7.1 --window redis=7.4:8.10` → 128 CVEs (123 app + 5 redis), where the redis
|
||||
five include a **critical** (CVE-2025-49844) that is invisible if the sidecar is left out.
|
||||
**Pass an `--image` for EVERY sidecar you upgraded** (redis, postgres, nginx …), not just the app —
|
||||
each image is judged by its own versions, and an image you don't name is not counted at all. Repeat
|
||||
the flag for each one and pass them **all in a single call** (the count is a union across images).
|
||||
e.g. discourse moving app 3.5.3→2026.7.1 *and* redis 7.4→8.10:
|
||||
|
||||
```
|
||||
... --from 3.5.3 --to 2026.7.1 --image redis 7.4 8.10
|
||||
```
|
||||
|
||||
→ 128 CVEs (123 app + 5 redis), where the redis five include a **critical** (CVE-2025-49844) that is
|
||||
invisible if the sidecar is left out. `<name>` is substring-matched against source repo names, so make
|
||||
it specific enough to hit exactly one.
|
||||
|
||||
It queries, per recipe: the **GitHub Security Advisories API** for every source repo in
|
||||
`cc-ci-plan/upstream/<recipe>.md` (CVE + GHSA + severity + vulnerable/patched ranges, so
|
||||
|
||||
@@ -19,14 +19,15 @@ Nothing in the pipeline queried an advisory source. This scan closes that hole.
|
||||
## Inputs
|
||||
|
||||
```
|
||||
advisory-scan.py <recipe> [--from <version>] [--to <version>] [--json] [--registry DIR]
|
||||
advisory-scan.py <recipe> [--from <version>] [--to <version>]
|
||||
[--image <name> <from> <to>]... [--json] [--registry DIR]
|
||||
```
|
||||
|
||||
| Input | Meaning |
|
||||
|---|---|
|
||||
| `<recipe>` | Recipe name; selects `cc-ci-plan/upstream/<recipe>.md` (the per-recipe URL registry) |
|
||||
| `--from` / `--to` | The **primary app image's** version window being upgraded across |
|
||||
| `--window KEY=FROM:TO` | A **sidecar's own** window (repeatable). `KEY` matches a source repo name, e.g. `--window redis=7.4:8.10`. Without it that image's advisories stay unclassified. |
|
||||
| `--image NAME FROM TO` | A **sidecar image and the versions it moved between** (repeatable). `NAME` is matched as a substring against source repo names, e.g. `--image redis 7.4 8.10`. Without it that image's advisories stay unclassified. |
|
||||
| `--registry` | Registry dir; also `CCCI_UPSTREAM_REGISTRY` |
|
||||
| `GITHUB_TOKEN` / `GITHUB_TOKEN_FILE` | Read-only token; **rate limit only** (60/hr anonymous → 5000/hr). Default file `/srv/cc-ci/.github-token`, mode 600. Public advisories need **no scopes**. |
|
||||
|
||||
@@ -101,15 +102,16 @@ published_at, context}`. A CVE seen by several sources keeps them all.
|
||||
|
||||
Two invariants govern this step, both learned from a wrong answer in production.
|
||||
|
||||
> **A. Every image is judged by its OWN window.** The app repo uses `--from/--to`; each sidecar uses
|
||||
> its own `--window KEY=FROM:TO`. An image with no window is **not** classified — its advisories are
|
||||
> **A. Every image is judged by its OWN versions.** The app repo uses `--from/--to`; each sidecar uses
|
||||
> its own `--image NAME FROM TO`. **Pass them all in ONE invocation** — the count is a union across
|
||||
> images, and the UNKNOWN guarantee in B only holds when a single run sees every one. An image with no window is **not** classified — its advisories are
|
||||
> listed as unclassified so they stay visible without inflating the count. The reported count is the
|
||||
> **union across windows**, and each window is classified independently (so one may use version
|
||||
> ranges while another falls back to dates).
|
||||
> *Why:* discourse once reported **133**, of which **34 were redis CVEs** — including
|
||||
> `CVE-2021-21309`, patched in redis 6.0.11 in 2021 — counted purely because 6.0.11 sits numerically
|
||||
> inside discourse's `3.5.3 → 2026.7.1` range. The fix is not to ignore sidecars but to give each one
|
||||
> the window it actually moved through: with `--window redis=7.4:8.10`, discourse scores
|
||||
> the versions it actually moved through: with `--image redis 7.4 8.10`, discourse scores
|
||||
> **128 = 123 (app, by date) + 5 (redis, by version range)** — and the redis five include
|
||||
> `CVE-2025-49844`, **critical**, which was invisible while sidecars went uncounted.
|
||||
>
|
||||
@@ -155,7 +157,7 @@ Markdown (default) for pasting into the per-recipe upgrade log, or `--json`.
|
||||
|---|---|
|
||||
| `cve_count_fixed` | Union across all windows, or **`null`** if ANY requested window could not be ordered (a partial number would understate) |
|
||||
| `count_known` | Distinguishes "counted zero" from "could not count" |
|
||||
| `windows` | Every source classified, with its from/to |
|
||||
| `windows` | Every source classified, with its from/to (the internal computed ranges) |
|
||||
| `classified_by` | **Per source**: `patched version ranges` or `advisory publish date (version scheme changed)` |
|
||||
| `date_window` | **Per source**, when 4b was used |
|
||||
| `fixed_by_this_upgrade[]` | CVE ids, with severity / GHSA / fixed-in per id |
|
||||
@@ -182,10 +184,13 @@ in this tool or stale registry data.
|
||||
|
||||
## Known limits
|
||||
|
||||
1. **Windows must be supplied.** An image with no `--window` is not counted — the scan will not
|
||||
1. **Versions must be supplied per image.** An image with no `--image` is not counted — the scan will not
|
||||
guess a version range it was not told. `/recipe-upgrade` passes one per image it bumped.
|
||||
2. **Date-based counts are temporal**, not exact — they assume publish-at-fix-time.
|
||||
3. **Registry-bound.** Unlisted vendor security pages are invisible; the scan cannot know what it was
|
||||
never pointed at.
|
||||
4. **Rate limit** without a token is 60/hr — a full weekly sweep will exhaust it and degrade to failed
|
||||
4. **`NAME` matching is substring-against-source-name**, so a short or generic name can attach to more
|
||||
than one repo (`postgres` matches `discourse/discourse-postgres`). The primary source is claimed
|
||||
first and cannot be stolen; be specific enough to hit one repo.
|
||||
5. **Rate limit** without a token is 60/hr — a full weekly sweep will exhaust it and degrade to failed
|
||||
sources (visibly, but degraded).
|
||||
|
||||
+13
-18
@@ -284,7 +284,7 @@ def osv(recipe: str, version: str | None) -> dict | None:
|
||||
|
||||
|
||||
def scan(recipe: str, v_from: str | None, v_to: str | None, registry_dir: str,
|
||||
extra_windows: list[tuple[str, str, str]] | None = None) -> dict:
|
||||
images: list[tuple[str, str, str]] | None = None) -> dict:
|
||||
urls, reg_path = registry_urls(recipe, registry_dir)
|
||||
report: dict = {
|
||||
"recipe": recipe,
|
||||
@@ -352,8 +352,9 @@ def scan(recipe: str, v_from: str | None, v_to: str | None, registry_dir: str,
|
||||
# false 133 (34 of them redis CVEs, incl. one patched in redis 6.0.11 in 2021). So each source
|
||||
# is classified against ITS OWN window, and the count is the union across windows.
|
||||
#
|
||||
# --from/--to → the PRIMARY app repo (first github source in the registry)
|
||||
# --window K=F:T → any other source whose name contains K (repeatable), e.g. redis=7.4:8.10
|
||||
# --from/--to → the PRIMARY app repo (first github source in the registry)
|
||||
# --image NAME FROM TO → any other source whose name contains NAME (repeatable),
|
||||
# e.g. --image redis 7.4 8.10
|
||||
#
|
||||
# A source with no window is not classified: its advisories are listed as unclassified so they
|
||||
# stay visible without inflating the count.
|
||||
@@ -364,7 +365,7 @@ def scan(recipe: str, v_from: str | None, v_to: str | None, registry_dir: str,
|
||||
windows = {} # source name -> (from, to)
|
||||
if primary:
|
||||
windows[primary] = (v_from, v_to)
|
||||
for key, wf, wt in (extra_windows or []):
|
||||
for key, wf, wt in (images or []):
|
||||
for src in gh_sources:
|
||||
if key.lower() in src.lower() and src not in windows:
|
||||
windows[src] = (wf, wt)
|
||||
@@ -414,7 +415,7 @@ def scan(recipe: str, v_from: str | None, v_to: str | None, registry_dir: str,
|
||||
if cve in fixed_set:
|
||||
continue
|
||||
if not any(src in e["sources"] for src in windows):
|
||||
e["classification"] = "unclassified: no version window given for this image"
|
||||
e["classification"] = "unclassified: no versions given for this image"
|
||||
unknown.append(cve)
|
||||
else:
|
||||
e.setdefault("classification", "outside-window")
|
||||
@@ -505,20 +506,14 @@ def main() -> int:
|
||||
ap.add_argument("--to", dest="v_to", default=None)
|
||||
ap.add_argument("--json", action="store_true", help="emit raw JSON instead of markdown")
|
||||
ap.add_argument("--registry", default=REGISTRY_DIR)
|
||||
ap.add_argument("--window", action="append", default=[], metavar="KEY=FROM:TO",
|
||||
help="extra image window, e.g. --window redis=7.4:8.10 (repeatable). "
|
||||
"KEY matches a source repo name; its advisories are then counted "
|
||||
"against ITS OWN bump instead of being left unclassified.")
|
||||
ap.add_argument("--image", action="append", default=[], nargs=3,
|
||||
metavar=("NAME", "FROM", "TO"),
|
||||
help="a sidecar image and the versions it moved between, e.g. "
|
||||
"--image redis 7.4 8.10 (repeatable). NAME matches a source repo name; "
|
||||
"its advisories are then counted against ITS OWN versions instead of "
|
||||
"being left unclassified.")
|
||||
a = ap.parse_args()
|
||||
wins = []
|
||||
for w in a.window:
|
||||
key, _, rng = w.partition('=')
|
||||
wf, _, wt = rng.partition(':')
|
||||
if key and wf and wt:
|
||||
wins.append((key, wf, wt))
|
||||
else:
|
||||
print(f'ignoring malformed --window {w!r} (expected KEY=FROM:TO)', file=sys.stderr)
|
||||
rep = scan(a.recipe, a.v_from, a.v_to, a.registry, wins)
|
||||
rep = scan(a.recipe, a.v_from, a.v_to, a.registry, [tuple(x) for x in a.image])
|
||||
print(json.dumps(rep, indent=2) if a.json else markdown(rep))
|
||||
return 0
|
||||
|
||||
|
||||
Reference in New Issue
Block a user