Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
14 changes: 14 additions & 0 deletions .github/workflows/build-test.yml
Original file line number Diff line number Diff line change
Expand Up @@ -139,6 +139,20 @@ jobs:
python3 -m unittest -v test_test_encoding test_test_package_artifact
echo "✓ Python harness unit tests passed"

# Audit sha e289021 F1/F7 regression coverage: no nginx build needed,
# so these run here rather than in the build/tests jobs.
- name: fuzz_seconds dispatch-input validation (F1 regression)
run: bash "$GITHUB_WORKSPACE"/tools/test_fuzz_seconds_validation.sh

- name: README/SECURITY/CONTRIBUTING vs live workflows (F7 regression)
run: bash "$GITHUB_WORKSPACE"/tools/test_docs_ci_drift.sh

# Audit sha e289021 F2/F3 regression coverage: exercises ci-build.sh's
# mv-guard and provenance checks directly (offline cases + one live
# clean-root angie build).
- name: ci-build.sh clean-root + provenance regressions (F2/F3)
run: bash "$GITHUB_WORKSPACE"/tools/test_ci_build_provenance.sh

- name: Shell script linting (shellcheck)
run: |
# Gate on error-severity findings (real bugs); style/info noise does
Expand Down
5 changes: 3 additions & 2 deletions README.md
Original file line number Diff line number Diff line change
Expand Up @@ -845,8 +845,8 @@ log_format zstd '$request in=$zstd_bytes_in out=$zstd_bytes_out '

Six workflows guard the module (badges at the top): five gate every
push & PR, and CI Deep runs the exhaustive monthly pass. A seventh,
Bump, runs weekly but only opens a version-bump PR — it does not gate
anything itself.
[`bump.yml`](.github/workflows/bump.yml) (Bump), runs weekly but only
opens a version-bump PR — it does not gate anything itself.

| Workflow | Cadence | What it does |
|---|---|---|
Expand All @@ -856,6 +856,7 @@ anything itself.
| **Valgrind** | every push & PR | A 60-second Memcheck-lite soak against a debug nginx build, catching uninitialised-value reads and leaks that ASAN cannot. |
| **CodeQL** | every push & PR + monthly | GitHub's semantic C/C++ analysis (`security-extended` query pack) over the filter/static module sources. |
| **CI Deep** | monthly + manual dispatch | The exhaustive run: a `build-flavors` matrix that compiles and runs the full `Test::Nginx::Socket` suite against **nginx mainline, nginx stable, and Angie** (the only workflow that builds Angie at all), hours-long fuzzing on the same target, full Memcheck and Helgrind soaks (a valgrind soak is ~20–50× slower than native), and the same security scanners. |
| **Bump** ([`bump.yml`](.github/workflows/bump.yml)) | weekly + manual dispatch | Checks nginx.org/angie.software for newer nginx-stable/Angie releases than what's pinned in CI Deep's `build-flavors` matrix, and opens a PR (never pushes directly to protected `master`) with the updated pin + a freshly-verified sha256 digest. Does not gate merges itself — normal required checks review the PR. |

The test suite includes a dedicated regression test for every known
historical bug class:
Expand Down
153 changes: 153 additions & 0 deletions tools/test_ci_build_provenance.sh
Original file line number Diff line number Diff line change
@@ -0,0 +1,153 @@
#!/usr/bin/env bash
#
# Regression tests for audit sha e289021 F2 and F3, both in tools/ci-build.sh:
#
# F2 - a fresh (or NO_CACHE=1) build always failed: the tarball's top-level
# dir name and the script's own SRCDIR were the same string, so
# extraction already landed at SRCDIR and the subsequent `mv` tried to
# move that dir onto itself ("cannot move to a subdirectory of
# itself"). This never actually built anything on a clean root.
# F3 - archive provenance: a wrong PGP signer, a corrupted/tampered
# (cache-hit) tarball, or a wrong pinned sha256 must all be rejected
# BEFORE extraction, not silently accepted.
#
# This test does not hit the network for the "must reject" cases -- it
# stages a fake $BUILD_ROOT by hand so it can run offline and fast. It DOES
# need network + real nginx.org access for the "F2: clean root actually
# reaches configure" case (skipped if offline).
#
# Usage: tools/test_ci_build_provenance.sh

