Fix Scan Finding

<!--
Copyright (c) Credible Data Inc.
SPDX-License-Identifier: MIT
-->

Fixing a security-scan finding

Audience: a contributor or agent working a CVE, Trivy, or code-scan finding in this repository -- fixing a red CRITICAL gate, or verifying a finding on a gate that is already green. No prior Trivy expertise assumed. Prerequisites: trivy on PATH (brew install trivy, or see the Trivy install docs), bun at the version in the root package.json engines, and Docker with buildx for image findings. No cloud credentials are needed -- everything here is local and read-only against public registries.

What CI scans

Each job uploads its full SARIF to Code Scanning, then runs a second scan scoped to CRITICAL with exit-code: 1. CRITICAL fails the job, and the secret gate fails on HIGH as well: a committed credential is an incident whatever its rating. GitHub's own "Trivy" Code Scanning check on a pull request is set, in the repository's code scanning settings, to fail only on critical alerts. It treats every alert in a file the PR touches as new, and a lockfile is one file, so at a high threshold every dependency PR went red on alerts already open on main. If that check goes red on high again, the setting was changed; that is the fix, not a lockfile edit. GitHub rates an alert by the SARIF's CVSS score, not Trivy's label, so the two can disagree: a Trivy MEDIUM can show as high, and a Trivy HIGH with CVSS 9.0 or more is critical to GitHub and still fails the check. HIGHs still land in Code Scanning, and one with a reachable fix is still worth fixing. Every job reads the same .trivyignore.yaml. On a pull request from a fork the SARIF upload is skipped (the token is read-only), but the gate still runs, so read the job log there.

The rule

Prefer the upstream fix. Accept only what you cannot fix. In order:

1. Refresh the lockfile if the manifest already allows a patched version. No ignore entry.
2. Move the parent that pins the vulnerable version, when a newer parent release in range fixes it. One move, N CVEs.
3. Force the transitive version with a root package.json resolutions entry only when 2 is genuinely blocked -- in range where possible, and only if it actually clears the finding (verify; see the traps below).
4. Remove the component from the image when nothing at runtime uses it.
5. Accept with expired_at when no reachable fix exists, with a statement: that is the review record.

Do not skip to 5 because it is quick. Do not do 2 or 3 without the delta check in step B.

A. Establish the real finding set

Do not trust the GitHub Code Scanning alert list. GitHub maps Trivy's CVE severity onto its own scale, so genuine CRITICALs can appear there as high. Always scan locally with the gate's flags:

trivy fs . --scanners vuln --severity CRITICAL \
  --ignorefile .trivyignore.yaml \
  --skip-dirs node_modules --skip-dirs .venv \
  --skip-dirs '**/dist' --skip-dirs '**/build' \
  --format json --quiet > /tmp/crit.json
jq -r '.Results[]? | .Target as $t | (.Vulnerabilities // [])[]
  | "\($t)\t\(.PkgName)@\(.InstalledVersion)\t\(.VulnerabilityID)\tfix=\(.FixedVersion)"' \
  /tmp/crit.json | sort -u

• Quote the **/... globs. Unquoted, zsh expands them, Trivy prints its usage text, and the run looks like a failure of the tree rather than of the command.
• Confirm the scan actually covered files. A Report Summary with no rows, or JSON with no Results key at all (what Trivy emits for a directory with no lockfiles), means nothing was scanned and exit=0 proves nothing. Running from the wrong directory is the usual cause. A gate that scanned nothing is the one failure mode that looks exactly like success.
• For image findings, build what CI builds. CI builds and scans both linux/amd64 and linux/arm64, with the DuckDB version derived from Malloy. A local build covers only the platform you ask for, and the per-architecture base images and prebuilt binaries differ enough that the two report different findings. Build the platform whose job is red (linux/amd64 or linux/arm64), and pass APT_REFRESH as the CI builds do, or a stale cached apt-get upgrade layer can report Debian packages CI has already upgraded:

  docker buildx build --platform <platform> --load \
    --build-arg DUCKDB_VERSION=$(node scripts/duckdb-version.js) \
    --build-arg APT_REFRESH=$(date -u +%F) -t publisher:scan .
  trivy image publisher:scan --scanners vuln --severity CRITICAL --ignorefile .trivyignore.yaml

• Run once more with --include-dev-deps. Trivy skips devDependencies in Node lockfiles by default, and so does the gate. On this repo the flag has surfaced CRITICALs in dev tooling in the root bun.lock (dompurify, ejs, shell-quote) that the gate does not see. A finding that appears only with the flag is not gating, but it is real.

Then group by root cause before touching anything -- see references/root-cause-grouping.md. Several CVEs against one lockfile are usually one lagging parent, not several pins.

B. Before any version move: the delta check

Never raise a pinned version or add a resolution without checking what the old version was protecting. If a pin carries a comment, that comment is the spec -- read it, then verify whether it still holds against the target version rather than assuming either way.

