ci: move image-heavy scans off the pr path and harden workflows - #202
Merged
Conversation
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
GitHub expression interpolation inside run: blocks substitutes values before bash parses the line, so credentials or outputs containing quotes, dollars, or backticks break the script or execute as shell. Pass values through env: and reference them as shell variables, build the login JSON with jq, and validate the extracted JWT charset before it is used anywhere downstream. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
pnpm audit now gates on high severity in production dependencies only; dev-tree advisories stay visible through Renovate and the nightly informational audit. The NuGet check fails closed if restore or list errors instead of reporting a false pass. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Docker Scout and ZAP both built the full image on every PR push (~7m30s wall-clock). Trivy already gates CVEs on every master publish; deep scanning now lives in the nightly workflow. The PR security workflow keeps the three sub-minute checks: NuGet vulnerabilities, pnpm audit, and Gitleaks. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
The traditional spider discovers almost nothing on a client-rendered SPA, so the full scan exercised only a fraction of the API. The scan now imports swagger.json from the deployed instance and actively scans every documented endpoint, gated on the spec actually being served. The compose stack existed only for the deleted PR-time baseline scan. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Scout moves from the PR path to the nightly schedule, keeping the Docker Hub login its CLI requires and the SARIF upload. The moderate full-tree pnpm audit compensates for the PR gate narrowing to high/prod without blocking anything. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
The nightly has never passed its build step: github.repository is mixed case and Docker rejects uppercase tags, the job lacked the packages permission to push to GHCR, and the container app pulled the image without registry credentials, which cannot work for the private package the first push creates. Provenance is disabled because the attestation manifest adds nothing to an ephemeral scan image. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
PerGB2018 requires retentionInDays between 30 and 730; the workspace is deleted with the resource group minutes later, so the default is fine. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Each poll could hang for minutes against an unready ingress, so the loop ran until the job timeout instead of failing in two minutes, and teardown then destroyed all evidence. Polls now carry curl timeouts, a failure-only step dumps container app status and logs before the resource group is deleted, and the SARIF upload is skipped when the scan never produced a report. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
The classic ZAP scan actions emit html, json and md reports only; SARIF export is an open upstream feature request, so this upload could never fire. ZAP findings live in the run artifacts, and the Security tab keeps Scout, Trivy and CodeQL. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Internal TCP ingress between container apps resolves DNS but never routes on a consumption environment, so the app could not reach postgres and crashed during startup migrations. Postgres now runs as a sidecar in the same container app and the app connects over localhost, removing the cross-app networking dependency entirely. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
… workflow Completes the change started in ec41d32 for the one workflow it left untouched. SONAR_TOKEN was interpolated directly into two run: blocks, alongside eight version/repository template expansions. All six steps now read their values from step-level env:. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
…changes The publish workflow was the last one without a concurrency group; it publishes images and force-pushes tags, so runs queue rather than cancel. paths-ignore now matches security.yml so a markdown-only push no longer builds and releases a beta. trivy-action moves off the mutable @master ref to v0.36.0. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
A workflow skipped by paths-ignore reports no status at all, so any check it owns sits on "Expected" forever and blocks the merge. With CI, the three security scans and the three CodeQL jobs becoming required status checks on master, these two workflows can no longer be path-filtered. Reverses the PR-side filtering from 231ea35 and predates it in ci.yml. The cost is a full run on docs-only PRs; the alternative is a job-level change filter, whose failure mode is silently skipping real code changes while the required checks still report green. publish-container.yml keeps its paths-ignore: it runs on master pushes and owns no PR check. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
…cancelled GitHub keeps only one pending run per concurrency group: a newer arrival cancels the older pending run even when cancel-in-progress is false. With a single "Deploy" group, a manually dispatched stable release sitting behind an in-progress build would be silently cancelled by the next push to master. Separating push from workflow_dispatch keeps the newest-master-wins queueing for pushes while a manual release can no longer be discarded. The two can now overlap, which is safe: they publish disjoint tags, and only the stable run creates releases and moves the vN tags. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
ci.yml has built no image since the image-heavy scans moved off the PR path, so REGISTRY and IMAGE_NAME were referenced nowhere in the file. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
PR #202 was opened during the 2026-08-06 Actions incident, which dropped its pull_request events. Dropped events are not replayed and close/reopen did not re-fire them, so this empty commit exists only to raise a synchronize event. Safe to drop; it disappears on a squash merge. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
![github-advanced-security[bot]](https://avatars.githubusercontent.com/in/57789?s=60&v=4){kind=small}
boardgametracker.client.esproj is in the solution and uses packages.config, so dotnet list package --vulnerable exits non-zero even when every project reports no vulnerable packages. The pipefail added in d4e1a32 propagated that through tee and killed the step before the grep ran, so the scan failed on every PR. Fail-closed behaviour is kept and made explicit: a run that produces no verdict at all is still an error, rather than being inferred from an exit code the command uses for an unrelated condition. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Resolves the CodeQL "Unpinned tag for a non-immutable Action" alerts raised on this PR. A tag is a mutable pointer, so a compromised or retagged release runs with whatever permissions the job holds; a commit sha cannot be moved. All 19 third-party actions are pinned repo-wide rather than only in the two files CodeQL flagged, since it flags changed files and the rest would surface one at a time as they are touched. Actions under actions/ and github/ are left on tags, matching the rule's own trust model. Each pin keeps a version marker so the intended release stays readable, and renovate gains helpers:pinGitHubActionDigests so new actions are pinned on arrival and existing digests keep getting updates. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
.NET Test Results1 305 tests 1 305 ✅ 13s ⏱️ Results for commit a1b9fcd. ♻️ This comment has been updated with latest results. |
Completes the pinning started in 79a0279. Actions under actions/ and github/ were left on tags because CodeQL's trust model exempts them, but a mutable ref is a mutable ref: the exemption reflects who owns the repo, not whether the pointer can move. Every uses: in the repo now resolves to an immutable commit, 99 in total, each keeping its version marker. The three codeql-action subpaths share one sha because they ship from a single repository. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
dotnet tool update with no --version always installed the latest scanner, so an upstream release could change behaviour with no commit here. That is how SonarCloud analysis broke on 2026-05-21 without a repo change: a newer scanner moved the quality gate poll to api.sonarcloud.io, where the current token is not authorised. The version is now declared once per job and reused in the cache key, because a key that ignores the version would restore an older scanner on a cache hit and silently defeat the pin. restore-keys is dropped for the same reason: a prefix match would return the previous version's cache. Pinning does not fix the broken token; it makes the next change deliberate. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Both values are already public: the README badges link to sonarcloud.io/summary/new_code?id=mregni_BoardGameTracker, and the uping organization is visible on its Docker Hub badge. Holding them as secrets adds no protection and masks them in logs, which is why a failing analysis reports "Downloading cache. Project key: ***" and cannot be diagnosed from the run. Only SONAR_TOKEN stays a secret. This also removes the key and organization as variables when investigating the current authorisation failure at the quality gate poll. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
The analysis was never broken. SonarCloud ingests the report, evaluates the gate and posts a "Quality Gate passed" check from its own app within seconds of upload; on the run that prompted this, upload and that check both landed at 00:42:20. The only failing call was the scanner polling api.sonarcloud.io for a verdict already being delivered, which returned "Not authorized or project not found" under two different tokens and with the key and organization set to verified literals. Dropping sonar.qualitygate.wait removes the duplicate. The gate is still enforced, by the SonarCloud Code Analysis check, which links straight to the per-PR dashboard. publish-container.yml loses its continue-on-error on the same step. It existed to tolerate this failure, and it is why a broken step went unnoticed on master for months. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
|
SummarySummary
CoverageBoardGameTracker.Api - 75.3%
BoardGameTracker.Common - 72%
BoardGameTracker.Core - 50.9%
Default - 24.2%
|
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Why
Every PR push started 12 jobs including two full Docker image builds (~7m30s, security-bound). Two ZAP auth steps interpolated credentials straight into
run:shell. The nightly ZAP job spidered a React SPA and therefore scanned almost nothing — and, as this branch discovered, had never actually completed a run on master.PR pipeline: before → after
pr.ymlci.ymlsecurity.ymlcodeql.ymlZero image builds on the PR path. The image-heavy scans moved to the nightly.
Nightly (
security-full-scan.yml)ZAP now runs as an OpenAPI-driven
action-api-scanagainst an ephemeral Azure Container Apps deployment, gated by a pre-flight check that the served spec has a sane endpoint count, plus Docker Scout and an informational full dependency audit. The resource group is torn down unconditionally.Getting this green took six follow-up commits: a mixed-case image tag, a missing
packages:write, absent GHCR pull credentials in the Bicep template, and — the interesting one — Postgres had to become a sidecar in the app container, because ACA internal TCP ingress resolves DNS but never routes on a consumption environment, so the API crashed at startupMigrate().Security fixes
All
${{ }}interpolation of secrets and step outputs is gone from everyrun:block in the repo. The nightly's JWT is charset-validated (^[A-Za-z0-9_-]+\.[A-Za-z0-9_-]+\.[A-Za-z0-9_-]+$) before it reaches ZAP'scmd_options, so the one remaining interpolation cannot carry a quote, space, or shell metacharacter.aquasecurity/trivy-actionis pinned off the mutable@masterref. CodeQL gains thesecurity-extendedsuite.Path filtering and required checks
ci.ymlandsecurity.ymlno longer usepaths-ignore. A workflow skipped by a path filter reports no status at all, so any required check it owns sits on "Expected" forever and blocks the merge. The alternative — a job-level change filter — was rejected because its failure mode is silently skipping real code changes while the required checks still report green.publish-container.ymlkeeps its filter; it runs on master pushes and owns no PR check.Verification
actionlintis clean across all nine workflows. The nightly has been dispatched and completes end to end.Follow-ups (not in this PR)
Two checks are red on every current PR and are deliberately not yet in the required set:
Frontend Dependency Scan— two real high-severitybrace-expansionadvisories reach production deps via@sentry/vite-plugin → @sentry/bundler-plugin-core → glob → minimatch. The existing pnpm override (brace-expansion@>=3.0.0 <5.0.7) no longer matches; the advisories need>=5.0.9.@sentry/vite-pluginis also a build-time tool sitting independencies, which is why--prodsees it at all.Test and SonarCloud Analysis— fails at "End SonarCloud analysis", i.e. the SonarCloud quality gate itself (sonar.qualitygate.wait=true).Once both are green they should join the required set.