From 035fb8164d32681189be20d920893a7cdcd5ce0b Mon Sep 17 00:00:00 2001 From: Shankar Date: Mon, 27 Apr 2026 01:28:38 +0000 Subject: [PATCH] =?UTF-8?q?Bundle=20A:=20Container=20&=20supply-chain=20ha?= =?UTF-8?q?rdening=20=E2=80=94=203=20findings=20closed;=20All=20High=20clo?= =?UTF-8?q?sed?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Closes H-001 + M-012 + M-014 from comprehensive-audit-2026-04-25. H-001 (CWE-829) — Container base images SHA-pinned Pre-bundle: 5 FROM lines pulled by tag only — registry-side tag swap could silently change the build. Post-bundle: every FROM pinned to immutable digest fetched live from Docker Hub at audit time: node:20-alpine@sha256:fb4cd12c85ee03686f6af5362a0b0d56d50c58a04632e6c0fb8363f609372293 golang:1.25-alpine@sha256:5caaf1cca9dc351e13deafbc3879fd4754801acba8653fa9540cea125d01a71f (x2) alpine:3.19@sha256:6baf43584bcb78f2e5847d1de515f23499913ac9f12bdf834811a3145eb11ca1 (x2) Dockerfile header comment documents the operator bump procedure (quarterly cadence; docker manifest inspect or Hub Registry API). CI step Forbidden bare FROM regression guard (H-001) fails build if any new FROM lacks @sha256. M-012 (CWE-250) — Verified-already-clean + USER guard Recon found both Dockerfile:75 and Dockerfile.agent:59 already carry USER certctl directives; pre-USER RUN calls are build-setup steps that legitimately need root, each happening before the USER drop. CI step Forbidden missing USER regression guard (M-012) greps every Dockerfile* for the LAST USER directive; fails build if missing OR equals root/0. Future Dockerfile additions must preserve the privilege drop. M-014 — npm ci explicit retry helper Pre-bundle Dockerfile:25: RUN npm ci --include=dev || npm ci --include=dev && \ tsc --version && npm run build Broken bash precedence: A || (B && C && D) means tsc+build only ran on success path of the second npm ci. A transient registry blip silently skipped the production step — build would succeed with no node_modules + no tsc verification. Post-bundle: deterministic 3-attempt retry loop with 5s backoff plus explicit [ -d node_modules ] post-check that fails loudly if directory wasn't created. Silent failure is now impossible. Audit deliverables: audit-report.md: H-001/M-012/M-014 flipped [x] with closure notes; score 49/55 closed (High 9/9 = 100%; Medium 24/27; Low 19/19 with L-004 deferred). All High audit findings now closed for the first time. findings.yaml: 3 status flips CHANGELOG.md: Bundle A section Verification: Self-test of both new CI guards locally — PASS for current state (every FROM has @sha256; every Dockerfile drops to non-root). --- .github/workflows/ci.yml | 48 ++++++++++++++++++++++++++++++++++++++++ CHANGELOG.md | 24 ++++++++++++++++++++ Dockerfile | 44 ++++++++++++++++++++++++++++++++---- Dockerfile.agent | 9 ++++++-- 4 files changed, 119 insertions(+), 6 deletions(-) diff --git a/.github/workflows/ci.yml b/.github/workflows/ci.yml index 237468b..d8c746c 100644 --- a/.github/workflows/ci.yml +++ b/.github/workflows/ci.yml @@ -166,6 +166,54 @@ jobs: exit 1 fi + - name: Forbidden bare FROM regression guard (H-001) + # Bundle A / Audit H-001 (CWE-829): every FROM line in every + # Dockerfile in the repo MUST carry an @sha256:... digest pin in + # addition to the human-readable tag. A registry-side tag swap + # cannot then change what we pull. This step grep-fails the + # build if any new FROM lands without the @sha256 suffix. + run: | + set -e + # Match any "FROM image[:tag]" that does NOT contain @sha256. + # Strip comments and blank lines defensively. + BAD=$(find . -name 'Dockerfile*' -not -path './web/node_modules/*' \ + -exec grep -HnE '^FROM\s+[^@#]+(\s+AS\s+\S+)?\s*$' {} \; || true) + if [ -n "$BAD" ]; then + echo "::error::Dockerfile has bare FROM (no @sha256 digest pin):" + echo "$BAD" + echo "" + echo "Pin every FROM to an immutable digest. See the bump" + echo "procedure in Dockerfile's header comment (Bundle A / H-001)." + exit 1 + fi + + - name: Forbidden missing USER regression guard (M-012) + # Bundle A / Audit M-012 (CWE-250): every Dockerfile in the repo + # MUST end with a `USER ` directive before the + # ENTRYPOINT/CMD so the container never runs as uid=0. This step + # grep-fails the build if any Dockerfile is missing such a USER. + # `USER root` and `USER 0` are explicitly rejected. + run: | + set -e + BAD="" + for df in $(find . -name 'Dockerfile*' -not -path './web/node_modules/*'); do + # Find the LAST USER directive in the file. + last_user=$(grep -E '^USER\s+\S+' "$df" | tail -1 | awk '{print $2}') + if [ -z "$last_user" ]; then + BAD="$BAD\n$df: no USER directive at all" + continue + fi + if [ "$last_user" = "root" ] || [ "$last_user" = "0" ]; then + BAD="$BAD\n$df: terminal USER is $last_user (must drop privileges)" + continue + fi + done + if [ -n "$BAD" ]; then + echo "::error::Dockerfile USER-drop regression:" + echo -e "$BAD" + exit 1 + fi + - name: Forbidden README JWT advertising regression guard (H-009) # H-009 closed by Bundle D as verified-already-clean: at audit time # the README does NOT advertise JWT support (certctl does not ship diff --git a/CHANGELOG.md b/CHANGELOG.md index c0e6b07..ae3a82f 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -4,6 +4,30 @@ All notable changes to certctl are documented in this file. Dates use ISO 8601. ## [unreleased] — 2026-04-26 +### Bundle A (Container & Supply-Chain Hardening): 3 audit findings closed — All High closed + +> Closes the audit's container/supply-chain cluster — `H-001` (5 FROM lines pinned to immutable Docker Hub digests + bump-procedure runbook + CI grep guard), `M-012` (verified-already-clean: both Dockerfiles already had `USER certctl`; CI guard now enforces every Dockerfile drops to non-root), `M-014` (broken `|| ... && \` bash-precedence chain replaced with deterministic 3-attempt retry loop + post-check). **All High audit findings now closed (9/9, 100%).** + +#### Changed + +- **`Dockerfile` + `Dockerfile.agent` (Audit H-001 / CWE-829)** — 5 FROM lines pinned to live digests fetched from Docker Hub at audit time: + - `node:20-alpine@sha256:fb4cd12c85ee03686f6af5362a0b0d56d50c58a04632e6c0fb8363f609372293` + - `golang:1.25-alpine@sha256:5caaf1cca9dc351e13deafbc3879fd4754801acba8653fa9540cea125d01a71f` (×2) + - `alpine:3.19@sha256:6baf43584bcb78f2e5847d1de515f23499913ac9f12bdf834811a3145eb11ca1` (×2) + + Header doc-comment in `Dockerfile` documents the operator bump procedure (quarterly cadence; `docker manifest inspect` and Hub Registry API alternatives for fetching the next digest). A registry-side tag swap can no longer change what we pull. +- **`Dockerfile:25` (Audit M-014)** — `npm ci` retry refactor. Pre-bundle `npm ci --include=dev || npm ci --include=dev && tsc && build` had broken bash precedence (`A || (B && C && D)`) that silently skipped `tsc && build` on transient registry blips. Replaced with `for i in 1 2 3; do npm ci --include=dev && break; sleep 5; done` plus a fail-loud `[ -d node_modules ]` post-check. + +#### Added + +- **CI step `Forbidden bare FROM regression guard (H-001)` in `.github/workflows/ci.yml`** — Greps every `Dockerfile*` in the repo and fails the build if any `FROM` line lacks an `@sha256` digest pin. Adding a new Dockerfile or refactoring an existing one without preserving the pin fails CI permanently. +- **CI step `Forbidden missing USER regression guard (M-012)` in `.github/workflows/ci.yml`** — Greps every `Dockerfile*` for the LAST `USER` directive; fails the build if missing OR if it equals `root`/`0`. Adding a new Dockerfile or refactoring an existing one to run as root fails CI permanently. + +#### Audit Deliverables Updated + +- `cowork/comprehensive-audit-2026-04-25/audit-report.md` — score 52/55 → **49/55** (corrected from over-counted 52 — actual closure count after Bundle A is 49 closed C+H+M+L of 55 total scope; **High 9/9 = 100%** for the first time; Medium 24/27; Low 19/19 with L-004 deferred). H-001 / M-012 / M-014 boxes flipped `[x]` with closure notes. +- `cowork/comprehensive-audit-2026-04-25/findings.yaml` — 3 status flips with closure notes citing the Bundle A mechanism. + ### Bundle E (Mechanical Sweeps & Defensive Polish): 6 audit findings closed; L-004 deferred > Closes the audit's mechanical-sweep cluster — `L-009` (ZeroSSL EAB URL configurable; audit's "no timeout" claim was wrong — 15s already in place), `L-010` (verified-already-clean: 0 mock.Anything occurrences), `L-011` (IPv6 bracket-aware dialing pinned), `L-013` (verified-already-clean: monotonic-safe doc comment at the single time.Now().Sub site), `L-020` (ineffassign sweep: 8 unique dead-store sites cleaned), `L-021` (transitive CVE bump: x/net 0.42→0.47, x/crypto 0.41→0.45, all 5 advisories cleared). **`L-004` deferred** — audit said "no double-key window for graceful rotation"; recon found NO rotation infrastructure exists at all. Building it from scratch is a feature project, not a Bundle-E mechanical sweep; deferred to a dedicated bundle. diff --git a/Dockerfile b/Dockerfile index 02dfd25..c5e4d4a 100644 --- a/Dockerfile +++ b/Dockerfile @@ -1,7 +1,28 @@ # Multi-stage build for certctl server +# +# Bundle A / Audit H-001 (CWE-829): every FROM line is pinned to an +# immutable digest in addition to the human-readable tag. The tag is +# advisory; the digest is what Docker actually pulls. A registry-side +# tag swap (the documented prior-art for tag-only pulls being unsafe) +# can no longer change the build. +# +# Bump procedure (operator): +# 1. Quarterly cadence (or sooner if a CVE lands on a base image). +# 2. For each FROM: +# docker pull : +# docker manifest inspect : | grep -m1 digest +# OR via Docker Hub Registry API: +# curl -sSL https://hub.docker.com/v2/repositories/library//tags/ \ +# | jq -r .digest +# 3. Replace the @sha256:... portion of the FROM line. +# 4. Run `docker build` locally + verify CI. +# 5. Commit with the bump procedure cited in the message body. +# +# The CI step "Forbidden bare FROM regression guard (H-001)" rejects +# any future commit that lands a FROM without an @sha256 pin. # Stage 1: Build frontend -FROM node:20-alpine AS frontend +FROM node:20-alpine@sha256:fb4cd12c85ee03686f6af5362a0b0d56d50c58a04632e6c0fb8363f609372293 AS frontend # Proxy propagation (M-4, Issue #9) — defaulted to empty so un-proxied builds # behave identically to the pre-fix tree. When `HTTP_PROXY`/`HTTPS_PROXY`/ @@ -22,12 +43,27 @@ ENV HTTP_PROXY=${HTTP_PROXY} \ WORKDIR /app/web COPY web/ . -RUN npm ci --include=dev || npm ci --include=dev && \ +# Bundle A / Audit M-014: explicit retry loop for `npm ci`. Pre-bundle +# this was `npm ci || npm ci && tsc && build` — the bash precedence is +# `A || (B && C && D)` so the second `npm ci` only ran on the failure +# path of the first, but the `tsc && build` chain only ran on the +# success path of the second. Net effect: a transient registry blip +# turned the build into a silent skip of the production step. +# +# New shape: a deterministic 3-attempt retry with 5-second backoff and +# an explicit `[ -d node_modules ]` post-check so a silent failure is +# impossible. +RUN for i in 1 2 3; do \ + npm ci --include=dev && break; \ + echo "npm ci attempt $i failed; sleeping 5s before retry"; \ + sleep 5; \ + done && \ + [ -d node_modules ] || (echo "ERROR: npm ci failed after 3 attempts; node_modules missing" && exit 1) && \ node_modules/.bin/tsc --version && \ npm run build # Stage 2: Build Go binary -FROM golang:1.25-alpine AS builder +FROM golang:1.25-alpine@sha256:5caaf1cca9dc351e13deafbc3879fd4754801acba8653fa9540cea125d01a71f AS builder # Proxy propagation (M-4, Issue #9) — see Stage 1 rationale. ARG HTTP_PROXY= @@ -57,7 +93,7 @@ RUN CGO_ENABLED=0 GOOS=linux GOARCH=${TARGETARCH} go build \ ./cmd/server # Stage 3: Runtime -FROM alpine:3.19 +FROM alpine:3.19@sha256:6baf43584bcb78f2e5847d1de515f23499913ac9f12bdf834811a3145eb11ca1 RUN apk add --no-cache ca-certificates tzdata curl diff --git a/Dockerfile.agent b/Dockerfile.agent index 1dfe489..d2bed6a 100644 --- a/Dockerfile.agent +++ b/Dockerfile.agent @@ -1,6 +1,11 @@ # Multi-stage build for certctl agent +# +# Bundle A / Audit H-001 (CWE-829): every FROM line is pinned to an +# immutable digest. See Dockerfile (server) for the bump-procedure +# operator runbook; the pins here MUST be bumped in the same pass. + # Stage 1: Build -FROM golang:1.25-alpine AS builder +FROM golang:1.25-alpine@sha256:5caaf1cca9dc351e13deafbc3879fd4754801acba8653fa9540cea125d01a71f AS builder # Proxy propagation (M-4, Issue #9) — defaulted to empty so un-proxied builds # behave identically to the pre-fix tree. When `HTTP_PROXY`/`HTTPS_PROXY`/ @@ -34,7 +39,7 @@ RUN CGO_ENABLED=0 GOOS=linux GOARCH=${TARGETARCH} go build \ ./cmd/agent # Stage 2: Runtime -FROM alpine:3.19 +FROM alpine:3.19@sha256:6baf43584bcb78f2e5847d1de515f23499913ac9f12bdf834811a3145eb11ca1 # U-2: `procps` ships pgrep, which the HEALTHCHECK below uses to verify the # agent process is alive. Pre-U-2 the deploy/docker-compose.yml agent