1. Read the pin rationale. A pinned version or resolutions entry often carries a reason in a nearby comment or in the commit that added it: git log -S'"<pkg>": "<ver>"' -- package.json.
2. Check the version line, not only the fixed version. A fix published only on a new major is unreachable when every parent release still declares the old major. Check what the parent's latest release declares, and what the registry actually has:

   npm view <pkg> dist-tags
   npm view <parent>@latest dependencies.<pkg>

   Always check the latest public release before accepting a finding as unfixable. The newest version on the registry may carry the fix even when the scanner's "Fixed Version" or your current major suggests otherwise, and a release may have shipped since the advisory was written. When the fix exists only on a release that is a breaking change (a new major, or a parent that has to move with it), do not decide alone: tell the user what the migration would involve -- which APIs or behavior change, which files and consumers are affected, whether the impact is runtime or build-time only, and what testing it needs -- and let them choose between migrating now and accepting with an expiry until the migration is scheduled. orval is the worked example: npm had 8.38.0 with the fix, but forcing it broke the only consumer, so the evidence went to the user as a choice rather than a silent accept.
3. Test the change against the real consumer, empirically. When a resolution moves a consumer across a major, run the consumer's own call against both versions in scratch installs and diff the output. The worked example is in references/ecosystems.md (one package on two majors).
4. Look for held-back companions. Grep for comments referencing the pinned version -- a package "held back to track X" must move with X or it is silently mismatched.
5. Check what consumes the output if the move could change behavior at a boundary: generated code, a wire format, a file another tool reads. A patch bump usually cannot; a major can.

Then classify and act:

• No behavioral impact -> apply, build, test, done.
• Impact found -> state the specific impact, the blast radius, and the migration steps, and get the user's approval before migrating. Do not silently absorb a migration into a CVE fix. Present it as a choice (move and migrate now / review the diff first / investigate deeper / stay and accept) with the consequence of each.
• Build-time-only impact (codegen, test tooling) still needs approval, but say so plainly -- it is a much smaller ask than a runtime change, and mislabeling it either way wastes the decision.

C. Apply the fix

Match the ecosystem; details and commands, including the Bun traps that make a targeted refresh rewrite half the tree, are in references/ecosystems.md.

Whatever you touch, the stale-pin comment is part of the diff. A pin comment that survives its own removal becomes a lie the next reader believes -- rewrite it to describe the new state and the invariant that still matters, not the change you made.

Run builds, image builds, and test suites with run_in_background: true; an image build for a platform other than your machine's runs under emulation and takes minutes.

D. Verify

The test suite is the oracle for "did I break it"; the scanner is the oracle for "did I fix it". Both, in that order, and neither substitutes for the other.

1. Re-run the step A scan and read what remains. A move that clears four of five CVEs is a partial fix, and the fifth is the one that matters.
2. Run what exercises the moved dependency: bun run test for server dependencies; bun install --frozen-lockfile && bun run generate-clients in packages/server/k6-tests for its codegen; an image build of the red job's platform (step A) for anything in the Dockerfile.
3. Do not trust a codegen exit code. bun run generate-clients exits 0 even when it generates no files. Count the output files, and diff them against the previous version.
4. For a Dockerfile change, confirm the component is gone from the built image, for example docker run --rm --entrypoint sh publisher:scan -c 'cd /publisher && bun why esbuild', then re-scan the image.

E. If it cannot be fixed: accept it properly

An acceptance has three parts, and skipping any one of them is how a suppression becomes permanent by accident:

1. A path-scoped entry. A bare id: suppresses the finding repo-wide, including in a lockfile added next month that inherits the same vulnerable dependency. paths: resolve against the scan root, which is the repository root: packages/server/k6-tests/bun.lock, not k6-tests/bun.lock. An entry with the wrong prefix parses fine and silently never matches. Verify the scoping holds: copy the affected bun.lock and package.json to a new directory, re-scan, and confirm the finding still fires there. Delete the copy.
2. expired_at on anything that will be fixed (yyyy-mm-dd; Trivy stops honouring the entry that day and the gate reds again). Omit it only for a genuine won't-fix, where an expiry would red the gate on a date certain for something nobody intends to change -- that is recurring noise, not a forcing function. A risk-accepted entry must name the compensating control, not assert safety.
3. A statement: that says the package and version, how it is reached, why the vulnerable path is not exercised here, and what upgrade retires the entry. The statement in .trivyignore.yaml, plus the PR description, is the review record -- an entry without one is an unexplained suppression. Follow the shape of the entries already in the file rather than inventing a new one.

OS-package (Debian) findings in an image scan cannot be path-scoped. They carry no package path, so no paths: value matches them: tested on the built image with CVE-2019-16224 (liblmdb0), a bare id: suppressed it, while paths: ["nonexistent"], paths: ["usr/share/doc/liblmdb0/copyright"], and paths: ["var/lib/dpkg/status"] all left it firing. So an OS-package acceptance must be a bare id: entry. That is safe here because the filesystem scan never reports OS packages, so the entry cannot hide a lockfile finding; to make up for the missing scope, the statement: must name the Debian package and the image it was found in. Prefer fixing these in the Dockerfile first (see references/ecosystems.md): Trivy Status: fixed means apt-get upgrade clears it; Status: affected means Debian has no fix yet, so first ask what installed the package (apt-cache rdepends --installed <pkg> inside the image) -- removing an unused parent package is option 4 of the rule, and is how this repo's unfixed liblmdb0 and libxml2 findings were cleared rather than accepted.