fix-scan-finding

Files (3)

• SKILL.md
• ecosystems.md · references/
• root-cause-grouping.md · references/

SKILL.md (first lines)

<!--
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

| Workflow | Job | Reads |
|---|---|---|
| `.github/workflows/security-scan.yml` | `Trivy filesystem scan (vulnerabilities)` | every lockfile in the tree |
| `.github/workflows/security-scan.yml` | `Trivy config scan (Dockerfiles)` | the Dockerfiles only: Trivy's misconfiguration scanner does not read GitHub Actions workflows |
| `.github/workflows/security-scan.yml` | `Trivy secret scan` | the working tree |
| `.github/workflows/image-scan.yml` | `Trivy image scan (built from source, <platform>)` | the `Dockerfile` built from the PR, once per platform the release publishes (`linux/amd64`, `linux/arm64`) |
| `.github/workflows/image-scan.yml` | `Trivy image scan (published latest, <platform>)` | `ms2data/malloy-publisher:latest` on both platforms, weekly schedule and dispatch |

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:

```bash
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:
  ```bash
  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](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:
   ```bash
   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

From malloydata/publisher, MIT.

More from malloydata/publisher