set -uo pipefail

SCRIPT_DIR="$(cd "$(dirname "${BASH_SOURCE[0]}")" && pwd)"
CI_BUILD="$SCRIPT_DIR/ci-build.sh"

fail=0
WORK="$(mktemp -d)"
trap 'rm -rf "$WORK"' EXIT

# ---------------------------------------------------------------------
# F3a: wrong sha256 pin must be rejected for a statically-pinned version,
# even though the tarball content and PGP signature (if any) are untouched.
# We fabricate a tiny "tarball" and inject a bogus pin table entry via a
# throwaway copy of the script so the real pin table isn't touched.
# ---------------------------------------------------------------------
test_wrong_sha256_rejected() {
local root="$WORK/f3-sha256"
mkdir -p "$root"
# angie path is simpler to fake: sha256-only, no PGP.
echo "not a real tarball" >"$root/angie-9.9.9.tar.gz"

local copy="$WORK/ci-build-badpin.sh"
cp "$CI_BUILD" "$copy"
# Force a pin that cannot possibly match the fake tarball's digest.
sed -i 's/\["1\.11\.5"\]=.*/["9.9.9"]="0000000000000000000000000000000000000000000000000000000000000"/' "$copy"

if BUILD_ROOT="$root" bash "$copy" angie 9.9.9 >"$root/out.log" 2>&1; then
echo "FAIL: ci-build.sh accepted a tarball with a mismatched pinned sha256"
cat "$root/out.log"
return 1
fi
if ! grep -q "sha256 mismatch" "$root/out.log"; then
echo "FAIL: rejection happened but not via the expected sha256-mismatch path:"
cat "$root/out.log"
return 1
fi
echo "✓ F3: wrong pinned sha256 rejected before extraction"
}

# ---------------------------------------------------------------------
# F3b: corrupted CACHED tarball must be re-verified and rejected, not
# silently reused just because a file with the right name already exists.
# (This was the literal pre-fix gap: verification only ran on fresh
# download.) Same fake-tarball approach, but this time the "cache" is
# pre-populated before the script runs.
# ---------------------------------------------------------------------
test_corrupted_cache_rejected() {
local root="$WORK/f3-cache"
mkdir -p "$root"
echo "corrupted cached tarball" >"$root/angie-1.11.5.tar.gz"

if BUILD_ROOT="$root" bash "$CI_BUILD" angie 1.11.5 >"$root/out.log" 2>&1; then
echo "FAIL: ci-build.sh accepted a corrupted CACHED tarball for a pinned version"
cat "$root/out.log"
return 1
fi
if ! grep -q "sha256 mismatch" "$root/out.log"; then
echo "FAIL: cache-hit path did not re-verify sha256:"
cat "$root/out.log"
return 1
fi
echo "✓ F3: corrupted cached tarball re-verified and rejected on cache-hit"
}

