From 1c6452e10e8d5dc221b68061e3277085278b7a27 Mon Sep 17 00:00:00 2001 From: Claude Date: Mon, 3 Aug 2026 16:15:12 -0400 Subject: [PATCH] ci(extension): shadow the derived version + verify real XPI contents Milestone #271 steps 2 and 3. Neither changes what gets published. STEP 2 -- shadow mode. build.yml's sign-extension and ci.yml's extension-version guard now log the version that WOULD be derived from git history alongside the hand-maintained one. Nothing reads the derived value, and neither site can fail because of it. This exists because `web-ext sign` is one-shot per version: AMO 409s on a repeat, so a wrong formula burns a real version number that cannot be reclaimed. Comparing the two across real builds is the only way to validate it at zero cost. sign-extension runs on main only, so main pushes are the sole source of truth for whether the derived number moves exactly when the shipped extension changes -- the dev-side log is a convenience, not the evidence. sign-extension now checks out with fetch-depth: 0. The derived version is a commit count and a depth-1 clone cannot produce one. STEP 3 -- XPI content verification. Every other packaging assertion checks our declaration against itself. This is the first that asks web-ext what it ACTUALLY wrote into the archive. That assumption was both unverified and fragile: `test/**` only survives to web-ext because callers `set -f` before substituting it, so losing that quoting would silently start shipping dev files with no other signal. The step builds the XPI and asserts test/, scripts/, vitest.config.js, package.json, package-lock.json, README.md and node_modules are absent -- and, because an over-matching exclusion would break the extension at runtime rather than at build time, that manifest.json, all four lib/*.js and every UI directory are present. unzip is installed only when missing; node:24-bookworm-slim may not carry it. Refs #2399, #2400 Co-Authored-By: Claude Opus 5 (1M context) --- .forgejo/workflows/build.yml | 24 ++++++++++++++++++ .forgejo/workflows/ci.yml | 11 +++++++++ .forgejo/workflows/extension.yml | 42 ++++++++++++++++++++++++++++++++ ci-requirements.md | 6 +++++ 4 files changed, 83 insertions(+) diff --git a/.forgejo/workflows/build.yml b/.forgejo/workflows/build.yml index 4d86212..a63cee9 100644 --- a/.forgejo/workflows/build.yml +++ b/.forgejo/workflows/build.yml @@ -40,6 +40,11 @@ jobs: image: git.fabledsword.com/bvandeusen/ci-python:3.14 steps: - uses: actions/checkout@v4 + with: + # Full history: the shadow-mode step below derives a version from a + # commit count, which a depth-1 clone cannot produce. Harmless for + # everything else in this job. + fetch-depth: 0 - name: Resolve extension version id: extver @@ -48,6 +53,25 @@ jobs: echo "version=$VERSION" >> "$GITHUB_OUTPUT" echo "Resolved extension version: $VERSION" + # --- shadow mode (milestone #271, step 2) --------------------------- + # Informational ONLY — nothing downstream reads this, and it must never + # fail the build. This is THE place the derived formula gets validated: + # `sign-extension` only runs on main, so main pushes are the sole source + # of truth for whether the derived version moves exactly when the shipped + # extension changes. Compare these lines across several main builds + # before step 4 lets the derived value control publishing. + - name: Shadow — derived version (informational) + run: | + set -u + DERIVED=$(sh extension/scripts/packaging.sh version 2>&1 || echo "UNAVAILABLE") + MANUAL=${{ steps.extver.outputs.version }} + echo "shadow: manual=$MANUAL derived=$DERIVED sha=$GITHUB_SHA" + if [ "$MANUAL" = "$DERIVED" ]; then + echo "shadow: manual and derived agree" + else + echo "shadow: DIVERGENT — expected until step 4 cuts over; derived is authoritative-to-be" + fi + - name: Check Forgejo release-asset cache id: cache env: diff --git a/.forgejo/workflows/ci.yml b/.forgejo/workflows/ci.yml index 71e5caa..fccfe19 100644 --- a/.forgejo/workflows/ci.yml +++ b/.forgejo/workflows/ci.yml @@ -77,6 +77,17 @@ jobs: test -n "$PKG" || { echo "ERROR: no version found in extension/package.json"; exit 1; } test -n "$MAN" || { echo "ERROR: no version found in extension/manifest.json"; exit 1; } + # --- shadow mode (milestone #271, step 2) ------------------------- + # Informational ONLY: nothing below reads DERIVED, and this must never + # fail the job. `web-ext sign` is one-shot per version (AMO 409s on a + # repeat), so a wrong formula would burn a real version number that + # can't be reclaimed. Logging it against real pushes first is the only + # way to validate it at zero cost. + # Placed before every early-exit path so it reports on all runs. + DERIVED=$(sh extension/scripts/packaging.sh version 2>&1 || echo "UNAVAILABLE") + echo "shadow: manual=$PKG derived=$DERIVED" + # ----------------------------------------------------------------- + # (1) Unconditional: the two version strings must agree. `web-ext sign` # reads manifest.json (package.json sits in --ignore-files and isn't # even inside the XPI), so AMO signs MAN and Firefox installs MAN. diff --git a/.forgejo/workflows/extension.yml b/.forgejo/workflows/extension.yml index a67c578..c933028 100644 --- a/.forgejo/workflows/extension.yml +++ b/.forgejo/workflows/extension.yml @@ -38,3 +38,45 @@ jobs: # package version-consistency checks. No browser, no network. - name: Unit tests run: cd extension && npm run test:unit + + # Everything else about packaging is asserted against our own declaration + # of what ships. This is the only check that asks web-ext what it ACTUALLY + # put in the archive. Until now that was an unverified assumption about + # glob semantics — and a fragile one: `test/**` reaches web-ext intact + # only because callers `set -f` first, so losing that quoting would + # silently start shipping dev files with no other signal. + - name: Verify XPI contents + run: | + set -eu + command -v unzip >/dev/null 2>&1 || { apt-get update -qq && apt-get install -y -qq unzip; } + cd extension + npm run build + ZIP=$(ls web-ext-artifacts/*.zip | head -1) + echo "=== packaged entries in $ZIP ===" + unzip -Z1 "$ZIP" | sort + echo "=== end ===" + ENTRIES=$(unzip -Z1 "$ZIP") + fail=0 + # Must NOT ship: repo infrastructure with no business in a user's browser. + for pat in 'test/' 'scripts/' 'vitest.config.js' 'package.json' 'package-lock.json' 'README.md' 'node_modules/' 'web-ext-artifacts/'; do + if echo "$ENTRIES" | grep -q "^$pat"; then + echo "ERROR: '$pat' was packaged into the XPI but must not be" + fail=1 + fi + done + # Must ship: if an exclusion pattern ever over-matches, the extension + # breaks at runtime rather than at build time, so assert presence too. + for req in 'manifest.json' 'lib/url.js' 'lib/api.js' 'lib/platforms.js' 'lib/cookies.js'; do + if ! echo "$ENTRIES" | grep -q "^$req$"; then + echo "ERROR: '$req' is missing from the XPI" + fail=1 + fi + done + for dir in 'background/' 'popup/' 'options/' 'content/' 'icons/'; do + if ! echo "$ENTRIES" | grep -q "^$dir"; then + echo "ERROR: nothing from '$dir' was packaged" + fail=1 + fi + done + [ "$fail" -eq 0 ] || exit 1 + echo "XPI contents verified." diff --git a/ci-requirements.md b/ci-requirements.md index 36d0119..2d69ef4 100644 --- a/ci-requirements.md +++ b/ci-requirements.md @@ -27,6 +27,10 @@ per `docs/process.md`'s "add deps to the image when used by >1 project". - `pip install -r requirements.txt pytest pytest-asyncio` — in `backend-lint-and-test` and `integration` jobs - `npm install --no-audit --no-fund` — in `frontend-build` job - `npm install --no-audit --no-fund` — in `extension.yml`'s `lint` job (web-ext + vitest) +- `unzip` — in `extension.yml`'s "Verify XPI contents" step, installed via apt + only when absent (`node:24-bookworm-slim` may or may not carry it). Debian + package, ~2s. Not worth baking into a shared image for a single consumer, per + `docs/process.md`'s ">1 project" rule. ## Notes @@ -55,6 +59,8 @@ per `docs/process.md`'s "add deps to the image when used by >1 project". pathspec in `ci.yml`'s `extension-version` guard, and the commit count that derives the extension version. Three hand-kept copies of that one fact is what allowed issue #2397. +- `build.yml`'s `sign-extension` checks out with `fetch-depth: 0` — the derived + extension version is a commit count, which a shallow clone cannot produce. - Callers MUST `set -f` before substituting the script's output. Without it the shell expands `test/**` against the working tree and silently narrows the pattern to whatever files exist at that moment — a failure that looks like