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.

The traps

• A printed "Fixed Version" can be unreachable through the parent. orval's 11 CRITICALs in packages/server/k6-tests are fixed only in 8.21.0+ (npm latest is 8.38.0), but @grafana/openapi-to-k6 0.3.2 and its latest, 0.4.1, both declare orval ^7.5.0. Forcing orval 8.38.0 makes bun run generate-clients fail under both ("Cannot read properties of undefined", no files written). So the finding is unfixable on this line, not un-upgraded, and was accepted with expired_at. Always check npm view <pkg> dist-tags and test forcing the fixed version against the real consumer before accepting -- and before planning a move around it.
• Forcing a version can look like a fix and not be one. A resolution to the newest 7.x of orval cleared the one finding 7.x fixes and left the 8.x-only ones standing. Always re-scan after a resolution; never assume the version you chose covers every CVE you targeted.
• bun update <transitive-pkg> adds it as a direct dependency. Run against basic-ftp and protobufjs, it wrote both into the root package.json dependencies at their latest majors (6.2.1 and 8.8.0) instead of refreshing them in range. Use resolutions for a transitive package.
• Deleting a lockfile entry does not re-resolve just that entry. Removing stale entries from bun.lock and running bun install --lockfile-only re-resolved most of the tree, moving unrelated packages such as the whole AWS SDK. That is not a targeted fix.
• Bun resolutions are top-level only. A nested key such as "snowflake-sdk/fast-xml-parser" is accepted and silently ignored; the lockfile does not change.
• Build-time-only dependencies are still reported. A CVE in codegen or test tooling (packages/server/k6-tests, e2e, examples/data-app each have their own lockfile) is not in the image. It still fails the filesystem gate, so fix it or accept it with that reasoning stated -- do not argue it away without an entry.
• A devDependencies-only CVE is invisible without --include-dev-deps. Neither the gate nor the default local scan passes that flag. When sweeping one vulnerable chain repo-wide, re-run with it before concluding a lockfile is unaffected -- a clean default scan is not evidence the chain is absent, only that nothing in it is a production dependency.
• A clean bun.lock scan can still hide nested copies. After this repo's lockfile refresh reported zero HIGH, the image scan of the same tree still found nested lodash 4.17.21, ip-address 10.0.1 and ws 5.2.4 that the filesystem scan never listed. Scan the built image before calling a Node finding gone, or list every lockfile key still at the vulnerable version.
• A production dependency's peers ship. @vitejs/plugin-react in packages/app dependencies pulled vite, and through it esbuild (with a Go stdlib CVE in its binary), into the image despite bun install --production. bun why <pkg> inside the built image names the path.
• Your architecture is not the whole gate. CI scans the image on both published platforms, and a local build scans only the one you built; the other can carry findings yours does not. Build the red job's --platform before concluding an image finding is gone.
• Yesterday's quiet CVE can red you today. A newly published advisory against a version you already had will fail a gate that was green last week, with no change on your side. A red gate is not evidence your branch introduced the finding -- check the advisory date before hunting your diff. The weekly scheduled runs exist to surface exactly this.
• Unfixed criticals are not exempt. The gates do not set ignore-unfixed, so a critical with no upstream patch blocks like any other. When there is nothing to upgrade to, the answer is an explicit path-scoped .trivyignore.yaml entry with a statement: -- a reviewed acceptance, not a silent one.
• Renaming a scan job changes its check name. Status checks match by job name. If a maintainer has made a Trivy ... check required in branch protection, a rename leaves the old required check pending forever. Ask a maintainer before renaming one.

Finishing

State plainly which findings are fixed (with the version that fixed them), which are accepted (with the expiry and why no fix is reachable), and which are still open. If a change needed approval and you got it, say what was approved. Never report a finding as fixed on the strength of a scan that covered no targets.

The handoff is not done until CI is green

A green local run is necessary and not sufficient. A dependency move changes the resolved tree for every consumer, and CI is the only place the image is built and scanned on both published platforms the way the gate runs it, and the only place the full build and test matrix runs against the new lockfile. Local runs also use your machine's node_modules and Docker cache, which can hold artifacts CI resolves differently.

So end every fix by handing the user the PR step and waiting for CI:

1. Ask the user whether they want you to push the branch and open a PR against main, or would rather do it themselves. Give the exact commands either way, for example:

   git add <changed files>
   git commit -m "fix(deps): <what moved and which findings it clears>"
   git push -u origin <branch>
   gh pr create --repo malloydata/publisher --base main --title "<title>" --body-file <file>
   gh pr checks <pr-number> --repo malloydata/publisher --watch

   From a fork, push to the fork and open the PR against malloydata/publisher main.
2. Wait for the Trivy ... checks to pass on that PR. That is the gate the work exists to clear, and it re-runs against the PR head with .trivyignore.yaml as committed.
3. Wait for the build and test jobs to pass as well. They exercise the moved dependency; the scan does not.
4. If any of those go red, the fix is not done: read the failure, and treat a break in a different package as the expected shape of a shared-dependency regression rather than an unrelated flake.

Report the fix as verified locally, pending CI until those checks report. Do not describe a dependency move as safe or complete on local evidence alone.