# ---------------------------------------------------------------------
# F2: clean-root / NO_CACHE=1 extraction must not fail with "mv onto
# itself". We can exercise the mv-guard logic in isolation (no network)
# by reproducing exactly what ci-build.sh does: extract a tarball whose
# top-level dir name equals $FLAVOR-$VERSION, then run the same guarded
# mv line. This proves the guard itself is correct without needing a real
# nginx/angie tarball.
# ---------------------------------------------------------------------
test_mv_guard_clean_root() {
local root="$WORK/f2-mv"
mkdir -p "$root"
local dir="fake-1.0.0"
mkdir -p "$root/$dir"
touch "$root/$dir/marker"
tar -czf "$root/$dir.tar.gz" -C "$root" "$dir"
rm -rf "${root:?}/$dir"

local srcdir="$root/$dir" # same string ci-build.sh derives: "$ROOT/${FLAVOR}-${VERSION}"

# Reproduce ci-build.sh's exact extraction + guarded-mv sequence.
if [ ! -d "$srcdir" ]; then
tar -xzf "$root/$dir.tar.gz" -C "$root"
if [ "$root/$dir" != "$srcdir" ]; then
mv "$root/$dir" "$srcdir"
fi
fi

if [ ! -f "$srcdir/marker" ]; then
echo "FAIL: F2 mv-guard: extracted tree missing after guarded mv (regressed to pre-fix unconditional mv-onto-self failure mode)"
return 1
fi
echo "✓ F2: guarded mv is a no-op when extraction already lands at SRCDIR (clean-root case)"
}

# ---------------------------------------------------------------------
# F2 (live): actually run ci-build.sh against angie on a clean root with
# NO_CACHE=1 and confirm it reaches configure (this is the case that was
# unconditionally broken pre-fix). Needs network; skipped if unreachable.
# ---------------------------------------------------------------------
test_clean_root_live() {
if ! curl -fsS -o /dev/null --max-time 5 https://download.angie.software/ 2>/dev/null; then
echo "SKIP: F2 live clean-root build (no network to download.angie.software)"
return 0
fi
local root="$WORK/f2-live"
mkdir -p "$root"
if ! NO_CACHE=1 BUILD_ROOT="$root" timeout 300 bash "$CI_BUILD" angie 1.11.5 >"$root/out.log" 2>&1; then
echo "FAIL: F2 live: clean-root NO_CACHE=1 angie build did not reach a successful build:"
tail -40 "$root/out.log"
return 1
fi
if ! grep -q "cannot move to a subdirectory of itself" "$root/out.log"; then
echo "✓ F2: live clean-root NO_CACHE=1 angie build succeeded, no mv-onto-self failure"
else
echo "FAIL: F2 regressed -- mv-onto-self error resurfaced in a live clean-root build"
return 1
fi
}

test_wrong_sha256_rejected || fail=1
test_corrupted_cache_rejected || fail=1
test_mv_guard_clean_root || fail=1
test_clean_root_live || fail=1

if [ "$fail" -ne 0 ]; then
echo "FAIL: ci-build.sh provenance/clean-root regression"
exit 1
fi
echo "✓ all ci-build.sh F2/F3 regression cases pass"
71 changes: 71 additions & 0 deletions tools/test_docs_ci_drift.sh
Original file line number Diff line number Diff line change
@@ -0,0 +1,71 @@
#!/usr/bin/env bash
#
# Regression test for audit sha e289021 F7: README/SECURITY/CONTRIBUTING
# drifted materially from the live CI workflows (stale badge count, dead
# AGENTS.md link, a workflow present in .github/workflows/ but undocumented,
# a false -Werror claim). The full fix was a one-time manual re-sync;
# nothing then stopped it drifting again on the next workflow add/rename.
#
# This does NOT try to keep exact TAP subtest counts in sync (that's
# exactly the kind of brittle exact-count claim the F7 fix already
# recommended dropping) -- it checks structural facts that are cheap to
# keep true and expensive to silently get wrong:
# - every workflow file under .github/workflows/ is referenced somewhere
# in README.md (by filename), so a new/renamed workflow can't go
# undocumented
# - every workflow referenced in README.md's badges/table actually exists
# under .github/workflows/, so a removed/renamed workflow can't leave a
# dead link/badge behind
# - SECURITY.md does not link the removed AGENTS.md
# - CONTRIBUTING.md does not claim the fuzz harness builds with -Werror
# unless fuzz/build.sh (or equivalent) actually passes it
#
# Usage: tools/test_docs_ci_drift.sh

set -uo pipefail

