From 0a3ddcf0ec8e9ba0eecd19d334c69c2bfb4049bc Mon Sep 17 00:00:00 2001 From: Build System Date: Fri, 31 Jul 2026 01:48:10 +0200 Subject: [PATCH 1/9] ci: version pins, laned suite, local lint gate Three changes that only make sense together, all in CI. Version pins were spread over 7 workflow env: blocks, 3 hardcoded ci-deep matrix cells, and sha256 digests in bash associative arrays in ci-build.sh. Two lists with different writers meant a bump could carry a stale digest, and an unpinned version still built with only a WARNING. They now live in .github/versions.env with version and digest on adjacent lines and a single writer, loaded into $GITHUB_ENV as the first step after checkout in all 13 nginx-building jobs. Verification is mandatory: an unpinned version is a hard failure, and an already-present tarball is re-verified, so a poisoned build cache is caught rather than reused. Six workflows each carried their own push/pull_request trigger, so a PR asked for all of them at once. Wall-clock on builder02 is queueing for a runner slot, not compute. They are workflow_call members of one ci.yml now, in three lanes under the 268s budget set by the longest single job: A Fuzzing (268) B A/UBSan (94) -> Valgrind (83) -> Security scanners (77) = 254 C Build&Test (~50) Nothing is chained behind Fuzzing: work placed there is pure critical path, work appended to a shorter lane is free. Lint, CodeQL and the changed-files gate run hosted, take no self-hosted slot, and so are not laned at all. The durations are measured, not estimated, and ci.yml's header carries the run ID they came from plus the command to re-derive them. A tracked pre-commit hook (core.hooksPath .githooks) runs ci/linter/, whose thresholds mirror security-scanners.yml so local green predicts remote green. A missing linter exits 2 and blocks the commit; it is never a silent skip. Defects found by verifying rather than assuming: - build-test.yml shared a concurrency group with its own orchestrator and cancelled it. Lane C produced zero jobs and !cancelled() read that as a partial pass. - The asan changed-files gate failed open. A shallow checkout gave "no merge base", the error was swallowed left of grep -q, the step exited 0, and the sanitizer was skipped while the suite stayed green. - fetch-verify.sh took the first line of a stream as the digest, so a spurious progress line landed as NGINX_MAINLINE_SHA256. It is now shape-validated against ^[0-9a-f]{64}$ rather than trusted by position. - The Angie ci-deep cell had never built. -Wshadow in ci-build.sh's --with-cc-opt is baked into CFLAGS for every object nginx/Angie compiles, including Angie's own core, where Angie's -Werror made its shadow warnings fatal. Neither flag alone does it. The same job hardcoded objs/nginx, but Angie names its binary objs/angie, so TEST_NGINX_BINARY pointed at a file that did not exist and only the .so was asserted. Angie now runs 55/55. - semgrep defaults to one OCaml domain per core, each opening an io_uring ring against an 8 MB RLIMIT_MEMLOCK shared with the six runner slots, and aborts when they are busy: 3/3 crashed on a busy box, 0/3 on an idle one. Pinned to --jobs=1 in both the hook and CI, so a neighbouring job cannot redden a clean diff. --- .githooks/pre-commit | 28 ++ .github/scripts/compute-versions.sh | 125 +++++++ .github/scripts/fetch-verify.sh | 46 +++ .github/scripts/load-versions.sh | 24 ++ .github/versions.env | 37 ++ .github/workflows/asan.yml | 25 +- .github/workflows/build-test.yml | 27 +- .github/workflows/ci-deep.yml | 78 +++- .github/workflows/ci.yml | 182 +++++++++ .github/workflows/codeql.yml | 17 +- .github/workflows/fuzzing.yml | 13 +- .github/workflows/lint.yml | 59 +++ .github/workflows/security-scanners.yml | 58 ++- .github/workflows/valgrind.yml | 13 +- .perlcriticrc | 23 ++ .pre-commit-config.yaml | 145 +++++++ .yamllint | 25 ++ README.md | 32 +- ci/PROMPT-standardize-module.md | 478 ++++++++++++++++++++++++ ci/linter/README.md | 280 ++++++++++++++ ci/linter/install-linters.sh | 148 ++++++++ ci/linter/lib.sh | 50 +++ ci/linter/lint-c.sh | 77 ++++ ci/linter/lint-nginx.sh | 78 ++++ ci/linter/lint-perl.sh | 37 ++ ci/linter/lint-python.sh | 30 ++ ci/linter/lint-sh.sh | 24 ++ ci/linter/lint-yaml.sh | 67 ++++ ci/linter/run-all.sh | 111 ++++++ ci/tools/bump-versions.sh | 215 +++-------- ci/tools/ci-build.sh | 149 +++++--- src/ngx_http_skel_module.c | 21 +- src/ngx_http_skel_scan.c | 3 +- 33 files changed, 2428 insertions(+), 297 deletions(-) create mode 100755 .githooks/pre-commit create mode 100755 .github/scripts/compute-versions.sh create mode 100755 .github/scripts/fetch-verify.sh create mode 100755 .github/scripts/load-versions.sh create mode 100644 .github/versions.env create mode 100644 .github/workflows/ci.yml create mode 100644 .github/workflows/lint.yml create mode 100644 .perlcriticrc create mode 100644 .pre-commit-config.yaml create mode 100644 .yamllint create mode 100644 ci/PROMPT-standardize-module.md create mode 100644 ci/linter/README.md create mode 100755 ci/linter/install-linters.sh create mode 100755 ci/linter/lib.sh create mode 100755 ci/linter/lint-c.sh create mode 100755 ci/linter/lint-nginx.sh create mode 100755 ci/linter/lint-perl.sh create mode 100755 ci/linter/lint-python.sh create mode 100755 ci/linter/lint-sh.sh create mode 100755 ci/linter/lint-yaml.sh create mode 100755 ci/linter/run-all.sh diff --git a/.githooks/pre-commit b/.githooks/pre-commit new file mode 100755 index 0000000..38a0960 --- /dev/null +++ b/.githooks/pre-commit @@ -0,0 +1,28 @@ +#!/usr/bin/env bash +# .githooks/pre-commit -- tracked, reviewable commit gate. +# +# Lives in the repo (unlike .git/hooks/, which is per-clone and invisible to +# review), so a change to what the gate checks arrives as a diff. +# +# Enable once per clone: +# git config core.hooksPath .githooks +# Emergency bypass: +# git commit --no-verify +# +# It lints only the STAGED files, so a commit is never blocked by a finding in +# a file it does not touch. Run the whole tree yourself with: +# ci/linter/run-all.sh +# +# Exit 2 (a linter is not installed) blocks the commit on purpose: a gate that +# skips itself when its tool is missing reports green while checking nothing. +set -uo pipefail + +ROOT="$(git rev-parse --show-toplevel)" +"$ROOT/ci/linter/run-all.sh" --staged +rc=$? +if [ "$rc" -eq 2 ]; then + echo "pre-commit: missing linters -- run ci/linter/install-linters.sh" >&2 +elif [ "$rc" -ne 0 ]; then + echo "pre-commit: lint failed -- fix, or commit with --no-verify" >&2 +fi +exit "$rc" diff --git a/.github/scripts/compute-versions.sh b/.github/scripts/compute-versions.sh new file mode 100755 index 0000000..6632664 --- /dev/null +++ b/.github/scripts/compute-versions.sh @@ -0,0 +1,125 @@ +#!/usr/bin/env bash +# compute-versions.sh -- resolve the latest upstream versions + their sha256 and +# rewrite .github/versions.env in place. Used by bump.yml (weekly). Prints a +# short summary of what it resolved to stdout, which bump.yml quotes into the +# PR body. +# +# Resolves: +# nginx mainline (odd minor) + stable (even minor) -- nginx.org download page +# Angie latest release -- GitHub API tag_name +# +# Angie is RESOLVED from the GitHub API (the only machine-readable index of +# Angie releases) but DOWNLOADED from download.angie.software, which is what +# ci/tools/ci-build.sh fetches. The two archives differ byte-for-byte, so the +# digest must come from the URL we actually build from -- hashing the GitHub +# tag archive here would pin a sha that never matches at build time. +# +# Run locally to preview a bump: bash .github/scripts/compute-versions.sh +# (it rewrites versions.env; `git diff` to review, `git checkout` to discard). +# +# Requires: curl, jq, sha256sum. GITHUB_TOKEN honoured for API rate limits -- +# unauthenticated api.github.com from a shared runner IP gets 403-throttled. +set -euo pipefail + +VERSIONS_FILE=".github/versions.env" +FV=".github/scripts/fetch-verify.sh" +tmp="$(mktemp -d)" +trap 'rm -rf "$tmp"' EXIT + +api() { + local url="$1" + if [ -n "${GITHUB_TOKEN:-}" ]; then + curl -fsSL -H "Authorization: Bearer $GITHUB_TOKEN" "$url" + else + curl -fsSL "$url" + fi +} + +# sha256 of a URL (download to scratch, hash). Fails the job on download error. +sha_of_url() { + local url="$1" out="$tmp/dl.$RANDOM" sha + # In "-" mode fetch-verify.sh sends progress text to stderr, so stdout is + # exactly one "SHA OUTFILE" line. Read the first field directly rather than + # picking the last line out of mixed output -- an extra stdout line there + # would otherwise be captured as a digest with no error. + read -r sha _ < <(bash "$FV" "$url" - "$out") + # Validate the shape rather than trusting position. Moving the progress text + # to stderr fixes today's contamination; this catches the next one, because a + # non-digest silently written into versions.env would pin every future build + # to a value that can never match. + if ! printf '%s' "$sha" | grep -qE '^[0-9a-f]{64}$'; then + echo "::error::expected a sha256 from $FV for $url, got: ${sha:-}" >&2 + return 1 + fi + printf '%s' "$sha" +} + +echo "resolving nginx versions from nginx.org..." +dl_html="$(curl -fsSL https://nginx.org/en/download.html)" +# nginx numbers mainline with an odd minor and stable with an even one. Take +# the highest of each rather than trusting page order. +NGX_MAINLINE="$(printf '%s' "$dl_html" | grep -oE 'nginx-1\.[0-9]+\.[0-9]+' \ + | awk -F. '$2%2==1' | sort -uV | tail -1 | sed 's/nginx-//')" +NGX_STABLE="$(printf '%s' "$dl_html" | grep -oE 'nginx-1\.[0-9]+\.[0-9]+' \ + | awk -F. '$2%2==0' | sort -uV | tail -1 | sed 's/nginx-//')" +[ -n "$NGX_MAINLINE" ] && [ -n "$NGX_STABLE" ] || { echo "::error::failed to resolve nginx versions" >&2; exit 1; } + +echo "resolving Angie latest release..." +ANGIE_TAG="$(api 'https://api.github.com/repos/webserver-llc/angie/releases/latest' | jq -r '.tag_name')" +[ -n "$ANGIE_TAG" ] && [ "$ANGIE_TAG" != "null" ] || { echo "::error::failed to resolve Angie" >&2; exit 1; } +# Upstream tags releases "Angie-1.12.1"; ci-build.sh wants the bare version. +ANGIE="${ANGIE_TAG#Angie-}" +if ! printf '%s' "$ANGIE" | grep -qE '^[0-9]+\.[0-9]+\.[0-9]+$'; then + echo "::error::unexpected Angie tag format: $ANGIE_TAG" >&2 + exit 1 +fi + +echo "hashing archives..." +NGX_MAINLINE_SHA="$(sha_of_url "https://nginx.org/download/nginx-${NGX_MAINLINE}.tar.gz")" +NGX_STABLE_SHA="$(sha_of_url "https://nginx.org/download/nginx-${NGX_STABLE}.tar.gz")" +ANGIE_SHA="$(sha_of_url "https://download.angie.software/files/angie-${ANGIE}.tar.gz")" + +cat > "$VERSIONS_FILE" <". +ANGIE_VERSION=${ANGIE} +ANGIE_SHA256=${ANGIE_SHA} +EOF + +echo "----- resolved -----" +echo "nginx mainline: ${NGX_MAINLINE}" +echo "nginx stable: ${NGX_STABLE}" +echo "angie: ${ANGIE}" diff --git a/.github/scripts/fetch-verify.sh b/.github/scripts/fetch-verify.sh new file mode 100755 index 0000000..feeabb8 --- /dev/null +++ b/.github/scripts/fetch-verify.sh @@ -0,0 +1,46 @@ +#!/usr/bin/env bash +# fetch-verify.sh URL EXPECTED_SHA256 OUTFILE +# +# Download URL to OUTFILE and verify its sha256 against EXPECTED_SHA256. +# On mismatch: print the actual sha and exit 1 (fails the CI job — a changed +# or tampered upstream archive never reaches the build). +# +# Pass EXPECTED_SHA256="-" to skip verification and just print the computed +# sha (used by bump.yml to harvest fresh hashes for a version bump). +# +# If OUTFILE already exists with the right sha (warm actions/cache hit) the +# download is skipped — the sha check still runs, so a poisoned cache is caught. +set -euo pipefail + +url="${1:?usage: fetch-verify.sh URL SHA256 OUTFILE}" +want="${2:?missing expected sha256}" +out="${3:?missing output path}" + +sha_of() { sha256sum "$1" | cut -d' ' -f1; } + +if [ -f "$out" ] && [ "$want" != "-" ] && [ "$(sha_of "$out")" = "$want" ]; then + echo "cache hit (sha ok): $out" + exit 0 +fi + +# stderr, not stdout: in "-" mode stdout must carry ONLY the final +# "$got $out" line, which compute-versions.sh reads as the digest. +echo "downloading: $url" >&2 +# -f: fail on HTTP errors; -S: show errors; -L: follow redirects; retries. +# --connect-timeout/--max-time: a stalled upstream must not hold a runner open. +curl -fSL --retry 3 --retry-delay 2 \ + --connect-timeout 30 --max-time 300 -o "$out" "$url" + +got="$(sha_of "$out")" +if [ "$want" = "-" ]; then + echo "$got $out" + exit 0 +fi + +if [ "$got" != "$want" ]; then + echo "::error::sha256 MISMATCH for $url" >&2 + echo " expected: $want" >&2 + echo " actual: $got" >&2 + exit 1 +fi +echo "sha256 verified: $out" diff --git a/.github/scripts/load-versions.sh b/.github/scripts/load-versions.sh new file mode 100755 index 0000000..98cad97 --- /dev/null +++ b/.github/scripts/load-versions.sh @@ -0,0 +1,24 @@ +#!/usr/bin/env bash +# load-versions.sh — export every pin from .github/versions.env into the +# workflow environment. Run as the first step of every CI job: +# +# - run: bash .github/scripts/load-versions.sh +# +# Skips comments/blanks; validates each line is KEY=value so a malformed +# versions.env fails loudly instead of injecting garbage into $GITHUB_ENV. +set -euo pipefail + +f=".github/versions.env" +[ -f "$f" ] || { echo "::error::$f not found" >&2; exit 1; } + +while IFS= read -r line || [ -n "$line" ]; do + case "$line" in + ''|\#*) continue ;; + esac + if ! printf '%s' "$line" | grep -qE '^[A-Za-z_][A-Za-z0-9_]*='; then + echo "::error::malformed line in $f: $line" >&2 + exit 1 + fi + echo "$line" >> "$GITHUB_ENV" + echo "loaded: ${line%%=*}" +done < "$f" diff --git a/.github/versions.env b/.github/versions.env new file mode 100644 index 0000000..9af3574 --- /dev/null +++ b/.github/versions.env @@ -0,0 +1,37 @@ +# Central version + sha256 pins for all CI workflows. +# +# SINGLE SOURCE OF TRUTH. Every workflow loads this file into $GITHUB_ENV as its +# first step (via .github/scripts/load-versions.sh); the weekly bump.yml job +# rewrites it and opens a PR. Tarballs are pinned by version string (release +# archives are immutable) AND verified against the sha256 recorded here, so a +# compromised or changed upstream archive fails the build instead of being +# compiled. +# +# Version and digest live on adjacent lines on purpose: they are bumped by one +# writer (compute-versions.sh) in one file, so a version can no longer move +# while its digest stays behind. +# +# Regenerate with .github/scripts/compute-versions.sh (bump.yml runs it weekly). +# Keep KEY=value, no spaces, no quotes -- this file is both `source`d by +# ci/tools/ci-build.sh and `cat`d into $GITHUB_ENV. + +# nginx mainline (odd minor) -- the default build everywhere; also the +# ci-deep "mainline" matrix cell. +NGINX_MAINLINE=1.31.3 +NGINX_MAINLINE_SHA256=a7657c50811c2d92d9895395e8b873ef60398142c4db21eb647811c38f6dd525 + +# nginx stable (even minor) -- ci-deep "stable" matrix cell only. +NGINX_STABLE=1.30.4 +NGINX_STABLE_SHA256=4261dc90e9e47c1c4041276e9aaa3d48ebe2e664f728e14fa95ae6c67d57a08b + +# nginx version used by every single-version job (build-test, asan, valgrind, +# codeql, fuzzing, security-scanners, ci-deep memcheck). Tracks mainline. +NGINX_VERSION=1.31.3 +NGINX_VERSION_SHA256=a7657c50811c2d92d9895395e8b873ef60398142c4db21eb647811c38f6dd525 + +# Angie (webserver-llc) -- ci-deep "angie" matrix cell. Pinned to the tarball +# from download.angie.software, NOT the GitHub tag archive: different bytes, +# so this digest is not interchangeable with a github.com/webserver-llc one. +# ANGIE_VERSION is the bare version; the upstream release tag is "Angie-". +ANGIE_VERSION=1.12.1 +ANGIE_SHA256=5f4f203be2aca6fe20770b489c720e46e51d337e521065e7e472b61e24e3d2f5 diff --git a/.github/workflows/asan.yml b/.github/workflows/asan.yml index 3e4bf73..3fa3627 100644 --- a/.github/workflows/asan.yml +++ b/.github/workflows/asan.yml @@ -20,26 +20,12 @@ name: A/UBSan env: FORCE_JAVASCRIPT_ACTIONS_TO_NODE24: true - NGINX_VERSION: "1.31.2" on: - push: - branches: [main] - paths: - - "src/**" - - "ci/t/**" - - "ci/tools/ci-build.sh" - - "ci/tools/soak.sh" - - ".github/workflows/asan.yml" - pull_request: - branches: [main] - paths: - - "src/**" - - "ci/t/**" - - "ci/tools/ci-build.sh" - - "ci/tools/soak.sh" - - ".github/workflows/asan.yml" - workflow_dispatch: {} + # Triggered by ci.yml, which lanes the PR suite so peak runner use stays + # bounded. No push/pull_request trigger here on purpose: two entry points + # would run this twice per PR and defeat the laning. + workflow_call: {} concurrency: group: asan-${{ github.workflow }}-${{ github.ref }} @@ -59,6 +45,9 @@ jobs: with: persist-credentials: false + - name: Load version pins + run: bash .github/scripts/load-versions.sh + - name: Install dependencies run: | sudo apt-get update diff --git a/.github/workflows/build-test.yml b/.github/workflows/build-test.yml index 6eb5c18..7531b35 100644 --- a/.github/workflows/build-test.yml +++ b/.github/workflows/build-test.yml @@ -11,17 +11,18 @@ name: Build&Test env: FORCE_JAVASCRIPT_ACTIONS_TO_NODE24: true - NGINX_VERSION: "1.31.2" on: - push: - branches: [main] - pull_request: - branches: [main] - workflow_dispatch: {} + # Triggered by ci.yml, which lanes the PR suite so peak runner use stays + # bounded. No push/pull_request trigger here on purpose: two entry points + # would run this twice per PR and defeat the laning. + workflow_call: {} concurrency: - group: ci-${{ github.workflow }}-${{ github.ref }} + # Prefix must be unique per workflow. As a workflow_call member this inherits + # the caller's github.workflow/github.ref, so a group string matching ci.yml's + # would cancel the caller rather than a stale copy of this workflow. + group: build-test-${{ github.workflow }}-${{ github.ref }} cancel-in-progress: true permissions: @@ -93,6 +94,9 @@ jobs: with: persist-credentials: false + - name: Load version pins + run: bash .github/scripts/load-versions.sh + - name: Install build dependencies run: | sudo apt-get update @@ -229,6 +233,9 @@ jobs: with: persist-credentials: false + - name: Load version pins + run: bash .github/scripts/load-versions.sh + - name: Install dependencies run: | sudo apt-get update @@ -284,6 +291,9 @@ jobs: with: persist-credentials: false + - name: Load version pins + run: bash .github/scripts/load-versions.sh + - name: Install build dependencies run: | sudo apt-get update @@ -345,6 +355,9 @@ jobs: with: persist-credentials: false + - name: Load version pins + run: bash .github/scripts/load-versions.sh + - name: Install dependencies run: | sudo apt-get update diff --git a/.github/workflows/ci-deep.yml b/.github/workflows/ci-deep.yml index a43fe7d..7eee199 100644 --- a/.github/workflows/ci-deep.yml +++ b/.github/workflows/ci-deep.yml @@ -30,7 +30,6 @@ name: CI Deep env: FORCE_JAVASCRIPT_ACTIONS_TO_NODE24: true - NGINX_VERSION: "1.31.2" on: schedule: @@ -52,21 +51,27 @@ permissions: jobs: build-flavors: - name: Build & Test::Nginx (${{ matrix.flavor }} ${{ matrix.version }}) + # matrix.label, not the version: the job name is evaluated before any step + # runs, so the resolved version is not available here. + name: Build & Test::Nginx (${{ matrix.flavor }} ${{ matrix.label }}) runs-on: ${{ github.event.pull_request.head.repo.fork && 'ubuntu-latest' || fromJSON('["self-hosted","builder02","lxc"]') }} timeout-minutes: 30 strategy: fail-fast: false + # A matrix is expanded before any step runs, so load-versions.sh cannot + # feed it. Each cell therefore names the versions.env KEY it wants, and + # the build step dereferences it after the pins are loaded -- keeping the + # versions themselves in one file instead of re-hardcoding them here. matrix: include: - flavor: nginx - version: "1.31.2" + version_key: NGINX_MAINLINE label: mainline - flavor: nginx - version: "1.30.3" + version_key: NGINX_STABLE label: stable - flavor: angie - version: "1.12.0" + version_key: ANGIE_VERSION label: angie steps: - name: Checkout module @@ -74,6 +79,9 @@ jobs: with: persist-credentials: false + - name: Load version pins + run: bash .github/scripts/load-versions.sh + - name: Install dependencies run: | sudo apt-get update @@ -82,28 +90,67 @@ jobs: cpanminus libio-socket-ssl-perl sudo cpanm --notest --quiet Test::Nginx + - name: Resolve matrix version + # The cell names a versions.env key (NGINX_MAINLINE/NGINX_STABLE/ + # ANGIE_VERSION); dereference it now that load-versions.sh has put the + # pins in the environment, so later steps can use env.MATRIX_VERSION. + env: + # Through env, not inline ${{ }}: an expression expanded straight into + # a run: block is template injection (zizmor template-injection). The + # matrix values are repo-controlled today, but the safe shape costs + # nothing and stops the pattern from being copied somewhere it is not. + MATRIX_VERSION_KEY: ${{ matrix.version_key }} + run: | + key="$MATRIX_VERSION_KEY" + value="${!key:-}" + if [ -z "$value" ]; then + echo "::error::$key is not set in .github/versions.env" + exit 1 + fi + echo "MATRIX_VERSION=$value" >> "$GITHUB_ENV" + echo "resolved $key=$value" + - name: Restore build caches uses: ./.github/actions/build-cache with: mode: debug flavor: ${{ matrix.flavor }} - nginx-version: ${{ matrix.version }} + nginx-version: ${{ env.MATRIX_VERSION }} - name: Build server and dynamic module + # angie names its binary objs/angie, nginx names it objs/nginx. Assert + # BOTH artifacts and export the server path, so the flavor's real binary + # name lives in one place instead of being hardcoded per step -- checking + # only the .so passes for angie while leaving the server unverified. + env: + MATRIX_FLAVOR: ${{ matrix.flavor }} # via env, see the note above run: | - bash ci/tools/ci-build.sh "${{ matrix.flavor }}" "${{ matrix.version }}" - build=".build/${{ matrix.flavor }}-${{ matrix.version }}-debug/objs" + set -euo pipefail + bash ci/tools/ci-build.sh "$MATRIX_FLAVOR" "$MATRIX_VERSION" + build=".build/$MATRIX_FLAVOR-${MATRIX_VERSION}-debug/objs" + case "$MATRIX_FLAVOR" in + angie) server="$build/angie" ;; + *) server="$build/nginx" ;; + esac test -f "$build/ngx_http_skel_module.so" + test -x "$server" + echo "SERVER_BIN=$GITHUB_WORKSPACE/$server" >> "$GITHUB_ENV" + echo "MODULE_SO=$GITHUB_WORKSPACE/$build/ngx_http_skel_module.so" >> "$GITHUB_ENV" - name: Run ci/t/ env: - TEST_NGINX_BINARY: ${{ github.workspace }}/.build/${{ matrix.flavor }}-${{ matrix.version }}-debug/objs/nginx - TEST_NGINX_LOAD_MODULES: ${{ github.workspace }}/.build/${{ matrix.flavor }}-${{ matrix.version }}-debug/objs/ngx_http_skel_module.so + # Set by the build step above -- it resolves the flavor's binary name + # (angie vs nginx) once, after the version is known. + TEST_NGINX_BINARY: ${{ env.SERVER_BIN }} + TEST_NGINX_LOAD_MODULES: ${{ env.MODULE_SO }} # See build-test.yml's identical setting for why this must be 20, not # the ~2s Test::Nginx::Socket default. TEST_NGINX_TIMEOUT: "20" TEST_NGINX_SERVROOT: ${{ github.workspace }}/ci/t/servroot - run: prove -v ci/t/ + # Test::Nginx probes a server on PATH for its version and Bailouts if it + # finds none -- TEST_NGINX_BINARY alone does not satisfy that probe, so + # the objs/ dir goes on PATH too. + run: PATH="$(dirname "$TEST_NGINX_BINARY"):$PATH" prove -v ci/t/ fuzz: name: Fuzz (scan core, long) @@ -115,6 +162,9 @@ jobs: with: persist-credentials: false + - name: Load version pins + run: bash .github/scripts/load-versions.sh + - name: Install build deps run: | sudo apt-get update @@ -199,6 +249,9 @@ jobs: with: persist-credentials: false + - name: Load version pins + run: bash .github/scripts/load-versions.sh + - name: Install dependencies run: | sudo apt-get update @@ -231,6 +284,9 @@ jobs: with: persist-credentials: false + - name: Load version pins + run: bash .github/scripts/load-versions.sh + - name: Install dependencies run: | sudo apt-get update diff --git a/.github/workflows/ci.yml b/.github/workflows/ci.yml new file mode 100644 index 0000000..8546e7f --- /dev/null +++ b/.github/workflows/ci.yml @@ -0,0 +1,182 @@ +name: CI + +# Single entry point for the PR suite. The seven checks below used to be +# independent workflows, each with its own push/pull_request trigger, so a PR +# asked for all of them at once. On the builder02 runners that is the wrong +# shape: CI wall-clock here is dominated by jobs QUEUEING for a label-matching +# slot, not by the jobs themselves, so ten simultaneous requests just means the +# tail waits. builder02 has SIX ephemeral slots (ci-ephemeral@runner-01..04, +# @docker-01..02); everything below is shaped around that number. +# +# The self-hosted work is arranged as three lanes. A lane starts with its +# longest job and releases the slot to a shorter, independent follow-up. +# +# A Fuzzing (282s) = 282s +# B A/UBSan (97s) -> Valgrind (79s) -> Security scanners (47s) = 223s +# C Build&Test (~60s) = 60s +# +# The budget is the longest single job -- 282s of fuzzing -- because no +# arrangement can finish sooner than that. Both other lanes are under it, so the +# suite is fuzz-bound and lane B has ~59s of headroom. If Valgrind or the +# scanners grow past that, move one of them to a fourth lane rather than letting +# lane B become the critical path. +# +# Durations are OBSERVED on this exact shape -- run 30591711577 (head 1b6d094, +# 2026-07-31), green, 286s wall clock, lane B finishing 39s ahead of Fuzzing. +# An earlier revision of this comment quoted 320/149/120/110/100/98s and paired +# Fuzzing with Security scanners, which put lane A at 348s: over budget, and the +# suite's actual critical path, for a 352s total. Security scanners reads 47s +# here against 77s on that shape because semgrep gained --jobs=1 --metrics=off +# in the same change. Re-measure with +# `gh run view -R myguard-labs/nginx-skeleton-module --json jobs` before +# trusting any number here; they drift with the runners. +# +# NOTE on "peak runner use": a lane is not a slot. Build&Test is a reusable +# workflow that fans out into FIVE concurrent self-hosted jobs, so the observed +# peak is 5 + Fuzzing + A/UBSan = 7 against 6 slots, and one job queues briefly +# at t=0. That is accepted -- it costs seconds and only at the start -- but do +# not read "three lanes" as "three runners". +# +# Lint, CodeQL and Detect-relevant-changes run on ubuntu-latest and take no +# self-hosted slot, so they are not laned at all and start immediately. +# +# Follow-ups use !cancelled() so a FAILING first check does not suppress an +# unrelated second one -- a red A/UBSan should still tell you whether Valgrind +# is clean. It also keeps the chain alive when A/UBSan is SKIPPED by the +# changed-files gate below, which a bare `needs:` would not. A superseded +# workflow is still cancelled as one unit, because cancel-in-progress below +# applies to the whole run. +# +# No `push:` trigger, by repo policy: CI runs on the PR, and the merge commit is +# identical to the tested PR head, so re-running it on merge buys nothing. + +on: + pull_request: + branches: [main] + workflow_dispatch: {} + +concurrency: + # "orchestrator-", not "ci-": build-test.yml already uses + # ci-${{ github.workflow }}-${{ github.ref }}, and a called workflow inherits + # the caller's github.workflow/github.ref -- so an identical group string + # makes the member cancel its own caller, and lane C dies before it starts a + # single job. The prefix is what keeps the two apart. + group: orchestrator-${{ github.workflow }}-${{ github.ref }} + cancel-in-progress: true + +permissions: + contents: read + +jobs: + # --- lane A: the budget-setting job, alone ------------------------------- + # Nothing follows Fuzzing. At 268s it is the longest job in the suite, so the + # suite cannot finish sooner; anything chained behind it is pure added + # critical path. Security scanners used to sit here and cost exactly that. + fuzzing: + name: Fuzzing + permissions: + contents: read + uses: ./.github/workflows/fuzzing.yml + + # --- lane B -------------------------------------------------------------- + # asan.yml previously carried its own paths: filter (src/**, ci/t/**, + # ci-build.sh, soak.sh, its own workflow). A reusable workflow cannot filter + # its own triggering, so that gate moves here as an explicit job-level `if` + # over the changed-files output -- dropping it silently would run the 149s + # sanitizer job on every docs-only PR. + changes: + name: Detect relevant changes + runs-on: ubuntu-latest + permissions: + contents: read + outputs: + code: ${{ steps.filter.outputs.code }} + steps: + - uses: actions/checkout@93cb6efe18208431cddfb8368fd83d5badbf9bfd # v5 + with: + persist-credentials: false + # Full history: a shallow clone has no common ancestor with the base, + # so `base...HEAD` dies with "no merge base" and the gate below would + # fall through to code=false -- skipping the sanitizer job on exactly + # the PRs that need it. + fetch-depth: 0 + - id: filter + env: + BASE_SHA: ${{ github.event.pull_request.base.sha }} + run: | + set -euo pipefail + # workflow_dispatch has no PR base; treat that as "run everything". + if [ -z "${BASE_SHA:-}" ]; then + echo "code=true" >> "$GITHUB_OUTPUT" + exit 0 + fi + # Fail loudly rather than fail open: this gate decides whether a + # sanitizer runs, so an unusable diff must stop the job, not quietly + # answer "no relevant changes". + if ! changed="$(git diff --name-only "$BASE_SHA"...HEAD)"; then + echo "::error::cannot diff $BASE_SHA...HEAD -- refusing to guess" >&2 + exit 1 + fi + if printf '%s\n' "$changed" | grep -qE \ + '^(src/|ci/t/|ci/tools/ci-build\.sh|ci/tools/soak\.sh|\.github/workflows/(asan|ci)\.yml|\.github/versions\.env|\.github/scripts/)'; then + echo "code=true" >> "$GITHUB_OUTPUT" + else + echo "code=false" >> "$GITHUB_OUTPUT" + fi + echo "changed files matching the asan gate:" + printf '%s\n' "$changed" | grep -E \ + '^(src/|ci/t/|ci/tools/ci-build\.sh|ci/tools/soak\.sh|\.github/workflows/(asan|ci)\.yml|\.github/versions\.env|\.github/scripts/)' || echo " (none)" + + asan: + name: A/UBSan + needs: changes + if: ${{ needs.changes.outputs.code == 'true' }} + permissions: + contents: read + uses: ./.github/workflows/asan.yml + + valgrind: + name: Valgrind + needs: asan + if: ${{ !cancelled() }} + permissions: + contents: read + uses: ./.github/workflows/valgrind.yml + + security-scanners: + name: Security scanners + # Tail of lane B, not of lane A. 94 + 83 + 77 = 254s still fits under the + # 268s fuzzing budget, so these 77s are free here and were critical path + # there. + needs: valgrind + if: ${{ !cancelled() }} + permissions: + contents: read + uses: ./.github/workflows/security-scanners.yml + + # --- lane C -------------------------------------------------------------- + build-test: + name: Build&Test + permissions: + contents: read + uses: ./.github/workflows/build-test.yml + + # --- hosted, unlaned ----------------------------------------------------- + # These run on ubuntu-latest, take no self-hosted slot, and so have no reason + # to wait behind anything. Lint is the fastest check in the suite and should + # report first. CodeQL used to hang off build-test to conserve a slot it never + # occupied -- that only delayed its result by ~50s. + lint: + name: Lint + permissions: + contents: read + uses: ./.github/workflows/lint.yml + + codeql: + name: CodeQL + permissions: + contents: read + # Granted per-job, not at workflow level: no other job here needs either. + security-events: write # CodeQL uploads its SARIF to code scanning + actions: read # the CodeQL action reads run metadata to attribute results + uses: ./.github/workflows/codeql.yml diff --git a/.github/workflows/codeql.yml b/.github/workflows/codeql.yml index d0c649f..81f4f70 100644 --- a/.github/workflows/codeql.yml +++ b/.github/workflows/codeql.yml @@ -20,18 +20,16 @@ name: CodeQL env: FORCE_JAVASCRIPT_ACTIONS_TO_NODE24: true - NGINX_VERSION: "1.31.2" on: - push: - branches: [main] - pull_request: - branches: [main] + # Triggered by ci.yml, which lanes the PR suite so peak runner use stays + # bounded. No push/pull_request trigger here on purpose: two entry points + # would run this twice per PR and defeat the laning. + workflow_call: {} schedule: - # Monthly, day 4 @ 04:47 UTC. Staggered away from ci-deep.yml (day 6) so the - # two heavy scheduled runs never contend for the same runners. + # Monthly, day 4 @ 04:47 UTC. Staggered away from ci-deep.yml (day 6) so + # the two heavy scheduled runs never contend for the same runners. - cron: "47 4 4 * *" - workflow_dispatch: {} concurrency: group: codeql-${{ github.ref }} @@ -54,6 +52,9 @@ jobs: with: persist-credentials: false + - name: Load version pins + run: bash .github/scripts/load-versions.sh + - name: Install build dependencies run: | sudo apt-get update diff --git a/.github/workflows/fuzzing.yml b/.github/workflows/fuzzing.yml index 98f73ae..ee8586e 100644 --- a/.github/workflows/fuzzing.yml +++ b/.github/workflows/fuzzing.yml @@ -14,14 +14,12 @@ name: Fuzzing env: FORCE_JAVASCRIPT_ACTIONS_TO_NODE24: true - NGINX_VERSION: "1.31.2" on: - push: - branches: [main] - pull_request: - branches: [main] - workflow_dispatch: {} + # Triggered by ci.yml, which lanes the PR suite so peak runner use stays + # bounded. No push/pull_request trigger here on purpose: two entry points + # would run this twice per PR and defeat the laning. + workflow_call: {} concurrency: group: fuzzing-${{ github.workflow }}-${{ github.ref }} @@ -41,6 +39,9 @@ jobs: with: persist-credentials: false + - name: Load version pins + run: bash .github/scripts/load-versions.sh + - name: Install build deps run: | sudo apt-get update diff --git a/.github/workflows/lint.yml b/.github/workflows/lint.yml new file mode 100644 index 0000000..72415c3 --- /dev/null +++ b/.github/workflows/lint.yml @@ -0,0 +1,59 @@ +name: Lint + +# The same ci/linter/ scripts a developer runs locally, run again on the PR -- +# so a clone that never enabled the hook (`git config core.hooksPath .githooks`) +# cannot land nginx-convention, shell, Perl, Python or workflow-syntax +# regressions. Local and remote run the SAME entry point on purpose: a CI-only +# reimplementation drifts from the hook and the two stop agreeing. +# +# LINT_ONLY leaves out the "c" checker: flawfinder/cppcheck/semgrep over src/ +# are already the job of security-scanners.yml, at the same thresholds +# (ci/linter/lint-c.sh mirrors that workflow). Running them twice per PR buys +# nothing but queue time. +# +# runs-on: ubuntu-latest, NOT the self-hosted pool. This job needs no nginx +# build, and ci.yml deliberately caps peak self-hosted use at three concurrent +# jobs; putting lint on a hosted runner keeps that cap intact while giving the +# fastest feedback in the suite. + +env: + FORCE_JAVASCRIPT_ACTIONS_TO_NODE24: true + +on: + # Triggered by ci.yml, like every other member of the suite. No + # push/pull_request trigger here: two entry points would run it twice per PR. + workflow_call: {} + +concurrency: + group: lint-${{ github.workflow }}-${{ github.ref }} + cancel-in-progress: true + +permissions: + contents: read + +jobs: + lint: + name: Linters + runs-on: ubuntu-latest + timeout-minutes: 15 + steps: + - name: Checkout module + uses: actions/checkout@93cb6efe18208431cddfb8368fd83d5badbf9bfd # v5 + with: + persist-credentials: false + + - name: Install linters + # One installer for CI and for a fresh clone; a second apt list here + # would be the thing that goes stale. + run: ci/linter/install-linters.sh + + - name: Verify the toolchain is complete + # A linter missing from PATH makes its check exit 2, which run-all.sh + # reports as a failure rather than skipping -- this step just surfaces + # WHICH one before the findings scroll past. + run: ci/linter/install-linters.sh --check + + - name: Run linters + env: + LINT_ONLY: nginx sh python perl yaml + run: ci/linter/run-all.sh diff --git a/.github/workflows/security-scanners.yml b/.github/workflows/security-scanners.yml index 7e89514..8b394a3 100644 --- a/.github/workflows/security-scanners.yml +++ b/.github/workflows/security-scanners.yml @@ -9,10 +9,19 @@ name: Security scanners # to ignore it. # clang-tidy -- blocks on cert-* and clang-analyzer-security.*. These are # path-sensitive and near-zero-FP on code this size. -# semgrep -- ADVISORY (|| true). It is the highest-FP of the three; its -# value is the uploaded report a human reads, not a red X. If a -# semgrep rule earns its keep, promote it to blocking here -# deliberately -- do not leave it advisory forever by default. +# semgrep -- blocks at WARNING and above; INFO stays advisory. Promoted +# from fully-advisory 2026-07-30. +# WARNING, not ERROR, is the gate level on purpose: the +# registry's ERROR tier has no C rules that fire on this kind +# of code (verified -- an ERROR-only gate matched 48 rules and +# 0 findings against a deliberate strcpy/format-string file, +# i.e. it could never go red). WARNING is the lowest tier with +# real signal: it catches that same file via +# insecure-use-string-copy-fn. src/ is clean at WARNING+ today, +# so this goes in green. +# Registry C coverage is thin either way (~9 rules reach these +# files) -- semgrep is the third line here, behind flawfinder +# and clang-tidy, which carry the real C weight. # # Action pins (keep in sync with build-test.yml): # actions/checkout@v5 -> 93cb6efe18208431cddfb8368fd83d5badbf9bfd @@ -20,14 +29,12 @@ name: Security scanners env: FORCE_JAVASCRIPT_ACTIONS_TO_NODE24: true - NGINX_VERSION: "1.31.2" on: - push: - branches: [main] - pull_request: - branches: [main] - workflow_dispatch: {} + # Triggered by ci.yml, which lanes the PR suite so peak runner use stays + # bounded. No push/pull_request trigger here on purpose: two entry points + # would run this twice per PR and defeat the laning. + workflow_call: {} concurrency: group: security-scanners-${{ github.workflow }}-${{ github.ref }} @@ -47,6 +54,9 @@ jobs: with: persist-credentials: false + - name: Load version pins + run: bash .github/scripts/load-versions.sh + - name: Install dependencies run: | sudo apt-get update @@ -112,11 +122,37 @@ jobs: done exit "$status" - - name: semgrep (advisory) + - name: semgrep (gate on >=WARNING) run: | + set -euo pipefail command -v semgrep >/dev/null || { echo "semgrep missing from PATH" >&2; exit 1; } + # --jobs=1 is a correctness flag on a self-hosted runner, not a speed + # flag. semgrep-core defaults to one OCaml domain per core (32 on this + # box), each opening an io_uring ring against an 8 MB RLIMIT_MEMLOCK + # shared with every other job on the host. Under runner load it + # exhausts and semgrep-core dies with `Unix_error: Cannot allocate + # memory io_uring_queue_init` -> exit 2, i.e. a load-dependent red + # that has nothing to do with the code under review. Reproduced 3/3 on + # a busy box and 0/3 on an idle one. src/ is three files, so the + # parallelism was buying nothing. ci/linter/lint-c.sh carries the same + # flag -- keep the two in sync or local stops predicting remote. + # --metrics=off drops the scan-summary POST (2.76s -> 1.27s measured). + # + # Pass 1: full scan, every severity, never blocking. This is what + # lands in the uploaded artifact for a human to read -- INFO findings + # stay visible even though they do not gate. semgrep scan --config p/c --config p/security-audit \ + --jobs=1 --metrics=off \ "$GITHUB_WORKSPACE/src/" 2>&1 | tee semgrep.log || true + # Pass 2: the gate. --error makes semgrep exit 1 on any match at + # WARNING or ERROR. Exit 2 (fatal: bad config, network, parse error) + # also fails the step rather than passing silently -- a scanner that + # could not run is not a clean scan. + semgrep scan --config p/c --config p/security-audit \ + --severity=WARNING --severity=ERROR --error \ + --jobs=1 --metrics=off \ + "$GITHUB_WORKSPACE/src/" 2>&1 | tee -a semgrep.log + echo "semgrep: clean at >=WARNING" - name: Upload reports if: always() diff --git a/.github/workflows/valgrind.yml b/.github/workflows/valgrind.yml index 24f5841..b3a04b4 100644 --- a/.github/workflows/valgrind.yml +++ b/.github/workflows/valgrind.yml @@ -19,14 +19,12 @@ name: Valgrind env: FORCE_JAVASCRIPT_ACTIONS_TO_NODE24: true - NGINX_VERSION: "1.31.2" on: - push: - branches: [main] - pull_request: - branches: [main] - workflow_dispatch: {} + # Triggered by ci.yml, which lanes the PR suite so peak runner use stays + # bounded. No push/pull_request trigger here on purpose: two entry points + # would run this twice per PR and defeat the laning. + workflow_call: {} concurrency: group: valgrind-${{ github.workflow }}-${{ github.ref }} @@ -46,6 +44,9 @@ jobs: with: persist-credentials: false + - name: Load version pins + run: bash .github/scripts/load-versions.sh + - name: Install dependencies run: | sudo apt-get update diff --git a/.perlcriticrc b/.perlcriticrc new file mode 100644 index 0000000..6cbebdc --- /dev/null +++ b/.perlcriticrc @@ -0,0 +1,23 @@ +# perlcritic config -- used by ci/linter/lint-perl.sh and by editor plugins. +# +# The Perl in this repo is a Test::Nginx::Socket suite (ci/t/*.t), not library +# code. Four default policies at severity >=4 describe a .pm and fire on every +# well-formed .t file; leaving them on lands the gate red on arrival, which +# only teaches everyone to --no-verify. Each exclusion below is about the file +# SHAPE, not about tolerating a real defect: +# +# RequireExplicitPackage a .t is a script, not a module; wrapping the suite +# in a package would hide Test::Nginx's exported DSL. +# RequireEndWithOne same reason -- a script has no module return value. +# RequireUseStrict Test::Nginx::Socket enables strict and warnings in +# RequireUseWarnings its import, so the policies see "code before +# strictures" for pragmas that ARE in effect. +# +# Everything else at severity >=4 stays live. Add a policy here only with the +# reason it cannot apply, not because it is inconvenient. +severity = 4 + +[-Modules::RequireExplicitPackage] +[-Modules::RequireEndWithOne] +[-TestingAndDebugging::RequireUseStrict] +[-TestingAndDebugging::RequireUseWarnings] diff --git a/.pre-commit-config.yaml b/.pre-commit-config.yaml new file mode 100644 index 0000000..8ce7ab9 --- /dev/null +++ b/.pre-commit-config.yaml @@ -0,0 +1,145 @@ +--- +# yamllint disable rule:line-length +# The exclude regex and the actionlint entry are single unsplittable +# strings; wrapping either changes what they match. +# Local pre-commit gates for nginx-skeleton-module. +# +# GENERATED by tools/gen-precommit-config.py -- re-run it rather than +# hand-editing paths, so every module's config stays derived from its real +# layout instead of drifting toward a copied-around guess. +# +# Purpose: catch the cheap half of what CI catches, before a push burns a +# remote round-trip. These hooks deliberately MIRROR the CI thresholds in +# .github/workflows/security-scanners.yml (flawfinder >=4, semgrep >=WARNING). +# If a threshold moves there, move it here in the same commit, or local-green +# stops predicting remote-green. +# +# Detected in this repo: +# C source dirs : src +# fuzz corpora : ci/fuzz/corpus, ci/fuzz/regressions +# +# What is NOT here, and why: +# clang-tidy -- needs ngx_auto_config.h, which only exists after nginx is +# configured. A hook cannot assume a built tree, and one that +# silently skips when the tree is missing is a vacuous gate. +# Stays CI-only on purpose. +# clang-format -- no .clang-format in these repos, so there is no agreed style +# to enforce; adding one would reformat existing code in an +# unrelated diff. +# codeql/asan/valgrind/fuzzing -- far too slow for a commit hook; CI-only. +# +# Fuzz harnesses (fuzz/, t/) are outside the lint scope: they deliberately do +# unsafe things with untrusted input and would trip flawfinder forever. +# +# Install (once per clone): +# pipx install pre-commit && pre-commit install +# Run against everything without committing: +# pre-commit run --all-files +# +# VERIFY BEFORE TRUSTING (a green hook proves nothing until seen red): +# printf '#!/bin/bash\ncd /tmp/x\nrm -rf "${B}/"*\n' > _p.sh +# pre-commit run shellcheck --files _p.sh # expect Failed +# rm _p.sh +# Note SC2086 is INFO and correctly does NOT trip shellcheck here -- probe with +# a real warning-severity finding (SC2164/SC2115) or you will misread the gate. + +# Fuzz corpus files are byte-exact test inputs: a stripped space or an added +# newline changes what is actually fuzzed. Never let a fixer touch them. +exclude: '^ci/fuzz/corpus/|^ci/fuzz/regressions/|\.traefik-ref/|\.claude/worktrees/|\.build/|(^|/)objs/|(^|/)vendor/|(^|/)node_modules/|(^|/)\.venv/|\.(bin|gz|zip|png|ico|der|pem|so|o|a)$' + +repos: + - repo: https://github.com/pre-commit/pre-commit-hooks + rev: v6.0.0 + hooks: + - id: trailing-whitespace + - id: end-of-file-fixer + - id: mixed-line-ending + args: [--fix=lf] + - id: check-merge-conflict + - id: check-yaml + - id: check-added-large-files + args: [--maxkb=512] + # Test vectors are not secrets. autocert's tests/unit/test_crypto.c and + # test_alpn.c carry locked PEM fixtures (an elided MIGH... blob and a + # documented P-256 vector); gating on those is a pure false positive, + # while the hook still guards every real path. + - id: detect-private-key + exclude: '^tests?/' + + - repo: local + hooks: + # Mirrors the CI "flawfinder (gate on >=4)" step: fires on exactly what + # CI blocks on. Noise below level 4 would train everyone to --no-verify. + - id: flawfinder + name: flawfinder (gate on >=4) + entry: flawfinder --minlevel=4 --error-level=4 --quiet + language: system + files: '^src/.*\.[ch]$' + pass_filenames: true + + # Mirrors the CI "semgrep (gate on >=WARNING)" step, same configs and + # same severity floor. WARNING not ERROR: the registry's ERROR tier has + # no C rules that fire on this kind of code, so an ERROR gate could never + # go red (verified 2026-07-30). + # + # require_serial IS LOAD-BEARING, do not remove it to "speed the hook up": + # pre-commit otherwise shards the file list across one process per core + # (32 here), and every semgrep-core allocates its own io_uring. That blows + # past RLIMIT_MEMLOCK (ulimit -l, 8192 KB on this box) and semgrep-core + # dies with + # Unix_error: Cannot allocate memory io_uring_queue_init + # surfacing as a bare "exit code 2" with no message. That failure is + # indistinguishable from a real finding at a glance, so it would have + # this gate permanently red for a reason that has nothing to do with the + # code. One process, one ring, no contention. + # + # --quiet is also deliberately ABSENT: it suppressed exactly the error + # text above and turned a diagnosable crash into a silent exit 2. + - id: semgrep + name: semgrep (gate on >=WARNING) + entry: semgrep scan --config p/c --config p/security-audit + --severity=WARNING --severity=ERROR --error + language: system + files: '^src/.*\.[ch]$' + pass_filenames: true + require_serial: true + + # CI workflows are code that breaks silently; actionlint catches bad if: + # expressions, unknown contexts and shell errors inside run: blocks. + # + # -ignore runner-label: actionlint 1.7.7 ships a FIXED list of GitHub + # runner images and flags anything newer as "unknown label". Several + # repos legitimately target ubuntu-26.04 / ubuntu-26.04-arm, which that + # version has never heard of. Every such report is a false positive about + # the linter's age, not the workflow, and leaving it on lands this hook + # red on arrival. Drop this ignore once actionlint is new enough. + # + # SHELLCHECK_OPTS applies to the shellcheck actionlint runs over `run:` + # blocks. Without it that embedded pass reports at INFO while the + # standalone shellcheck hook below gates at warning -- so the same SC2086 + # would be ignored in a .sh file and blocking in a workflow. Same floor + # in both places. + - id: actionlint + name: actionlint (workflow syntax) + entry: env SHELLCHECK_OPTS=-Swarning actionlint -ignore 'label ".+" is unknown' + language: system + files: ^\.github/workflows/.*\.ya?ml$ + pass_filenames: true + + # -S warning: block on warning/error, not info. The info tier (e.g. + # SC2015) fires on existing scripts and would land this red on arrival, + # which just teaches everyone to --no-verify. + # + # NOTE some repos have pre-existing warning-level findings in build/test + # scripts (cache-turbo ci/tests/unit/extract_shm.sh SC2043, + # http-sentinel fuzz/build.sh SC2010, coraza-nginx build.sh SC2089/2090). + # They do NOT block normal work, because a commit hook only ever sees the + # files being committed -- they surface on `pre-commit run --all-files`. + # They are real and worth fixing; they are deliberately NOT suppressed + # here, so touching one of those scripts makes you fix it. + - id: shellcheck + name: shellcheck (gate on >=warning) + entry: shellcheck -S warning + language: system + files: \.(sh|bash)$ + pass_filenames: true diff --git a/.yamllint b/.yamllint new file mode 100644 index 0000000..ee6e4c2 --- /dev/null +++ b/.yamllint @@ -0,0 +1,25 @@ +# yamllint config for nginx-skeleton-module -- used by ci/linter/lint-yaml.sh +# and by any editor integration, so both agree. +# +# GitHub Actions workflows are not hand-written prose YAML; three default rules +# fight their syntax rather than catching bugs: +# document-start a workflow never starts with ---, and adding one everywhere +# is churn with no reader benefit. +# truthy `on:` is the trigger key, not the boolean `on`. check-keys +# off keeps the rule live for VALUES (yes/no/on/off), which is +# where a real YAML 1.1 surprise bites. +# line-length inline `run:` shell and ${{ }} expressions legitimately run +# long. 120 as a WARNING: visible, not blocking. lint-yaml.sh +# deliberately does not pass --strict, so warnings do not fail +# the gate -- errors (real syntax/structure problems) do. +extends: default + +rules: + document-start: disable + truthy: + check-keys: false + comments: + min-spaces-from-content: 1 + line-length: + max: 120 + level: warning diff --git a/README.md b/README.md index 58e04c2..d8a5c47 100644 --- a/README.md +++ b/README.md @@ -56,7 +56,12 @@ ci/ everything that only exists to test/build the module soak.sh sustained matching/benign storm under valgrind/ASan valgrind.supp nginx-core-only suppressions rename-module.sh skel -> your name (delete after use) -.github/workflows/ eight workflows, see below + linter/ local lint gate, mirrors the CI thresholds + PROMPT-standardize-module.md prompt: bring an existing module to this standard + run-all.sh every checker; what the pre-commit hook runs + install-linters.sh apt-get -> pipx -> cpan -> upstream binary +.githooks/pre-commit tracked commit gate (opt in: core.hooksPath) +.github/workflows/ nine workflows, see below ``` `ci/t/` uses the same `Test::Nginx::Socket` framework as upstream nginx's own @@ -75,13 +80,34 @@ That split is why `ci/fuzz/fuzz_scan.c` can link and drive the *real* code path rather than a reimplementation of it. A fuzzer that tests a copy of the parser tests nothing; it just drifts from production quietly and reports green. +## Linting + +`ci/linter/` is the local half of CI: nginx source conventions, C, shell, +Python, Perl and workflow YAML, at the same thresholds the remote scanners +gate on. It runs from a tracked pre-commit hook so a finding costs two seconds +locally instead of a red PR. + +```sh +ci/linter/install-linters.sh # apt-get, then pipx/cpan for what apt lacks +git config core.hooksPath .githooks +ci/linter/run-all.sh # or --staged, what the hook runs +``` + +Full setup (per-tool apt/pip/cpan commands, hook install, how the gate was +verified red, how to extend it): **[ci/linter/README.md](ci/linter/README.md)**. + +`.githooks/pre-commit` replaces the `pre-commit`-framework hook if you enable +it — `core.hooksPath` makes git ignore `.git/hooks/` entirely. Pick one; the +linter README explains what each covers. + ## CI -Eight workflows. A failure surfaces as a red run plus the uploaded artifact — no +Nine workflows. A failure surfaces as a red run plus the uploaded artifact — no chat notifications wired. | Workflow | Trigger | Gates | |---|---|---| +| `lint.yml` | PR (via `ci.yml`) | the `ci/linter/` gate: nginx conventions, shellcheck, ruff, perlcritic + `perl -c`, yamllint, actionlint, **zizmor** (workflow security) — hosted runner, no self-hosted slot | | `build-test.yml` | PR + push | shellcheck/cppcheck/actionlint, build, **.so dlopens**, **bad config is rejected**, **`-T` survives merged multi-context config**, `-Werror` strict compile, Test::Nginx, ASan+UBSan, **rename smoke** | | `asan.yml` | PR + push (path-gated: `src/` + build/soak harness) | 60s ASan/UBSan request-storm soak (static `--add-module`) | | `fuzzing.yml` | PR + push | replay every past crash, then 120s fresh fuzz | @@ -226,6 +252,8 @@ Build: `build-essential curl libpcre2-dev zlib1g-dev` Tests: `cpanminus` + `cpanm Test::Nginx` Fuzz: `clang` (needs `-fsanitize=fuzzer`) Soak: `valgrind` +Lint: `ci/linter/install-linters.sh` (shellcheck, cppcheck, flawfinder, +yamllint, clang-tidy, perlcritic, ruff, semgrep, actionlint, Test::Nginx) ## Contributing diff --git a/ci/PROMPT-standardize-module.md b/ci/PROMPT-standardize-module.md new file mode 100644 index 0000000..82da74f --- /dev/null +++ b/ci/PROMPT-standardize-module.md @@ -0,0 +1,478 @@ +# Prompt — bring an existing nginx module up to myguard skeleton standard + +Copy the whole file into a fresh session, replace `` with the module +path, and run it. It is written to be executed by an agent with repo write +access, but it reads as a checklist for a human too. + +The reference implementation is `/opt/myguard/labs/nginx-skeleton-module` +(this repo). **Read the reference before changing the target** — the point is +not to make the target look similar, it is to give it the same *gates*. + +--- + +## Context you are given + +- **Target module:** `` (e.g. `/opt/myguard/labs/nginx-http-shield-module`) +- **Reference:** `/opt/myguard/labs/nginx-skeleton-module` +- **Memory mirror:** `/opt/myguard/memory/labs//` — read + `index.md`, `issues.md`, `lessons.md` FIRST. A trap already recorded there + outranks anything you infer from the code. +- Owner is the `myguard-labs` org; verify with + `git -C remote get-url origin`, never from a list. + +## Ground rules + +1. **One PR per phase**, in the phase order below. Each PR is independently + revertible and independently green. Do not open the next phase's PR until + the previous is merged — later phases move files the earlier ones edit. +2. **Remote CI green before merge.** No `[skip ci]`, no disabled workflows. +3. **Never weaken a gate to make it pass.** If the target genuinely cannot meet + a threshold, say so in the PR body with the file:line that proves it, and + leave the gate at the honest value with a comment naming the reason. +4. **Every gate must be seen red once.** A check you never observed failing is + a check you have not verified. Record the probe you used, in a comment or + the PR body. This applies to coverage, to fuzz harnesses and to linters. +5. **Existing behaviour is not in scope.** You are moving CI, not rewriting the + module. If you find a real bug, file it in the memory mirror's `issues.md` + and keep going; fix it only if it blocks a gate. +6. Comments explain **why**, at the decision, in the target's existing voice. + A rule with no recorded reason gets deleted by the next person. + +--- + +## Phase 0 — inventory and baseline (no changes) + +Produce a short written baseline before touching anything: + +```bash +cd +ls -d t tests fuzz ci src scripts .githooks 2>/dev/null +ls .github/workflows +git log --oneline -10 +gh run list -R myguard-labs/ --limit 20 \ + --json name,conclusion,startedAt,updatedAt,workflowName +``` + +Record, in the memory mirror's `index.md`: + +- current layout (which of `t/`, `tests/`, `fuzz/`, `ci/` exist) +- current workflow list and which of the reference's nine are missing +- **measured wall-clock per workflow** from `gh run list` — you need real + numbers for the lane work in Phase 7. Estimates are not acceptable there. +- current coverage number, if any tooling exists (usually none) + +Layouts seen across the org, so you know what you are walking into: +`t/` + `fuzz/` at the root (most modules), `tests/` + `fuzz/` (autocert), +already-migrated `ci/` (cache-turbo, label-autoconf). + +--- + +## Phase 1 — move all CI material under `ci/` + +Target layout, matching the reference exactly: + +```text +ci/ + t/ Test::Nginx suite (was t/ or tests/) + fuzz/ libFuzzer targets, dict, corpus/, regressions/ + vendor/nginx-tests/ upstream suite submodule + tools/ ci-build.sh, soak.sh, valgrind.supp, helpers + linter/ local lint gate (Phase 6) +``` + +Rules: + +- `git mv`, never copy-then-delete — blame must survive. +- A directory move breaks **every relative path that climbs out of it**. After + the move, grep for and fix, in this order: `../` in C `#include`s, `$PWD`/ + `dirname` logic in shell, `paths:` filters in workflows, `hashFiles()` keys, + `prove` invocations, fuzz corpus paths, `.gitmodules` submodule paths, + `.gitignore`, coverage exclude patterns, README references. + A missed climb compiles fine and silently tests the wrong tree. +- `git submodule update --init` still working after moving + `ci/vendor/nginx-tests` is a required check — the `.gitmodules` `path:` must + be edited, not just the directory moved. +- Run the suite locally after the move and before the workflow edits, so a + failure is attributable to the move and not to both at once: + `TEST_NGINX_TIMEOUT=20 prove -v ci/t/` +- Leave a dated pointer line in the memory mirror if any note references an old + path. + +**Acceptance:** local `prove` green, fuzz targets still build +(`ci/fuzz/build.sh`), no path outside `ci/` refers to `t/`, `tests/` or `fuzz/`. + +--- + +## Phase 2 — workflows and badges + +Bring the target to the reference's **nine** workflows. Do not copy blindly: +each one carries reference-specific paths and pins that must be re-derived for +the target. + +| Workflow | What it must gate in the target | +|---|---| +| `ci.yml` | orchestrator; the ONLY `pull_request` entry point | +| `lint.yml` | the `ci/linter/` gate (Phase 6), hosted runner | +| `build-test.yml` | build, `.so` dlopens, bad config rejected, `-T` survives merged multi-context config, `-Werror`, Test::Nginx, ASan+UBSan | +| `asan.yml` | ASan/UBSan request-storm soak, static `--add-module` | +| `fuzzing.yml` | replay every past crash, then fresh fuzz | +| `valgrind.yml` | memcheck soak | +| `security-scanners.yml` | flawfinder ≥4 blocks, clang-tidy blocks, semgrep ≥WARNING | +| `codeql.yml` | CodeQL over the **module TU only** | +| `ci-deep.yml` | monthly: long fuzz, memcheck, helgrind, nginx mainline+stable+angie matrix | +| `bump.yml` | weekly pin bump + `ci/vendor/nginx-tests` submodule update | + +Also port, adapting paths: + +- `.github/versions.env` — single source of truth for version **and sha256** + pins. Tarballs verified by digest, not just version string. +- `.github/scripts/{load-versions,compute-versions,fetch-verify}.sh` +- `.github/actions/build-cache/` composite action +- `.github/actionlint.yaml` — declares the self-hosted runner labels, otherwise + actionlint flags every `runs-on` and the lint step becomes noise people skip. + +Members are `workflow_call:`-only; only `ci.yml` has `pull_request:`. Two entry +points run everything twice per PR and defeat the laning. + +**Badges:** README badge block must list the workflows in the SAME ORDER as the +`## CI` table in that README, and both must match reality — a badge for a +workflow that no longer exists renders a permanent grey "no status" and is +worse than no badge. Order used by the reference README: +Build&Test, Security Scanners, Fuzzing, Valgrind, CodeQL, A/UBSan, CI Deep — +insert Lint where the CI table puts it, and keep the two lists in lockstep. + +**Acceptance:** `actionlint` clean; every badge resolves to a real workflow +file; the CI table and the badge row are in identical order. + +--- + +## Phase 3 — coverage to the honest maximum + +No coverage tooling exists in the reference yet. Add it to the target as part +of this work, and (recommendation) upstream it back to the skeleton afterwards. + +Wiring: + +- Build the scan core with `--coverage` (`-fprofile-arcs -ftest-coverage`) in a + dedicated `coverage` mode in `ci/tools/ci-build.sh` — a new mode, not a flag + bolted onto `debug`, so the cache keys stay separable. +- Report with `gcovr` over `src/` only. Exclude `ci/`, the fuzz harnesses and + vendored code — a harness inflating the number is the classic self-deception. +- Count coverage from BOTH drivers: Test::Nginx (`ci/t/`) and the fuzz targets + replaying `ci/fuzz/corpus/` + `regressions/`. The scan core is reachable from + both; measuring one hides the other's gaps. +- Gate at a floor slightly under the achieved number (the reference's sibling + repos use "achieved minus a couple of points"), so noise does not flap the + gate but a real regression trips it. Raise the floor in the same PR that + raises coverage; never lower it to make a red run green. + +**"Without cheating" is the hard part. These are rejected outright:** + +- a test whose assertion holds in both the pass and the fail state + (tell: a captured variable that is never compared) +- a control that hardcodes the verdict instead of calling the real function +- asserting a *precondition* rather than the claim +- one shared counter asserted at N call sites — it pins none of them +- a test written from the same misunderstanding as the code +- excluding a hard file from the coverage config to lift the percentage +- tests that only execute lines without asserting on the result + +**Required per new test:** a negative control. Break the code the test claims +to guard (flip a comparison, delete the bound check, swap a constant), confirm +the test FAILS, restore. A test that passes against the mutated code guards +nothing. Note the mutation you used in the test's comment. + +Push toward the maximum by targeting, in order: error paths, allocation +failure, malformed/truncated input, boundary values at every `MAX_*` constant, +cross-buffer seams, and the branches your gcovr report shows as never taken. +100% is not a goal; every *reachable* branch having a meaningful assertion is. + +**Acceptance:** coverage job in CI, floor enforced, report uploaded as an +artifact, and each added test observed failing against a stated mutation. + +--- + +## Phase 4 — ASan and fuzzing, retargeted to this module + +Fuzzing is per-module work; a copied harness that drives the skeleton's rule +table proves nothing about the target. + +- The fuzz target must call the **real** decision function with + `(const uint8_t *, size_t)`, not a reimplementation. If the target module has + no such seam — decision logic entangled with `ngx_http_request_t` — extract + it first (the reference's "one structural rule"). That refactor is in scope + here; it is what makes everything else measurable. +- Seed corpus from the module's actual domain: real headers/bodies/config + values it parses, plus every past crash under `ci/fuzz/regressions/`. +- `fuzz.dict` with the module's real tokens — separators, markers, keywords. + A dictionary of the skeleton's tokens actively misdirects the fuzzer. +- Replay-then-fuzz order in `fuzzing.yml`: every recorded regression first + (fast, deterministic), then the time-boxed fresh run. A crash that returns + must fail in seconds, not after the fresh budget. +- ASan soak (`asan.yml`) must drive the module's real request shape — its + directives enabled, its body path exercised — not a default config where the + handler never runs. Verify by checking the soak actually reaches the module + (a counter, a log line, or coverage from the soak build). +- Keep the ASan build static (`--add-module`); a dynamic module under ASan + loses interception on the parts that matter. +- Adapt the neighbours as needed: `valgrind.supp` needs target-specific + nginx-core suppressions; `codeql.yml`'s TU filter needs the target's file + names; `ci-deep.yml`'s matrix needs the target's nginx/angie compatibility + range. + +**Acceptance:** fuzz target links against production code, replays all +regressions, and a deliberately reintroduced past bug is caught by the replay +step (verify once, then revert). + +--- + +## Phase 5 — caching, all layers + +Every build goes through `ci/tools/ci-build.sh` as the single chokepoint; no +workflow duplicates cache logic. `.github/actions/build-cache` restores caches +for one mode. Layers, cheapest first: + +| Layer | Saves | Keyed on | +|---|---|---| +| **apt / packages** | package install per job | package set hash; on self-hosted, prefer a pre-baked runner image over re-installing | +| **ccache** | recompilation | content (`CCACHE_COMPILERCHECK=content`) | +| **mold** | link time | used when present; **skipped under ASan** | +| **eatmydata** | `configure` + apt fsync stalls | wrap the configure/install steps; never wrap something whose durability matters | +| **build tree** (`.build/nginx--`) | `./configure` | mode + version + `hashFiles(ci-build.sh, config, src/**)` | +| **source tarball** | the download | version (+ sha256 verified after restore) | + +Rules that are load-bearing: + +- nginx's `configure` **ignores a bare `CC=`** — ccache must be wired through + the configure argument the reference uses, not via env. +- ccache may use a `restore-keys` fallback ladder (content-hashed, a partial + hit cannot serve a wrong object). The **build-tree cache must stay + exact-match only** — do not "fix" that for consistency. +- Hybrid restore (on-disk warm dirs + `actions/cache` fallback) stays. Deleting + the fallback because the runners are persistent is how this silently degrades + the day they become ephemeral. +- GitHub scopes caches **by ref**: a PR run writes `refs/pull/N/merge` and + cannot read a branch's entries. A cold PR run is not a bug. +- A cache must never be able to serve a stale artifact into a green result. If + a key cannot express what invalidates it, do not cache that layer. +- State the honest win in the README. If caching saves 5s on a 2.5-minute gate, + say so — the reason to keep it is the heavier module that comes later. + +--- + +## Phase 6 — pre-commit linters + +Port `ci/linter/` from the reference and follow its README verbatim: +**[ci/linter/README.md](linter/README.md)** — per-tool `apt-get` (preferred), +then `pipx` for what Debian lacks, then `cpan` for Perl modules, then upstream +binary for actionlint. `ci/linter/install-linters.sh` is the single installer; +CI and a fresh clone use the same one. + +- Tracked hook at `.githooks/pre-commit`, enabled with + `git config core.hooksPath .githooks`. It lints STAGED files only. +- Checkers: C (flawfinder ≥4, cppcheck, semgrep ≥WARNING), nginx conventions + (libc vs `ngx_*`, tabs, 80 cols, include order), shell (shellcheck + `-S warning`), Python (ruff), Perl (`perl -c` + perlcritic ≥4), YAML + (yamllint + actionlint). +- Thresholds **mirror `security-scanners.yml`**. Move one there, move it here + in the same commit, or local-green stops predicting remote-green. +- A missing tool exits 2 and BLOCKS. Never a silent skip. +- Relaxations live in `.yamllint` / `.perlcriticrc` at the repo root, each with + its reason. If the target has pre-existing warning-level findings, fix them + or record why — do not add a blanket suppression. +- `lint.yml` runs the same `run-all.sh` on a hosted runner, so a clone that + never enabled the hook still cannot land a regression. + +### Speed budget: the whole hook under ~2s on a one-file commit + +A commit gate people wait on is a commit gate people bypass with `--no-verify`. +Measure it (`time ci/linter/run-all.sh `), and if it is over budget +the fix is scoping the slow checker — **never** dropping one, and never a +default-on skip flag. + +Carry these three from the reference; each was measured, not assumed: + +- **`semgrep --metrics=off`.** The end-of-scan telemetry POST to semgrep.dev + was 2.76s of a 2.76s scan; without it, 1.27s. More than half the gate was + upload. +- **`semgrep --jobs=1`** — a *correctness* flag, not a speed one. semgrep-core + defaults to one OCaml domain per core and each domain opens its own io_uring + ring against the host's `RLIMIT_MEMLOCK` (8 MB on builder02, shared with + every other job). When the runners are busy it exhausts and semgrep-core + aborts with `Unix_error: Cannot allocate memory io_uring_queue_init`, exit 2 + — a red gate caused by a *neighbouring* job, on a scan of three files where + the parallelism bought nothing. Reproduced 3/3 busy, 0/3 idle, so an idle-box + green tells you nothing here. The same flags go in `security-scanners.yml`: + same host, same crash, and the two must stay in sync anyway. +- **`run-all.sh` fans the checkers out** (`LINT_JOBS`, default one slot per + checker, `LINT_JOBS=1` to bisect a hang). Two requirements that are easy to + get wrong: + - Buffer each checker's output to its own file and replay it whole, in fixed + glob order — **never stream them interleaved.** Findings carry a + `file:line` but not a checker name, so interleaved output cannot be + attributed, which is the entire reason to buffer. Fixed order also makes + two runs of the same dirty tree byte-comparable. + - Each child writes its exit status to a file. The reaping `wait` is + collective, so per-child statuses are not otherwise recoverable, and a + **missing** status file (child SIGKILLed) must count as a failure, never as + a pass. + +Reference measurements, whole checkout: 3.8s → 1.45s, one C file 2.9s → 1.31s. +Yours will differ; record yours — **and check `/proc/loadavg` before you take +them.** The build host also runs the self-hosted CI slots; at load ~50 the same +full-tree run varied 2.2s–12.4s over six back-to-back attempts, a spread wider +than the whole improvement. A busy-box A/B measures the neighbouring jobs. This +is the same shared-resource contention as the semgrep memlock crash above, and +it is why every number in this phase has to say what the box was doing. + +**Acceptance:** run every probe in the linter README's "Verify before trusting" +section against the target and observe each one red — *after* the speed work, +not before. `--jobs`/`--metrics` are exactly the kind of flag that can silently +turn a checker into a no-op, so the semgrep probe in particular must still +fire (`insecure-use-string-copy-fn` on the malloc/strcpy file). Then run +`run-all.sh` with two different checkers failing at once and confirm both +appear in the output and both are named in the `== FAIL:` line. + +--- + +## Phase 7 — runner topology: lanes, at most four + +The reference lanes the suite so peak self-hosted use stays bounded — CI +wall-clock on builder02 is dominated by jobs QUEUEING for a label-matching +slot, not by the jobs themselves. Ten simultaneous requests just means the tail +waits. + +**Measure first, and measure the target, not the reference.** Take real per-job +durations from a recent green run: + +```sh +gh run list -R myguard-labs/ --workflow=ci.yml --limit 5 \ + --json databaseId,headSha,conclusion +gh run view -R myguard-labs/ --json jobs \ + -q '.jobs[] | [.name, .conclusion, + (((.completedAt|fromdate)-(.startedAt|fromdate))|tostring)+"s", + .startedAt, .completedAt] | @tsv' +``` + +Keep `startedAt`/`completedAt`, not just the durations — the gaps are what show +you queueing, and which lane is actually the critical path. + +Then: + +1. Identify the longest single **job**. That is the budget: no arrangement can + finish sooner. Chain **nothing** behind it — every second added there is a + second on the suite's critical path, whereas the same job appended to a + shorter lane is free. Pairing the longest job with a follow-up "to keep the + lane busy" is the single most common way this gets worse; it is what put the + reference's lane A at 348s against a 268s budget. +2. Build the **fewest lanes that fit**, four maximum, each a chain of `needs:` + where a long job releases its slot to a shorter, independent follow-up. No + lane's total may exceed the budget. Three lanes that all fit beat four that + also fit — fewer chains, fewer `!cancelled()` edges to reason about. Note + the headroom of the fullest lane in the comment, so the next person knows + how much a job can grow before the shape stops holding. +3. If it does not fit in four, the honest fixes are: move a check out-of-band + (monthly), time-box it, or put it on a hosted runner — not "add a fifth". +4. **A lane is not a slot.** Count the target's real slots + (`systemctl list-units | grep ci-ephemeral` on the runner host — six on + builder02), and remember a reusable workflow can fan out into many + concurrent jobs: the reference's Build&Test is *five*, so the observed peak + is 7 against 6 slots. Brief oversubscription at t=0 is acceptable; writing + "caps peak runner use at three" when it is seven is not. +5. Hosted jobs (lint, CodeQL) take no self-hosted slot, so they are **not laned + at all** — no `needs:`, start immediately, fastest feedback. Chaining a + hosted job behind a self-hosted one to "conserve a slot" conserves nothing + and just delays its result. +6. Follow-ups use `if: ${{ !cancelled() }}` so a failing first check does not + suppress an unrelated second one. A red ASan should still tell you whether + Valgrind is clean. It is also what keeps a chain alive when an earlier job + is *skipped* by a changed-files gate, which a bare `needs:` would not. +7. Concurrency groups must not collide. A called workflow inherits the caller's + `github.workflow`/`github.ref`, so an identical group string makes a member + cancel its own caller and a whole lane dies before it starts. Prefix the + orchestrator's group distinctly. +8. Path-gating a reusable workflow does not work — a called workflow cannot + filter its own triggering. Gates move to a `changes` job in the orchestrator + with an explicit job-level `if`. That diff job must **fail loudly** on an + unusable diff, never fall through to "no relevant changes" — failing open + skips the sanitizer on exactly the PRs that need it. + +The orchestrator's header comment is the only place this design is written +down, so it is part of the deliverable, not documentation of it. It must carry +the lane map, the measured durations, the run ID and date they came from, and +the command above to re-derive them. **Any lane change rewrites that comment in +the same commit** — a stale lane map reads as measurement and gets trusted. +Also record the lane map and timings in the memory mirror. + +--- + +## Phase 8 — self-hosted runner exposure + +Applies whenever `runs-on` includes `self-hosted`. A self-hosted runner +executing untrusted code is arbitrary code execution on the build host. + +Required: + +- **Fork routing.** Every self-hosted job uses the reference's expression: + `runs-on: ${{ github.event.pull_request.head.repo.fork && 'ubuntu-latest' || fromJSON('["self-hosted","builder02","lxc"]') }}` + A fork PR never reaches the build host. +- **No `pull_request_target`**, ever, in a repo with self-hosted runners. It + runs with a writable token in the base-repo context; combined with a fork's + code it is a full compromise. If something seems to need it, it does not. +- **Least-privilege tokens.** `permissions: contents: read` at workflow level; + widen per-job only where genuinely needed (`security-events: write` for + CodeQL). Never `write-all`. +- `persist-credentials: false` on every checkout, so a later step cannot reuse + the token. +- **Pin every third-party action to a full commit SHA**, with the version in a + trailing comment. A tag is mutable. +- Pin every downloaded tool version (semgrep, actionlint, nginx tarballs) and + verify tarballs by sha256. This is code executing on a persistent host. +- Never expose secrets to a job that can run untrusted code. Prefer no secrets + at all in the PR lane; `bump.yml`-style writers run only from the default + branch on a schedule. +- Repo settings (check with `gh api`, fix or report): require approval for + first-time-contributor workflow runs, restrict which actions may run, + branch protection with required checks, and no self-hosted runner registered + at org level where a public repo can grab it. +- Runner containers are LXC/incus and persistent: assume a job can see the + previous job's leftovers. Nothing sensitive may be left in `$HOME` or the + work dir, and cleanup must not depend on a job succeeding. + +- **`zizmor --persona=pedantic --offline`** over `.github/workflows/`, already + wired into the reference's `ci/linter/lint-yaml.sh` and therefore into both + the hook and `lint.yml`. It mechanises most of this section: template + injection, dangerous triggers, `artipacked`, `unpinned-uses`, + `excessive-permissions`, plus a `self-hosted-runner` audit. Port it with the + rest of the linter dir; expect the target to be red on first run and fix + each finding rather than ignoring it. `# zizmor: ignore[rule]` at the line, + with a reason, is the only acceptable suppression. +- `${{ }}` interpolation of any attacker-controlled field (PR title, branch + name, body) directly into a `run:` block is template injection. Pass through + `env:` and quote. The same shape is required for `matrix.*` even though it is + repo-controlled — the safe form costs nothing and stops the unsafe one being + copied somewhere it matters. + +--- + +## Finishing + +- README rewritten, not appended to: badge row, `## CI` table, layout tree, + Requirements, and a Linting section linking `ci/linter/README.md`. +- `CONTRIBUTING.md` tells a contributor how to enable the hook. +- `CHANGES` entry describing the standardisation. +- Memory mirror updated: `index.md` (layout, lane map, measured times), + `issues.md` (anything found and not fixed), `lessons.md` (every trap that + cost you a red CI round-trip — `[RECURRING]` if it has bitten before). +- Any trap that is a *class* rather than a typo goes into the matching + `.claude/skills/audit-*/` reference, not only into memory. The skill is what + runs unprompted next time. +- Improvements you made that the skeleton lacks (coverage tooling, eatmydata) + get a PR back to `nginx-skeleton-module`. The template is only + worth keeping if it stays ahead of its clones. + +## Report back + +State plainly, per phase: what landed, what is red, what you left undone and +why. Include the measured before/after wall-clock and the coverage +before/after. Do not report a phase complete on a gate you never saw fail. diff --git a/ci/linter/README.md b/ci/linter/README.md new file mode 100644 index 0000000..9cca52e --- /dev/null +++ b/ci/linter/README.md @@ -0,0 +1,280 @@ +# ci/linter — local lint gate + +Mirrors the cheap half of remote CI so a push does not burn a round-trip on a +finding shellcheck could have named in two seconds. Every script is standalone; +`run-all.sh` runs them all and `.githooks/pre-commit` runs `run-all.sh --staged`. + +## Layout + +| Script | Covers | Gate | +|---|---|---| +| `lint-c.sh` | `src/*.[ch]` | flawfinder ≥4, cppcheck (warning/performance/portability), semgrep ≥WARNING (`p/c`, `p/security-audit`) | +| `lint-nginx.sh` | `src/*.[ch]` | nginx conventions: libc alloc/str/num/io instead of `ngx_*`, hard tabs, >80 columns, trailing whitespace, `ngx_config.h` include order | +| `lint-sh.sh` | `*.sh`, `*.bash`, `.githooks/*` | shellcheck `-S warning` | +| `lint-python.sh` | `*.py` | `ruff check` + `ruff format --check` | +| `lint-perl.sh` | `ci/t/*.t`, `*.pl`, `*.pm` | `perl -c` + perlcritic severity ≥4 | +| `lint-yaml.sh` | `*.yml`, `*.yaml` | yamllint (errors block, warnings visible), actionlint + zizmor (`--persona=pedantic`) on `.github/workflows/` | +| `run-all.sh` | all of the above | runs every check, reports once | +| `install-linters.sh` | — | apt-get → pipx → cpan → upstream binary | +| `lib.sh` | — | sourced helpers (file selection, missing-tool failure) | + +Rule config lives at the repo root so editors and these scripts agree: +`.yamllint` (workflow-shaped YAML), `.perlcriticrc` (Test::Nginx-shaped Perl). +Both carry the reason for every relaxation; read them before adding another. + +Thresholds deliberately match `.github/workflows/security-scanners.yml`. Move +one there and move it here **in the same commit**, or local-green stops +predicting remote-green — the only reason this directory exists. + +`clang-tidy` is **CI-only**: it needs `ngx_auto_config.h`, which exists only in +a configured nginx tree. A hook cannot assume one, and a check that skips +itself when the tree is missing is a vacuous gate. + +## 1. Install the linters + +```sh +ci/linter/install-linters.sh # install what is missing +ci/linter/install-linters.sh --check # report only +``` + +Preference order, and why each tool lands where it does: + +**apt-get (preferred — distro-managed, no PEP 668 fight)** + +```sh +sudo apt-get update +sudo apt-get install -y --no-install-recommends \ + shellcheck cppcheck flawfinder yamllint clang-tidy \ + libperl-critic-perl perl pipx cpanminus +``` + +**pip / pipx (Python tools Debian does not carry at the needed version)** + +Use `pipx`, not `pip3`: Debian 12+ marks the system interpreter +externally-managed, so a bare `pip3 install` fails and +`--break-system-packages` is a worse answer than a venv per tool. + +```sh +pipx install ruff +pipx install 'semgrep==1.169.0' # pinned to the CI version on purpose +``` + +`semgrep` is pinned because an unpinned upgrade changes findings under you and +local stops matching CI. Bump it here and in `security-scanners.yml` together. + +**cpan (Perl modules apt does not carry on every target release)** + +```sh +sudo cpanm --notest Test::Nginx::Socket # also what makes `perl -c` work on ci/t/*.t +sudo cpanm --notest Perl::Critic # only if libperl-critic-perl was unavailable +``` + +`--notest`: Test::Nginx's own suite wants a live nginx and a free port, which +an install step has no business demanding. + +```sh +pipx install zizmor # GitHub Actions security audit +``` + +`zizmor` is deliberately **not** pinned, unlike semgrep: its rule set is the +whole point, and a frozen security scanner stops finding what it was added for. +A new rule going red is a finding to triage, not drift to suppress. + +**upstream binary (no apt/pip/cpan source)** + +```sh +ver=1.7.7 +curl -fsSL "https://github.com/rhysd/actionlint/releases/download/v${ver}/actionlint_${ver}_linux_amd64.tar.gz" \ + | tar -xz actionlint && sudo install -m0755 actionlint /usr/local/bin/ +``` + +Make sure `~/.local/bin` is on `PATH` for the pipx-installed tools. + +## 2. Enable the pre-commit hook + +The hook is tracked at `.githooks/pre-commit` so a change to the gate arrives +as a reviewable diff. Git does not use it until you point `core.hooksPath` at +the directory — once per clone: + +```sh +git config core.hooksPath .githooks +``` + +Verify it is live: + +```sh +git config --get core.hooksPath # -> .githooks +``` + +Bypass in an emergency with `git commit --no-verify`. + +**This replaces the `pre-commit` framework hook.** `core.hooksPath` makes git +ignore `.git/hooks/` entirely, including the hook `pre-commit install` writes +there. The repo's `.pre-commit-config.yaml` still exists and covers overlapping +ground (whitespace fixers, private-key detection, flawfinder, semgrep, +shellcheck, actionlint). Pick one: + +- `git config core.hooksPath .githooks` — this directory: also covers Perl, + Python and the nginx conventions, no Python framework needed. +- `pipx install pre-commit && pre-commit install` — the framework: also runs + the whitespace/EOF fixers and `detect-private-key`, but not `lint-nginx.sh` + or `lint-perl.sh`. Then leave `core.hooksPath` unset. + +Running both means running flawfinder and semgrep twice per commit. + +## 3. Use it + +```sh +ci/linter/run-all.sh # every tracked file +ci/linter/run-all.sh --staged # what the hook runs +ci/linter/run-all.sh src/foo.c # named files +LINT_ONLY="c nginx" ci/linter/run-all.sh +LINT_SKIP_SEMGREP=1 ci/linter/run-all.sh # loud opt-out of the slowest pass +LINT_JOBS=1 ci/linter/run-all.sh # serial, for bisecting a hang +ci/linter/run-all.sh --list +``` + +Exit codes: `0` clean, `1` findings, `2` a linter is missing. + +### Speed, and why it is shaped this way + +Measured 2026-07-31 on builder02 (i9-14900HX, 32 threads) with **no CI job +running** — see the caveat below before comparing against your own numbers: + +| | before | after | +|---|---|---| +| full tree | 3.8s | **1.45s** | +| one C file | 2.9s | **1.31s** | +| full tree, `LINT_JOBS=1` | 3.2s | 2.57s | + +Re-measure on an idle box or not at all. This host also runs six self-hosted CI +runner slots, and at load average ~50 the same full-tree run took 2.2s to 12.4s +across six back-to-back attempts — the run-to-run spread is wider than the +entire improvement, so a busy-box A/B measures the neighbours, not the change. +Check `/proc/loadavg` first. + +Two changes, only one of which is really about speed: + +- **`semgrep --metrics=off`.** The end-of-scan POST to semgrep.dev was 2.76s of + a 2.76s scan; without it the same scan is 1.27s. More than half the hook's + wall clock was telemetry. +- **`semgrep --jobs=1`.** A *correctness* fix. semgrep-core defaults to one + OCaml domain per core and each domain opens its own io_uring ring against + this host's 8 MB `RLIMIT_MEMLOCK`, which is shared with the self-hosted CI + runners. When the runners are busy it exhausts and semgrep-core aborts with + `Unix_error: Cannot allocate memory io_uring_queue_init`, exit 2 — a red + commit gate caused by neighbouring load, not by the diff. Reproduced 3/3 on a + busy box, 0/3 on an idle one. `src/` is three files, so nothing was gained by + the parallelism in the first place. `security-scanners.yml` carries the same + two flags; they run on the same host and must stay in sync. +- **`run-all.sh` fans the checkers out** (`LINT_JOBS`, default one slot per + checker). Each checker's output is buffered and replayed whole in glob order, + never streamed: findings carry a `file:line` but not a checker name, so + interleaved output is unattributable. Fixed order also keeps two runs of the + same dirty tree byte-comparable. + +The floor is now semgrep's own startup. If this creeps back over ~2s, scope +semgrep — do not drop a checker. + +Suppress one justified `lint-nginx.sh` finding with a trailing +`/* NOLINT-nginx */` on that line. Whole-rule suppression is deliberately not +supported: the exception belongs next to the code that needs it, where review +can see the reason. + +## Verify before trusting + +A green gate proves nothing until it has been seen red. Every probe below was +run against this tree and observed failing; re-run them after changing a +threshold. + +```sh +# shell: SC2164 + SC2115 -> exit 1 +printf '#!/bin/bash\ncd /tmp/x\nrm -rf "${B}/"*\n' > _p.sh +LINT_ONLY=sh ci/linter/run-all.sh _p.sh ; rm _p.sh + +# C + nginx conventions: malloc/strcpy -> exit 1 from both lint-c and lint-nginx +printf '#include \nvoid f(void){char*p=malloc(4);strcpy(p,"ab");}\n' > src/_probe.c +LINT_ONLY="c nginx" ci/linter/run-all.sh src/_probe.c ; rm src/_probe.c + +# python: unused import -> exit 1 +printf 'import os\nx=1\n' > _p.py +LINT_ONLY=python ci/linter/run-all.sh _p.py ; rm _p.py + +# perl: string eval + interpolated system() -> exit 1 +printf 'my $x = 1;\nsystem("ls $x");\neval "1";\n' > _p.pl +LINT_ONLY=perl ci/linter/run-all.sh _p.pl ; rm _p.pl + +# yaml: unterminated flow sequence -> exit 1 +printf 'a: [1,\n' > _p.yml +LINT_ONLY=yaml ci/linter/run-all.sh _p.yml ; rm _p.yml + +# workflow security: zizmor -> exit 1 on template-injection, artipacked, +# unpinned-uses and excessive-permissions, all from these seven lines +cat > .github/workflows/_probe.yml <<'EOF' +name: probe +on: [pull_request] +jobs: + p: + runs-on: ubuntu-latest + steps: + - uses: actions/checkout@v4 + - run: echo "${{ github.event.pull_request.title }}" +EOF +LINT_ONLY=yaml ci/linter/run-all.sh .github/workflows/_probe.yml +rm .github/workflows/_probe.yml + +# missing tool -> exit 2, never a silent skip +printf 'x = 1\n' > _p.py +PATH="$(echo "$PATH" | tr : '\n' | grep -v "$HOME/.local/bin" | paste -sd:)" \ + LINT_ONLY=python ci/linter/run-all.sh _p.py ; rm _p.py + +# the hook itself blocks the commit +printf '#!/bin/bash\ncd /tmp/x\n' > _bad.sh && git add _bad.sh +git commit -m probe # -> blocked +git reset -q HEAD _bad.sh && rm _bad.sh +``` + +Note SC2086 is INFO and correctly does **not** trip `lint-sh.sh`; probe with a +warning-severity finding or you will misread the gate. + +### Workflow security (zizmor) + +`actionlint` reads a workflow as syntax; `zizmor` reads it as an attack +surface — template injection into `run:`, `pull_request_target`, credentials +persisted by `actions/checkout`, actions pinned to a mutable tag, over-broad +`permissions:`. On a repo with **self-hosted runners** that class of mistake is +arbitrary code execution on the build host, which is why it is a gate and not +advice. + +Run at `--persona=pedantic`: the default persona already passes on this tree, +so gating on it could never go red. Pedantic is what caught the `matrix.*` +interpolations in `ci-deep.yml` (now passed through `env:`) and the +undocumented CodeQL permissions. + +`--offline`, so a commit hook never needs a token. The online audits only add +repo-settings context; that belongs in a periodic review. + +Inapplicable finding → `# zizmor: ignore[rule]` on the line, with the reason. +Never a blanket disable in a `zizmor.yml`. + +## In CI + +`.github/workflows/lint.yml` runs `install-linters.sh` then +`LINT_ONLY="nginx sh python perl yaml" run-all.sh` — the same entry point as +the hook, so a clone that never enabled `core.hooksPath` still cannot land a +regression. It is wired into the `ci.yml` orchestrator and runs on +`ubuntu-latest`, taking no self-hosted slot. + +The `c` checker is left out there because `security-scanners.yml` already runs +flawfinder/clang-tidy/semgrep over `src/` at the same thresholds. That is also +why `lint-c.sh` must be edited in the same commit as that workflow. + +## Extending + +- New file type: drop a `ci/linter/lint-.sh` in place — `run-all.sh` + picks it up by glob. Keep "no files of this kind" exiting 0, and fail with + exit 2 (via `need`) when the tool is absent. +- New nginx convention: one more `rule ` call in + `lint-nginx.sh`. +- New dependency: add it to `install-linters.sh` **and** to the apt/pip/cpan + lists above, so a fresh clone is one command from armed. diff --git a/ci/linter/install-linters.sh b/ci/linter/install-linters.sh new file mode 100755 index 0000000..db519f0 --- /dev/null +++ b/ci/linter/install-linters.sh @@ -0,0 +1,148 @@ +#!/usr/bin/env bash +# ci/linter/install-linters.sh -- install every linter ci/linter/ needs. +# +# Order of preference, per tool: apt-get (distro-managed, no PEP 668 fight) +# -> pipx/pip (Python tools apt does not carry) -> cpan (Perl modules apt does +# not carry) -> upstream binary (actionlint). Nothing here is a silent skip: +# a tool that fails to install prints why and the script exits non-zero, so a +# half-installed toolchain cannot masquerade as a working gate. +# +# Usage: +# ci/linter/install-linters.sh install what is missing +# ci/linter/install-linters.sh --check report only, install nothing +# ci/linter/install-linters.sh --force reinstall even if present +# +# Needs sudo for the apt-get and cpan steps; the pipx step installs into +# ~/.local/bin (make sure it is on PATH). +# +# Side effects: apt-get install, pipx install, cpanm install, and one curl of +# the actionlint release tarball into /usr/local/bin. +# +# Extend: add a line to the APT/PIPX/CPAN lists. Anything needing a bespoke +# install gets its own function at the bottom, next to install_actionlint. + +set -uo pipefail + +CHECK=0 FORCE=0 +case "${1:-}" in + --check) CHECK=1 ;; + --force) FORCE=1 ;; + -h|--help) sed -n '2,25p' "$0"; exit 0 ;; + "") ;; + *) echo "unknown argument: $1" >&2; exit 2 ;; +esac + +SUDO="" +[ "$(id -u)" -eq 0 ] || SUDO="sudo" + +# toolapt package -- checked with command -v +APT_TOOLS=( + "shellcheck:shellcheck" # sh/bash + "cppcheck:cppcheck" # C + "flawfinder:flawfinder" # C, risky-API scan + "yamllint:yamllint" # YAML + "clang-tidy:clang-tidy" # C, CI-only (needs a configured nginx tree) + "perlcritic:libperl-critic-perl" # Perl test suite + "perl:perl" +) +# Not in Debian/Ubuntu at the versions this repo targets -> Python packaging. +# pipx, not pip: PEP 668 marks the system interpreter externally-managed, so a +# bare `pip3 install` fails on Debian 12+ and `--break-system-packages` is a +# worse answer than an isolated venv per tool. +PIPX_TOOLS=( + "ruff:ruff" # Python lint + format check + "zizmor:zizmor" # GitHub Actions security audit. Not + # pinned: its rule set is the point, and a + # frozen security scanner stops finding + # what it was added for. A new rule going + # red is a finding to triage, not drift. + "semgrep:semgrep==1.169.0" # C, pinned to the CI version on purpose: + # an unpinned semgrep changes findings + # under you and local stops matching CI. +) +# apt has no libtest-nginx-perl on every target release; cpan always does. +CPAN_MODULES=( + "Test::Nginx::Socket" # the ci/t/*.t suite; also makes `perl -c` work + "Perl::Critic" # fallback if libperl-critic-perl was missing +) + +have() { command -v "$1" >/dev/null 2>&1; } +step() { printf '\n== %s\n' "$*"; } +rc=0 + +report() { + printf '%-14s %s\n' "$1" "$(command -v "$1" 2>/dev/null || echo MISSING)" +} + +if [ "$CHECK" -eq 1 ]; then + step "linter status" + for e in "${APT_TOOLS[@]}" "${PIPX_TOOLS[@]}"; do report "${e%%:*}"; done + report actionlint + report zizmor + perl -MTest::Nginx::Socket -e1 2>/dev/null \ + && printf '%-14s ok\n' 'Test::Nginx' \ + || { printf '%-14s MISSING\n' 'Test::Nginx'; rc=1; } + exit "$rc" +fi + +step "apt-get" +NEED=() +for e in "${APT_TOOLS[@]}"; do + tool="${e%%:*}"; pkg="${e##*:}" + if [ "$FORCE" -eq 1 ] || ! have "$tool"; then NEED+=("$pkg"); fi +done +if [ "${#NEED[@]}" -gt 0 ]; then + echo "installing: ${NEED[*]}" + $SUDO apt-get update -qq || rc=1 + $SUDO apt-get install -y --no-install-recommends "${NEED[@]}" || rc=1 +else + echo "nothing to do" +fi + +step "pipx" +have pipx || $SUDO apt-get install -y --no-install-recommends pipx || rc=1 +for e in "${PIPX_TOOLS[@]}"; do + tool="${e%%:*}"; spec="${e#*:}" + if [ "$FORCE" -eq 1 ] || ! have "$tool"; then + echo "installing: $spec" + pipx install "$spec" || pipx install --force "$spec" || rc=1 + fi +done + +step "cpan" +have cpanm || $SUDO apt-get install -y --no-install-recommends cpanminus || rc=1 +for m in "${CPAN_MODULES[@]}"; do + if [ "$FORCE" -eq 1 ] || ! perl -M"$m" -e1 2>/dev/null; then + echo "installing: $m" + # --notest: Test::Nginx's own suite wants a live nginx binary and a + # free port, which is not something an install step should demand. + $SUDO cpanm --notest "$m" || rc=1 + fi +done + +install_actionlint() { + # No apt package on the target releases and no pip/cpan equivalent: a Go + # binary from the upstream release, pinned, checksum-visible in the log. + local ver="1.7.7" tmp + tmp="$(mktemp -d)" + curl -fsSL -o "$tmp/al.tgz" \ + "https://github.com/rhysd/actionlint/releases/download/v${ver}/actionlint_${ver}_linux_amd64.tar.gz" || return 1 + sha256sum "$tmp/al.tgz" + tar -xzf "$tmp/al.tgz" -C "$tmp" actionlint || return 1 + $SUDO install -m0755 "$tmp/actionlint" /usr/local/bin/actionlint || return 1 + rm -rf "$tmp" +} + +step "actionlint" +if [ "$FORCE" -eq 1 ] || ! have actionlint; then + install_actionlint || { echo "actionlint install failed" >&2; rc=1; } +else + echo "present: $(command -v actionlint)" +fi + +step "result" +if [ "$rc" -ne 0 ]; then + echo "one or more installs FAILED -- ci/linter/ is not fully armed" >&2 + exit 1 +fi +echo "all linters installed; verify with: ci/linter/install-linters.sh --check" diff --git a/ci/linter/lib.sh b/ci/linter/lib.sh new file mode 100755 index 0000000..6f03988 --- /dev/null +++ b/ci/linter/lib.sh @@ -0,0 +1,50 @@ +#!/usr/bin/env bash +# ci/linter/lib.sh -- shared helpers for the ci/linter/lint-*.sh scripts. +# +# Sourced, never executed. Provides: +# repo_root absolute path of the checkout +# lint_files emit the file list to lint, one per line, NUL-safe +# callers use `mapfile -t`. Honours LINT_MODE: +# staged (default in the git hook) -- staged files only +# all -- every tracked file +# and an explicit file list passed in "$@" by run-all.sh. +# need hard-fail with an install hint when a linter is absent +# say / warn / die consistent output +# +# Missing tools are a HARD FAILURE, never a silent skip: a gate that quietly +# disappears when its tool is uninstalled reports green while checking nothing. +# Run ci/linter/install-linters.sh to get them all. + +set -euo pipefail + +repo_root() { git rev-parse --show-toplevel; } + +say() { printf ' %s\n' "$*"; } +warn() { printf ' WARN: %s\n' "$*" >&2; } +die() { printf 'ERROR: %s\n' "$*" >&2; exit 2; } + +need() { + command -v "$1" >/dev/null 2>&1 && return 0 + die "$1 not found. Install it: $2 (or run ci/linter/install-linters.sh)" +} + +# Paths never linted: byte-exact fuzz inputs, vendored trees, build output. +LINT_EXCLUDE_RE='^ci/fuzz/corpus/|^ci/fuzz/regressions/|^ci/vendor/|(^|/)objs/|(^|/)\.build/|(^|/)node_modules/|(^|/)\.venv/' + +# lint_files [explicit files...] +lint_files() { + local match_re="$1"; shift + local list + if [ "$#" -gt 0 ]; then + list=$(printf '%s\n' "$@") + elif [ "${LINT_MODE:-all}" = "staged" ]; then + list=$(git diff --cached --name-only --diff-filter=ACMR) + else + list=$(git ls-files) + fi + printf '%s\n' "$list" \ + | grep -Ev "$LINT_EXCLUDE_RE" \ + | grep -E "$match_re" \ + | while read -r f; do [ -f "$f" ] && printf '%s\n' "$f"; done \ + || true +} diff --git a/ci/linter/lint-c.sh b/ci/linter/lint-c.sh new file mode 100755 index 0000000..103ee8c --- /dev/null +++ b/ci/linter/lint-c.sh @@ -0,0 +1,77 @@ +#!/usr/bin/env bash +# ci/linter/lint-c.sh -- C static analysis for src/*.[ch]. +# +# Mirrors .github/workflows/security-scanners.yml exactly: +# flawfinder gate at >=4 (below 4 is risky-API grep noise; gating on it +# trains everyone to --no-verify) +# semgrep gate at >=WARNING with p/c + p/security-audit +# plus cppcheck, which CI does not run and which is cheap enough locally. +# +# If a threshold moves in that workflow, move it here in the SAME commit -- +# otherwise local-green stops predicting remote-green, which is the only +# reason this script exists. +# +# NOT here, deliberately: +# clang-tidy -- needs ngx_auto_config.h, i.e. a configured nginx tree. A +# local hook cannot assume one, and a check that skips itself +# when the tree is missing is a vacuous gate. CI-only. +# +# Usage: ci/linter/lint-c.sh [files...] (no args => LINT_MODE, default all) +# Env: LINT_MODE=staged|all +# LINT_SKIP_SEMGREP=1 explicit, loud opt-out for slow machines +# +# Extend: add a scanner as one more block below; keep the CI mirror comment +# accurate or the header above becomes a lie. + +# shellcheck source=ci/linter/lib.sh +. "$(git rev-parse --show-toplevel)/ci/linter/lib.sh" + +mapfile -t FILES < <(lint_files '^src/.*\.[ch]$' "$@") +[ "${#FILES[@]}" -gt 0 ] || { echo "lint-c: no C files to check"; exit 0; } + +echo "lint-c: ${#FILES[@]} file(s)" +rc=0 + +need flawfinder "apt-get install flawfinder" +say "flawfinder (gate >=4)" +flawfinder --minlevel=4 --error-level=4 --quiet "${FILES[@]}" || rc=1 + +need cppcheck "apt-get install cppcheck" +say "cppcheck (error,warning)" +# --error-exitcode=1 makes any reported id fail. missingInclude/unusedFunction +# are suppressed: this is a module compiled INTO nginx, so its headers and its +# ngx_http_* callbacks are never resolvable or called from this tree alone. +cppcheck --quiet --error-exitcode=1 \ + --enable=warning,performance,portability \ + --inline-suppr \ + --suppress=missingInclude --suppress=missingIncludeSystem \ + --suppress=unusedFunction --suppress=unknownMacro \ + --suppress=normalCheckLevelMaxBranches \ + "${FILES[@]}" || rc=1 + +if [ -n "${LINT_SKIP_SEMGREP:-}" ]; then + warn "semgrep SKIPPED via LINT_SKIP_SEMGREP -- CI still gates on it" +else + need semgrep "pipx install semgrep==1.169.0" + say "semgrep (gate >=WARNING)" + # --quiet is deliberately absent: it hides semgrep-core's own crash text + # (io_uring/RLIMIT_MEMLOCK) and turns a diagnosable failure into a bare + # exit 2 that reads like a real finding. + # + # --jobs=1 is a correctness flag, not a speed flag. semgrep-core defaults to + # one OCaml domain per core (32 here), each of which opens its own io_uring + # ring against the 8 MB RLIMIT_MEMLOCK this host shares with the self-hosted + # CI runners. Under runner load it exhausts and semgrep-core dies with + # `Unix_error: Cannot allocate memory io_uring_queue_init` -> exit 2, which + # this script reports as a finding. Observed 3/3 crashed on a 3-file scan + # while runners were busy, 0/3 on an idle box: a load-dependent false RED. + # src/ is three files, so the parallelism was buying nothing to begin with. + # + # --metrics=off: no scan-summary POST to semgrep.dev. Measured 2.76s -> 1.27s + # on this tree, i.e. more than half the wall clock was that upload. + semgrep scan --config p/c --config p/security-audit \ + --severity=WARNING --severity=ERROR --error \ + --jobs=1 --metrics=off "${FILES[@]}" || rc=1 +fi + +exit "$rc" diff --git a/ci/linter/lint-nginx.sh b/ci/linter/lint-nginx.sh new file mode 100755 index 0000000..4e74078 --- /dev/null +++ b/ci/linter/lint-nginx.sh @@ -0,0 +1,78 @@ +#!/usr/bin/env bash +# ci/linter/lint-nginx.sh -- nginx-specific source conventions for src/*.[ch]. +# +# What no generic C linter knows: an nginx module must use the nginx allocator, +# the nginx string/number helpers and the nginx source style, because it is +# compiled into a worker with a pool-based lifetime and a shared coding +# standard. flawfinder/cppcheck/semgrep all pass code that leaks a malloc() +# into a request pool or calls atoi() on attacker input. +# +# Checks (each one line per finding, "file:line: rule: text"): +# libc-alloc malloc/calloc/realloc/free -> ngx_palloc/ngx_pcalloc/ngx_pfree +# libc-str strcpy/strcat/sprintf/strncpy -> ngx_cpymem/ngx_snprintf +# libc-num atoi/atol/strtol on request data -> ngx_atoi/ngx_atoof +# libc-io bare printf/fprintf(stderr) -> ngx_log_error +# tabs hard tab in source -> nginx style is 4 spaces +# width line >80 columns -> nginx style limit +# trailing trailing whitespace +# include .c must include ngx_config.h before ngx_core.h +# +# Suppress a single justified line with a trailing /* NOLINT-nginx */ . +# Suppressing a whole rule is not supported on purpose: the exception belongs +# next to the code that needs it, where review can see the reason. +# +# Usage: ci/linter/lint-nginx.sh [files...] Env: LINT_MODE=staged|all +# Extend: add a rule as one more `rule ` call. + +# shellcheck source=ci/linter/lib.sh +. "$(git rev-parse --show-toplevel)/ci/linter/lib.sh" + +mapfile -t FILES < <(lint_files '^src/.*\.[ch]$' "$@") +[ "${#FILES[@]}" -gt 0 ] || { echo "lint-nginx: no C files to check"; exit 0; } + +echo "lint-nginx: ${#FILES[@]} file(s)" +rc=0 + +# rule -- report every matching line not marked NOLINT. +rule() { + local name="$1" re="$2" msg="$3" hits + hits=$(grep -nE "$re" "${FILES[@]}" 2>/dev/null | grep -v 'NOLINT-nginx' || true) + [ -n "$hits" ] || return 0 + printf '%s\n' "$hits" | sed "s/^/ /; s/$/ [$name: $msg]/" + rc=1 +} + +rule libc-alloc '(^|[^_[:alnum:]])(malloc|calloc|realloc|free)[[:space:]]*\(' \ + 'use ngx_palloc/ngx_pcalloc/ngx_pfree' +rule libc-str '(^|[^_[:alnum:]])(strcpy|strcat|sprintf|strncpy|strncat)[[:space:]]*\(' \ + 'use ngx_cpymem/ngx_snprintf' +rule libc-num '(^|[^_[:alnum:]])(atoi|atol|strtol|strtoul)[[:space:]]*\(' \ + 'use ngx_atoi/ngx_atoof' +rule libc-io '(^|[^_[:alnum:]])(printf|fprintf|perror)[[:space:]]*\(' \ + 'use ngx_log_error/ngx_conf_log_error' +rule tabs $'\t' 'nginx style is 4 spaces, no hard tabs' +rule trailing '[[:space:]]+$' 'trailing whitespace' +rule width '^.{81,}$' 'nginx style limit is 80 columns' + +for f in "${FILES[@]}"; do + case "$f" in + *.c) + # ngx_config.h defines the feature macros every later nginx header + # reads; including ngx_core.h first silently changes the build. Look at + # the FIRST ngx_ include, not a fixed head -N window: these files open + # with a long licence/design comment that would push the includes out + # of any window and make the check vacuous. + # Angle brackets only: a local "ngx_http__*.h" is this module's + # own header and carries its own ngx_config.h include -- matching it + # here reported every well-formed file. + first_ngx=$(grep -nE '^[[:space:]]*#[[:space:]]*include[[:space:]]*/dev/null || rc=1 +done + +need perlcritic "apt-get install libperl-critic-perl (or: cpan Perl::Critic)" +say "perlcritic (severity >=4)" +perlcritic --severity 4 --quiet "${FILES[@]}" || rc=1 + +exit "$rc" diff --git a/ci/linter/lint-python.sh b/ci/linter/lint-python.sh new file mode 100755 index 0000000..7cd7d27 --- /dev/null +++ b/ci/linter/lint-python.sh @@ -0,0 +1,30 @@ +#!/usr/bin/env bash +# ci/linter/lint-python.sh -- ruff lint + format check over tracked *.py. +# +# ruff replaces flake8/pyflakes/isort/black here: one binary, no venv, and it +# is what the superrepo's tools/ scripts are already checked with. The module +# tree carries no Python today; this gate exists so the first helper script +# committed under ci/tools/ is checked from its first commit instead of after +# it has grown. +# +# `ruff format --check` reports formatting WITHOUT rewriting: a linter that +# edits files behind a commit hook changes what you are about to commit. +# +# Usage: ci/linter/lint-python.sh [files...] Env: LINT_MODE=staged|all +# Extend: per-rule config goes in a [tool.ruff] block in pyproject.toml at the +# repo root, not in flags here, so editors and CI see the same rules. + +# shellcheck source=ci/linter/lib.sh +. "$(git rev-parse --show-toplevel)/ci/linter/lib.sh" + +mapfile -t FILES < <(lint_files '\.py$' "$@") +[ "${#FILES[@]}" -gt 0 ] || { echo "lint-python: no Python files to check"; exit 0; } + +echo "lint-python: ${#FILES[@]} file(s)" +need ruff "apt-get install ruff (or: pipx install ruff)" +rc=0 +say "ruff check" +ruff check "${FILES[@]}" || rc=1 +say "ruff format --check" +ruff format --check "${FILES[@]}" || rc=1 +exit "$rc" diff --git a/ci/linter/lint-sh.sh b/ci/linter/lint-sh.sh new file mode 100755 index 0000000..79c6ea4 --- /dev/null +++ b/ci/linter/lint-sh.sh @@ -0,0 +1,24 @@ +#!/usr/bin/env bash +# ci/linter/lint-sh.sh -- shellcheck over every *.sh / *.bash in the tree. +# +# -S warning, not info: the info tier (SC2015, SC2086 in safe positions) fires +# on existing build scripts and would land this gate red on arrival, which only +# teaches everyone to --no-verify. Same floor as the shellcheck the actionlint +# hook runs inside `run:` blocks (SHELLCHECK_OPTS=-Swarning), so an SC2164 is +# not ignored in a .sh file while blocking in a workflow. +# +# Usage: ci/linter/lint-sh.sh [files...] Env: LINT_MODE=staged|all +# Extend: raise to -S info only together with a pass that fixes the backlog. + +# shellcheck source=ci/linter/lib.sh +. "$(git rev-parse --show-toplevel)/ci/linter/lib.sh" + +# .githooks/* has no extension but is bash; an unchecked commit hook is the +# one script whose bug silently disables every other check here. +mapfile -t FILES < <(lint_files '\.(sh|bash)$|^\.githooks/' "$@") +[ "${#FILES[@]}" -gt 0 ] || { echo "lint-sh: no shell files to check"; exit 0; } + +echo "lint-sh: ${#FILES[@]} file(s)" +need shellcheck "apt-get install shellcheck" +shellcheck -S warning -x "${FILES[@]}" +say "clean" diff --git a/ci/linter/lint-yaml.sh b/ci/linter/lint-yaml.sh new file mode 100755 index 0000000..6b624a7 --- /dev/null +++ b/ci/linter/lint-yaml.sh @@ -0,0 +1,67 @@ +#!/usr/bin/env bash +# ci/linter/lint-yaml.sh -- yamllint over every *.yml/*.yaml, plus actionlint +# over .github/workflows/*. +# +# Workflows are code that fails silently: a bad `if:` expression, an unknown +# context or a shell error inside a `run:` block does not stop the run, it +# makes the job pass while checking nothing. actionlint is the only checker +# here that reads them as GitHub Actions rather than as YAML. +# +# -ignore 'label ".+" is unknown' +# actionlint 1.7.7 carries a FIXED list of runner images and flags newer +# ones (ubuntu-26.04, ubuntu-26.04-arm) as unknown. Every such report is +# about the linter's age, not the workflow. Drop the ignore once +# actionlint is new enough to know the labels this repo targets. +# SHELLCHECK_OPTS=-Swarning +# actionlint's embedded shellcheck otherwise reports at info while +# lint-sh.sh gates at warning -- same finding, two verdicts. +# +# No --strict on yamllint, on purpose: warnings (long inline `run:` lines) stay +# visible without failing the gate, errors still block. The rule set lives in +# .yamllint at the repo root so editors and this script agree. +# +# actionlint and zizmor are not alternatives: actionlint reads a workflow as +# SYNTAX (bad if:, unknown context, shell error in run:), zizmor reads it as an +# ATTACK SURFACE (template injection, dangerous triggers, leaked credentials, +# unpinned actions, over-broad permissions). This repo targets self-hosted +# runners, where a workflow-level mistake is arbitrary code execution on the +# build host -- so the security pass is not optional here. +# +# --persona=pedantic, not the default: the default already passes on this tree, +# so gating on it would never go red. Pedantic is what caught the matrix +# interpolations in ci-deep.yml and the undocumented CodeQL permissions. Findings +# that are genuinely inapplicable get a `# zizmor: ignore[rule]` at the line, +# with the reason -- never a blanket rule disable in zizmor.yml. +# +# --offline: no API calls from a commit hook. The online audits need a token and +# only add repo-settings context, which belongs in a periodic review, not here. +# +# Usage: ci/linter/lint-yaml.sh [files...] Env: LINT_MODE=staged|all +# Extend: yamllint rules live in .yamllint at the repo root. + +# shellcheck source=ci/linter/lib.sh +. "$(git rev-parse --show-toplevel)/ci/linter/lib.sh" + +mapfile -t FILES < <(lint_files '\.ya?ml$' "$@") +[ "${#FILES[@]}" -gt 0 ] || { echo "lint-yaml: no YAML files to check"; exit 0; } + +echo "lint-yaml: ${#FILES[@]} file(s)" +rc=0 + +need yamllint "apt-get install yamllint" +say "yamllint" +yamllint "${FILES[@]}" || rc=1 + +mapfile -t WF < <(printf '%s\n' "${FILES[@]}" | grep -E '^\.github/workflows/' || true) +if [ "${#WF[@]}" -gt 0 ]; then + need actionlint "go install github.com/rhysd/actionlint/cmd/actionlint@latest (see install-linters.sh)" + say "actionlint (${#WF[@]} workflow(s))" + SHELLCHECK_OPTS=-Swarning \ + actionlint -ignore 'label ".+" is unknown' "${WF[@]}" || rc=1 + + need zizmor "pipx install zizmor" + say "zizmor (workflow security, pedantic)" + zizmor --offline --persona=pedantic --no-progress "${WF[@]}" || rc=1 +fi + +exit "$rc" diff --git a/ci/linter/run-all.sh b/ci/linter/run-all.sh new file mode 100755 index 0000000..5c5781f --- /dev/null +++ b/ci/linter/run-all.sh @@ -0,0 +1,111 @@ +#!/usr/bin/env bash +# ci/linter/run-all.sh -- run every ci/linter/lint-*.sh and report once. +# +# This is what .githooks/pre-commit calls and what a human should call before +# pushing. It does NOT stop at the first failure: seeing all four failures in +# one run beats four commit attempts. +# +# Usage: +# ci/linter/run-all.sh lint every tracked file +# ci/linter/run-all.sh --staged lint only staged files (hook mode) +# ci/linter/run-all.sh --list list the checks and exit +# ci/linter/run-all.sh ... lint exactly these files +# +# Env: +# LINT_MODE=staged|all same as --staged / default +# LINT_ONLY="c sh" run only these checkers (names below) +# LINT_SKIP_SEMGREP=1 loud opt-out of the slowest C pass +# LINT_JOBS=N parallel checkers (default 0 = one per checker, +# 1 = serial; use 1 when bisecting a hang) +# +# Exit: 0 all clean, 1 findings, 2 a linter is missing (install-linters.sh). +# +# The checkers are independent, so they run concurrently and each one's output +# is buffered to its own file and replayed whole, in the fixed glob order. Never +# stream them interleaved: findings carry a file:line but not a checker name, so +# interleaving makes them unattributable, which is the whole reason to buffer. +# Order stays glob order and not completion order so two runs of a dirty tree +# produce the same transcript. +# +# Extend: drop a new ci/linter/lint-.sh in place -- it is picked up by +# glob, no edit here. Keep the "no files of this kind" case exiting 0. + +set -uo pipefail + +ROOT="$(git rev-parse --show-toplevel)" +# || exit 2, not a bare cd: every checker resolves its paths relative to the +# repo root, so failing to get there would lint the wrong tree -- or nothing at +# all -- and report clean. Same "could not run" status as a missing linter. +cd "$ROOT" || exit 2 + +MODE="${LINT_MODE:-all}" +FILES=() +for a in "$@"; do + case "$a" in + --staged) MODE=staged ;; + --all) MODE=all ;; + --list) ls ci/linter/lint-*.sh; exit 0 ;; + -h|--help) sed -n '2,30p' "$0"; exit 0 ;; + *) FILES+=("$a") ;; + esac +done +export LINT_MODE="$MODE" + +echo "== lint (mode: $MODE) ==" + +# Selected checkers, in glob order. Built first so the replay loop below can +# walk the same list it launched. +SEL=() +for s in ci/linter/lint-*.sh; do + name="$(basename "$s" .sh)"; name="${name#lint-}" + if [ -n "${LINT_ONLY:-}" ] && [[ " $LINT_ONLY " != *" $name "* ]]; then + continue + fi + SEL+=("$name") +done + +JOBS="${LINT_JOBS:-0}" +[ "$JOBS" -eq 0 ] 2>/dev/null && JOBS="${#SEL[@]}" +[ "$JOBS" -ge 1 ] 2>/dev/null || JOBS=1 + +# mktemp -d, not a fixed path: two checkouts (or a hook racing a manual run) +# would otherwise share buffers. Trap covers the Ctrl-C path too. +BUF="$(mktemp -d "${TMPDIR:-/tmp}/lint-buf.XXXXXX")" +trap 'rm -rf "$BUF"' EXIT INT TERM + +for name in "${SEL[@]}"; do + # Bounded fan-out: block until a slot frees. `wait -n` needs bash 4.3+, + # which install-linters.sh already assumes. + while [ "$(jobs -rp | wc -l)" -ge "$JOBS" ]; do wait -n; done + { + "ci/linter/lint-$name.sh" "${FILES[@]+"${FILES[@]}"}" \ + > "$BUF/$name.out" 2>&1 + # Exit status travels in a file, not in $?: the reaping `wait` below is + # collective, so per-child statuses are not otherwise recoverable. + echo "$?" > "$BUF/$name.rc" + } & +done +wait + +rc=0 missing=0 failed=() +for name in "${SEL[@]}"; do + cat "$BUF/$name.out" + # A missing .rc means the subshell died before writing one (OOM-killer, + # SIGKILL). Treat it as a failure, never as a pass. + crc="$(cat "$BUF/$name.rc" 2>/dev/null || echo 1)" + case "$crc" in + 0) ;; + 2) missing=1; failed+=("$name(tool missing)") ;; + *) rc=1; failed+=("$name") ;; + esac +done + +if [ "$missing" -eq 1 ]; then + echo "== FAIL: ${failed[*]} -- run ci/linter/install-linters.sh ==" >&2 + exit 2 +fi +if [ "$rc" -ne 0 ]; then + echo "== FAIL: ${failed[*]} ==" >&2 + exit 1 +fi +echo "== all linters clean ==" diff --git a/ci/tools/bump-versions.sh b/ci/tools/bump-versions.sh index 612835b..d89915b 100755 --- a/ci/tools/bump-versions.sh +++ b/ci/tools/bump-versions.sh @@ -1,26 +1,32 @@ #!/usr/bin/env bash # -# Check nginx.org/angie.software for newer releases than what's pinned in this -# repo, and rewrite every pin in place. Called by .github/workflows/bump.yml on -# a schedule; also runnable locally to preview a bump before it lands. +# Refresh every upstream pin in this repo. Called by .github/workflows/bump.yml +# on a schedule; also runnable locally to preview a bump before it lands. # # ci/tools/bump-versions.sh [--dry-run] # -# What gets bumped, and why each one has to move together: -# - NGINX_VERSION (mainline pin) -- .github/workflows/{build-test,ci-deep,codeql,valgrind,asan,fuzzing,security-scanners}.yml -# (EVERY workflow with an NGINX_VERSION env -# -- miss one and its gate silently tests a -# stale nginx after a bump) -# - nginx stable + angie pins -- ci-deep.yml's build-flavors matrix -# - NGINX_SHA256 / ANGIE_SHA256 -- ci/tools/ci-build.sh (this script computes -# the digest itself from the same tarball -# ci-build.sh will later verify against) -# - ci/vendor/nginx-tests submodule -- `git submodule update --remote` +# Two things move here: +# - .github/versions.env -- nginx mainline/stable + angie versions +# AND their sha256s, rewritten wholesale +# by .github/scripts/compute-versions.sh +# - ci/vendor/nginx-tests submodule -- `git submodule update --remote` # -# A version bump with a stale sha256 pin is worse than no pin (ci-build.sh -# treats a missing pin as "print a warning", but a WRONG pin is a hard FATAL -- -# so every version edit here is paired with a digest computed from the exact -# tarball that version resolves to, never carried over from a previous entry. +# This script used to sed a version literal into each of seven workflow files, +# patch ci-deep.yml's matrix, and insert a matching digest into a table in +# ci-build.sh. Those pins now live on adjacent lines in one file with one +# writer, so the failure modes that design had -- a bumped version carrying a +# stale digest, or one workflow missed by the sed and silently gating on an old +# nginx -- are gone by construction, and so is the code that managed them. +# +# --dry-run reports what WOULD change without writing anything: versions.env is +# regenerated into a scratch copy and diffed, and the submodule is left alone. +# +# Exit status is 0 whether or not anything changed; the caller decides what to +# do with a dirty tree. Prints CHANGED=0|1 as its last line. +# +# GH_TOKEN is honoured (passed through to compute-versions.sh as GITHUB_TOKEN): +# the runners share an egress IP, so unauthenticated api.github.com calls are +# routinely rate-limited to 403s. set -euo pipefail @@ -29,165 +35,36 @@ DRY_RUN=0 cd "$(dirname "$0")/../.." -# --- discover latest versions ------------------------------------------- - -# nginx.org/en/download.html lists Mainline then Stable then Legacy, each as -# its own section header followed by a table whose first tarball link is that -# section's current release -- no JSON feed exists, so parse the one page -# nginx itself treats as authoritative. -latest_nginx() { - local branch="$1" # mainline | stable - local page section - page="$(curl -fsSL https://nginx.org/en/download.html)" - case "$branch" in - mainline) section="Mainline version" ;; - stable) section="Stable version" ;; - esac - # The page is one long line; cut everything before the section header, then - # take the first tarball link after it -- that link is that section's - # current release, per nginx.org's own page layout. - echo "${page#*"$section"}" \ - | grep -oE 'nginx-[0-9]+\.[0-9]+\.[0-9]+\.tar\.gz' | head -1 \ - | grep -oE '[0-9]+\.[0-9]+\.[0-9]+' -} - -latest_angie() { - local json - # The runners share an egress IP, so the unauthenticated API allowance - # (60/hr) is routinely exhausted and this call 403s. Send GH_TOKEN when - # one is present -- authenticated requests get their own, far larger quota. - local -a auth=() - [ -n "${GH_TOKEN:-}" ] && auth=(-H "Authorization: Bearer $GH_TOKEN") - if ! json="$(curl -fsSL "${auth[@]}" https://api.github.com/repos/webserver-llc/angie/releases/latest)"; then - echo "error: could not query the angie release API (rate limit? set GH_TOKEN)" >&2 - return 1 - fi - echo "$json" | grep -m1 '"tag_name"' | grep -oE '[0-9]+\.[0-9]+\.[0-9]+' -} - -NEW_MAINLINE="$(latest_nginx mainline)" -NEW_STABLE="$(latest_nginx stable)" -NEW_ANGIE="$(latest_angie)" - -for v in NEW_MAINLINE NEW_STABLE NEW_ANGIE; do - if [ -z "${!v}" ]; then - echo "FATAL: could not determine $v -- refusing to bump with a blank version" >&2 - exit 1 - fi -done - -echo "latest: nginx mainline=$NEW_MAINLINE stable=$NEW_STABLE angie=$NEW_ANGIE" - -# Each matrix entry is "version:" immediately followed by "label:" (see -# ci-deep.yml's build-flavors job) -- pair them up rather than assuming -# ordering, so a future reordering of the matrix can't silently swap pins. -matrix_version_for_label() { - awk -v want="$1" ' - /version:/ { match($0, /"[0-9.]+"/); v = substr($0, RSTART+1, RLENGTH-2); next } - /label:/ { split($0, a, ":"); l = a[2]; gsub(/[ \t]/, "", l); if (l == want) { print v; exit } } - ' .github/workflows/ci-deep.yml -} - -CUR_MAINLINE="$(grep -m1 'NGINX_VERSION:' .github/workflows/build-test.yml | grep -oE '[0-9]+\.[0-9]+\.[0-9]+')" -CUR_STABLE="$(matrix_version_for_label stable)" -CUR_ANGIE="$(matrix_version_for_label angie)" - -echo "pinned: nginx mainline=$CUR_MAINLINE stable=$CUR_STABLE angie=$CUR_ANGIE" +# compute-versions.sh reads GITHUB_TOKEN; bump.yml historically set GH_TOKEN. +export GITHUB_TOKEN="${GITHUB_TOKEN:-${GH_TOKEN:-}}" CHANGED=0 +VERSIONS_FILE=".github/versions.env" -# --- sha256 helper -------------------------------------------------------- -sha256_for() { - local flavor="$1" version="$2" url tmp digest - case "$flavor" in - nginx) url="https://nginx.org/download/nginx-${version}.tar.gz" ;; - angie) url="https://download.angie.software/files/angie-${version}.tar.gz" ;; - esac - tmp="$(mktemp)" - curl -fsSL "$url" -o "$tmp" - digest="$(sha256sum "$tmp" | awk '{print $1}')" - rm -f "$tmp" - echo "$digest" -} - -# --- bump a version everywhere it's pinned -------------------------------- -bump_nginx_workflow_pin() { - local old="$1" new="$2" - [ "$old" = "$new" ] && return 0 - # Every workflow that pins NGINX_VERSION -- keep this list == the set that - # `grep -l 'NGINX_VERSION:' .github/workflows/*.yml` returns, or a bumped - # mainline leaves the omitted gate testing a stale nginx (silent + green). - for f in .github/workflows/build-test.yml .github/workflows/ci-deep.yml \ - .github/workflows/codeql.yml .github/workflows/valgrind.yml \ - .github/workflows/asan.yml .github/workflows/fuzzing.yml \ - .github/workflows/security-scanners.yml; do - sed -i "s/NGINX_VERSION: \"${old}\"/NGINX_VERSION: \"${new}\"/" "$f" - done - CHANGED=1 -} - -bump_matrix_pin() { - local label="$1" old="$2" new="$3" - [ "$old" = "$new" ] && return 0 - # Matrix entries are unique per label (mainline/stable/angie) in ci-deep.yml. - python3 - "$label" "$old" "$new" <<'PYEOF' -import re, sys -label, old, new = sys.argv[1:4] -path = ".github/workflows/ci-deep.yml" -text = open(path).read() -pattern = re.compile( - r'(version:\s*"' + re.escape(old) + r'"\n\s*label:\s*' + re.escape(label) + r')' -) -replaced = pattern.sub(lambda m: m.group(1).replace(old, new), text) -if replaced == text: - print(f"WARNING: no matrix entry matched for label={label} old={old}", file=sys.stderr) -open(path, "w").write(replaced) -PYEOF - CHANGED=1 -} - -bump_sha256_pin() { - local table="$1" old="$2" new="$3" digest="$4" - grep -q "\[\"${new}\"\]" ci/tools/ci-build.sh && return 0 # already pinned - # Insert the new pin right after the table's opening line; leave old - # entries in place (ci-build.sh keys by version, older callers still work). - sed -i "/declare -A ${table}=(/a\\ [\"${new}\"]=\"${digest}\"" ci/tools/ci-build.sh - CHANGED=1 -} - -if [ "$NEW_MAINLINE" != "$CUR_MAINLINE" ]; then - echo "bump nginx mainline: $CUR_MAINLINE -> $NEW_MAINLINE" - if [ "$DRY_RUN" = 0 ]; then - DIGEST="$(sha256_for nginx "$NEW_MAINLINE")" - echo " sha256 $DIGEST" - bump_nginx_workflow_pin "$CUR_MAINLINE" "$NEW_MAINLINE" - bump_matrix_pin mainline "$CUR_MAINLINE" "$NEW_MAINLINE" - bump_sha256_pin NGINX_SHA256 "$CUR_MAINLINE" "$NEW_MAINLINE" "$DIGEST" - else +# --- version + sha256 pins ------------------------------------------------- +if [ "$DRY_RUN" = 0 ]; then + # Tolerate a missing file: compute-versions.sh creates it from scratch, so + # bootstrapping (or regenerating after a delete) should report "changed" + # rather than dying here under set -e. + before="$(cat "$VERSIONS_FILE" 2>/dev/null || true)" + bash .github/scripts/compute-versions.sh + if [ "$before" != "$(cat "$VERSIONS_FILE")" ]; then + echo "--- versions.env changed ---" + git --no-pager diff -- "$VERSIONS_FILE" || true CHANGED=1 - fi -fi - -if [ "$NEW_STABLE" != "$CUR_STABLE" ]; then - echo "bump nginx stable: $CUR_STABLE -> $NEW_STABLE" - if [ "$DRY_RUN" = 0 ]; then - DIGEST="$(sha256_for nginx "$NEW_STABLE")" - echo " sha256 $DIGEST" - bump_matrix_pin stable "$CUR_STABLE" "$NEW_STABLE" - bump_sha256_pin NGINX_SHA256 "$CUR_STABLE" "$NEW_STABLE" "$DIGEST" else - CHANGED=1 + echo "versions.env already up to date" fi -fi - -if [ "$NEW_ANGIE" != "$CUR_ANGIE" ]; then - echo "bump angie: $CUR_ANGIE -> $NEW_ANGIE" - if [ "$DRY_RUN" = 0 ]; then - DIGEST="$(sha256_for angie "$NEW_ANGIE")" - echo " sha256 $DIGEST" - bump_matrix_pin angie "$CUR_ANGIE" "$NEW_ANGIE" - bump_sha256_pin ANGIE_SHA256 "$CUR_ANGIE" "$NEW_ANGIE" "$DIGEST" +else + # Regenerate into a scratch copy so the working tree is untouched. + scratch="$(mktemp -d)" + trap 'rm -rf "$scratch"' EXIT + cp -a .github "$scratch/.github" + ( cd "$scratch" && bash .github/scripts/compute-versions.sh >/dev/null ) + if diff -u "$VERSIONS_FILE" "$scratch/$VERSIONS_FILE"; then + echo "(dry-run: versions.env already up to date)" else + echo "(dry-run: versions.env would change as shown above)" CHANGED=1 fi fi diff --git a/ci/tools/ci-build.sh b/ci/tools/ci-build.sh index c6aef30..c4a894c 100755 --- a/ci/tools/ci-build.sh +++ b/ci/tools/ci-build.sh @@ -56,9 +56,39 @@ set -euo pipefail FLAVOR="${1:-nginx}" -VERSION="${2:-1.31.2}" MODE="${3:-debug}" +# Version pins (and their sha256s) all come from .github/versions.env -- see +# the integrity block below. Sourced this early so the default version tracks +# the pinned one instead of being a literal that silently rots. +VERSIONS_FILE="${VERSIONS_FILE:-$PWD/.github/versions.env}" +if [ ! -f "$VERSIONS_FILE" ]; then + echo "FATAL: $VERSIONS_FILE not found (run from the module root)" >&2 + exit 1 +fi +# Validate before sourcing. load-versions.sh applies the same KEY=value check +# on the CI side, but this script `source`s the file directly -- so without a +# check here, a stray line that is not a pin would be executed as shell rather +# than rejected. Same rule enforced in both consumers. +while IFS= read -r line || [ -n "$line" ]; do + case "$line" in + ''|\#*) continue ;; + esac + if ! printf '%s' "$line" | grep -qE '^[A-Za-z_][A-Za-z0-9_]*=[A-Za-z0-9._-]*$'; then + echo "FATAL: malformed line in $VERSIONS_FILE: $line" >&2 + exit 1 + fi +done < "$VERSIONS_FILE" +# shellcheck source=/dev/null +. "$VERSIONS_FILE" + +case "$FLAVOR" in + nginx) DEFAULT_VERSION="$NGINX_VERSION" ;; + angie) DEFAULT_VERSION="$ANGIE_VERSION" ;; + *) DEFAULT_VERSION="" ;; +esac +VERSION="${2:-$DEFAULT_VERSION}" + case "$MODE" in debug|asan|module) ;; *) @@ -87,61 +117,70 @@ esac NO_CACHE="${NO_CACHE:-0}" -# --- integrity: pinned SHA-256 for nginx tarballs we've actually verified --- +# --- integrity: sha256 pins come from .github/versions.env ------------------- # nginx.org serves plain HTTP-adjacent PGP signatures, not a sha256sum file, so # "verify against the vendor" means pinning a known-good digest for each source -# tarball we build, computed once from a tarball fetched over HTTPS from -# nginx.org and recorded here -- not fabricated. A version not in this table -# builds anyway (this skeleton tracks a moving nginx release, and refusing to -# build an unpinned version would break every future version bump until someone -# updates this table first) but prints a loud warning instead of silently -# skipping verification, so the gap is visible rather than assumed-safe. -declare -A NGINX_SHA256=( - ["1.31.1"]="9fcaaeb8f22544b09a19a761f3412c4112215422401634bebdd1296a403cc4bc" - ["1.31.2"]="af2a957c41da636ddc4f883e4523c6d140b4784dbce42000c364ae5092aa473c" - ["1.30.3"]="e5823dc6f45610993def93ebf6cfce68264af4958c77e874b7d20f3709001b8f" -) - -# Same idea as NGINX_SHA256, for the angie flavor. -declare -A ANGIE_SHA256=( - ["1.12.0"]="cd7867d200b22a80165b93696c30a1ac3a28c1162544b7f43c71232b19814ef6" -) +# tarball we build. Those digests used to live in a table right here, which put +# the version (in seven workflow files) and its digest (here) in different +# places under different writers -- so a bumped version with a stale digest was +# a live failure mode. Both now live on adjacent lines in .github/versions.env, +# written by one tool (.github/scripts/compute-versions.sh). +# +# Verification is MANDATORY: a version with no digest here is a hard failure, +# not a warning. compute-versions.sh always emits version+digest together, so +# the only way to reach this error is a hand-edit that half-did the job. +# +# (versions.env was already sourced near the top, to default $VERSION.) +# +# Pick the digest matching the flavor+version being built. The single-version +# jobs all build NGINX_VERSION; ci-deep's matrix also builds NGINX_STABLE and +# ANGIE_VERSION. Anything else has no pin and must not be built silently. +EXPECTED="" +if [ -z "$VERSION" ]; then + # Guard before the case below: an empty $VERSION would match an empty + # "${NGINX_STABLE:-}" pattern and silently adopt the wrong digest. + echo "FATAL: no version given and no default pin for flavor '$FLAVOR'" >&2 + exit 1 +fi +case "$FLAVOR" in + nginx) + case "$VERSION" in + "${NGINX_VERSION:-}") EXPECTED="${NGINX_VERSION_SHA256:-}" ;; + "${NGINX_MAINLINE:-}") EXPECTED="${NGINX_MAINLINE_SHA256:-}" ;; + "${NGINX_STABLE:-}") EXPECTED="${NGINX_STABLE_SHA256:-}" ;; + esac + ;; + angie) + case "$VERSION" in + "${ANGIE_VERSION:-}") EXPECTED="${ANGIE_SHA256:-}" ;; + esac + ;; +esac + +if [ -z "$EXPECTED" ]; then + echo "FATAL: no pinned sha256 for $FLAVOR $VERSION" >&2 + echo " $VERSIONS_FILE pins:" >&2 + echo " nginx ${NGINX_VERSION:-?} (mainline ${NGINX_MAINLINE:-?}, stable ${NGINX_STABLE:-?})" >&2 + echo " angie ${ANGIE_VERSION:-?}" >&2 + echo " Regenerate it with .github/scripts/compute-versions.sh, or add the" >&2 + echo " version + its sha256 there -- never build an unverified tarball." >&2 + exit 1 +fi # The mode is in the tree path. See the CACHING block above -- sharing one tree # across modes is what lets a cached debug objs/ silently disarm the asan job. SRCDIR="$ROOT/${DIR}-${MODE}" mkdir -p "$ROOT" -if [ ! -f "$ROOT/${DIR}.tar.gz" ]; then - curl -fsSL "$URL" -o "$ROOT/${DIR}.tar.gz" - - # A fresh download is the only time a bad tarball can enter the cache, so - # this is where verification belongs -- a tarball already sitting in - # .build/ from a prior verified run doesn't need re-checking every - # invocation. Pinned digests are computed once from an HTTPS fetch and - # recorded in NGINX_SHA256/ANGIE_SHA256 above; this is a stopgap for - # nginx.org's plain HTTP-adjacent PGP signatures (not a sha256sum file) -- - # see https://nginx.org/en/pgp_keys.html for the upstream-recommended - # `gpg --verify` method against nginx's published release-signing keys. - case "$FLAVOR" in - nginx) EXPECTED="${NGINX_SHA256[$VERSION]:-}" ;; - angie) EXPECTED="${ANGIE_SHA256[$VERSION]:-}" ;; - esac - if [ -n "$EXPECTED" ]; then - ACTUAL="$(sha256sum "$ROOT/${DIR}.tar.gz" | awk '{print $1}')" - if [ "$ACTUAL" != "$EXPECTED" ]; then - echo "FATAL: sha256 mismatch for ${DIR}.tar.gz" >&2 - echo " expected: $EXPECTED" >&2 - echo " actual: $ACTUAL" >&2 - rm -f "$ROOT/${DIR}.tar.gz" - exit 1 - fi - echo "sha256: OK ($VERSION)" - else - echo "WARNING: no pinned sha256 for $FLAVOR $VERSION -- add one to" \ - "${FLAVOR^^}_SHA256 in tools/ci-build.sh (downloaded tarball is" \ - "UNVERIFIED)" >&2 - fi +# fetch-verify.sh downloads (with retries/timeouts) and checks the digest. It +# re-checks a tarball already present in .build/ rather than trusting it, so a +# poisoned build cache is caught too -- and deletes nothing it did not verify. +if ! bash "$MODULE_DIR/.github/scripts/fetch-verify.sh" \ + "$URL" "$EXPECTED" "$ROOT/${DIR}.tar.gz"; then + # A tarball that fails verification must not survive to be picked up as a + # "cache hit" by the next run. + rm -f "$ROOT/${DIR}.tar.gz" + exit 1 fi if [ "$NO_CACHE" = "1" ]; then rm -rf "$SRCDIR" @@ -151,8 +190,18 @@ if [ ! -d "$SRCDIR" ]; then mv "$ROOT/$DIR" "$SRCDIR" fi -# Strict flags: this is hostile-input parser code, so warnings are errors. -CC_OPT="-g -Wall -Wextra -Wshadow" +# Whole-tree flags: --with-cc-opt lands in CFLAGS for EVERY object configure +# compiles -- the upstream core included, not just our module. So only warnings +# that upstream is expected to be clean under belong here. +# +# -Wshadow deliberately does NOT: angie's configure puts -Werror in its own +# default CFLAGS, so -Wshadow here turns shadow warnings in ANGIE's sources +# (ngx_http_client_module.c, ngx_http_prometheus_module.c) into hard build +# failures in code we neither own nor patch. Our own sources ARE kept clean +# under -Wshadow -Werror by the "Strict module compile" step in build-test.yml, +# which applies the full strict set to src/*.c alone -- that is the right place +# for it, and the reason this list is the laxer one. +CC_OPT="-g -Wall -Wextra" LD_OPT="" ADD_MODULE="--add-dynamic-module=$MODULE_DIR" diff --git a/src/ngx_http_skel_module.c b/src/ngx_http_skel_module.c index 5eb8b28..8de06b5 100644 --- a/src/ngx_http_skel_module.c +++ b/src/ngx_http_skel_module.c @@ -39,7 +39,8 @@ typedef struct { * handler just returns the verdict the body handler recorded here. */ typedef struct { - ngx_int_t status; /* verdict from the body pass; NGX_DECLINED = pass */ + /* verdict from the body pass; NGX_DECLINED = pass */ + ngx_int_t status; } ngx_http_skel_ctx_t; @@ -194,8 +195,9 @@ ngx_http_skel_handler(ngx_http_request_t *r) * reads and buffers the WHOLE body before the handler resumes -- in memory * up to client_body_buffer_size, then spooled to a temp file. The bound is * client_max_body_size, NOT skel_max_body: we inspect only the first - * skel_max_body bytes, but nginx still buffers everything first. So enabling - * the body scan (a) defeats request streaming (proxy_request_buffering off) + * skel_max_body bytes, but nginx still buffers everything first. So + * enabling the body scan (a) defeats request streaming + * (proxy_request_buffering off) * and (b) makes every upload up to client_max_body_size hit disk. Keep * client_max_body_size sane on body-scanned routes; do not read this as a * reason to raise skel_max_body -- that only widens the inspected prefix. @@ -242,9 +244,10 @@ ngx_http_skel_body_handler(ngx_http_request_t *r) : NGX_DECLINED; } - /* preserve_body: the content handler (proxy_pass, a POST target) still needs - * the bytes we just buffered; without this they are discarded and it sees an - * empty body. write_event_handler: the resume point once the engine parks. */ + /* preserve_body: the content handler (proxy_pass, a POST target) still + * needs the bytes we just buffered; without this they are discarded and + * it sees an empty body. write_event_handler: the resume point once the + * engine parks. */ r->preserve_body = 1; r->write_event_handler = ngx_http_core_run_phases; @@ -442,7 +445,8 @@ ngx_http_skel_scan_body(ngx_http_request_t *r, size_t max) scanned += len; } - /* End of the (bounded) body: flush any escape held mid-token at the seam. */ + /* End of the (bounded) body: flush any escape held mid-token at the + * seam. */ return ngx_http_skel_stream_final(&st); } @@ -496,7 +500,8 @@ ngx_http_skel_init(ngx_conf_t *cf) if (ngx_http_skel_scan_rules_valid() != NGX_OK) { ngx_log_error(NGX_LOG_EMERG, cf->log, 0, "skel: a scan rule is too long for the %d-byte " - "cross-buffer seam carry; raise NGX_HTTP_SKEL_MAX_RULE_LEN " + "cross-buffer seam carry; raise " + "NGX_HTTP_SKEL_MAX_RULE_LEN " "so that 3 * longest_rule < it", (int) NGX_HTTP_SKEL_MAX_RULE_LEN); return NGX_ERROR; diff --git a/src/ngx_http_skel_scan.c b/src/ngx_http_skel_scan.c index 11bcfbd..ded52a0 100644 --- a/src/ngx_http_skel_scan.c +++ b/src/ngx_http_skel_scan.c @@ -204,7 +204,8 @@ ngx_http_skel_normalize(u_char *dst, u_char *src, size_t len) * token and 'A' is literal. Whether a trailing '%' opens a token depends on the * decoder's state arriving there, so this walks ngx_unescape_uri()'s exact * three-state machine (type 0 path) over the whole window and reports only the - * END state -- it decides where a token is still open, never what it decodes to. + * END state -- it decides where a token is still open, never what it decodes + * to. */ static size_t ngx_http_skel_partial_escape(const u_char *data, size_t len) From 4fcb7e264f2c29ecee7fddf9a5aedaadc7a6fe75 Mon Sep 17 00:00:00 2001 From: Build System Date: Fri, 31 Jul 2026 02:13:05 +0200 Subject: [PATCH 2/9] ci(linter): fail closed on an empty checker selection, verify actionlint Two ways the lint gate could report clean without having checked anything. An unmatched LINT_ONLY left SEL empty, so both loops in run-all.sh iterated zero times and the script printed "all linters clean" and exited 0. lint.yml narrows the run with LINT_ONLY, so a renamed or mistyped checker would have made the whole job vacuous. An empty selection is now exit 2 -- the same "could not run" status as a missing linter -- naming the value and the known checkers. install-linters.sh printed sha256sum of the actionlint tarball and installed it regardless: a check-shaped no-op, worse than no check because it reads as done. The digest is now pinned beside the version and compared, matching the mandatory-verification policy the repo already applies in versions.env. Same fix in the README, which piped the tarball straight into tar. ci/linter/selftest.sh carries the negative controls for the selector and runs in lint.yml ahead of the linters; with the guard reverted, three of its four cases fail. --- .github/workflows/lint.yml | 7 ++++ ci/linter/README.md | 11 +++++- ci/linter/install-linters.sh | 21 ++++++++-- ci/linter/run-all.sh | 15 ++++++++ ci/linter/selftest.sh | 75 ++++++++++++++++++++++++++++++++++++ 5 files changed, 124 insertions(+), 5 deletions(-) create mode 100755 ci/linter/selftest.sh diff --git a/.github/workflows/lint.yml b/.github/workflows/lint.yml index 72415c3..b080f98 100644 --- a/.github/workflows/lint.yml +++ b/.github/workflows/lint.yml @@ -53,6 +53,13 @@ jobs: # WHICH one before the findings scroll past. run: ci/linter/install-linters.sh --check + - name: Selftest the gate + # Before trusting the next step's verdict, prove the gate can still say + # no: a selector that matches nothing must exit 2, not report clean. + # The step below narrows the run with LINT_ONLY, so a silent selector + # is exactly the failure that would make this whole job vacuous. + run: ci/linter/selftest.sh + - name: Run linters env: LINT_ONLY: nginx sh python perl yaml diff --git a/ci/linter/README.md b/ci/linter/README.md index 9cca52e..aed666c 100644 --- a/ci/linter/README.md +++ b/ci/linter/README.md @@ -84,10 +84,17 @@ A new rule going red is a finding to triage, not drift to suppress. ```sh ver=1.7.7 -curl -fsSL "https://github.com/rhysd/actionlint/releases/download/v${ver}/actionlint_${ver}_linux_amd64.tar.gz" \ - | tar -xz actionlint && sudo install -m0755 actionlint /usr/local/bin/ +sha=023070a287cd8cccd71515fedc843f1985bf96c436b7effaecce67290e7e0757 +curl -fsSL -o actionlint.tgz \ + "https://github.com/rhysd/actionlint/releases/download/v${ver}/actionlint_${ver}_linux_amd64.tar.gz" +echo "$sha actionlint.tgz" | sha256sum -c - # must pass before the next line +tar -xzf actionlint.tgz actionlint && sudo install -m0755 actionlint /usr/local/bin/ ``` +Do not pipe the tarball straight into `tar`: that installs whatever the network +returned. The digest is from the release's `actionlint_${ver}_checksums.txt` and +is pinned beside the version in `install-linters.sh` too — bump both together. + Make sure `~/.local/bin` is on `PATH` for the pipx-installed tools. ## 2. Enable the pre-commit hook diff --git a/ci/linter/install-linters.sh b/ci/linter/install-linters.sh index db519f0..de278ec 100755 --- a/ci/linter/install-linters.sh +++ b/ci/linter/install-linters.sh @@ -122,12 +122,27 @@ done install_actionlint() { # No apt package on the target releases and no pip/cpan equivalent: a Go - # binary from the upstream release, pinned, checksum-visible in the log. - local ver="1.7.7" tmp + # binary from the upstream release, pinned by version AND sha256. + # + # The digest is COMPARED, not printed. Printing `sha256sum` and moving on + # reads as a verification step but installs whatever arrived -- worse than + # no check, because it stops anyone from adding a real one. Version and + # digest sit on adjacent lines for the same reason .github/versions.env + # does it: a version must not be able to move while its digest stays behind. + # From the upstream release's actionlint__checksums.txt. + local ver="1.7.7" + local sha="023070a287cd8cccd71515fedc843f1985bf96c436b7effaecce67290e7e0757" + local tmp tmp="$(mktemp -d)" curl -fsSL -o "$tmp/al.tgz" \ "https://github.com/rhysd/actionlint/releases/download/v${ver}/actionlint_${ver}_linux_amd64.tar.gz" || return 1 - sha256sum "$tmp/al.tgz" + if ! printf '%s %s\n' "$sha" "$tmp/al.tgz" | sha256sum -c - >/dev/null 2>&1; then + echo "actionlint ${ver}: sha256 mismatch" >&2 + echo " expected: $sha" >&2 + echo " got: $(sha256sum < "$tmp/al.tgz" | cut -d' ' -f1)" >&2 + rm -rf "$tmp" + return 1 + fi tar -xzf "$tmp/al.tgz" -C "$tmp" actionlint || return 1 $SUDO install -m0755 "$tmp/actionlint" /usr/local/bin/actionlint || return 1 rm -rf "$tmp" diff --git a/ci/linter/run-all.sh b/ci/linter/run-all.sh index 5c5781f..3531dec 100755 --- a/ci/linter/run-all.sh +++ b/ci/linter/run-all.sh @@ -55,15 +55,30 @@ echo "== lint (mode: $MODE) ==" # Selected checkers, in glob order. Built first so the replay loop below can # walk the same list it launched. +ALL=() SEL=() for s in ci/linter/lint-*.sh; do name="$(basename "$s" .sh)"; name="${name#lint-}" + ALL+=("$name") if [ -n "${LINT_ONLY:-}" ] && [[ " $LINT_ONLY " != *" $name "* ]]; then continue fi SEL+=("$name") done +# An empty selection is "could not run", never "clean". A typo in LINT_ONLY +# used to leave both loops below iterating zero times, printing "all linters +# clean" and exiting 0 -- a gate that silently lints nothing is worse than no +# gate. Same exit 2 as a missing linter. +if [ "${#SEL[@]}" -eq 0 ]; then + if [ "${#ALL[@]}" -eq 0 ]; then + echo "no ci/linter/lint-*.sh found -- wrong tree?" >&2 + else + echo "LINT_ONLY=\"${LINT_ONLY:-}\" matched no checker; known: ${ALL[*]}" >&2 + fi + exit 2 +fi + JOBS="${LINT_JOBS:-0}" [ "$JOBS" -eq 0 ] 2>/dev/null && JOBS="${#SEL[@]}" [ "$JOBS" -ge 1 ] 2>/dev/null || JOBS=1 diff --git a/ci/linter/selftest.sh b/ci/linter/selftest.sh new file mode 100755 index 0000000..1338d31 --- /dev/null +++ b/ci/linter/selftest.sh @@ -0,0 +1,75 @@ +#!/usr/bin/env bash +# ci/linter/selftest.sh -- negative controls for the lint gate itself. +# +# The gate is the thing that decides whether everything else is allowed to +# land, so "the gate ran and said clean" has to be distinguishable from "the +# gate ran nothing and said clean". That distinction is not observable from a +# green lint job: a selector typo used to make run-all.sh print +# "== all linters clean ==" and exit 0 having executed zero checkers. +# +# Every case here asserts the FAILING direction -- a check that only ever +# asserts the passing direction cannot detect its own disarming. +# +# Usage: ci/linter/selftest.sh +# Exit: 0 all controls held, 1 one or more did not. +# +# Runs in about a second: no case invokes a real checker (LINT_ONLY values are +# either bogus or rejected before dispatch), so no linter needs to be installed. +# +# Extend: add a case() line. Keep each case asserting a specific exit status, +# and prefer a case where the OLD, broken behaviour would have passed. + +set -uo pipefail + +ROOT="$(git rev-parse --show-toplevel)" +cd "$ROOT" || exit 2 + +rc=0 + +# case -- command... +case_() { + local want="$1" desc="$2"; shift 2 + local out got + out="$("$@" 2>&1)"; got=$? + if [ "$got" -eq "$want" ]; then + echo "ok $desc (exit $got)" + else + echo "FAIL $desc: expected exit $want, got $got" >&2 + echo "$out" | sed 's/^/ | /' >&2 + rc=1 + fi +} + +# The regression itself: an unmatched selector must be "could not run" (2), +# not "clean" (0). +case_ 2 "unknown LINT_ONLY exits 2" \ + env LINT_ONLY=nosuchchecker ci/linter/run-all.sh + +# ...including when only SOME of the listed names are bogus in a way that +# leaves nothing selected. +case_ 2 "LINT_ONLY of only-bogus names exits 2" \ + env LINT_ONLY="c-lang shellscript" ci/linter/run-all.sh + +# The error names the offending value and the known checkers, or nobody can +# act on it. +# Captured first, not piped into grep: `set -o pipefail` would otherwise hand +# the pipeline run-all.sh's exit 2 and the assertion would read as failed no +# matter what the message said. +msg="$(env LINT_ONLY=nosuchchecker ci/linter/run-all.sh 2>&1)" +if printf '%s\n' "$msg" | grep -q 'matched no checker; known: .*sh'; then + echo "ok unmatched-selector message names value and known checkers" +else + echo "FAIL unmatched-selector message is not actionable" >&2 + rc=1 +fi + +# Positive control: the selector still selects. --list is used rather than a +# real run so this stays independent of which linters are installed. +case_ 0 "--list works" ci/linter/run-all.sh --list + +if [ "$rc" -eq 0 ]; then + echo "== lint gate selftest: all controls held ==" +else + echo "== lint gate selftest: FAILED ==" >&2 +fi +exit "$rc" From 6466df80714059e8b8cc3d6573f718e73d01b1ca Mon Sep 17 00:00:00 2001 From: Build System Date: Fri, 31 Jul 2026 02:14:25 +0200 Subject: [PATCH 3/9] ci: correct the lane header -- lane B is at budget, not under it The lane map was measured from one green run. The next green run of the same tree put Security scanners at 94s against the 47s recorded here, and lane B finished 1s ahead of Fuzzing rather than 39s. builder02 runs the package builds too, so a single sample is not a duration. Header now carries the spread across both runs and drops the "~59s of headroom" claim: lane B is roughly at budget, so new work there needs a fourth lane. --- .github/workflows/ci.yml | 38 ++++++++++++++++++++++---------------- 1 file changed, 22 insertions(+), 16 deletions(-) diff --git a/.github/workflows/ci.yml b/.github/workflows/ci.yml index 8546e7f..58856a2 100644 --- a/.github/workflows/ci.yml +++ b/.github/workflows/ci.yml @@ -11,25 +11,31 @@ name: CI # The self-hosted work is arranged as three lanes. A lane starts with its # longest job and releases the slot to a shorter, independent follow-up. # -# A Fuzzing (282s) = 282s -# B A/UBSan (97s) -> Valgrind (79s) -> Security scanners (47s) = 223s +# A Fuzzing (277-282s) = 277-282s +# B A/UBSan (84-97s) -> Valgrind (79-85s) -> Scanners (47-94s) = 228-269s # C Build&Test (~60s) = 60s # -# The budget is the longest single job -- 282s of fuzzing -- because no -# arrangement can finish sooner than that. Both other lanes are under it, so the -# suite is fuzz-bound and lane B has ~59s of headroom. If Valgrind or the -# scanners grow past that, move one of them to a fourth lane rather than letting -# lane B become the critical path. +# The budget is the longest single job -- fuzzing, which is a fixed 120s of +# fuzz plus its build -- because no arrangement can finish sooner than that. +# Lane B stays under it, so the suite is fuzz-bound. # -# Durations are OBSERVED on this exact shape -- run 30591711577 (head 1b6d094, -# 2026-07-31), green, 286s wall clock, lane B finishing 39s ahead of Fuzzing. -# An earlier revision of this comment quoted 320/149/120/110/100/98s and paired -# Fuzzing with Security scanners, which put lane A at 348s: over budget, and the -# suite's actual critical path, for a 352s total. Security scanners reads 47s -# here against 77s on that shape because semgrep gained --jobs=1 --metrics=off -# in the same change. Re-measure with -# `gh run view -R myguard-labs/nginx-skeleton-module --json jobs` before -# trusting any number here; they drift with the runners. +# The ranges are not padding. They are the SPREAD BETWEEN TWO CONSECUTIVE GREEN +# RUNS of the same tree, runs 30591711577 (head 1b6d094, 286s) and 30592009272 +# (head 0a3ddcf, 280s), which differ only by a comment. Lane B finished 54s +# ahead of Fuzzing in the first and 1s ahead in the second; Security scanners +# alone read 47s and then 94s. builder02 also runs the package builds, so a job +# here is timed against whatever else the box is doing -- one sample tells you +# nothing, which is why the earlier "~59s of headroom" claim in this header was +# wrong the day after it was measured. +# +# So: treat lane B as roughly at budget, not comfortably under it. Adding work +# to it needs a fourth lane, not the headroom this comment used to promise. +# An earlier revision quoted 320/149/120/110/100/98s and paired Fuzzing with +# Security scanners, which put lane A at 348s: over budget, and the suite's +# actual critical path, for a 352s total. Scanners came down from 77s because +# semgrep gained --jobs=1 --metrics=off in the same change. Re-measure with +# `gh run view -R myguard-labs/nginx-skeleton-module --json jobs` over +# SEVERAL runs before trusting any number here. # # NOTE on "peak runner use": a lane is not a slot. Build&Test is a reusable # workflow that fans out into FIVE concurrent self-hosted jobs, so the observed From e741934d7963ed77496ca5d213f3305b0a75f99c Mon Sep 17 00:00:00 2001 From: Build System Date: Fri, 31 Jul 2026 02:22:10 +0200 Subject: [PATCH 4/9] ci(lint): surface git/env failures instead of masking as empty lint_files() ate a failed git diff/ls-files as an empty file list -- 0 files reads as clean, not as an error. ci-build.sh's DEFAULT_VERSION lookup dereferenced NGINX_VERSION/ANGIE_VERSION unguarded, dying with an unbound-variable error under set -u instead of reaching the FATAL block that already exists for a missing pin. --- ci/linter/lib.sh | 4 ++-- ci/tools/ci-build.sh | 4 ++-- 2 files changed, 4 insertions(+), 4 deletions(-) diff --git a/ci/linter/lib.sh b/ci/linter/lib.sh index 6f03988..5d64ad9 100755 --- a/ci/linter/lib.sh +++ b/ci/linter/lib.sh @@ -38,9 +38,9 @@ lint_files() { if [ "$#" -gt 0 ]; then list=$(printf '%s\n' "$@") elif [ "${LINT_MODE:-all}" = "staged" ]; then - list=$(git diff --cached --name-only --diff-filter=ACMR) + list=$(git diff --cached --name-only --diff-filter=ACMR) || die "git diff --cached failed" else - list=$(git ls-files) + list=$(git ls-files) || die "git ls-files failed" fi printf '%s\n' "$list" \ | grep -Ev "$LINT_EXCLUDE_RE" \ diff --git a/ci/tools/ci-build.sh b/ci/tools/ci-build.sh index c4a894c..03c5f30 100755 --- a/ci/tools/ci-build.sh +++ b/ci/tools/ci-build.sh @@ -83,8 +83,8 @@ done < "$VERSIONS_FILE" . "$VERSIONS_FILE" case "$FLAVOR" in - nginx) DEFAULT_VERSION="$NGINX_VERSION" ;; - angie) DEFAULT_VERSION="$ANGIE_VERSION" ;; + nginx) DEFAULT_VERSION="${NGINX_VERSION:-}" ;; + angie) DEFAULT_VERSION="${ANGIE_VERSION:-}" ;; *) DEFAULT_VERSION="" ;; esac VERSION="${2:-$DEFAULT_VERSION}" From 49aeabf3739a10dc9d7a3912cd50a11fc138c754 Mon Sep 17 00:00:00 2001 From: Build System Date: Fri, 31 Jul 2026 02:22:14 +0200 Subject: [PATCH 5/9] ci(scripts): guard GITHUB_ENV, give compute-versions.sh the fetch-verify retry budget load-versions.sh dereferenced $GITHUB_ENV under set -u with no guard, so running it outside a workflow step died on a bare 'unbound variable' instead of a usable message. compute-versions.sh's api() and the nginx.org download-page fetch had no --retry/--connect-timeout/--max-time, unlike fetch-verify.sh -- a transient blip failed the weekly bump job or hung a runner instead of retrying. --- .github/scripts/compute-versions.sh | 10 +++++++--- .github/scripts/load-versions.sh | 4 ++++ 2 files changed, 11 insertions(+), 3 deletions(-) diff --git a/.github/scripts/compute-versions.sh b/.github/scripts/compute-versions.sh index 6632664..e8c208a 100755 --- a/.github/scripts/compute-versions.sh +++ b/.github/scripts/compute-versions.sh @@ -28,10 +28,13 @@ trap 'rm -rf "$tmp"' EXIT api() { local url="$1" + # Same resilience budget as fetch-verify.sh: a transient blip or a stalled + # connection must retry, not fail the weekly bump job or hang the runner. + local -a opts=(-fsSL --retry 3 --retry-delay 2 --connect-timeout 30 --max-time 300) if [ -n "${GITHUB_TOKEN:-}" ]; then - curl -fsSL -H "Authorization: Bearer $GITHUB_TOKEN" "$url" + curl "${opts[@]}" -H "Authorization: Bearer $GITHUB_TOKEN" "$url" else - curl -fsSL "$url" + curl "${opts[@]}" "$url" fi } @@ -55,7 +58,8 @@ sha_of_url() { } echo "resolving nginx versions from nginx.org..." -dl_html="$(curl -fsSL https://nginx.org/en/download.html)" +dl_html="$(curl -fsSL --retry 3 --retry-delay 2 --connect-timeout 30 --max-time 300 \ + https://nginx.org/en/download.html)" # nginx numbers mainline with an odd minor and stable with an even one. Take # the highest of each rather than trusting page order. NGX_MAINLINE="$(printf '%s' "$dl_html" | grep -oE 'nginx-1\.[0-9]+\.[0-9]+' \ diff --git a/.github/scripts/load-versions.sh b/.github/scripts/load-versions.sh index 98cad97..28ae7c2 100755 --- a/.github/scripts/load-versions.sh +++ b/.github/scripts/load-versions.sh @@ -10,6 +10,10 @@ set -euo pipefail f=".github/versions.env" [ -f "$f" ] || { echo "::error::$f not found" >&2; exit 1; } +# Outside a GitHub Actions step $GITHUB_ENV is unset, and set -u turns that +# into a bare "unbound variable" -- give anyone running this by hand a +# message that says what actually went wrong. +[ -n "${GITHUB_ENV:-}" ] || { echo "::error::GITHUB_ENV unset -- run this inside a GitHub Actions step" >&2; exit 1; } while IFS= read -r line || [ -n "$line" ]; do case "$line" in From 7b6e72d2ab8745d0a2c7064965e0faa7e415e92d Mon Sep 17 00:00:00 2001 From: Build System Date: Fri, 31 Jul 2026 02:22:19 +0200 Subject: [PATCH 6/9] ci: extract the asan changed-paths gate pattern to one variable The alternation was written twice, once to decide and once to log -- same fail-closed behaviour, same matched-paths logging, just one regex instead of two that could drift apart. --- .github/workflows/ci.yml | 9 +++++---- 1 file changed, 5 insertions(+), 4 deletions(-) diff --git a/.github/workflows/ci.yml b/.github/workflows/ci.yml index 58856a2..3fa6786 100644 --- a/.github/workflows/ci.yml +++ b/.github/workflows/ci.yml @@ -123,15 +123,16 @@ jobs: echo "::error::cannot diff $BASE_SHA...HEAD -- refusing to guess" >&2 exit 1 fi - if printf '%s\n' "$changed" | grep -qE \ - '^(src/|ci/t/|ci/tools/ci-build\.sh|ci/tools/soak\.sh|\.github/workflows/(asan|ci)\.yml|\.github/versions\.env|\.github/scripts/)'; then + # One pattern, decision and log read the same variable, so they + # cannot drift the way two copies of the same regex eventually do. + gate='^(src/|ci/t/|ci/tools/ci-build\.sh|ci/tools/soak\.sh|\.github/workflows/(asan|ci)\.yml|\.github/versions\.env|\.github/scripts/)' + if printf '%s\n' "$changed" | grep -qE "$gate"; then echo "code=true" >> "$GITHUB_OUTPUT" else echo "code=false" >> "$GITHUB_OUTPUT" fi echo "changed files matching the asan gate:" - printf '%s\n' "$changed" | grep -E \ - '^(src/|ci/t/|ci/tools/ci-build\.sh|ci/tools/soak\.sh|\.github/workflows/(asan|ci)\.yml|\.github/versions\.env|\.github/scripts/)' || echo " (none)" + printf '%s\n' "$changed" | grep -E "$gate" || echo " (none)" asan: name: A/UBSan From 0265fbeb06d3f3063a15c35d8a1e4a219a0260ef Mon Sep 17 00:00:00 2001 From: Build System Date: Fri, 31 Jul 2026 02:22:23 +0200 Subject: [PATCH 7/9] ci: wire the actionlint pre-commit hook to .github/actionlint.yaml explicitly Relied on actionlint's cwd-based auto-discovery of the config file; pre-commit hooks don't guarantee cwd the way an interactive shell invocation does. -config-file makes the self-hosted-runner-label allowlist explicit. The separate -ignore for unknown GitHub-hosted runner images stays -- there's no config-file section for that, only for self-hosted labels. --- .pre-commit-config.yaml | 21 +++++++++++++++------ 1 file changed, 15 insertions(+), 6 deletions(-) diff --git a/.pre-commit-config.yaml b/.pre-commit-config.yaml index 8ce7ab9..13f048b 100644 --- a/.pre-commit-config.yaml +++ b/.pre-commit-config.yaml @@ -107,12 +107,21 @@ repos: # CI workflows are code that breaks silently; actionlint catches bad if: # expressions, unknown contexts and shell errors inside run: blocks. # + # -config-file: declares our self-hosted-runner labels (builder02, lxc, + # b02lxc, docker) explicitly rather than relying on actionlint finding + # .github/actionlint.yaml by cwd-guessing -- pre-commit hooks don't + # guarantee cwd the way a bare `actionlint` invocation from a shell does. + # # -ignore runner-label: actionlint 1.7.7 ships a FIXED list of GitHub - # runner images and flags anything newer as "unknown label". Several - # repos legitimately target ubuntu-26.04 / ubuntu-26.04-arm, which that - # version has never heard of. Every such report is a false positive about - # the linter's age, not the workflow, and leaving it on lands this hook - # red on arrival. Drop this ignore once actionlint is new enough. + # *hosted* runner images and flags anything newer as "unknown label". + # That's a different gap than the self-hosted labels above -- there's no + # config-file section for "GitHub-hosted images this actionlint release + # doesn't know yet" -- so this stays even with the config file wired in. + # Several repos legitimately target ubuntu-26.04 / ubuntu-26.04-arm, + # which that version has never heard of. Every such report is a false + # positive about the linter's age, not the workflow, and leaving it on + # lands this hook red on arrival. Drop this ignore once actionlint is + # new enough. # # SHELLCHECK_OPTS applies to the shellcheck actionlint runs over `run:` # blocks. Without it that embedded pass reports at INFO while the @@ -121,7 +130,7 @@ repos: # in both places. - id: actionlint name: actionlint (workflow syntax) - entry: env SHELLCHECK_OPTS=-Swarning actionlint -ignore 'label ".+" is unknown' + entry: env SHELLCHECK_OPTS=-Swarning actionlint -config-file .github/actionlint.yaml -ignore 'label ".+" is unknown' language: system files: ^\.github/workflows/.*\.ya?ml$ pass_filenames: true From 672eb15de5cfe509d6705f247eb1cac50842c890 Mon Sep 17 00:00:00 2001 From: Build System Date: Fri, 31 Jul 2026 02:22:26 +0200 Subject: [PATCH 8/9] ci(linter): pin ruff, matching the semgrep pin ruff floated to whatever PyPI served on each install, so local and CI findings could diverge the same way an unpinned semgrep would. zizmor stays unpinned on purpose -- its rule set is the point -- and the README explanation now says so against both pinned tools instead of just semgrep. --- ci/linter/README.md | 14 ++++++++------ ci/linter/install-linters.sh | 5 ++++- 2 files changed, 12 insertions(+), 7 deletions(-) diff --git a/ci/linter/README.md b/ci/linter/README.md index aed666c..0b7fd5a 100644 --- a/ci/linter/README.md +++ b/ci/linter/README.md @@ -55,12 +55,14 @@ externally-managed, so a bare `pip3 install` fails and `--break-system-packages` is a worse answer than a venv per tool. ```sh -pipx install ruff +pipx install 'ruff==0.16.1' # pinned to the CI version on purpose pipx install 'semgrep==1.169.0' # pinned to the CI version on purpose ``` -`semgrep` is pinned because an unpinned upgrade changes findings under you and -local stops matching CI. Bump it here and in `security-scanners.yml` together. +`ruff` and `semgrep` are pinned because an unpinned upgrade changes findings +under you and local stops matching CI. Bump each here and in its CI consumer +together -- `ruff` in `install-linters.sh`, `semgrep` in +`security-scanners.yml` too. **cpan (Perl modules apt does not carry on every target release)** @@ -76,9 +78,9 @@ an install step has no business demanding. pipx install zizmor # GitHub Actions security audit ``` -`zizmor` is deliberately **not** pinned, unlike semgrep: its rule set is the -whole point, and a frozen security scanner stops finding what it was added for. -A new rule going red is a finding to triage, not drift to suppress. +`zizmor` is deliberately **not** pinned, unlike ruff and semgrep: its rule set +is the whole point, and a frozen security scanner stops finding what it was +added for. A new rule going red is a finding to triage, not drift to suppress. **upstream binary (no apt/pip/cpan source)** diff --git a/ci/linter/install-linters.sh b/ci/linter/install-linters.sh index de278ec..645c42e 100755 --- a/ci/linter/install-linters.sh +++ b/ci/linter/install-linters.sh @@ -50,7 +50,10 @@ APT_TOOLS=( # bare `pip3 install` fails on Debian 12+ and `--break-system-packages` is a # worse answer than an isolated venv per tool. PIPX_TOOLS=( - "ruff:ruff" # Python lint + format check + "ruff:ruff==0.16.1" # Python lint + format check, pinned: + # an unpinned ruff changes findings under + # you and local stops matching CI, same + # reasoning as the semgrep pin below. "zizmor:zizmor" # GitHub Actions security audit. Not # pinned: its rule set is the point, and a # frozen security scanner stops finding From 8b47982edfdd3beda5ceeef0bc0b4dc126ca17cb Mon Sep 17 00:00:00 2001 From: Build System Date: Fri, 31 Jul 2026 02:23:59 +0200 Subject: [PATCH 9/9] ci: let detect-private-key see .pem files again pre-commit applies the top-level exclude before any per-hook rule, so every suffix in that regex is invisible to every hook. .pem and .der were in it to keep the whitespace fixers from rewriting key material -- and took detect-private-key, whose whole job is finding a .pem, out with them. Probe, identical fake key in both files: probe-key.pem -> detect private key ... (no files to check) Skipped, exit 0 probe-key.txt -> detect private key ... Failed, Private key found Both suffixes move to per-hook excludes on the three fixers that rewrite files. After the change the .pem probe Fails and trailing-whitespace still skips it. Also drops two job-level comments that quoted the old single-sample durations the header no longer claims. --- .github/workflows/ci.yml | 12 ++++++------ .pre-commit-config.yaml | 16 +++++++++++++++- 2 files changed, 21 insertions(+), 7 deletions(-) diff --git a/.github/workflows/ci.yml b/.github/workflows/ci.yml index 3fa6786..6ea5292 100644 --- a/.github/workflows/ci.yml +++ b/.github/workflows/ci.yml @@ -75,9 +75,9 @@ permissions: jobs: # --- lane A: the budget-setting job, alone ------------------------------- - # Nothing follows Fuzzing. At 268s it is the longest job in the suite, so the - # suite cannot finish sooner; anything chained behind it is pure added - # critical path. Security scanners used to sit here and cost exactly that. + # Nothing follows Fuzzing. It is the longest job in the suite, so the suite + # cannot finish sooner; anything chained behind it is pure added critical + # path. Security scanners used to sit here and cost exactly that. fuzzing: name: Fuzzing permissions: @@ -152,9 +152,9 @@ jobs: security-scanners: name: Security scanners - # Tail of lane B, not of lane A. 94 + 83 + 77 = 254s still fits under the - # 268s fuzzing budget, so these 77s are free here and were critical path - # there. + # Tail of lane B, not of lane A. The lane still fits under the fuzzing + # budget, so this job is free here and was critical path there. Header has + # the measured spread; lane B has less room than it looks. needs: valgrind if: ${{ !cancelled() }} permissions: diff --git a/.pre-commit-config.yaml b/.pre-commit-config.yaml index 13f048b..04f0cd9 100644 --- a/.pre-commit-config.yaml +++ b/.pre-commit-config.yaml @@ -45,16 +45,30 @@ # Fuzz corpus files are byte-exact test inputs: a stripped space or an added # newline changes what is actually fuzzed. Never let a fixer touch them. -exclude: '^ci/fuzz/corpus/|^ci/fuzz/regressions/|\.traefik-ref/|\.claude/worktrees/|\.build/|(^|/)objs/|(^|/)vendor/|(^|/)node_modules/|(^|/)\.venv/|\.(bin|gz|zip|png|ico|der|pem|so|o|a)$' +# +# .pem and .der are NOT in this list, deliberately. pre-commit applies the +# top-level exclude before any per-hook rule, so a suffix named here is +# invisible to EVERY hook -- including detect-private-key, whose entire job is +# catching a .pem. Verified: an identical fake key reads "(no files to check) +# Skipped" as probe.pem and "Private key found" as probe.txt. They are excluded +# per-hook on the three fixers below instead, which is the only reason they were +# ever in this list. +exclude: '^ci/fuzz/corpus/|^ci/fuzz/regressions/|\.traefik-ref/|\.claude/worktrees/|\.build/|(^|/)objs/|(^|/)vendor/|(^|/)node_modules/|(^|/)\.venv/|\.(bin|gz|zip|png|ico|so|o|a)$' repos: - repo: https://github.com/pre-commit/pre-commit-hooks rev: v6.0.0 hooks: + # The three fixers rewrite files in place, which is why keys and DER blobs + # have to stay out of their reach. Per-hook, not top-level: a top-level + # exclude would also blind detect-private-key below. - id: trailing-whitespace + exclude: '\.(pem|der)$' - id: end-of-file-fixer + exclude: '\.(pem|der)$' - id: mixed-line-ending args: [--fix=lf] + exclude: '\.(pem|der)$' - id: check-merge-conflict - id: check-yaml - id: check-added-large-files