P1: Supply-chain security — Dependabot, npm audit gate, SBOM (#175) - #192
Conversation
4b3df1e to
ae4e571
Compare
Pins both Dockerfile stages to node:22-alpine@sha256:16e22a55… (v22.23.1, verified by pull and build). Without this, moving to a floating 22 tag would silently drop the digest pin #189 adds, reverting its reproducible/tamper-evident build hardening depending on merge order. Once #192 lands, Dependabot's docker ecosystem keeps the digest fresh. Bumps @types/node to ^22 so the types match the runtime rather than staying on 20. Adds tests pinning the GraphQL-over-HTTP behaviour the mina-explorer depends on: validation errors return 200 (its client throws on non-2xx before reading the body) and carry the literal "Cannot query field" (its fallbacks key on that string). Both were verified by hand across the 4 → 5 upgrade and are unchanged, but they rest on an implicit content-negotiation default — yoga only returns 400 under an Accept header that client never sends — so a later bump could flip them unnoticed. Now a standing guard rather than a one-time check. Addresses review feedback on #194. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
|
Verdict: MERGEABLE ✅ CI-only ( What I checked
Non-blocking nits
None of these change the answer: this is a net-positive, honestly-wired gate that will not block the merge train. Rebase ( Automated second-pass review — focus: downstream compatibility with mina-explorer / mina-explorer-api. |
SanabriaRusso
left a comment
There was a problem hiding this comment.
Approving on the basis of the second-pass review comment above: no mid-to-high severity security, compatibility, or degradation issue found, and the downstream contract with mina-explorer / mina-explorer-api holds — GraphQL validation error text reaches errors[].message verbatim, the browser SPA's cross-origin access is preserved, and the real consumer query shapes (including the 2000-block analytics query and the 500-row page crawl) still pass.
Two things this approval does not mean:
- It does not close the non-blocking items in the review comment. Several are worth fixing before or shortly after merge; they are written up there with patches.
- It does not by itself mean the branch is ready to merge.
mainrequires branches to be up to date, so this needs an update-branch (or a rebase, if the branch is conflicting) first, and a few PRs in this series have cross-PR ordering constraints called out in their review comments.
Automated second-pass review — focus: downstream compatibility with mina-explorer / mina-explorer-api.
Pins both Dockerfile stages to node:22-alpine@sha256:16e22a55… (v22.23.1, verified by pull and build). Without this, moving to a floating 22 tag would silently drop the digest pin #189 adds, reverting its reproducible/tamper-evident build hardening depending on merge order. Once #192 lands, Dependabot's docker ecosystem keeps the digest fresh. Bumps @types/node to ^22 so the types match the runtime rather than staying on 20. Adds tests pinning the GraphQL-over-HTTP behaviour the mina-explorer depends on: validation errors return 200 (its client throws on non-2xx before reading the body) and carry the literal "Cannot query field" (its fallbacks key on that string). Both were verified by hand across the 4 → 5 upgrade and are unchanged, but they rest on an implicit content-negotiation default — yoga only returns 400 under an Accept header that client never sends — so a later bump could flip them unnoticed. Now a standing guard rather than a one-time check. Addresses review feedback on #194. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
eb0422f to
e61d725
Compare
SanabriaRusso
left a comment
There was a problem hiding this comment.
Approving. e61d725e fixes the green-by-construction problem, and I verified the gate is real without being permanently red.
The SBOM job is no longer green-by-construction. continue-on-error does not appear anywhere in .github/workflows/security.yaml now — I read the whole file at head, not just the diff. if-no-files-found is error (:52), so a failed SBOM generation fails the job instead of warning. anchore/sbom-action is SHA-pinned (e22c389904149dbc22b58101806040fa8d37a610 # v0.24.0).
The audit gate genuinely fails the build. security.yaml:29-30 runs npm audit --omit=dev --audit-level=critical with no || true and no continue-on-error. The only || true remaining is line 33, the explicitly-labelled "Full audit (informational)" step — that's a deliberate gate/signal split, not a masked gate.
And it won't block every merge. I ran the exact gated command against main's lockfile: exit 0, with prod-only severity counts {low: 0, moderate: 8, high: 2, critical: 0}. Scoping to --omit=dev and gating at critical is the right pragmatic calibration for a first gate — a gate that is always red is worse than none.
Security posture of the workflow itself is good. Workflow-level permissions: contents: read (:12-13) inherited by both jobs with no job-level escalation; trigger is pull_request, not pull_request_target (:4), so the classic fork-code-with-write-token hole is absent; no secrets referenced; and npm ci --ignore-scripts (:27) stops a malicious PR's lifecycle scripts executing on the runner. concurrency with cancel-in-progress is set. Nicely constructed.
Dependabot volume is bounded. Three ecosystems, all interval: weekly, with open-pull-requests-limit 5 / 3 / 2 = 10 PRs per week worst case, and the grouping (production-dependencies / development-dependencies for npm, patterns: ['*'] for actions) collapses most weeks to 2–4. Nowhere near a firehose. The dependency-type: production|development group syntax is valid.
Non-blocking nits
-
The docker ecosystem entry is inert right now.
.github/dependabot.yml:28-32targets/and the comment says it keeps "the pinned base-image digest" fresh — butmain'sDockerfilecurrently uses a floatingnode:20-alpinetag, which the docker updater won't bump. It starts doing anything once #189 or #194 lands a digest pin. Worth knowing so it isn't later mistaken for broken. -
The informational audit reports green.
npm audit || trueshows a green check even when it finds things, so nobody will click into it. Same non-blocking semantics, better visibility:- # Informational: surface the full picture (incl. dev tooling) without blocking. - - name: Full audit (informational) - run: npm audit || true + # Informational: surface the full picture (incl. dev tooling) without blocking. + # continue-on-error (rather than `|| true`) so a finding shows as a visible + # non-blocking failure instead of a green check nobody opens. + - name: Full audit (informational) + continue-on-error: true + run: npm audit --audit-level=high
-
criticalis loose given today's baseline. There are already 2 HIGH prod advisories (picomatch ReDoS + POSIX-class glob matching, and uuid buffer bounds viajaeger-client).npm auditreports the picomatch one as fixable without a breaking change. Tightening to--audit-level=highis a reasonable follow-up once those are cleared — I wouldn't do it in this PR, since it would land red. -
node-version: '20'atsecurity.yaml:22will be stale once #194 moves everything to 22. New file, so no textual conflict — worth a follow-up sweep. (nightly-devnet-dump.yaml:56is in the same boat; I've noted it on #194.) -
Inconsistent action pinning:
anchore/sbom-actionis SHA-pinned whileactions/checkout@v4,setup-node@v4andupload-artifact@v4are tag-pinned (mutable). Defensible for first-party actions — just flagging that the file applies two standards. -
The title still advertises Trivy, which isn't in the diff (dropped deliberately per the earlier review). Worth editing the title so the commit history doesn't claim something the repo doesn't have.
One note on scope, not a request: this workflow is not among main's required status checks (Run-Tests and Linting only), so even a red audit run won't hard-block a merge today. If the intent is a real gate rather than a signal, npm audit needs adding to branch protection — same observation I made on #198 about Check Schema.
Downstream: none. CI/repo configuration only — no application code, no schema, no runtime behaviour. Zero risk to either consumer.
Pins both Dockerfile stages to node:22-alpine@sha256:16e22a55… (v22.23.1, verified by pull and build). Without this, moving to a floating 22 tag would silently drop the digest pin #189 adds, reverting its reproducible/tamper-evident build hardening depending on merge order. Once #192 lands, Dependabot's docker ecosystem keeps the digest fresh. Bumps @types/node to ^22 so the types match the runtime rather than staying on 20. Adds tests pinning the GraphQL-over-HTTP behaviour the mina-explorer depends on: validation errors return 200 (its client throws on non-2xx before reading the body) and carry the literal "Cannot query field" (its fallbacks key on that string). Both were verified by hand across the 4 → 5 upgrade and are unchanged, but they rest on an implicit content-negotiation default — yoga only returns 400 under an Accept header that client never sends — so a later bump could flip them unnoticed. Now a standing guard rather than a one-time check. Addresses review feedback on #194. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
e61d725 to
537b63b
Compare
There were no supply-chain controls: no Dependabot, no audit gate, no SBOM.
- `.github/dependabot.yml`: weekly updates for npm (production/development
grouped), GitHub Actions, and Docker (keeps the pinned base-image digest
fresh).
- `.github/workflows/security.yaml`:
- npm audit — hard gate on **critical** advisories in production deps (what
actually ships), plus a full informational audit. Production deps currently
have 0 critical, so the gate passes; the 4 highs are OpenTelemetry/fast-uri
transitives that Dependabot / the Yoga 5 upgrade (#176) will clear.
- CycloneDX SBOM generation, uploaded as an artifact.
Dependency/image vulnerability scanning (Trivy/Grype) is deferred so the scanner
action can be verified separately rather than shipped red — Dependabot (npm +
docker) already surfaces vulnerable deps and base images in the meantime.
Refs #175.
Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01QSuak9smCHbp4N17xjjLF6
537b63b to
66dd648
Compare
Pins both Dockerfile stages to node:22-alpine@sha256:16e22a55… (v22.23.1, verified by pull and build). Without this, moving to a floating 22 tag would silently drop the digest pin #189 adds, reverting its reproducible/tamper-evident build hardening depending on merge order. Once #192 lands, Dependabot's docker ecosystem keeps the digest fresh. Bumps @types/node to ^22 so the types match the runtime rather than staying on 20. Adds tests pinning the GraphQL-over-HTTP behaviour the mina-explorer depends on: validation errors return 200 (its client throws on non-2xx before reading the body) and carry the literal "Cannot query field" (its fallbacks key on that string). Both were verified by hand across the 4 → 5 upgrade and are unchanged, but they rest on an implicit content-negotiation default — yoga only returns 400 under an Accept header that client never sends — so a later bump could flip them unnoticed. Now a standing guard rather than a one-time check. Addresses review feedback on #194. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Pins both Dockerfile stages to node:22-alpine@sha256:16e22a55… (v22.23.1, verified by pull and build). Without this, moving to a floating 22 tag would silently drop the digest pin #189 adds, reverting its reproducible/tamper-evident build hardening depending on merge order. Once #192 lands, Dependabot's docker ecosystem keeps the digest fresh. Bumps @types/node to ^22 so the types match the runtime rather than staying on 20. Adds tests pinning the GraphQL-over-HTTP behaviour the mina-explorer depends on: validation errors return 200 (its client throws on non-2xx before reading the body) and carry the literal "Cannot query field" (its fallbacks key on that string). Both were verified by hand across the 4 → 5 upgrade and are unchanged, but they rest on an implicit content-negotiation default — yoga only returns 400 under an Accept header that client never sends — so a later bump could flip them unnoticed. Now a standing guard rather than a one-time check. Addresses review feedback on #194. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Pins both Dockerfile stages to node:22-alpine@sha256:16e22a55… (v22.23.1, verified by pull and build). Without this, moving to a floating 22 tag would silently drop the digest pin #189 adds, reverting its reproducible/tamper-evident build hardening depending on merge order. Once #192 lands, Dependabot's docker ecosystem keeps the digest fresh. Bumps @types/node to ^22 so the types match the runtime rather than staying on 20. Adds tests pinning the GraphQL-over-HTTP behaviour the mina-explorer depends on: validation errors return 200 (its client throws on non-2xx before reading the body) and carry the literal "Cannot query field" (its fallbacks key on that string). Both were verified by hand across the 4 → 5 upgrade and are unchanged, but they rest on an implicit content-negotiation default — yoga only returns 400 under an Accept header that client never sends — so a later bump could flip them unnoticed. Now a standing guard rather than a one-time check. Addresses review feedback on #194. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
What & why
Part of the production-readiness epic (#163). Refs #175.
There were no supply-chain controls: no Dependabot, no audit gate, no SBOM.
Changes
.github/dependabot.yml— weekly updates for npm (production/development grouped), GitHub Actions, and Docker (keeps the pinned base-image digest from P1: Container hardening — node as PID 1 / tini, HEALTHCHECK, pin digest #171 fresh)..github/workflows/security.yaml:Notes
fast-uritransitives that Dependabot and the Yoga 5 upgrade (P1: Dependency upgrades — graphql-yoga 4→5, Node 20→22 LTS #176) will clear. The gate blocks prod criticals today, surfaces everything else, and can be tightened tohighonce P1: Dependency upgrades — graphql-yoga 4→5, Node 20→22 LTS #176 lands.Testing
npm audit --omit=dev --audit-level=critical→ exit 0 (the blocking gate passes)prettier --debug-check .— clean; YAML validated🤖 Generated with Claude Code