SCRIPT_DIR="$(cd "$(dirname "${BASH_SOURCE[0]}")" && pwd)"
MODULE_DIR="$(dirname "$SCRIPT_DIR")"
cd "$MODULE_DIR" || exit 1

fail=0

# --- every real workflow file is mentioned in README.md ---
for wf in .github/workflows/*.yml; do
name="$(basename "$wf")"
if ! grep -q "$name" README.md; then
echo "FAIL: .github/workflows/$name exists but is not referenced in README.md (undocumented workflow)"
fail=1
fi
done

# --- every workflow filename README.md references actually exists ---
while IFS= read -r name; do
[ -f ".github/workflows/$name" ] || {
echo "FAIL: README.md references .github/workflows/$name, which does not exist (stale/dead link or badge)"
fail=1
}
done < <(grep -oE 'workflows/[A-Za-z0-9_-]+\.yml' README.md | sed 's#workflows/##' | sort -u)

# --- SECURITY.md must not link the removed AGENTS.md ---
if [ -f SECURITY.md ] && grep -q 'AGENTS\.md' SECURITY.md; then
echo "FAIL: SECURITY.md still links AGENTS.md, which was removed (PR #79)"
fail=1
fi

# --- CONTRIBUTING.md's -Werror claim about the fuzz harness must match
# reality: only assert this if a fuzz build step actually exists and does
# NOT pass -Werror, since a future CI change legitimately flipping this on
# would make the check obsolete rather than wrong. ---
if [ -f CONTRIBUTING.md ] && grep -qi 'fuzz.*-Werror\|-Werror.*fuzz' CONTRIBUTING.md; then
if ! grep -rq -- '-Werror' fuzz/ 2>/dev/null && ! grep -rlq -- '-Werror' .github/workflows/*.yml 2>/dev/null; then
echo "FAIL: CONTRIBUTING.md claims the fuzz harness builds with -Werror, but no fuzz build step passes it"
fail=1
fi
fi
Comment on lines +60 to +65

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🎯 Functional Correctness | 🟠 Major | ⚡ Quick win

Bind the -Werror check to the fuzz build.

An unrelated workflow containing -Werror satisfies Line 61, so this regression test can pass while the fuzz harness no longer uses that flag. Restrict the workflow check to the fuzz build step/workflow.

🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@tools/test_docs_ci_drift.sh` around lines 60 - 65, Update the -Werror
validation in the drift check around the grep conditions so workflow matches are
restricted to the fuzz build step or fuzz-specific workflow, rather than any
.github workflow containing -Werror. Preserve the existing direct fuzz/ source
check and failure message, ensuring unrelated workflow flags cannot satisfy the
test.


if [ "$fail" -ne 0 ]; then
echo "FAIL: docs/CI drift detected (audit sha e289021 F7 regression)"
exit 1
fi
echo "✓ README/SECURITY/CONTRIBUTING workflow references match .github/workflows/ contents"
103 changes: 103 additions & 0 deletions tools/test_fuzz_seconds_validation.sh
Original file line number Diff line number Diff line change
@@ -0,0 +1,103 @@
#!/usr/bin/env bash
#
# Regression test for audit sha e289021 F1: ci-deep.yml's "Select fuzz
# duration" step used to splice workflow_dispatch's fuzz_seconds input
# directly into a run: shell command, letting a quoted payload (e.g.
# `3600"; id; #`) execute on the self-hosted builder02 runner. The fix
# passes the raw value through env: and validates it in Bash before use.
#
# This script extracts that exact validation logic (kept byte-identical
# below to .github/workflows/ci-deep.yml's "Select fuzz duration" step) and
# drives it against malicious/malformed/boundary inputs, asserting each is
# rejected, plus a couple of valid inputs that must be accepted.
#
# Usage: tools/test_fuzz_seconds_validation.sh

set -uo pipefail

SCRIPT_DIR="$(cd "$(dirname "${BASH_SOURCE[0]}")" && pwd)"
WORKFLOW="$SCRIPT_DIR/../.github/workflows/ci-deep.yml"

fail=0

# Runs the same validation the workflow step performs, given
# RAW_FUZZ_SECONDS, and prints either "FUZZ_SECS=<n>" (accept) or nothing
# with a nonzero exit (reject) -- mirrors the workflow's own exit-1-on-error
# shape, minus the GitHub Actions ::error:: annotation syntax.
validate() {
local RAW_FUZZ_SECONDS="$1"
case "$RAW_FUZZ_SECONDS" in
'' | *[!0-9]*)
return 1
;;
esac
if [ "$RAW_FUZZ_SECONDS" -gt 14400 ]; then
return 1
fi
printf 'FUZZ_SECS=%s\n' "$RAW_FUZZ_SECONDS"
Comment on lines +29 to +37

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🩺 Stability & Availability | 🟠 Major | ⚡ Quick win

🧩 Analysis chain

🏁 Script executed:

#!/usr/bin/env bash
raw='999999999999999999999999999999999999'
if [ "$raw" -gt 14400 ]; then
  echo rejected
else
  echo accepted
fi

Repository: myguard-labs/nginx-zstd-module

Length of output: 260


🏁 Script executed:

#!/usr/bin/env bash
set -euo pipefail

printf '=== tools/test_fuzz_seconds_validation.sh ===\n'
wc -l tools/test_fuzz_seconds_validation.sh
sed -n '1,140p' tools/test_fuzz_seconds_validation.sh

printf '\n=== search FUZZ_SECS / fuzz seconds references ===\n'
rg -n "FUZZ_SECS|RAW_FUZZ_SECONDS|fuzz seconds|fuzz_seconds|14400" . -g '!**/.git/**' -g '!**/node_modules/**'

printf '\n=== tools/test_ci_build_provenance.sh ===\n'
if [ -f tools/test_ci_build_provenance.sh ]; then
  wc -l tools/test_ci_build_provenance.sh
  sed -n '1,220p' tools/test_ci_build_provenance.sh
fi

Repository: myguard-labs/nginx-zstd-module

Length of output: 12071


Reject oversized digit strings before the -gt compare. A very long numeric input makes [ "$RAW_FUZZ_SECONDS" -gt 14400 ] error out, so validate can still emit FUZZ_SECS for an over-budget duration. Add a range-safe guard before the numeric comparison and a regression case for an oversized digit string.

🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@tools/test_fuzz_seconds_validation.sh` around lines 29 - 37, Update the
validation logic around RAW_FUZZ_SECONDS in validate to reject digit strings
exceeding the supported range before invoking the [ numeric comparison,
preventing arithmetic errors from reaching the FUZZ_SECS output. Add a
regression case in the test script using an oversized all-digit input and verify
validation fails without emitting FUZZ_SECS.

}

check_rejected() {
local input="$1" desc="$2"
if out="$(validate "$input" 2>/dev/null)"; then
echo "FAIL: expected reject for $desc (input=$input), got accepted: $out"
fail=1
else
echo "✓ rejected: $desc"
fi
}

check_accepted() {
local input="$1" expect="$2"
out="$(validate "$input" 2>/dev/null)"
if [ "$out" != "FUZZ_SECS=$expect" ]; then
echo "FAIL: expected accept for input=$input -> FUZZ_SECS=$expect, got: $out"
fail=1
else
echo "✓ accepted: $input -> $out"
fi
}

# --- injection payloads (the actual F1 exploit shape) ---
check_rejected '3600"; id; #' "quote-breakout shell injection"
check_rejected '$(id)' "command substitution"
check_rejected '`id`' "backtick substitution"
check_rejected '3600; rm -rf /' "semicolon-chained command"
check_rejected '3600 && id' "shell-and injection"
check_rejected $'3600\nid' "embedded newline"

# --- malformed / non-numeric ---
check_rejected '' "empty string"
check_rejected 'abc' "non-numeric"
check_rejected '-100' "negative number"
check_rejected '3600.5' "decimal"
check_rejected ' 3600' "leading whitespace"
check_rejected '3600 ' "trailing whitespace"

# --- boundary: over budget ---
check_rejected '14401' "one over the 14400s budget"
check_rejected '999999999' "way over budget"

# --- must still accept legitimate values ---
check_accepted '3600' 3600
check_accepted '0' 0
check_accepted '14400' 14400

# --- drift guard: fail loudly if the workflow's validation logic changes
# shape without this test being updated to match ---
if ! grep -q "'\*\[!0-9\]\*'" "$WORKFLOW" 2>/dev/null; then
if ! grep -q '\*\[!0-9\]\*' "$WORKFLOW"; then
echo "FAIL: ci-deep.yml's non-numeric guard pattern not found -- validate() above may have drifted from the live workflow"
fail=1
fi
fi
if ! grep -q '14400' "$WORKFLOW"; then
echo "FAIL: ci-deep.yml no longer mentions the 14400s budget -- validate() above may have drifted from the live workflow"
fail=1
fi
Comment on lines +86 to +97

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🎯 Functional Correctness | 🔵 Trivial | 🏗️ Heavy lift

🧩 Analysis chain

🏁 Script executed:

#!/bin/bash
set -euo pipefail

printf '\n== repo files ==\n'
git ls-files tools/test_fuzz_seconds_validation.sh tools/test_ci_build_provenance.sh tools/ci-build.sh .github/workflows/ci-deep.yml

printf '\n== outlines ==\n'
for f in tools/test_fuzz_seconds_validation.sh tools/test_ci_build_provenance.sh tools/ci-build.sh .github/workflows/ci-deep.yml; do
  [ -f "$f" ] && { echo "--- $f"; wc -l "$f"; sed -n '1,220p' "$f" | cat -n | sed -n '1,220p'; }
done

Repository: myguard-labs/nginx-zstd-module

Length of output: 32213


🏁 Script executed:

#!/bin/bash
set -euo pipefail

printf '\n== exact references ==\n'
rg -n "grep -q|env|dispatch|validated|non-numeric|14400|guard|tar -xzf|guarded-mv|mv " tools/test_fuzz_seconds_validation.sh tools/test_ci_build_provenance.sh tools/ci-build.sh .github/workflows/ci-deep.yml

printf '\n== structural map for ci-build ==\n'
ast-grep outline tools/ci-build.sh --view expanded || true

Repository: myguard-labs/nginx-zstd-module

Length of output: 4678


Assert the workflow wiring, not just the copied guard.

  • tools/test_fuzz_seconds_validation.sh can still pass if fuzz_seconds is interpolated directly into run:; add a check for the env: RAW_FUZZ_SECONDS handoff.
  • tools/test_ci_build_provenance.sh’s offline F2 path is still a local replay; if you want it to catch drift in tools/ci-build.sh itself, cover the real script with a fixture-based offline case.
📍 Affects 2 files
  • tools/test_fuzz_seconds_validation.sh#L86-L97 (this comment)
  • tools/test_ci_build_provenance.sh#L104-L110
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@tools/test_fuzz_seconds_validation.sh` around lines 86 - 97, The workflow
validation test must verify the env: RAW_FUZZ_SECONDS handoff rather than only
the copied guard and budget patterns; update
tools/test_fuzz_seconds_validation.sh at lines 86-97 to assert that wiring.
Strengthen the offline F2 coverage in tools/test_ci_build_provenance.sh at lines
104-110 by executing the real tools/ci-build.sh with a fixture-based offline
case instead of only replaying equivalent local logic.


if [ "$fail" -ne 0 ]; then
echo "FAIL: fuzz_seconds validation regression"
exit 1
fi
echo "✓ all fuzz_seconds validation cases pass (injection rejected, valid values accepted)"
Loading