diff --git a/.github/CI_SETUP.md b/.github/CI_SETUP.md deleted file mode 100644 index 17c30f1..0000000 --- a/.github/CI_SETUP.md +++ /dev/null @@ -1,170 +0,0 @@ -# CI/CD Pipeline - -Automated build, test, and security analysis for the zstd-nginx-module. - -## Workflows - -The pipeline is split across four workflow files: - -| File | Purpose | -|---|---| -| [`build-test.yml`](workflows/build-test.yml) | Lint/validation, build matrix (nginx mainline + Angie), Perl/Python tests, ASAN/UBSAN | -| [`codeql.yml`](workflows/codeql.yml) | CodeQL `security-extended` analysis | -| [`security-scanners.yml`](workflows/security-scanners.yml) | flawfinder, clang-tidy, semgrep + SARIF upload | -| [`fuzzing.yml`](workflows/fuzzing.yml) | libFuzzer Accept-Encoding parser fuzzing | - -Common triggers (`build-test.yml`, `codeql.yml`, `security-scanners.yml`): - -- **Push** to `master`, `main`, `dev` -- **Pull requests** to `master`, `main` -- **Weekly schedule** — Monday 04:17 UTC, catches nginx API drift against - newly released nginx versions even with no commits -- **Manual** — `workflow_dispatch` - -Workflow-level hardening: - -- `concurrency` — superseded runs on the same ref are cancelled, no pile-up -- `permissions: contents: read` by default; `codeql` and `secure` request - `security-events: write` only where needed -- `timeout-minutes` on every job — a hung nginx test cannot burn runner hours -- All third-party actions are **pinned to commit SHAs** (see the header - comment in `build-test.yml`), not floating tags - -## nginx versions - -| Role | Version | Where | -|---|---|---| -| nginx mainline (default + artifact + tests) | **latest, resolved at run time** | `resolve` job → `needs.resolve.outputs.nginx_version` | -| Angie (nginx fork, also packaged) | **1.11.5** (pinned) | `resolve` job matrix JSON | - -nginx is **not pinned**. A `resolve` job scrapes `nginx.org/en/download.html` -for the current mainline release and exposes it as -`needs.resolve.outputs.nginx_version` plus a ready build-matrix JSON; every -other job (`build`, `build-old-libzstd`, `build-asan`, `tests`) consumes those -so the whole run agrees on one version. This keeps the weekly cron testing the -module against new releases as they ship, and avoids 404s once nginx.org drops -an old mainline tarball. `codeql.yml` and `valgrind.yml` resolve the same way -via a per-job "Resolve latest mainline nginx" step. `tools/ci-build.sh` with no -argument resolves the latest too (pass an explicit version to override). - -The `build` job uses a dynamic -`strategy.matrix: ${{ fromJSON(needs.resolve.outputs.matrix) }}` (entries with -`flavor`/`version`/`url`/`dir`) so the -module is compiled against both nginx mainline and Angie — the actively -developed fork this module also ships packages for (see -`tools/test_package_artifact.py`). The shared test binary is the nginx -mainline one. - -There is **no special "CI module" for nginx** — CI builds nginx from source -with `--add-module` (full binary) or `--with-compat` + nginx-dev headers (for -lint tools). The realistic extra HTTP modules the zstd filter runs alongside -are compiled in so the module is exercised in a real nginx, not a stripped one: -`ssl`, `v2`, `v3`, `gzip_static`, `realip`, `sub`, `addition`, `stub_status`, -`auth_request`, plus `--with-threads` and `--with-file-aio`. - -### Debug compile flags - -The `build` job configures with full debug flags: - -- `--with-debug` — nginx `ngx_log_debug*` logging compiled in -- `-g3` — maximum debug info including macro definitions -- `-O0` — no optimisation, accurate line/variable info -- `-fno-omit-frame-pointer` — reliable gdb/valgrind backtraces -- `-funwind-tables` — unwind info for crash backtraces -- `-DNGX_DEBUG_PALLOC=1` — pool-allocator debug bookkeeping - -Module sources additionally get a strict compile pass with -`-Wall -Wextra -Wshadow -Wstrict-aliasing -Wunreachable-code -Wunused --Wwrite-strings -Werror`. - -## Jobs - -| Workflow | Job | Depends on | Purpose | -|---|---|---|---| -| build-test | `validation` | — | actionlint, shellcheck, cppcheck, clang static analyzer, Python harness unit tests | -| build-test | `build` | — | Build matrix (nginx mainline + Angie), strict module compile, ccache, upload artifact | -| build-test | `build-asan` | — | Build nginx with `-fsanitize=address,undefined` | -| build-test | `tests` | `build` | Perl `Test::Nginx::Socket` suites + Python end-to-end smoke tests | -| build-test | `tests-asan` | `build-asan` | Re-run smoke tests under ASAN+UBSAN, fail on any memory/UB error | -| codeql | `codeql` | — | GitHub first-party C/C++ security analysis (`security-extended`) | -| security-scanners | `secure` | — | flawfinder, clang-tidy, semgrep — results uploaded as SARIF to the Security tab | - -Within `build-test`, `validation`, `build`, and `build-asan` start in -parallel; only `tests`/`tests-asan` wait on their respective build job. -`codeql` and `security-scanners` are independent workflows running in -parallel with it. - -### `tests` coverage - -- `t/00-filter.t` — filter module Perl suite -- `t/01-static.t` — static module Perl suite -- `tools/test_encoding.py` — truncation, Vary, boundary-size (1900 lines), - repeated-request, concurrent-request smoke tests -- `tools/test_terminal_frame.py` — empty-output `ZSTD_e_end` terminal-frame - regression (the bug fixed in `a209f96`) - -### Caching - -- apt archives per job -- nginx source tarballs keyed by version -- **ccache** for the nginx + module compile, keyed on source/header hashes -- Perl modules (`~/perl5`) -- nginx-dev generated headers -- semgrep rules and pip cache - -## Security analysis - -Five layers, results surfaced in the GitHub **Security → Code scanning** tab -via SARIF (not just buried in artifacts): - -| Tool | Job | Output | -|---|---|---| -| CodeQL (`security-extended`) | `codeql` | SARIF (native) | -| flawfinder | `secure` | SARIF + log | -| semgrep (`p/c`, `p/security-audit`) | `secure` | SARIF + log | -| clang-tidy (`cert-*`, `bugprone-*`, `clang-analyzer-security.*`) | `secure` | log | -| cppcheck / clang static analyzer | `validation` | log artifacts | - -ASAN+UBSAN (`tests-asan`) is the runtime memory-safety layer — it directly -targets the lifetime/UB bug classes in this module's history (per-request -context handling, terminal-frame emission). - -## Local testing - -Build against nginx locally before pushing: - -```bash -bash tools/ci-build.sh # default: latest mainline (resolved from nginx.org) -bash tools/ci-build.sh 1.29.8 # specific nginx version -``` - -Run the test suites locally (requires `Test::Nginx::Socket`): - -```bash -cd t -perl 00-filter.t -perl 01-static.t -python3 ../tools/test_encoding.py --nginx-binary /path/to/nginx -python3 ../tools/test_terminal_frame.py --nginx-binary /path/to/nginx -``` - -Run the Python harness unit tests: - -```bash -cd tools -python3 -m unittest test_test_encoding test_test_package_artifact -``` - -## Status badge - -```markdown -[![Build & Test](https://github.com/OWNER/REPO/actions/workflows/build-test.yml/badge.svg)](https://github.com/OWNER/REPO/actions/workflows/build-test.yml) -``` - -## See also - -- [`workflows/build-test.yml`](workflows/build-test.yml) — main build/test workflow definition -- `tools/ci-build.sh` — local build script -- `tools/test_encoding.py` — end-to-end encoding tester -- `tools/test_terminal_frame.py` — terminal-frame regression test -- `valgrind.suppress` — suppression file for local valgrind runs diff --git a/.github/workflows/bump.yml b/.github/workflows/bump.yml new file mode 100644 index 0000000..e5df458 --- /dev/null +++ b/.github/workflows/bump.yml @@ -0,0 +1,63 @@ +name: Bump versions + +# Weekly: check nginx.org/angie.software for newer nginx-stable/angie +# releases than what's pinned in ci-deep.yml's build-flavors matrix, and pull +# the vendored nginx-tests submodule to its upstream HEAD. nginx mainline is +# NOT touched here -- it's resolved dynamically at CI run time (see +# build-test.yml's `resolve` job / ci-deep.yml's per-job resolve steps), so +# there's no static mainline pin to bump. +# +# tools/bump-versions.sh does the actual work (also runnable locally with +# --dry-run to preview). Commits and pushes straight to master -- this repo +# has no branch protection gate for this path, and a version/digest/submodule +# bump is exactly the kind of low-risk, easily-revertable change that doesn't +# need a human in the loop before landing. The next build-test.yml run on +# master is what actually proves the bump didn't break anything. +# +# Action pins (keep in sync with build-test.yml): +# actions/checkout@v5 -> 93cb6efe18208431cddfb8368fd83d5badbf9bfd + +on: + schedule: + # Weekly, Monday 04:00 UTC. + - cron: "0 4 * * 1" + workflow_dispatch: {} + +concurrency: + group: bump-${{ github.workflow }}-${{ github.ref }} + cancel-in-progress: true + +permissions: + contents: write + +jobs: + bump: + name: Bump nginx-stable/angie/nginx-tests pins + runs-on: [self-hosted, builder02, lxc] + timeout-minutes: 15 + steps: + - name: Checkout + uses: actions/checkout@93cb6efe18208431cddfb8368fd83d5badbf9bfd # v5 + with: + persist-credentials: true + submodules: true + fetch-depth: 0 + + - name: Run bump script + id: bump + run: bash tools/bump-versions.sh + + - name: Commit and push if anything changed + run: | + set -euo pipefail + if [ -z "$(git status --porcelain)" ]; then + echo "nothing changed, skipping commit" + exit 0 + fi + git config user.name "myguard-bump-bot" + git config user.email "actions@users.noreply.github.com" + git add -A + git commit -m "chore: bump nginx-stable/angie/nginx-tests pins + + Automated weekly version check (tools/bump-versions.sh)." + git push origin HEAD:master diff --git a/.github/workflows/ci-deep.yml b/.github/workflows/ci-deep.yml index 070fc8e..3376183 100644 --- a/.github/workflows/ci-deep.yml +++ b/.github/workflows/ci-deep.yml @@ -9,11 +9,20 @@ name: CI Deep # nothing on a private repo, so the SARIF-upload path is removed too — the # scanners still run and GATE on findings, they just don't upload. # -# Jobs: fuzz (14400s/target) · memcheck soak · helgrind soak · scanners. +# Jobs: build-flavors (nginx mainline/stable + angie) · fuzz (14400s/target) · +# memcheck soak · helgrind soak · scanners. # * memcheck AND helgrind run under tools/soak.sh, which already passes # --suppressions=valgrind.suppress and --error-exitcode=99 for BOTH tools # (see tools/soak.sh) — the drift-fix baseline for every module. # +# build-flavors exists because build-test.yml (the fast PR gate) only builds +# ONE pinned nginx version (mainline) — the module's README documents Angie +# support and tracks nginx stable too, but nothing in CI actually built angie +# or nginx stable until this job. It is here (monthly), not in the PR gate, +# because each leg is a from-scratch configure+compile+Test::Nginx run and +# three of them would blow the PR gate's budget. Uses tools/ci-build.sh, which +# supports flavor+version args and sha256-verifies the tarball it downloads. +# # Action pins (keep in sync with build-test.yml header): # actions/checkout@v5 -> 93cb6efe18208431cddfb8368fd83d5badbf9bfd # actions/upload-artifact@v5 -> 330a01c490aca151604b8cf639adc76d48f6c5d4 @@ -42,6 +51,71 @@ permissions: contents: read jobs: + build-flavors: + name: Build & Test::Nginx (${{ matrix.flavor }} ${{ matrix.version }}) + runs-on: [self-hosted, builder02, lxc] + timeout-minutes: 30 + strategy: + fail-fast: false + matrix: + include: + - flavor: nginx + version: "1.31.2" + label: mainline + - flavor: nginx + version: "1.30.3" + label: stable + - flavor: angie + version: "1.11.5" + label: angie + steps: + - name: Checkout module + uses: actions/checkout@93cb6efe18208431cddfb8368fd83d5badbf9bfd # v5 + + - name: Install dependencies + run: | + sudo apt-get update + sudo apt-get install -y \ + build-essential curl libpcre2-dev libzstd-dev pkg-config \ + wget zstd libperl-dev cpanminus + + - name: Build server and dynamic modules + run: bash tools/ci-build.sh "${{ matrix.flavor }}" "${{ matrix.version }}" + + - name: Verify built modules + run: | + build=".build/${{ matrix.flavor }}-${{ matrix.version }}/objs" + test -f "$build/${{ matrix.flavor }}" + test -f "$build/ngx_http_zstd_filter_module.so" + test -f "$build/ngx_http_zstd_static_module.so" + echo "TEST_NGINX_BINARY=$GITHUB_WORKSPACE/$build/${{ matrix.flavor }}" >> "$GITHUB_ENV" + + - name: Cache Perl modules + uses: actions/cache@0057852bfaa89a56745cba8c7296529d2fc39830 # v4 + with: + path: ~/perl5 + key: perl-modules-${{ runner.os }}-Test-Nginx-Socket + + - name: Install Test::Nginx::Socket + run: | + cpanm -l ~/perl5 --notest Test::Nginx::Socket + echo "PERL5LIB=$HOME/perl5/lib/perl5:$PERL5LIB" >> "$GITHUB_ENV" + + - name: Run t/ (filter + static + conf-warn) + env: + # See build-test.yml's identical setting for why this must be 20, + # not the ~2s Test::Nginx::Socket default. + TEST_NGINX_TIMEOUT: "20" + run: | + export PERL5LIB="$HOME/perl5/lib/perl5:${PERL5LIB:-}" + export TEST_NGINX_SERVROOT="/tmp/nginx-servroot-${{ matrix.flavor }}-${{ matrix.version }}" + mkdir -p "$TEST_NGINX_SERVROOT" + cd "$GITHUB_WORKSPACE" + # t/01-static.t asserts on fixture mtimes (Last-Modified/If-Modified-Since); + # pin them to a fixed timestamp, same as build-test.yml's tests job. + touch -d @1541504307 t/suite/test t/suite/test.zst + prove -v t/00-filter.t t/01-static.t t/02-conf-warn.t + fuzz: name: Fuzz (Accept-Encoding, long) runs-on: [self-hosted, builder02, lxc] diff --git a/.gitignore b/.gitignore index 28eac68..ff82218 100644 --- a/.gitignore +++ b/.gitignore @@ -13,6 +13,10 @@ # nginx build tree (when building out-of-tree inside this repo) objs/ +# tools/ci-build.sh persistent build tree (nginx/angie source + compiled +# binaries, one dir per flavor-version) +.build/ + # Test runner working directories (nginx test::nginx servroot) t/servroot/ t/servroot-*/ diff --git a/AGENTS.md b/AGENTS.md deleted file mode 100644 index 9c5ea4a..0000000 --- a/AGENTS.md +++ /dev/null @@ -1,287 +0,0 @@ -# AGENTS.md — working in zstd-nginx-module - -Guidance for AI agents (and humans) modifying this repository. Everything -here was verified against the source at the time of writing; if code and -this file disagree, **trust the code and fix this file**. - -## What this is - -An nginx C module providing Zstandard compression, in two parts compiled -together: - -| Module | File | Role | -|---|---|---| -| `ngx_http_zstd_filter_module` | `filter/ngx_http_zstd_filter_module.c` | On-the-fly response compression (HTTP body filter) | -| `ngx_http_zstd_static_module` | `static/ngx_http_zstd_static_module.c` | Serves pre-compressed `.zst` files (gzip_static equivalent) | -| shared | `ngx_http_zstd_common.h` | `static inline` helpers used by both — notably `ngx_http_zstd_accept_encoding()` (RFC 7231 parser) and `ngx_http_zstd_ok()` | - -Build glue: top-level `config` sources `filter/config` + `static/config` -(autoconf-style nginx add-module). `ZSTD_INC` / `ZSTD_LIB` env vars -select a custom libzstd; unset = system zstd. Dynamic linking is -preferred by the configs. - -## Hard rules - -1. **No AI co-author trailers.** Never add `Co-Authored-By: Claude ...` - (or any AI attribution) to commits, squash messages, or PR bodies. - This is a standing project rule (also in `/opt/packages/CLAUDE.md`). -2. **Never commit to `master` directly.** Branch, PR, let CI go green, - squash-merge. The repo takes no required status checks today, but - merging red is not acceptable. -3. **Don't fabricate work.** If an investigation shows a proposed change - is unnecessary (e.g. the audited `disable_symlinks` "gap" that turned - out to already be correct), drop it and say so — do not invent a - diff to make a task look done. -4. **Verify, don't assume.** Build it, run the suite, exercise the path - against a real nginx before claiming it works. Local YAML/lint - passing is necessary, not sufficient. - -## Repository layout - -``` -filter/ngx_http_zstd_filter_module.c filter module (~1.2k LoC) -static/ngx_http_zstd_static_module.c static module -ngx_http_zstd_common.h shared inline helpers -config, filter/config, static/config nginx add-module build glue -t/00-filter.t Perl Test::Nginx::Socket suite (filter) -t/01-static.t Perl Test::Nginx::Socket suite (static) -t/suite/ test fixture (`test`, `test.zst`) -tools/test_encoding.py end-to-end smoke tester (decompresses, asserts) -tools/test_terminal_frame.py empty-output ZSTD_e_end regression -tools/test_reload_leak.sh CDict reload-leak check (run under ASAN) -tools/test_test_*.py unit tests for the Python harness itself -tools/ci-build.sh local "build nginx + module" helper -fuzz/ libFuzzer harness (see below) -.github/workflows/ build-test / codeql / security-scanners / fuzzing -valgrind.suppress local valgrind suppressions -.clang-format C style -``` - -## Directives (source of truth: the `ngx_command_t` tables) - -Filter module — context `http, server, location` unless noted: - -| Directive | Notes | -|---|---| -| `zstd on\|off` | also valid in `if` (LIF) | -| `zstd_comp_level` | int; supports negative levels (custom setter) | -| `zstd_types` | mime list; default is common textual web formats (see README) | -| `zstd_buffers` | num size | -| `zstd_min_length` | size; default `1024` | -| `zstd_max_length` | size; default unset. Enforced in the header filter on known `Content-Length` **and** in the body filter on chunked/no-length responses (aborts the request) | -| `zstd_target_cblock_size` | size; needs libzstd ≥ 1.5.6 | -| `zstd_window_log` | int exponent; caps `ZSTD_c_windowLog` → per-request memory ceiling | -| `zstd_dict_file` | **`http` context only** (`NGX_HTTP_MAIN_CONF`) | - -Static module: `zstd_static on|off|always`. - -Variables (log-phase, backed by `ctx->bytes_in/bytes_out`): `$zstd_ratio`, -`$zstd_bytes_in`, `$zstd_bytes_out`. - -> Note: there is **no `zstd_bypass`** on `master` as of this writing -> (it exists on a feature branch / open PR). Check the command table -> before assuming a directive exists. Conversely, do not "add" a -> directive that an open PR already introduces — check open PRs first. - -## Adding a directive — the established pattern - -Mirror `zstd_target_cblock_size` / `zstd_window_log` exactly: - -1. Field in `ngx_http_zstd_loc_conf_t` (or `_main_conf_t` for http-only). -2. Entry in `ngx_http_zstd_filter_commands[]` (right slot fn: - `ngx_conf_set_num_slot` for ints, `_size_slot` for sizes, - `ngx_http_set_predicate_slot` for variable lists, etc.). -3. Init in `create_loc_conf` (`NGX_CONF_UNSET` / `NGX_CONF_UNSET_PTR`). -4. `ngx_conf_merge_*` in `merge_loc_conf` with the documented default. -5. If it drives the encoder: apply via `ZSTD_CCtx_setParameter` in - `ngx_http_zstd_filter_init_cctx`, **always** guarded by - `ZSTD_isError(rc)` → log `NGX_LOG_ALERT` → `return NGX_ERROR` - (graceful per-request fallback to uncompressed). -6. Document in `README.md` (directive section **and** the TOC). -7. Add a regression test (see below) and update the plan constant. - -Prefer nginx's own primitives over hand-rolled logic (e.g. -`ngx_http_test_predicates`, `ngx_http_set_disable_symlinks`). - -## Testing - -Build a test nginx (the suites need a binary, not just headers): - -```bash -bash tools/ci-build.sh # nginx 1.31.0 by default -# or a full configure with --add-module=$(pwd) + extra http modules -``` - -Run the Perl suites (require `Test::Nginx::Socket`, typically under -`~/perl5`, so set `PERL5LIB`): - -```bash -export PERL5LIB=$HOME/perl5/lib/perl5 -export TEST_NGINX_BINARY=/path/to/built/nginx -export TEST_NGINX_SERVROOT=/tmp/srv && mkdir -p "$TEST_NGINX_SERVROOT" -prove t/00-filter.t t/01-static.t -``` - -End-to-end + harness unit tests: - -```bash -python3 tools/test_encoding.py --nginx-binary /path/to/nginx -python3 tools/test_terminal_frame.py --nginx-binary /path/to/nginx -(cd tools && python3 -m unittest test_test_encoding test_test_package_artifact) -bash tools/test_reload_leak.sh /path/to/asan-built/nginx # real under ASAN -``` - -### Perl suite gotchas (these will bite you) - -- **`plan tests => repeat_each() * (blocks() * 3) + N;`** — `blocks()` - auto-counts, but the `+ N` constant is a hand-maintained "extra - assertions" budget. Adding/removing a test, or using - `--- ignore_response`, changes the real subtest count. Run the suite, - read the `planned X but ran Y` line, adjust `N` by the delta. A "Bad - plan" with `0 Failed` is *only* the constant being off. - Current: filter `+147`, static `+63`. (38 filter tests, 20 static.) -- `--- config` is injected inside `server{}`. `http`-context directives - (`map`, `zstd_dict_file`, `log_format`) must go in `--- http_config`. -- `$TEST_NGINX_HTML_DIR` is **not** a shell env var and the literal - token (even inside a `#` comment in a config section) triggers a - Test::Nginx substitution bail. Use `$TEST_NGINX_SERVER_ROOT` for - absolute paths; `--- user_files` land in `/html`. -- A deliberately aborted request needs `--- ignore_response` plus - `--- error_log` (assert the log line) — not `--- response_headers`. -- Static suite needs `touch -d @1541504307 t/suite/test t/suite/test.zst` - and a servroot positioned so `root ../../t/suite` resolves (CI uses - `TEST_NGINX_SERVROOT=$GITHUB_WORKSPACE/t/servroot-static`). - -## Fuzzing (`fuzz/`) - -Target: `ngx_http_zstd_accept_encoding()` and the -`ngx_http_zstd_eval_qvalue()` helper it calls. Both are **sliced from -the shipped header at build time** by `fuzz/extract_parser.sh` into -`generated_parser.inc` (gitignored), in definition order — there is -intentionally **no hand-maintained copy**. `fuzz/ngx_shim.h` reproduces the few nginx -primitives the parser needs, copied faithfully from upstream -`src/core/ngx_string.{c,h}` with citations. - -```bash -bash fuzz/build.sh && ./fuzz/fuzz_accept_encoding -max_total_time=60 fuzz/corpus/ -``` - -If you change the parser signature/body and the fuzz build fails, -`extract_parser.sh`'s anchor regex needs updating — it fails loudly on -purpose rather than fuzz nothing. Don't commit `fuzz/corpus/` units a -local run discovered; only the curated `NN_name` seeds are tracked -(enforced by `.gitignore`). - -## CI (4 workflows; actionlint gates them) - -| File | Name | Scope | -|---|---|---| -| `build-test.yml` | Build & Test | actionlint + lint, build matrix **nginx 1.31.0 mainline + Angie 1.11.5**, Perl/Python tests, ASAN/UBSAN | -| `codeql.yml` | CodeQL | `security-extended`; needs a full `make` (static `--add-module` compiles into the binary — `make modules` sees nothing) | -| `security-scanners.yml` | Security Scanners | flawfinder / clang-tidy / semgrep → SARIF | -| `fuzzing.yml` | Fuzzing | nightly + bounded-PR libFuzzer | - -CI-specific facts that have caused failures before: - -- **Actions expression contexts**: `${{ env.X }}` is **not** allowed in - a job-level `name:` (it is fine in `with:`/`run:`/`path:`/`key:`). - `${{ matrix.X }}` *is* allowed in a job `name:`. actionlint catches - this; run it locally on every workflow edit. -- All third-party actions are **pinned to commit SHAs**; the canonical - pin table is the header comment in `build-test.yml` — keep all four - files consistent with it. -- `run:` blocks are shellcheck'd by actionlint. Quote variables; for - flag-list variables that *must* word-split into compiler args, use a - scoped `# shellcheck disable=SC2086` **with a rationale comment**. -- CodeQL "default setup" (a repo Settings toggle) conflicts with the - advanced `codeql.yml` and rejects its SARIF. It is currently disabled - via the API; this is a repo setting, not in any diff. - -Validate locally before pushing: - -```bash -actionlint .github/workflows/*.yml # must exit 0 -python3 -c "import glob,yaml;[yaml.safe_load(open(f)) for f in glob.glob('.github/workflows/*.yml')]" -``` - -## Module behaviour worth knowing before you touch it - -- Per-request `ZSTD_CCtx` is created/reset per request and freed via a - pool cleanup (a recurring historical bug area — `774b4a5`, `a209f96`). - Don't reintroduce worker-global compression state. -- The header filter gate (status/length/type/`header_only`) decides - eligibility; `ngx_http_zstd_ok()` checks `Accept-Encoding`. The body - filter runs the streaming compress loop with a `failed:` path that - sets `ctx->done` and returns `NGX_ERROR`. -- Once compression starts the client is mid-`Content-Encoding: zstd` - stream — you **cannot** switch to passthrough; the only safe failure - is aborting the request (this is why the chunked `max_length` cap - aborts rather than passes through). -- `$zstd_ratio` etc. are log-phase only — `not_found` until - `ctx->done`. They are not usable in `add_header`. - -## PR conventions - -- One logical change per PR, scoped commit message, no AI co-author. -- If multiple in-flight PRs each append tests to `t/00-filter.t` off - `master`, they will collide on test numbering + the plan constant. - Flag this in the PR body; whichever merges later gets a trivial - renumber rebase. Don't hide the conflict. - -### Stacked PRs (one PR per issue/change) - -When a single piece of work decomposes into several independent -changes, ship them as a **stack** — one PR per logical change, each -based on the previous PR's branch rather than all on `master`. This -keeps every PR small and individually reviewable while preserving the -real dependency order, instead of one sprawling diff or several PRs -that secretly conflict. - -Rules: - -- **One PR per issue/change.** Each branch contains exactly one - logical change and its docs (README reference **and** TOC) — never - bundle an unrelated fix because it was convenient. -- **Push and open each PR immediately, before starting the next one.** - Don't batch the work and push at the end. The moment a PR's code is - committed and locally verified, push the branch and open the PR so - CI starts running on it while you build the next PR in the stack. CI - here takes several minutes (CodeQL, ASAN/UBSAN, multi-target builds); - pushing eagerly overlaps that wall-clock time with development - instead of serialising it. By the time the stack is fully written, - the lower PRs' CI is usually already green. -- **Base each PR on the one below it**, not on `master`. The base - branch of PR _n_ is the head branch of PR _n−1_. Set it explicitly: - `gh pr create --base `. -- **Order by dependency, lowest-risk first.** Pure refactors and - hot-path cleanups go at the bottom of the stack; new directives and - default changes on top. A reviewer reading bottom-up sees each diff - in isolation. -- **State the stack in every PR body.** List the full stack with links - and mark this PR's position, e.g. `Stack: #12 ← #13 (this) ← #14`, - and note "merge bottom-up". -- **Merge strictly bottom-up, and do NOT delete the base branch on - merge.** Squash-merge the base PR with the branch **kept** - (`--squash` without `--delete-branch`). Deleting the base branch - does **not** auto-retarget the next PR — GitHub *closes* it (a PR - whose base branch no longer exists cannot stay open and cannot be - reopened against the missing base; you must recreate it). Instead, - after the base merges: retarget the next PR to `master` explicitly - via the REST API (`gh api -X PATCH .../pulls/N -f base=master` — - `gh pr edit --base` can fail on the projects-classic GraphQL - deprecation), then rebase its branch onto the new `master` and - force-push so CI re-runs against the real post-merge base (a - retarget alone does not re-trigger the workflows). Only delete all - the stack branches at the very end, once everything is merged. - Never merge a higher PR before its base — that pulls the lower - change's diff in with it. -- **A rejected lower PR collapses the stack.** If the base PR needs - rework, rebase the rest of the stack onto its updated branch before - continuing; don't let higher PRs drift onto stale bases. -- Update `README.md` (reference **and** TOC) in the same PR as any - user-visible directive/variable change. Keep `.github/CI_SETUP.md` - truthful if you change CI structure. -- `gh` may resolve the wrong repo here (multiple remotes — `origin` = - `myguard-labs/nginx-zstd-module`, `upstream` = `tokers/zstd-nginx-module`). Pass - `--repo myguard-labs/nginx-zstd-module` or rely on the configured - default. diff --git a/CHANGES b/CHANGES new file mode 100644 index 0000000..dbd69ff --- /dev/null +++ b/CHANGES @@ -0,0 +1,13 @@ +Changes with http-zstd + + *) Added CI matrix building nginx mainline, nginx stable, and angie + (monthly, in ci-deep.yml's build-flavors job), running the Perl + Test::Nginx suite against each. + + *) Extended tools/ci-build.sh to accept a flavor argument (nginx|angie) + and persist the built server + both dynamic modules under .build/, + instead of only building pinned nginx mainline into a /tmp dir that + got deleted on exit. + + *) Added tools/bump-versions.sh + weekly bump.yml workflow to keep the + nginx-stable/angie pins current. diff --git a/tools/bump-versions.sh b/tools/bump-versions.sh new file mode 100755 index 0000000..30e7487 --- /dev/null +++ b/tools/bump-versions.sh @@ -0,0 +1,149 @@ +#!/usr/bin/env bash +# +# Check nginx.org/angie.software for newer releases than what's pinned in +# ci-deep.yml's build-flavors matrix, and rewrite those pins in place. Called +# by .github/workflows/bump.yml on a schedule; also runnable locally to +# preview a bump before it lands. +# +# tools/bump-versions.sh [--dry-run] +# +# What gets bumped, and why nginx MAINLINE is deliberately NOT one of them: +# - nginx mainline is resolved at CI run time (NGINX_VERSION: "" in +# build-test.yml / ci-deep.yml's env: block, filled by each workflow's own +# curl-nginx.org scrape — see build-test.yml's `resolve` job and +# ci-deep.yml's per-job "Resolve latest mainline nginx" steps). There is +# no static mainline pin in this repo to bump. +# - nginx STABLE pin -- ci-deep.yml's build-flavors matrix (label: stable) +# - angie pin -- ci-deep.yml's build-flavors matrix (label: angie) +# - ANGIE_SHA256 -- tools/ci-build.sh (nginx tarballs are PGP-verified +# at build time instead, so no nginx sha256 table +# exists to bump) +# +# 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 +# for angie) -- so every angie version edit here is paired with a digest +# computed from the exact tarball that version resolves to, never carried +# over from a previous entry. + +set -euo pipefail + +DRY_RUN=0 +[ "${1:-}" = "--dry-run" ] && DRY_RUN=1 + +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_stable() { + local page + page="$(curl -fsSL https://nginx.org/en/download.html)" + echo "${page#*"Stable version"}" \ + | 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 + json="$(curl -fsSL https://api.github.com/repos/webserver-llc/angie/releases/latest)" + echo "$json" | grep -m1 '"tag_name"' | grep -oE '[0-9]+\.[0-9]+\.[0-9]+' +} + +NEW_STABLE="$(latest_nginx_stable)" +NEW_ANGIE="$(latest_angie)" + +for v in 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 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_STABLE="$(matrix_version_for_label stable)" +CUR_ANGIE="$(matrix_version_for_label angie)" + +echo "pinned: nginx stable=$CUR_STABLE angie=$CUR_ANGIE" + +CHANGED=0 + +# --- sha256 helper (angie only -- nginx is PGP-verified at build time) --- +sha256_for_angie() { + local version="$1" url tmp digest + url="https://download.angie.software/files/angie-${version}.tar.gz" + tmp="$(mktemp)" + curl -fsSL "$url" -o "$tmp" + digest="$(sha256sum "$tmp" | awk '{print $1}')" + rm -f "$tmp" + echo "$digest" +} + +# --- bump a version in ci-deep.yml's build-flavors matrix ---------------- +bump_matrix_pin() { + local label="$1" old="$2" new="$3" + [ "$old" = "$new" ] && return 0 + 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_angie_sha256_pin() { + local old="$1" new="$2" digest="$3" + grep -q "\[\"${new}\"\]" 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 ANGIE_SHA256=(/a\\ [\"${new}\"]=\"${digest}\"" tools/ci-build.sh + CHANGED=1 +} + +if [ "$NEW_STABLE" != "$CUR_STABLE" ]; then + echo "bump nginx stable: $CUR_STABLE -> $NEW_STABLE" + if [ "$DRY_RUN" = 0 ]; then + bump_matrix_pin stable "$CUR_STABLE" "$NEW_STABLE" + else + CHANGED=1 + 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_angie_sha256_pin "$CUR_ANGIE" "$NEW_ANGIE" "$DIGEST" + else + CHANGED=1 + fi +fi + +if [ "$CHANGED" = 0 ]; then + echo "everything up to date, nothing to bump" +fi + +echo "CHANGED=$CHANGED" diff --git a/tools/ci-build.sh b/tools/ci-build.sh old mode 100644 new mode 100755 index db32f93..f1a09b6 --- a/tools/ci-build.sh +++ b/tools/ci-build.sh @@ -1,102 +1,168 @@ #!/bin/bash -# Build and test zstd-nginx-module against nginx -# Usage: ./tools/ci-build.sh [nginx-version] -# Default: latest mainline, resolved from nginx.org (not pinned). - -set -e +# Build and test zstd-nginx-module against nginx or angie. +# +# tools/ci-build.sh [flavor] [version] +# flavor : nginx (default) | angie +# version: source version, e.g. 1.31.2. Omit (or pass "" with flavor=nginx) +# to resolve the current mainline release from nginx.org. +# +# Built tree persists at .build/-/objs/{,ngx_http_zstd_*.so} +# so a caller (a CI job) can find the binary and both dynamic modules after +# the script exits — unlike the old single-nginx version of this script, which +# built under /tmp and deleted it on exit. NO_CACHE=1 forces a from-scratch +# rebuild of a given flavor/version pair. +# +# nginx tarballs are verified via PGP (nginx.org's own recommended method, +# https://nginx.org/en/pgp_keys.html) — no sha256sum file is published for +# them. angie.software does not publish a PGP signature for release tarballs, +# so angie tarballs are instead checked against a pinned sha256 recorded +# below, computed once from an HTTPS fetch (same approach the sibling +# nginx-skeleton-module repo uses for both flavors). A version not in the +# table still builds (this script tracks moving releases; refusing to build +# an unpinned version would break every future version bump until someone +# updates the table first) but prints a loud warning so the gap is visible. + +set -euo pipefail SCRIPT_DIR="$(cd "$(dirname "${BASH_SOURCE[0]}")" && pwd)" MODULE_DIR="$(dirname "$SCRIPT_DIR")" -if [ -n "${1:-}" ]; then - NGINX_VERSION="$1" -else + +FLAVOR="${1:-nginx}" +VERSION="${2:-}" + +case "$FLAVOR" in + nginx|angie) ;; + *) + echo "ERROR: unsupported flavor: $FLAVOR (want: nginx|angie)" >&2 + exit 2 + ;; +esac + +if [ -z "$VERSION" ]; then + if [ "$FLAVOR" != "nginx" ]; then + echo "ERROR: a version is required for flavor=$FLAVOR" >&2 + exit 2 + fi # Resolve the current mainline release; nginx.org only keeps the newest # mainline tarball, so a hardcoded version eventually 404s. - NGINX_VERSION="$(curl -fsSL https://nginx.org/en/download.html \ + VERSION="$(curl -fsSL https://nginx.org/en/download.html \ | grep -oP 'nginx-\K[0-9]+\.[0-9]+\.[0-9]+(?=\.tar\.gz)' \ | sort -V | tail -1)" - if [ -z "$NGINX_VERSION" ]; then + if [ -z "$VERSION" ]; then echo "ERROR: could not resolve mainline nginx version from nginx.org" >&2 exit 1 fi fi -BUILD_DIR="/tmp/nginx-build-$$" + +# --- pinned sha256 for angie tarballs we've actually verified --- +declare -A ANGIE_SHA256=( + ["1.11.5"]="b5f297c6df2a74b9d0091a7cdd747fffd2d0e1d0be43632da61c1c7539db2043" +) + +ROOT="${BUILD_ROOT:-$MODULE_DIR/.build}" +NO_CACHE="${NO_CACHE:-0}" ZSTD_MODULE_DIR="$MODULE_DIR" +case "$FLAVOR" in + nginx) + DIR="nginx-${VERSION}" + URL="https://nginx.org/download/${DIR}.tar.gz" + ;; + angie) + DIR="angie-${VERSION}" + URL="https://download.angie.software/files/${DIR}.tar.gz" + ;; +esac + +SRCDIR="$ROOT/${FLAVOR}-${VERSION}" +TARBALL="$ROOT/${DIR}.tar.gz" + echo "==========================================================================" echo " zstd-nginx-module CI Build" echo "==========================================================================" echo "" echo "Module: $ZSTD_MODULE_DIR" -echo "Nginx version: $NGINX_VERSION" -echo "Build directory: $BUILD_DIR" +echo "Flavor: $FLAVOR Version: $VERSION" +echo "Build tree: $SRCDIR" echo "" -# Cleanup on exit -cleanup() { - echo "" - echo "Cleaning up..." - rm -rf "$BUILD_DIR" -} -trap cleanup EXIT +mkdir -p "$ROOT" -# Create build directory -mkdir -p "$BUILD_DIR" -cd "$BUILD_DIR" +if [ "$NO_CACHE" = "1" ]; then + rm -rf "$SRCDIR" "$TARBALL" +fi -# Download nginx echo "==========================================================================" -echo "Phase 1: Downloading nginx $NGINX_VERSION" +echo "Phase 1: Downloading $FLAVOR $VERSION" echo "==========================================================================" echo "" -tarball="nginx-${NGINX_VERSION}.tar.gz" - -# Always fetch over HTTPS and verify the detached PGP signature against the -# nginx release-signing keys before unpacking. A plain HTTP download lets a -# network attacker swap the source that we then configure and compile. -if [ ! -f "$tarball" ]; then - wget -q "https://nginx.org/download/${tarball}" - echo "✓ Downloaded ${tarball}" +if [ ! -f "$TARBALL" ]; then + wget -q -O "$TARBALL" "$URL" + echo "✓ Downloaded ${DIR}.tar.gz" + + if [ "$FLAVOR" = "nginx" ]; then + # Always fetch over HTTPS and verify the detached PGP signature + # against the nginx release-signing keys before unpacking. A plain + # HTTP download lets a network attacker swap the source that we then + # configure and compile. + wget -q "${URL}.asc" -O "${TARBALL}.asc" + + gnupghome="$(mktemp -d)" + export GNUPGHOME="$gnupghome" + chmod 700 "$gnupghome" + for key in nginx_signing mdounin maxim sb thresh pluknet arut; do + wget -q "https://nginx.org/keys/${key}.key" -O - 2>/dev/null \ + | gpg --quiet --import 2>/dev/null || true + done + + if gpg --quiet --verify "${TARBALL}.asc" "$TARBALL"; then + echo "✓ PGP signature verified for ${DIR}.tar.gz" + else + echo "✗ PGP signature verification FAILED for ${DIR}.tar.gz" >&2 + rm -rf "$gnupghome" "$TARBALL" "${TARBALL}.asc" + exit 1 + fi + rm -rf "$gnupghome" + unset GNUPGHOME + else + EXPECTED="${ANGIE_SHA256[$VERSION]:-}" + if [ -n "$EXPECTED" ]; then + ACTUAL="$(sha256sum "$TARBALL" | awk '{print $1}')" + if [ "$ACTUAL" != "$EXPECTED" ]; then + echo "✗ sha256 mismatch for ${DIR}.tar.gz" >&2 + echo " expected: $EXPECTED" >&2 + echo " actual: $ACTUAL" >&2 + rm -f "$TARBALL" + exit 1 + fi + echo "✓ sha256 verified for ${DIR}.tar.gz" + else + echo "WARNING: no pinned sha256 for angie $VERSION -- add one to" \ + "ANGIE_SHA256 in tools/ci-build.sh (downloaded tarball is" \ + "UNVERIFIED)" >&2 + fi + fi else - echo "✓ Using cached ${tarball}" + echo "✓ Using cached ${DIR}.tar.gz" fi -# Detached signature. -wget -q "https://nginx.org/download/${tarball}.asc" -O "${tarball}.asc" - -# nginx release-signing public keys (served over HTTPS by nginx.org). Import -# whichever exist so verification keeps working as the active signer rotates. -gnupghome="$(mktemp -d)" -export GNUPGHOME="$gnupghome" -chmod 700 "$gnupghome" -for key in nginx_signing mdounin maxim sb thresh pluknet arut; do - wget -q "https://nginx.org/keys/${key}.key" -O - 2>/dev/null \ - | gpg --quiet --import 2>/dev/null || true -done - -if gpg --quiet --verify "${tarball}.asc" "$tarball"; then - echo "✓ PGP signature verified for ${tarball}" -else - echo "✗ PGP signature verification FAILED for ${tarball}" >&2 - rm -rf "$gnupghome" - exit 1 +if [ ! -d "$SRCDIR" ]; then + tar -xzf "$TARBALL" -C "$ROOT" + mv "$ROOT/$DIR" "$SRCDIR" fi -rm -rf "$gnupghome" -unset GNUPGHOME -tar xzf "$tarball" -cd "nginx-${NGINX_VERSION}" +cd "$SRCDIR" echo "" echo "==========================================================================" -echo "Phase 2: Configuring nginx with zstd module" +echo "Phase 2: Configuring $FLAVOR with zstd module" echo "==========================================================================" echo "" ./configure \ --prefix=/etc/nginx \ - --sbin-path=/usr/sbin/nginx \ + --sbin-path=/usr/sbin/"$FLAVOR" \ --with-http_ssl_module \ --with-http_gzip_static_module \ --with-http_realip_module \ @@ -108,11 +174,11 @@ echo "✓ Configuration complete" echo "" echo "==========================================================================" -echo "Phase 3: Compiling nginx" +echo "Phase 3: Compiling $FLAVOR" echo "==========================================================================" echo "" -make -j$(nproc) 2>&1 | tail -10 +make -j"$(nproc)" 2>&1 | tail -10 echo "" echo "==========================================================================" @@ -136,14 +202,17 @@ else exit 1 fi +if [ ! -f "objs/$FLAVOR" ]; then + echo "✗ $FLAVOR binary NOT found!" + exit 1 +fi + echo "" echo "==========================================================================" echo "✓ Build successful!" echo "==========================================================================" echo "" -echo "Modules ready:" -echo " - objs/ngx_http_zstd_filter_module.so" -echo " - objs/ngx_http_zstd_static_module.so" -echo "" -echo "Build directory: $BUILD_DIR" -echo "(Will be cleaned up on exit)" +echo "Server + modules ready in: $SRCDIR/objs/" +echo " - $FLAVOR" +echo " - ngx_http_zstd_filter_module.so" +echo " - ngx_http_zstd_static_module.so"