ci: gate NuGet publish behind a required-reviewer environment (#299)

.agents/build-release-ci.md

@@ -51,9 +51,38 @@ per-package **CycloneDX SBOM** (`*.bom.json`, via the `dotnet CycloneDX` local t
 creates **SLSA build-provenance attestations**, pushes `.nupkg`+`.snupkg` to NuGet.org with
 `--skip-duplicate`, and attaches the packages, SBOMs, and attestation bundle to the GitHub Release.
 
-> **OPS note (B8):** there is **no manual approval gate** — a `v*` tag push publishes to NuGet.org
-> immediately. The human checkpoint is the interactive `release.sh` prompt. Treat tag creation as the
-> point of no return.
+The publish job runs in the **`release` GitHub Environment** (a required reviewer must approve the
+deployment before it runs), and `NUGET_API_KEY` is a `release` **environment** secret — so only the
+gated job can read it. `workflow_dispatch` **dry-runs stay ungated** (the job's `environment:`
+expression resolves to no environment when `dry_run` is left at its `true` default), so they pack +
+SBOM + attest without prompting. A real `v*` tag push or a `dry_run=false` manual run pauses on
+"Review deployments" until approved.
+
+> **OPS note:** approving the `release` deployment is the point of no return — `--skip-duplicate`
+> means a same-version re-run is a no-op, but a published version cannot be unpublished from NuGet.org.
+
+### One-time `release` environment setup
+
+Repo-admin steps (recorded here so they're reproducible). `prevent_self_review=false` lets a solo
+maintainer approve their own release (set `true` and use a distinct reviewer for a team). No
+deployment-branch policy is set: the reviewer gate already governs every deployment, and leaving it
+open keeps both the tag-push and the manual `workflow_dispatch` publish (from `master`) working.
+
+```bash
+REVIEWER_ID=$(gh api users/maximn --jq .id)
+gh api --method PUT repos/maximn/google-maps/environments/release --input - <<JSON
+{
+  "wait_timer": 0,
+  "prevent_self_review": false,
+  "reviewers": [{ "type": "User", "id": $REVIEWER_ID }],
+  "deployment_branch_policy": null
+}
+JSON
+
+# Scope the publish key to the gated job: add it under the environment, then drop the repo copy.
+gh secret set NUGET_API_KEY --env release --repo maximn/google-maps   # prompts for value
+gh secret delete NUGET_API_KEY --repo maximn/google-maps              # only after a gated publish succeeds
+```
 
 ## CI (`.github/workflows/dotnet.yml`)
 
@@ -73,9 +102,9 @@ Two heavy jobs never gate PRs — both `workflow_dispatch`, and run on demand or
 - `renovate.json` — auto-merges non-major GitHub Actions and test/dev dependency updates and patch
   updates for runtime deps, with a 3-day `minimumReleaseAge`.
 
-> **OPS note (B8):** Renovate auto-merge assumes branch protection ("require status checks") is
-> configured on `master`; verify that in repo settings, otherwise an update could merge without CI
-> passing.
+> **OPS note:** Renovate auto-merge relies on branch protection requiring CI. Verified on `master`
+> (2026-06-29): `required_status_checks` enforces `build` + `Analyze (csharp)` + `Analyze (actions)`,
+> so a non-major update can't auto-merge without a green build + CodeQL.
 
 ## Commands
 

.agents/known-issues.md

@@ -13,7 +13,6 @@ When you fix one, remove it from this list (and update the relevant `.agents/*.m
 | **B4** | low | HTTP status tag is set on `Activity.Current` instead of the captured `activity`, so it can misattribute under unusual ambient-Activity nesting. | `Engine/MapsAPIGenericEngine.cs:122` | Capture `activity` and tag it directly; pass it down or set the tag inside the parent scope. See [`observability.md`](observability.md). |
 | **B5** | low | Observability is tracing-only — no `ILogger` integration and no metrics (counters/histograms). | `Engine/MapsAPIGenericEngine.cs` | Optional: add an `ILogger`-based debug log and OTel metrics (request count, duration histogram, error count). |
 | **B7** | low | Inconsistent SSL enforcement (`TimeZoneRequest` forces HTTPS, others don't) and mixed enum handling (some `[EnumMember]`, some bare names). | `Entities/TimeZone/Request/TimeZoneRequest.cs`; various `Entities/**/Response/Status*.cs` | Decide one SSL policy; standardize on `[EnumMember]` (or document when it's intentionally omitted). |
-| **B8** | medium | No manual approval gate before NuGet publish (a `v*` tag push publishes immediately), and Renovate auto-merge assumes branch protection is configured. | `.github/workflows/nuget.yml`, `renovate.json` | Confirm `master` branch protection requires CI; optionally add a release approval environment. (Process decision for the maintainer.) See [`build-release-ci.md`](build-release-ci.md). |
 
 ## Documented elsewhere (not defects — cross-references)
 

.github/workflows/nuget.yml

@@ -21,6 +21,8 @@ permissions:
 jobs:
   publish:
     runs-on: ubuntu-latest
+    # Gate real publishes behind the 'release' environment (required reviewer); leave dry-runs ungated.
+    environment: ${{ (github.event_name == 'push' || inputs.dry_run == false) && 'release' || '' }}
     permissions:
       contents: write       # create GitHub Release
       id-token: write       # Sigstore OIDC

RELEASING.md

@@ -13,7 +13,8 @@ Releases are **tag-driven**: pushing a git tag matching `v*` to `origin` trigger
   git pull
   git status   # should be clean
   ```
-- The `NUGET_API_KEY` secret is configured in repo settings (one-time setup; already in place)
+- The `NUGET_API_KEY` secret is configured as a `release` **environment** secret (one-time setup — see [`.agents/build-release-ci.md`](.agents/build-release-ci.md))
+- You can approve the `release` environment deployment (the publish run pauses for a required reviewer)
 
 ## Option 1: Use the release script (recommended)
 
@@ -69,6 +70,8 @@ Both options end at the same place — a `v*` tag in `origin` — and trigger th
 
 [`.github/workflows/nuget.yml`](.github/workflows/nuget.yml) runs on `push` of any `v*` tag:
 
+0. **Waits for approval** — the publish job runs in the `release` environment, so the run pauses on
+   "Review deployments" until a required reviewer approves (this is the human gate; nothing below runs first)
 1. Checks out the repo with full history and tags (`fetch-depth: 0`) at the tagged commit
 2. Installs the .NET SDKs needed to build all target frameworks
 3. `dotnet restore` → `dotnet build -c Release` → `dotnet pack -c Release` — [MinVer](https://github.com/adamralph/minver) derives the package version from the `v*` tag at build time (`v1.4.6` → `1.4.6`); no csproj is patched. Commits without a tag get a unique prerelease version (e.g. `1.4.7-alpha.0.3`), so only tagged builds publish a stable release.
@@ -79,6 +82,8 @@ Both options end at the same place — a `v*` tag in `origin` — and trigger th
 ## After pushing the tag
 
 - Watch the run: <https://github.com/maximn/google-maps/actions>
+- **Approve the deployment** — the run pauses on "Review deployments"; approve the `release`
+  environment to let the publish proceed (a reviewer can also reject it to abort)
 - Once green, the new version appears at <https://www.nuget.org/packages/GoogleMapsApi/> (usually within a few minutes)
 
 ## Recovery