diff --git a/.forgejo/workflows/build-push.yml b/.forgejo/workflows/build-push.yml index e9e1fc3..a3f7893 100644 --- a/.forgejo/workflows/build-push.yml +++ b/.forgejo/workflows/build-push.yml @@ -131,6 +131,26 @@ jobs: fi docker build "${tag_args[@]}" "${{ matrix.domain }}/" + # Build -> TEST -> push. Without this the only pytest run in CI was + # pr-build's, so anything reaching a branch by another route (a direct + # push, a workflow_dispatch, a v*.*.* tag) published an image that CI had + # never tested — and deploy.yml then rolls that exact tag onto beta/prod. + # Gating the artifact rather than the branch is what makes the deploy safe, + # since deploy.yml consumes a tag, not a commit status. + # + # Same invocation as build.yml's, deliberately: -u 0 because the image's + # default user cannot write to site-packages, --entrypoint sh because the + # real entrypoint is alembic-migrate-then-serve and would swallow the + # command, and `unset THERMOGRAPH_BASE` because the image bakes the prod + # root mount (/) while the tests are written against the /thermograph + # prefix. The Go daemon and Go frontend suites need no step here: both run + # inside their Dockerfiles, so `docker build` above already gated them. + - name: Test the built image (backend only) + if: steps.image.outputs.changed == 'true' && matrix.domain == 'backend' + run: | + docker run --rm -u 0 --entrypoint sh "${{ steps.image.outputs.sha_tag }}" \ + -c "unset THERMOGRAPH_BASE; pip install -q --no-cache-dir pytest==8.4.1 && python -m pytest tests -q" + - name: Push (SHA tag, every push) if: steps.image.outputs.changed == 'true' run: docker push "${{ steps.image.outputs.sha_tag }}" diff --git a/.forgejo/workflows/pr-build.yml b/.forgejo/workflows/pr-build.yml index 578aa9d..9147952 100644 --- a/.forgejo/workflows/pr-build.yml +++ b/.forgejo/workflows/pr-build.yml @@ -19,7 +19,11 @@ name: PR build (required check) on: pull_request: - branches: [dev, main] + # `release` included: it is the branch that deploys to PROD, and it used to + # be the least-checked branch in the repo. A PR into release ran neither a + # build nor a test — only the two standalone lint workflows — so the intended + # promotion path was the one with no gate on it. + branches: [dev, main, release] concurrency: group: pr-build-${{ github.event.pull_request.number }} @@ -73,18 +77,32 @@ jobs: if: needs.changes.outputs.observability == 'true' uses: ./.forgejo/workflows/observability-validate.yml + # NOT domain-gated and NOT `needs: changes`: both are repo-wide invariants that + # must hold on every PR regardless of which directories it touches. A plaintext + # secret or a broken script can arrive in a commit that changes no app domain at + # all — precisely the case where every domain job skips and `gate` used to pass + # vacuously. + lint-shell: + uses: ./.forgejo/workflows/shell-lint.yml + + guard-secrets: + uses: ./.forgejo/workflows/secrets-guard.yml + gate: name: gate # always(): skipped domain jobs (nothing changed there) must not skip the # gate itself -- the required check has to report on EVERY PR. if: always() - needs: [changes, build-backend, build-frontend, validate-observability] + needs: [changes, build-backend, build-frontend, validate-observability, + lint-shell, guard-secrets] runs-on: docker steps: - name: Reduce domain results run: | set -euo pipefail ok=1 + # Domain jobs: `skipped` is a pass — it means the PR did not touch that + # domain, which is the whole point of the path filtering. for pair in \ "backend=${{ needs.build-backend.result }}" \ "frontend=${{ needs.build-frontend.result }}" \ @@ -95,4 +113,17 @@ jobs: *) ok=0 ;; esac done - [ "$ok" = 1 ] || { echo "a changed domain's check failed"; exit 1; } + # Repo-wide invariants: `skipped` is a FAILURE. These have no `if:` and + # no `needs:`, so they run on every PR; a skip means the job did not + # execute and the invariant is therefore unverified, which must not be + # reported as a pass. + for pair in \ + "shellcheck=${{ needs.lint-shell.result }}" \ + "secrets-encrypted=${{ needs.guard-secrets.result }}"; do + echo "$pair" + case "${pair#*=}" in + success) ;; + *) ok=0 ;; + esac + done + [ "$ok" = 1 ] || { echo "a required check failed"; exit 1; } diff --git a/.forgejo/workflows/secrets-guard.yml b/.forgejo/workflows/secrets-guard.yml index 09b2049..9b65967 100644 --- a/.forgejo/workflows/secrets-guard.yml +++ b/.forgejo/workflows/secrets-guard.yml @@ -11,6 +11,11 @@ name: secrets-guard # runs when you expect it to isn't a backstop. on: + # workflow_call so pr-build.yml can fold this into the single required `gate` + # status. This is the one that mattered most: branch protection requires only + # `gate`, so a commit adding a PLAINTEXT secrets file failed this check as a + # separate status and was still mergeable. + workflow_call: {} pull_request: push: branches: [dev, main, release] diff --git a/.forgejo/workflows/shell-lint.yml b/.forgejo/workflows/shell-lint.yml index 6aa69eb..0c1af34 100644 --- a/.forgejo/workflows/shell-lint.yml +++ b/.forgejo/workflows/shell-lint.yml @@ -20,6 +20,11 @@ name: shell-lint # to fix any new findings in the same PR as the bump. on: + # workflow_call so pr-build.yml can fold this into the single required `gate` + # status. It stays independently triggered as well: branch protection is + # configured to require only `gate`, so before this was reachable from there a + # shellcheck regression reported as a separate status that nothing enforced. + workflow_call: {} pull_request: push: branches: [dev, main, release] diff --git a/backend/Dockerfile b/backend/Dockerfile index ab97519..96d5f6e 100644 --- a/backend/Dockerfile +++ b/backend/Dockerfile @@ -15,6 +15,17 @@ WORKDIR /src COPY daemon/go.mod daemon/go.sum ./ RUN go mod download COPY daemon/ ./ +# gofmt + vet + the full daemon test suite BEFORE the build, so no published +# image can contain a daemon that failed its own tests — the same guarantee +# frontend/Dockerfile already gives the Go frontend, and the reason the tests +# live here rather than in a workflow step: build-push.yml builds this image on +# every push to dev/main/release with no test job of its own, so a check that is +# not part of `docker build` does not gate the artifact that actually deploys. +# +# These 48 tests (gateway, cron, apiclient, config) previously ran nowhere: +# `grep -rn "go test" .forgejo/workflows/` found nothing. The daemon owns the +# prod Discord gateway websocket and every recurring-job timer. +RUN test -z "$(gofmt -l .)" && go vet ./... && go test ./... # -trimpath keeps the build reproducible (identical sources -> identical # binary), which is what lets the final stage's COPY layer cache-hit when only # Python code changed.