From 4035432839382496a14a64a18bc8f261c159ea4c Mon Sep 17 00:00:00 2001 From: shiny-code-bot Date: Sat, 3 Oct 2026 11:12:29 -0400 Subject: [PATCH 1/8] Separate release approval from the single secret environment --- .github/github.json | 2 +- .../app-store-connect-companion-upload.yml | 31 +++- .../workflows/app-store-connect-upload.yml | 31 +++- .github/workflows/release.yml | 31 +++- .github/workflows/ship.yml | 25 ++- .github/workflows/submit-app-store-review.yml | 21 ++- .../testflight-beta-distribution.yml | 31 +++- .../upload-app-store-screenshots.yml | 21 ++- Tests/ScriptsTests/test_release_approvals.py | 41 ++++- docs/release.md | 145 ++++++++++-------- docs/repo-settings.md | 39 ++--- scripts/check-release-approvals.py | 105 ++++++++----- scripts/release-approval-config.sh | 7 + 13 files changed, 372 insertions(+), 158 deletions(-) create mode 100755 scripts/release-approval-config.sh diff --git a/.github/github.json b/.github/github.json index e54550c..aafb126 100644 --- a/.github/github.json +++ b/.github/github.json @@ -50,7 +50,7 @@ }, "validate": { "default": "scripts/commit-gate.sh", - "releaseApprovalPolicy": "uv run scripts/check-release-approvals.py (structural dry-run: one Ship approval; standalone review retained; operator config and RELEASE_CHANNELS_CONFIGURED=true required)", + "releaseApprovalPolicy": "uv run scripts/check-release-approvals.py (structural dry-run: one secretless Ship approval; standalone gates; one release secret store; owner role move and RELEASE_APPROVALS_CONFIGURED=true required)", "ciChangeScope": "python3 scripts/ci-change-scope.py --base (pull requests only; skips swift build, swift test, and the companion builds when every changed path is documentation or validation tooling; unknown paths run everything; pushes to main and CodeQL always run in full)", "companionUploadFixtureTools": "CONTEXT_PANEL_UPLOAD_FIXTURE_TOOLS_DIR= scripts/upload-app-store-connect-companion-app.sh --export-only (test-only seam for fixture xcodebuild and codesign; refused for uploads)", "cloudKitCompanionSchema": "scripts/validate-cloudkit-companion-schema.sh", diff --git a/.github/workflows/app-store-connect-companion-upload.yml b/.github/workflows/app-store-connect-companion-upload.yml index 825ac90..7b06774 100644 --- a/.github/workflows/app-store-connect-companion-upload.yml +++ b/.github/workflows/app-store-connect-companion-upload.yml @@ -91,13 +91,36 @@ jobs: --version "${INPUT_VERSION}" \ --build-number "${INPUT_BUILD_NUMBER}" + - name: Require Completed Approval Setup + env: + RELEASE_APPROVALS_CONFIGURED: ${{ vars.RELEASE_APPROVALS_CONFIGURED }} + run: scripts/release-approval-config.sh + + approve: + name: Approve Release Intent + needs: guard + # Ship already reviewed this exact caller intent; all other callers need review. + if: ${{ !(github.workflow_ref == format('{0}/.github/workflows/ship.yml@refs/heads/main', github.repository)) }} + runs-on: ubuntu-latest + environment: release-approval + permissions: + contents: read + steps: + - name: Record Approved Intent + env: + INPUT_VERSION: ${{ inputs.version }} + run: | + printf 'Approved release intent for version %s at %s\n' "$INPUT_VERSION" "$GITHUB_SHA" + upload: name: Upload Companion App - needs: guard + needs: [guard, approve] runs-on: macos-26 - # Reusable workflows receive the caller identity; direct dispatch keeps review. - environment: >- - ${{ github.workflow_ref == format('{0}/.github/workflows/ship.yml@refs/heads/main', github.repository) && vars.RELEASE_CHANNELS_CONFIGURED == 'true' && 'release-channels' || 'release' }} + if: >- + ${{ !cancelled() && needs.guard.result == 'success' && + (needs.approve.result == 'success' || + (github.workflow_ref == format('{0}/.github/workflows/ship.yml@refs/heads/main', github.repository) && needs.approve.result == 'skipped')) }} + environment: release outputs: build_number: ${{ steps.build.outputs.build_number }} app_store_platform: ${{ steps.platform.outputs.app_store_platform }} diff --git a/.github/workflows/app-store-connect-upload.yml b/.github/workflows/app-store-connect-upload.yml index b31fbc4..06b3a75 100644 --- a/.github/workflows/app-store-connect-upload.yml +++ b/.github/workflows/app-store-connect-upload.yml @@ -74,13 +74,36 @@ jobs: --version "${INPUT_VERSION}" \ --build-number "${INPUT_BUILD_NUMBER}" + - name: Require Completed Approval Setup + env: + RELEASE_APPROVALS_CONFIGURED: ${{ vars.RELEASE_APPROVALS_CONFIGURED }} + run: scripts/release-approval-config.sh + + approve: + name: Approve Release Intent + needs: guard + # Ship already reviewed this exact caller intent; all other callers need review. + if: ${{ !(github.workflow_ref == format('{0}/.github/workflows/ship.yml@refs/heads/main', github.repository)) }} + runs-on: ubuntu-latest + environment: release-approval + permissions: + contents: read + steps: + - name: Record Approved Intent + env: + INPUT_VERSION: ${{ inputs.version }} + run: | + printf 'Approved release intent for version %s at %s\n' "$INPUT_VERSION" "$GITHUB_SHA" + upload: name: Upload Mac App - needs: guard + needs: [guard, approve] runs-on: macos-26 - # Reusable workflows receive the caller identity; direct dispatch keeps review. - environment: >- - ${{ github.workflow_ref == format('{0}/.github/workflows/ship.yml@refs/heads/main', github.repository) && vars.RELEASE_CHANNELS_CONFIGURED == 'true' && 'release-channels' || 'release' }} + if: >- + ${{ !cancelled() && needs.guard.result == 'success' && + (needs.approve.result == 'success' || + (github.workflow_ref == format('{0}/.github/workflows/ship.yml@refs/heads/main', github.repository) && needs.approve.result == 'skipped')) }} + environment: release outputs: build_number: ${{ steps.build.outputs.build_number }} diff --git a/.github/workflows/release.yml b/.github/workflows/release.yml index ae0c5bd..d815d86 100644 --- a/.github/workflows/release.yml +++ b/.github/workflows/release.yml @@ -82,13 +82,36 @@ jobs: --version "${INPUT_VERSION}" \ --build-number "${INPUT_BUILD_NUMBER}" + - name: Require Completed Approval Setup + env: + RELEASE_APPROVALS_CONFIGURED: ${{ vars.RELEASE_APPROVALS_CONFIGURED }} + run: scripts/release-approval-config.sh + + approve: + name: Approve Release Intent + needs: guard + # Ship already reviewed this exact caller intent; all other callers need review. + if: ${{ !(github.workflow_ref == format('{0}/.github/workflows/ship.yml@refs/heads/main', github.repository)) }} + runs-on: ubuntu-latest + environment: release-approval + permissions: + contents: read + steps: + - name: Record Approved Intent + env: + INPUT_VERSION: ${{ inputs.version }} + run: | + printf 'Approved release intent for version %s at %s\n' "$INPUT_VERSION" "$GITHUB_SHA" + macos: name: Build macOS App - needs: guard + needs: [guard, approve] runs-on: macos-26 - # Reusable workflows receive the caller identity; direct dispatch keeps review. - environment: >- - ${{ github.workflow_ref == format('{0}/.github/workflows/ship.yml@refs/heads/main', github.repository) && vars.RELEASE_CHANNELS_CONFIGURED == 'true' && 'release-channels' || 'release' }} + if: >- + ${{ !cancelled() && needs.guard.result == 'success' && + (needs.approve.result == 'success' || + (github.workflow_ref == format('{0}/.github/workflows/ship.yml@refs/heads/main', github.repository) && needs.approve.result == 'skipped')) }} + environment: release permissions: contents: write diff --git a/.github/workflows/ship.yml b/.github/workflows/ship.yml index 4c934cd..8684997 100644 --- a/.github/workflows/ship.yml +++ b/.github/workflows/ship.yml @@ -105,9 +105,28 @@ jobs: --version "${INPUT_VERSION}" \ --build-number "${INPUT_BUILD_NUMBER}" + - name: Require Completed Approval Setup + env: + RELEASE_APPROVALS_CONFIGURED: ${{ vars.RELEASE_APPROVALS_CONFIGURED }} + run: scripts/release-approval-config.sh + + approve: + name: Approve Release Intent + needs: guard + runs-on: ubuntu-latest + environment: release-approval + permissions: + contents: read + steps: + - name: Record Approved Intent + env: + INPUT_VERSION: ${{ inputs.version }} + run: | + printf 'Approved release intent for version %s at %s\n' "$INPUT_VERSION" "$GITHUB_SHA" + validate: name: Validate Release Intent - needs: guard + needs: approve runs-on: ubuntu-latest environment: release outputs: @@ -152,7 +171,6 @@ jobs: create_github_release: true notarize: ${{ inputs.notarize_github_release }} cloudkit_schema_receipt_base64: ${{ inputs.cloudkit_schema_receipt_base64 }} - secrets: inherit app-store-upload: name: Mac App Store Connect Build Upload Channel @@ -164,7 +182,6 @@ jobs: build_number: ${{ needs.validate.outputs.build_number }} upload: ${{ inputs.app_store_channel == 'upload' }} cloudkit_schema_receipt_base64: ${{ inputs.cloudkit_schema_receipt_base64 }} - secrets: inherit companion-app-store-upload: name: Companion App Store Connect Build Upload Channel @@ -177,7 +194,6 @@ jobs: platform: ${{ inputs.companion_platform }} upload: ${{ inputs.companion_app_store_channel == 'upload' }} cloudkit_schema_receipt_base64: ${{ inputs.cloudkit_schema_receipt_base64 }} - secrets: inherit testflight-beta: name: TestFlight Beta Distribution Channel @@ -208,7 +224,6 @@ jobs: include_internal_beta_groups: ${{ inputs.include_internal_testflight_groups }} dry_run: false cloudkit_schema_receipt_base64: ${{ inputs.cloudkit_schema_receipt_base64 }} - secrets: inherit summary: name: Release Summary diff --git a/.github/workflows/submit-app-store-review.yml b/.github/workflows/submit-app-store-review.yml index 8a7faa3..6c41bf2 100644 --- a/.github/workflows/submit-app-store-review.yml +++ b/.github/workflows/submit-app-store-review.yml @@ -285,9 +285,28 @@ jobs: --version "${INPUT_VERSION}" \ --build-number "${INPUT_BUILD_NUMBER}" + - name: Require Completed Approval Setup + env: + RELEASE_APPROVALS_CONFIGURED: ${{ vars.RELEASE_APPROVALS_CONFIGURED }} + run: scripts/release-approval-config.sh + + approve: + name: Approve Release Intent + needs: guard + runs-on: ubuntu-latest + environment: release-approval + permissions: + contents: read + steps: + - name: Record Approved Intent + env: + INPUT_VERSION: ${{ inputs.version }} + run: | + printf 'Approved release intent for version %s at %s\n' "$INPUT_VERSION" "$GITHUB_SHA" + submit: name: Submit App Store Review - needs: guard + needs: [guard, approve] runs-on: macos-26 environment: release diff --git a/.github/workflows/testflight-beta-distribution.yml b/.github/workflows/testflight-beta-distribution.yml index fe8e6ca..0e21499 100644 --- a/.github/workflows/testflight-beta-distribution.yml +++ b/.github/workflows/testflight-beta-distribution.yml @@ -101,13 +101,36 @@ jobs: --version "${INPUT_VERSION}" \ --build-number "${INPUT_BUILD_NUMBER}" + - name: Require Completed Approval Setup + env: + RELEASE_APPROVALS_CONFIGURED: ${{ vars.RELEASE_APPROVALS_CONFIGURED }} + run: scripts/release-approval-config.sh + + approve: + name: Approve Release Intent + needs: guard + # Ship already reviewed this exact caller intent; all other callers need review. + if: ${{ !(github.workflow_ref == format('{0}/.github/workflows/ship.yml@refs/heads/main', github.repository)) }} + runs-on: ubuntu-latest + environment: release-approval + permissions: + contents: read + steps: + - name: Record Approved Intent + env: + INPUT_VERSION: ${{ inputs.version }} + run: | + printf 'Approved release intent for version %s at %s\n' "$INPUT_VERSION" "$GITHUB_SHA" + distribute: name: Distribute TestFlight Beta - needs: guard + needs: [guard, approve] runs-on: ubuntu-latest - # Reusable workflows receive the caller identity; direct dispatch keeps review. - environment: >- - ${{ github.workflow_ref == format('{0}/.github/workflows/ship.yml@refs/heads/main', github.repository) && vars.RELEASE_CHANNELS_CONFIGURED == 'true' && 'release-channels' || 'release' }} + if: >- + ${{ !cancelled() && needs.guard.result == 'success' && + (needs.approve.result == 'success' || + (github.workflow_ref == format('{0}/.github/workflows/ship.yml@refs/heads/main', github.repository) && needs.approve.result == 'skipped')) }} + environment: release steps: - name: Checkout diff --git a/.github/workflows/upload-app-store-screenshots.yml b/.github/workflows/upload-app-store-screenshots.yml index df526aa..d5265bb 100644 --- a/.github/workflows/upload-app-store-screenshots.yml +++ b/.github/workflows/upload-app-store-screenshots.yml @@ -63,9 +63,28 @@ jobs: run: | scripts/release-workflow-guard.sh --version "${INPUT_VERSION}" + - name: Require Completed Approval Setup + env: + RELEASE_APPROVALS_CONFIGURED: ${{ vars.RELEASE_APPROVALS_CONFIGURED }} + run: scripts/release-approval-config.sh + + approve: + name: Approve Release Intent + needs: guard + runs-on: ubuntu-latest + environment: release-approval + permissions: + contents: read + steps: + - name: Record Approved Intent + env: + INPUT_VERSION: ${{ inputs.version }} + run: | + printf 'Approved release intent for version %s at %s\n' "$INPUT_VERSION" "$GITHUB_SHA" + upload: name: Upload Screenshots - needs: guard + needs: [guard, approve] runs-on: ubuntu-latest environment: release diff --git a/Tests/ScriptsTests/test_release_approvals.py b/Tests/ScriptsTests/test_release_approvals.py index 90af77d..850261f 100644 --- a/Tests/ScriptsTests/test_release_approvals.py +++ b/Tests/ScriptsTests/test_release_approvals.py @@ -2,6 +2,8 @@ import importlib.util from pathlib import Path import unittest +import os +import subprocess from typing import Any spec = importlib.util.spec_from_file_location( @@ -17,24 +19,29 @@ def guard() -> dict[str, Any]: return {"steps": [ {"uses": "actions/checkout@v7", "with": {"fetch-depth": 0}}, {"run": 'scripts/release-workflow-guard.sh --version "${INPUT_VERSION}" --build-number "${INPUT_BUILD_NUMBER}"'}, + {"run": "scripts/release-approval-config.sh", + "env": {"RELEASE_APPROVALS_CONFIGURED": "${{ vars.RELEASE_APPROVALS_CONFIGURED }}"}}, ]} def fixture() -> dict[str, Any]: - ship = {"guard": guard(), "validate": {"environment": "release", "needs": "guard"}} + approval = {"environment": "release-approval", "needs": "guard"} + ship = {"guard": guard(), "approve": copy.deepcopy(approval), + "validate": {"environment": "release", "needs": "approve"}} documents = {"ship.yml": {"jobs": ship}} for channel, filename in policy.CHANNELS.items(): ship[channel] = {"uses": f"./.github/workflows/{filename}", "needs": "validate"} documents[filename] = { "on": {"workflow_dispatch": {}, "workflow_call": {}}, - "jobs": {"guard": guard(), "channel": { - "needs": "guard", "environment": policy.CHANNEL_ENVIRONMENT, + "jobs": {"guard": guard(), "approve": dict(approval, **{"if": policy.STANDALONE_APPROVAL}), + "channel": { + "needs": ["guard", "approve"], "environment": "release", "if": policy.CHANNEL_READY, "env": {"SIGNING_KEY": "${{ secrets.SIGNING_KEY }}"}, }}, } for filename in policy.STANDALONE_ONLY: - documents[filename] = {"jobs": {"guard": guard(), "submit": { - "environment": "release", "needs": "guard", "env": {"KEY": "${{ secrets.KEY }}"}, + documents[filename] = {"jobs": {"guard": guard(), "approve": copy.deepcopy(approval), "submit": { + "environment": "release", "needs": ["guard", "approve"], "env": {"KEY": "${{ secrets.KEY }}"}, }}} return documents @@ -43,16 +50,26 @@ class ReleaseApprovalTests(unittest.TestCase): def test_full_ship_plan_requires_one_approval(self) -> None: plan = policy.check(fixture()) self.assertEqual(plan["ship"]["approval_count"], 1) - self.assertTrue(all(env == "release" for env in plan["standalone"].values())) + self.assertTrue(all(path["secret_environment"] == "release" and path["approval_count"] == 1 + for path in plan["standalone"].values())) + self.assertEqual(plan["secret_names_by_environment"]["release-approval"], []) def test_approval_bypasses_and_extra_prompts_fail(self) -> None: mutations = [ lambda d: d["ship.yml"]["jobs"]["github-release"].update(needs="guard"), lambda d: d["ship.yml"]["jobs"]["validate"].update(**{"continue-on-error": True}), lambda d: d["ship.yml"]["jobs"]["validate"].update(**{"if": "${{ false }}"}), + lambda d: d["ship.yml"]["jobs"]["validate"].update(needs="guard"), + lambda d: d["ship.yml"]["jobs"]["approve"].update(env={"KEY": "${{ secrets.KEY }}"}), + lambda d: d["ship.yml"]["jobs"]["github-release"].update(secrets="inherit"), + lambda d: d["release.yml"]["jobs"]["approve"].update(**{"if": "${{ inputs.skip_approval }}"}), + lambda d: d["release.yml"]["jobs"]["channel"].update(**{"if": "${{ always() }}"}), + lambda d: d["release.yml"]["jobs"]["guard"]["steps"][2].update(**{"if": "${{ false }}"}), + lambda d: d["release.yml"]["jobs"]["guard"]["steps"][2]["env"].update(RELEASE_APPROVALS_CONFIGURED="true"), + lambda d: d[policy.STANDALONE_ONLY[0]]["jobs"]["approve"].update(**{"if": "${{ false }}"}), lambda d: d["ship.yml"]["jobs"].update(extra={"environment": "release"}), lambda d: d["ship.yml"]["jobs"]["github-release"].update(**{"if": "${{ always() }}"}), - lambda d: d["release.yml"]["jobs"]["channel"].update(environment="release"), + lambda d: d["release.yml"]["jobs"]["channel"].update(environment="release-approval"), lambda d: d["release.yml"]["jobs"]["channel"].update(environment="release-channels"), lambda d: d["release.yml"]["jobs"]["channel"].update(environment="${{ inputs.environment }}"), lambda d: d["release.yml"]["jobs"]["channel"].update(needs=[]), @@ -89,3 +106,13 @@ def test_testflight_join_requires_success_even_when_upload_is_skipped(self) -> N call["if"] = "${{ !cancelled() && inputs.testflight_beta && needs.validate.result != 'success' && inputs.optional }}" with self.assertRaises(ValueError): policy.check(documents) + + def test_setup_confirmation_fails_closed(self) -> None: + script = Path(__file__).resolve().parents[2] / "scripts/release-approval-config.sh" + for value, succeeds in [(None, False), ("false", False), ("TRUE", False), ("true", True)]: + with self.subTest(value=value): + env = {"PATH": os.defpath} + if value is not None: + env["RELEASE_APPROVALS_CONFIGURED"] = value + result = subprocess.run([str(script)], env=env, capture_output=True, text=True) + self.assertEqual(result.returncode == 0, succeeds) diff --git a/docs/release.md b/docs/release.md index 76ba962..bee3281 100644 --- a/docs/release.md +++ b/docs/release.md @@ -28,72 +28,85 @@ App Store Review submission is intentionally separate from `Ship`. Run it only after the TestFlight build has been validated and the App Store release decision is explicit. -A normal `Ship` run requires **one approval**: `Validate Release Intent` uses -GitHub's `release` environment with Chris as required reviewer. Its channel -calls depend on successful intent validation. GitHub packaging, Mac and -companion uploads, and TestFlight distribution then use `release-channels`, -which has no required reviewer and permits only the protected `main` branch. -Re-running a failed channel in that same approved Ship run does not request -another approval; a new Ship dispatch requires its own intent approval. -Cancelling a run prevents its TestFlight join from starting or continuing. - -Channel workflows choose the environment from `github.workflow_ref`, GitHub's -caller workflow identity, not a user-supplied input, -and the repository variable -`RELEASE_CHANNELS_CONFIGURED=true` set by Chris after environment setup. Only -this repository's -`ship.yml` on `refs/heads/main` with completed setup selects -`release-channels`. -Until that variable is `true`, all channels retain reviewed `release`; this -avoids auto-creating an unconfigured environment during the transition. A direct -dispatch of -Release, either upload workflow, or TestFlight uses `release` and still needs -review. App Store Review submission and screenshot upload always use `release` -and remain separate approvals, even if called by another workflow. - -Dispatch every release workflow from `main`; task branches, arbitrary refs, and -tags are rejected by a separate secretless guard before signing or App Store -Connect credentials are available. That guard validates the protected ref, -checked-out commit, main ancestry, version, and build number. The one-approval -contract assumes the operator environment setup below; GitHub environment rules -are external configuration, not created by a workflow. Protected-main workflow -changes remain trusted: GitHub environment branch rules do not restrict secrets -to a particular workflow file. When adding a release channel, classify its -workflow and approval dependencies in the checker with the same model review -required for approval changes; the checker rejects unclassified direct access -to `release-channels`. - -`scripts/check-release-approvals.py` checks the parsed workflow graph in the -CI and prints the all-channel approval plan without running a release. -It also prints the referenced secret names per environment without values. -It rejects channel calls without successful intent dependencies, additional -reviewed Ship jobs, and unreviewed standalone paths. This is a structural dry -run, not evidence of a live GitHub approval prompt. - -## One-time channel environment setup (owner only) - -Wait until the active 1.0.69 Ship run and any standalone release jobs finish. -In repository **Settings → Environments → New environment**, create -`release-channels`. Leave required reviewers and wait timer off. Under -**Deployment branches and tags**, choose **Selected branches and tags**, click -**Add deployment branch or tag rule**, choose **Branch**, enter `main`, and save. -Keep `main` protected and leave the existing `release` reviewer unchanged. - -Under **Environment secrets → Add environment secret**, add the channel secret -names referenced by the four channel workflows, using their original private -credential sources. GitHub cannot reveal an existing secret value; agents must -never read or copy one. Populate matching secrets in `release` too when they -currently exist only at -repository level; retain existing environment entries for standalone -recovery workflows and Ship's App Store version preflight. Remove any -repository-level duplicates only after both environments are populated, so -other workflows cannot inherit signing/upload secrets without an environment. -Do not rotate or invent credentials as part of this setup. The owner confirms -names, branch restrictions, and reviewers only; secret values stay private. -Finally open **Settings → Secrets and variables → Actions → Variables → New -repository variable**, name it `RELEASE_CHANNELS_CONFIGURED`, value `true`, and -click **Add variable**. Set it only after setup is complete. Delete this variable -to restore per-channel review without changing or removing credentials. +A configured `Ship` run requires **one approval**: the secretless `Approve +Release Intent` job uses `release-approval`, with Chris as required reviewer. +After that job succeeds, `Validate Release Intent` preflights App Store versions +and resolves the build number using the `release` environment. All signing, +notarization, upload, CloudKit-receipt, and TestFlight secrets live only in +`release` in GitHub. That environment has no reviewer or wait timer and accepts +only the protected `main` branch. The gate environment contains no secrets. + +The four reusable channels identify Ship through `github.workflow_ref`, +GitHub's caller workflow identity, rather than a dispatch input. Only this +repository's `ship.yml` on `refs/heads/main` skips their standalone `approve` +job; their caller jobs must depend on successful Ship validation. Every other +caller and direct dispatch requires its own secretless reviewed gate before +using `release`. App Store Review submission and screenshots always require +that gate, including reusable calls. Channel jobs explicitly reject failed or +cancelled guards and failed standalone approvals; the skipped-gate path is +limited to the exact Ship caller. Ship's TestFlight join also requires successful +validation and the selected upload before distributing a build. + +Dispatch release workflows from `main`. Before any environment job, the +secretless trust guard verifies the protected ref, checked-out commit, main +ancestry, version, and build number. A second secretless check refuses new runs +unless the repository variable `RELEASE_APPROVALS_CONFIGURED` is exactly `true`. +Set it only after the owner finishes the role move below. Removing it pauses new +release runs; it does not re-add an environment reviewer or affect an already +approved run. The previous `RELEASE_CHANNELS_CONFIGURED` variable and +`release-channels` environment are unused by this design. + +Retry failed channel jobs within the original approved run to retain its review; +a new Ship dispatch needs its own approval. Re-running the approval job itself +can request review again. Live prompt counts are GitHub-owned evidence and must +be checked during a later authorized release, never by dispatching solely to +test this change. + +Protected-main workflow changes remain trusted: environment branch rules do +not restrict secrets to a workflow filename. Classify new release jobs and their +approval dependencies in the checker, and obtain the model review required for +approval changes. `scripts/check-release-approvals.py` lints the parsed workflow +graph in CI and prints an all-channel structural dry-run plus secret names, +never values. It checks one secretless Ship gate, standalone review paths, +success dependencies and a single secret environment. It cannot verify live +reviewer settings or actual secret placement. Computed environment names in +future unclassified workflows remain a recorded detection limit (#750). + +## One-time environment role move (owner only) + +Wait until the active 1.0.69 Ship run and all standalone release jobs finish, +and this redesign has merged. Do not follow the earlier duplicate-secret setup. +The preferred hand step leaves the existing `release` secret entries in place: +no secret value is read, copied, rotated, or re-entered. + +1. In **Settings → Environments → release**, inspect secret **names only**. + Confirm the enabled channels' names from the checker are already here. If a + needed name exists only in repository secrets or another environment, stop + and report the name on #747 before removing any reviewer or activating. The + agent cannot verify this inventory or recover GitHub's write-only values. +2. In **Settings → Environments → New environment**, enter `release-approval` + and click **Configure environment**. Enable **Required reviewers**, select + `cbusillo`, and save protection rules. Keep self-review allowed for the solo + operator; disable administrator bypass. Leave wait timer off and add no + secrets. Under **Deployment branches and tags**, choose **Selected branches + and tags** → **Add deployment branch or tag rule** → **Branch**, enter `main`, + and save. +3. Return to **release**. Set the same selected **Branch** rule for `main` only. + Uncheck **Required reviewers**, keep wait timer off, and save protection rules. + Leave every environment secret untouched. The required reviewer now sits on + `release-approval`, while all secret jobs continue reading `release`. +4. In **Settings → Secrets and variables → Actions → Repository secrets**, + remove channel-secret duplicates only when the matching name is confirmed in + `release`. If an earlier setup created `release-channels`, remove only its + confirmed duplicates too, after all older runs have finished. Keep unrelated + automation secrets. Missing names require an owner inventory reply, not + rotation or deletion of the only copy. +5. In **Settings → Secrets and variables → Actions → Variables**, delete the + obsolete `RELEASE_CHANNELS_CONFIGURED` variable if present. Click **New + repository variable**, name it `RELEASE_APPROVALS_CONFIGURED`, enter `true`, + and click **Add variable** (or edit/save an existing variable). Do this last. +6. Confirm configuration and names on #747 from `cbusillo`, never values. No + release dispatch is required or authorized for this setup task. The lower-level workflows remain callable for recovery and validation: diff --git a/docs/repo-settings.md b/docs/repo-settings.md index 505adf1..d81f1fd 100644 --- a/docs/repo-settings.md +++ b/docs/repo-settings.md @@ -28,25 +28,26 @@ Expected GitHub settings: `errors` or security alerts at `high_or_higher`. - Code quality gate on `main`: enabled for `errors`. - Force pushes and branch deletion: blocked for `main`. -- Release environment: active environment named `release` with a required - repository-owner review, administrator bypass disabled, and deployment - branches restricted to protected branches only. Because `main` is the only - protected branch, secret-bearing release jobs cannot run from tags or task - branches. -- Ship channel environment: `release-channels`, with no required reviewer or - wait timer, and a selected deployment **Branch** rule for `main` only. Channel - signing/upload secrets belong here and in `release` for standalone recovery; - repository-level copies are removed by the owner after setup. -- Activation variable: repository Actions variable `RELEASE_CHANNELS_CONFIGURED` - is `true` only after the owner confirms channel environment setup. Unset or - false retains reviewed `release` for each channel; deleting it rolls approval - routing back without deleting credentials. -- Release environment secret: - `CONTEXT_PANEL_CLOUDKIT_SCHEMA_RECEIPT_KEY` in both environments, containing at - least 32 bytes of - high-entropy key material shared with the operator Keychain entry used to seal - Production CloudKit schema receipts. Do not store the value in repository - files or repository-level secrets. +- Review environment: `release-approval`, with `cbusillo` as required reviewer, + self-review allowed, administrator bypass disabled, no wait timer and no + secrets. Selected deployment **Branch** rule: `main` only. +- Secret environment: existing `release`, with no reviewer or wait timer and a + selected deployment **Branch** rule for protected `main` only. Every GitHub + release secret lives only here. Ship preflight and all six standalone/channel + workflows use it after a secretless review gate. No duplicated secret store. +- Activation variable: repository Actions variable `RELEASE_APPROVALS_CONFIGURED` + is exactly `true` only after the owner confirms the role move and secret-name + inventory. Unset/false pauses new release workflows before environment jobs. + Removing it does not restore reviewer settings or stop an already running job. + Previous `RELEASE_CHANNELS_CONFIGURED` is obsolete and should be removed. +- `release-channels` is unused; confirmed duplicate secrets there and at + repository level are removed by the owner after older runs finish. Leave + existing `release` values untouched; stop on a missing sole-store name. +- Release environment secret: `CONTEXT_PANEL_CLOUDKIT_SCHEMA_RECEIPT_KEY`, at + least 32 bytes of high-entropy key material shared with the operator Keychain + entry that seals Production CloudKit schema receipts. Its sole GitHub copy + belongs in `release`; never store its value in repository files. + - Immutable Releases: enabled so newly published GitHub Releases lock their tag, title, notes, and assets after publication. diff --git a/scripts/check-release-approvals.py b/scripts/check-release-approvals.py index 73fac96..1695a42 100644 --- a/scripts/check-release-approvals.py +++ b/scripts/check-release-approvals.py @@ -16,9 +16,12 @@ import re import shlex -CHANNEL_ENVIRONMENT = ( - "${{ github.workflow_ref == format('{0}/.github/workflows/ship.yml@refs/heads/main', " - "github.repository) && vars.RELEASE_CHANNELS_CONFIGURED == 'true' && 'release-channels' || 'release' }}" +SHIP_CALLER = "github.workflow_ref == format('{0}/.github/workflows/ship.yml@refs/heads/main', github.repository)" +STANDALONE_APPROVAL = "${{ !(" + SHIP_CALLER + ") }}" +CHANNEL_READY = ( + "${{ !cancelled() && needs.guard.result == 'success' && " + "(needs.approve.result == 'success' || (" + SHIP_CALLER + + " && needs.approve.result == 'skipped')) }}" ) CHANNELS = { "github-release": "release.yml", @@ -101,21 +104,38 @@ def guard_check(jobs: dict, workflow_name: str) -> None: f"{workflow_name}: trust guard must be invoked directly with release inputs") require(not guard.get("if") and not guard.get("continue-on-error"), f"{workflow_name}: trust guard must run and succeed") + setup_steps = [step for step in steps if step.get("run") == "scripts/release-approval-config.sh"] + require(len(setup_steps) == 1 and not setup_steps[0].get("if") + and not setup_steps[0].get("continue-on-error") + and setup_steps[0].get("env", {}).get("RELEASE_APPROVALS_CONFIGURED") + == "${{ vars.RELEASE_APPROVALS_CONFIGURED }}", + f"{workflow_name}: owner-confirmed setup is required before environment jobs") + + def approval_check(jobs: dict, workflow_name: str, reusable: bool = False) -> None: + approval = jobs["approve"] + require(environment(approval) == "release-approval" and not secret_bearing(approval), + f"{workflow_name}: review gate must be secretless in release-approval") + require(needs(approval) == ["guard"] and not approval.get("continue-on-error"), + f"{workflow_name}: review gate must follow successful trust guard") + require(approval.get("if", "") == (STANDALONE_APPROVAL if reusable else ""), + f"{workflow_name}: only the exact Ship caller may skip standalone review") ship = workflows["ship.yml"]["jobs"] guard_check(ship, "ship.yml") + approval_check(ship, "ship.yml") intent = ship["validate"] - require(environment(intent) == "release", "Ship intent must require release review") + require(environment(intent) == "release", "Ship preflight must use the sole release secret store") require(not intent.get("if") and not intent.get("continue-on-error"), - "Ship intent cannot be optional or tolerate failure") - require(needs(intent) == ["guard"], "Ship intent must follow trusted-source guard") + "Ship preflight cannot be optional or tolerate failure") + require(needs(intent) == ["approve"], "Ship preflight must follow approved intent") for job_id, job in ship.items(): - require(job_id == "validate" or environment(job) is None, - f"Ship/{job_id}: only intent may attach an environment") + require(job_id in ("approve", "validate") or environment(job) is None, + f"Ship/{job_id}: only review and preflight may attach environments") require(not secret_bearing(job) or job_id == "validate", - f"Ship/{job_id}: secrets must stay in intent or guarded channels") + f"Ship/{job_id}: only guarded preflight may reference secrets") if "uses" in job: require(job_id in CHANNELS, f"unclassified Ship channel: {job_id}") + require("secrets" not in job, f"Ship/{job_id}: channels must use environment secrets directly") for channel, filename in CHANNELS.items(): call = ship[channel] require(call.get("uses") == f"./.github/workflows/{filename}", @@ -140,53 +160,54 @@ def guard_check(jobs: dict, workflow_name: str) -> None: require("workflow_dispatch" in document["on"] and "workflow_call" in document["on"], f"{filename}: recovery dispatch and reusable call must remain supported") guard_check(document["jobs"], filename) + approval_check(document["jobs"], filename, reusable=True) count = 0 for job_id, job in document["jobs"].items(): - if environment(job) is not None: + if job_id not in ("guard", "approve"): count += 1 - require(environment(job) == CHANNEL_ENVIRONMENT, - f"{filename}/{job_id}: caller identity must select channel environment") - require(needs(job) == ["guard"] and not job.get("if"), - f"{filename}/{job_id}: must require successful guard") - require(not secret_bearing(job) or environment(job) == CHANNEL_ENVIRONMENT, - f"{filename}/{job_id}: secrets require guarded environment") + require(environment(job) == "release", + f"{filename}/{job_id}: channel must use the sole release secret store") + require(needs(job) == ["guard", "approve"] + and re.sub(r"\s+", " ", job.get("if", "")).strip() == CHANNEL_READY + and not job.get("continue-on-error"), + f"{filename}/{job_id}: must require guard and standalone or Ship approval") + require(not secret_bearing(job) or environment(job) == "release", + f"{filename}/{job_id}: secrets require the single guarded environment") require("uses" not in job, f"{filename}: nested channels require approval policy review") require(count == 1, f"{filename}: expected one guarded channel job") for filename in STANDALONE_ONLY: - guard_check(workflows[filename]["jobs"], filename) - for job_id, job in workflows[filename]["jobs"].items(): - if secret_bearing(job): + jobs = workflows[filename]["jobs"] + guard_check(jobs, filename) + approval_check(jobs, filename) + require(len(jobs) == 3, f"{filename}: classify additional jobs before adding them") + for job_id, job in jobs.items(): + if job_id not in ("guard", "approve"): require(environment(job) == "release", - f"{filename}/{job_id}: standalone secrets require release review") - require(needs(job) == ["guard"] and not job.get("if"), - f"{filename}/{job_id}: standalone secrets require successful guard") - channel_names: set[str] = set() - for filename in CHANNELS.values(): - channel_names.update(secret_names(workflows[filename])) - reviewed_names = channel_names | secret_names(intent) - for filename in STANDALONE_ONLY: - reviewed_names.update(secret_names(workflows[filename])) + f"{filename}/{job_id}: standalone secrets require the sole release store") + require(needs(job) == ["guard", "approve"] and not job.get("if") + and not job.get("continue-on-error"), + f"{filename}/{job_id}: standalone secrets require successful review") + all_names = set().union(*(secret_names(workflows[filename]) for filename in + ("ship.yml", *CHANNELS.values(), *STANDALONE_ONLY))) classified = {"ship.yml", *CHANNELS.values(), *STANDALONE_ONLY} for filename, document in workflows.items(): if filename not in classified: for job in document.get("jobs", {}).values(): - require("release-channels" not in (environment(job) or ""), - f"{filename}: unclassified workflow may not select release-channels") - reviewed_jobs = [job_id for job_id, job in ship.items() if environment(job) == "release"] - standalone = { - filename: next("release" if environment(job) == CHANNEL_ENVIRONMENT else environment(job) - for job in workflows[filename]["jobs"].values() if secret_bearing(job)) - for filename in (*CHANNELS.values(), *STANDALONE_ONLY) - } + require(not any(name in (environment(job) or "") for name in + ("release", "release-approval", "release-channels")), + f"{filename}: classify workflows before selecting release environments") + reviewed_jobs = [job_id for job_id, job in ship.items() + if environment(job) == "release-approval"] return { "proof": "structural dry-run; live environment configuration is owner-confirmed", - "secret_names_by_environment": {"release": sorted(reviewed_names), - "release-channels": sorted(channel_names)}, - "activation": "Repository variable RELEASE_CHANNELS_CONFIGURED=true after owner setup", - "fallback": "Unset/false activation keeps every channel on reviewed release", + "secret_names_by_environment": {"release": sorted(all_names), "release-approval": []}, + "activation": "RELEASE_APPROVALS_CONFIGURED=true only after owner moves reviewer role", + "fallback": "Unset/false activation refuses new release workflows before any environment job", "ship": {"reviewed_jobs": reviewed_jobs, "approval_count": len(reviewed_jobs), - "channel_environment": "release-channels", "channels": list(CHANNELS)}, - "standalone": standalone, + "secret_environment": "release", "channels": list(CHANNELS)}, + "standalone": {filename: {"reviewed_jobs": ["approve"], "approval_count": 1, + "secret_environment": "release"} + for filename in (*CHANNELS.values(), *STANDALONE_ONLY)}, } diff --git a/scripts/release-approval-config.sh b/scripts/release-approval-config.sh new file mode 100755 index 0000000..1587cb4 --- /dev/null +++ b/scripts/release-approval-config.sh @@ -0,0 +1,7 @@ +#!/usr/bin/env bash +# No environment or secret is attached until the owner confirms the role move. +set -euo pipefail +if [[ "${RELEASE_APPROVALS_CONFIGURED:-}" != "true" ]]; then + echo "Release approval setup is incomplete; follow docs/release.md before enabling RELEASE_APPROVALS_CONFIGURED." >&2 + exit 1 +fi From 66870b4f9385c656be19417f5d5793c5c5c8abf9 Mon Sep 17 00:00:00 2001 From: shiny-code-bot Date: Sat, 3 Oct 2026 11:20:39 -0400 Subject: [PATCH 2/8] Account for migration review and workflow-level secret leaks --- Tests/ScriptsTests/test_release_approvals.py | 2 ++ docs/release.md | 13 +++++++++++++ docs/repo-settings.md | 3 ++- scripts/check-release-approvals.py | 15 ++++++++++++--- 4 files changed, 29 insertions(+), 4 deletions(-) diff --git a/Tests/ScriptsTests/test_release_approvals.py b/Tests/ScriptsTests/test_release_approvals.py index 850261f..c12d639 100644 --- a/Tests/ScriptsTests/test_release_approvals.py +++ b/Tests/ScriptsTests/test_release_approvals.py @@ -56,6 +56,8 @@ def test_full_ship_plan_requires_one_approval(self) -> None: def test_approval_bypasses_and_extra_prompts_fail(self) -> None: mutations = [ + lambda d: d["release.yml"].update(env={"KEY": "${{ secrets.KEY }}"}), + lambda d: d["release.yml"].update(defaults={"run": {"working-directory": "${{ secrets.KEY }}"}}), lambda d: d["ship.yml"]["jobs"]["github-release"].update(needs="guard"), lambda d: d["ship.yml"]["jobs"]["validate"].update(**{"continue-on-error": True}), lambda d: d["ship.yml"]["jobs"]["validate"].update(**{"if": "${{ false }}"}), diff --git a/docs/release.md b/docs/release.md index bee3281..d52ed6f 100644 --- a/docs/release.md +++ b/docs/release.md @@ -79,6 +79,19 @@ and this redesign has merged. Do not follow the earlier duplicate-secret setup. The preferred hand step leaves the existing `release` secret entries in place: no secret value is read, copied, rotated, or re-entered. +Historical runs use their original workflow commit when re-run. For up to +[30 days after the initial run](https://docs.github.com/en/actions/how-tos/manage-workflow-runs/re-run-workflows-and-jobs), +a pre-redesign run can still reference `release` without the new secretless gate. +Before removing its reviewer, Chris decides the historical-run treatment in +[direction#24](https://github.com/cbusillo/direction/issues/24) and records it on +#747. Waiting for that window to end preserves logs; deleting runs would require +Chris's explicit choice and evidence preservation. The agent performs neither. +Do not remove the reviewer or activate the final one-approval configuration +before that decision. If Chris chooses a waiting period, he can first configure +the new gate and confirm the secret inventory, leave `release` reviewed, and set +the new variable to continue releasing with extra approvals during the transition. +The one-approval contract applies after the role move is complete. + 1. In **Settings → Environments → release**, inspect secret **names only**. Confirm the enabled channels' names from the checker are already here. If a needed name exists only in repository secrets or another environment, stop diff --git a/docs/repo-settings.md b/docs/repo-settings.md index d81f1fd..e9f0e76 100644 --- a/docs/repo-settings.md +++ b/docs/repo-settings.md @@ -39,6 +39,8 @@ Expected GitHub settings: is exactly `true` only after the owner confirms the role move and secret-name inventory. Unset/false pauses new release workflows before environment jobs. Removing it does not restore reviewer settings or stop an already running job. + The owner may explicitly enable an interim route with both environments + reviewed while historical runs age out; that route still has extra prompts. Previous `RELEASE_CHANNELS_CONFIGURED` is obsolete and should be removed. - `release-channels` is unused; confirmed duplicate secrets there and at repository level are removed by the owner after older runs finish. Leave @@ -47,7 +49,6 @@ Expected GitHub settings: least 32 bytes of high-entropy key material shared with the operator Keychain entry that seals Production CloudKit schema receipts. Its sole GitHub copy belongs in `release`; never store its value in repository files. - - Immutable Releases: enabled so newly published GitHub Releases lock their tag, title, notes, and assets after publication. diff --git a/scripts/check-release-approvals.py b/scripts/check-release-approvals.py index 1695a42..c5f0b7f 100644 --- a/scripts/check-release-approvals.py +++ b/scripts/check-release-approvals.py @@ -120,6 +120,10 @@ def approval_check(jobs: dict, workflow_name: str, reusable: bool = False) -> No require(approval.get("if", "") == (STANDALONE_APPROVAL if reusable else ""), f"{workflow_name}: only the exact Ship caller may skip standalone review") + for filename in ("ship.yml", *CHANNELS.values(), *STANDALONE_ONLY): + document = workflows[filename] + require(not secret_bearing({"env": document.get("env"), "defaults": document.get("defaults")}), + f"{filename}: workflow-level secrets would reach the secretless gates") ship = workflows["ship.yml"]["jobs"] guard_check(ship, "ship.yml") approval_check(ship, "ship.yml") @@ -205,9 +209,14 @@ def approval_check(jobs: dict, workflow_name: str, reusable: bool = False) -> No "fallback": "Unset/false activation refuses new release workflows before any environment job", "ship": {"reviewed_jobs": reviewed_jobs, "approval_count": len(reviewed_jobs), "secret_environment": "release", "channels": list(CHANNELS)}, - "standalone": {filename: {"reviewed_jobs": ["approve"], "approval_count": 1, - "secret_environment": "release"} - for filename in (*CHANNELS.values(), *STANDALONE_ONLY)}, + "standalone": { + filename: {"reviewed_jobs": [job_id for job_id, job in workflows[filename]["jobs"].items() + if environment(job) == "release-approval"], + "approval_count": sum(environment(job) == "release-approval" + for job in workflows[filename]["jobs"].values()), + "secret_environment": "release"} + for filename in (*CHANNELS.values(), *STANDALONE_ONLY) + }, } From 74fa6ce0086f383fcf44a5405168055b5c2f0296 Mon Sep 17 00:00:00 2001 From: shiny-code-bot Date: Sat, 3 Oct 2026 11:32:28 -0400 Subject: [PATCH 3/8] Clarify single-store migration and inspect explicit release paths --- Tests/ScriptsTests/test_release_approvals.py | 3 +++ docs/release.md | 19 +++++++++++++------ docs/repo-settings.md | 5 ++++- scripts/check-release-approvals.py | 16 +++++++++------- 4 files changed, 29 insertions(+), 14 deletions(-) diff --git a/Tests/ScriptsTests/test_release_approvals.py b/Tests/ScriptsTests/test_release_approvals.py index c12d639..5658dc2 100644 --- a/Tests/ScriptsTests/test_release_approvals.py +++ b/Tests/ScriptsTests/test_release_approvals.py @@ -56,6 +56,9 @@ def test_full_ship_plan_requires_one_approval(self) -> None: def test_approval_bypasses_and_extra_prompts_fail(self) -> None: mutations = [ + lambda d: d["release.yml"]["jobs"]["approve"].update(**{"continue-on-error": True}), + lambda d: d["release.yml"]["jobs"]["approve"].update(needs=[]), + lambda d: d["release.yml"]["jobs"].pop("approve"), lambda d: d["release.yml"].update(env={"KEY": "${{ secrets.KEY }}"}), lambda d: d["release.yml"].update(defaults={"run": {"working-directory": "${{ secrets.KEY }}"}}), lambda d: d["ship.yml"]["jobs"]["github-release"].update(needs="guard"), diff --git a/docs/release.md b/docs/release.md index d52ed6f..a614d64 100644 --- a/docs/release.md +++ b/docs/release.md @@ -52,8 +52,9 @@ secretless trust guard verifies the protected ref, checked-out commit, main ancestry, version, and build number. A second secretless check refuses new runs unless the repository variable `RELEASE_APPROVALS_CONFIGURED` is exactly `true`. Set it only after the owner finishes the role move below. Removing it pauses new -release runs; it does not re-add an environment reviewer or affect an already -approved run. The previous `RELEASE_CHANNELS_CONFIGURED` variable and +release runs; it does not re-add an environment reviewer. Each pending channel +guard reads the variable again, so removing it can also block a channel in an +approved run. Jobs whose guards already passed are unaffected. The previous `RELEASE_CHANNELS_CONFIGURED` variable and `release-channels` environment are unused by this design. Retry failed channel jobs within the original approved run to retain its review; @@ -94,9 +95,14 @@ The one-approval contract applies after the role move is complete. 1. In **Settings → Environments → release**, inspect secret **names only**. Confirm the enabled channels' names from the checker are already here. If a - needed name exists only in repository secrets or another environment, stop - and report the name on #747 before removing any reviewer or activating. The - agent cannot verify this inventory or recover GitHub's write-only values. + needed name exists only elsewhere, use its original private source yourself: + **Environment secrets → Add environment secret** in `release`, enter its name + and value, and click **Add secret** once. Then remove the old copy after + confirming this destination name. This exception needs one entry, never two + maintained copies. If its original source is unavailable, report the name on + #747 before removing any reviewer or activating; do not rotate it. The agent + never reads, copies or re-enters a value. Skip this entry entirely when the + existing `release` inventory is complete. 2. In **Settings → Environments → New environment**, enter `release-approval` and click **Configure environment**. Enable **Required reviewers**, select `cbusillo`, and save protection rules. Keep self-review allowed for the solo @@ -104,7 +110,8 @@ The one-approval contract applies after the role move is complete. secrets. Under **Deployment branches and tags**, choose **Selected branches and tags** → **Add deployment branch or tag rule** → **Branch**, enter `main`, and save. -3. Return to **release**. Set the same selected **Branch** rule for `main` only. +3. Only after the direction#24 decision is recorded on #747, return to + **release**. Set the same selected **Branch** rule for `main` only. Uncheck **Required reviewers**, keep wait timer off, and save protection rules. Leave every environment secret untouched. The required reviewer now sits on `release-approval`, while all secret jobs continue reading `release`. diff --git a/docs/repo-settings.md b/docs/repo-settings.md index e9f0e76..3abfbb0 100644 --- a/docs/repo-settings.md +++ b/docs/repo-settings.md @@ -38,7 +38,10 @@ Expected GitHub settings: - Activation variable: repository Actions variable `RELEASE_APPROVALS_CONFIGURED` is exactly `true` only after the owner confirms the role move and secret-name inventory. Unset/false pauses new release workflows before environment jobs. - Removing it does not restore reviewer settings or stop an already running job. + Removing it does not restore reviewer settings or stop an already running job; + it blocks any channel guard that has not yet passed, even in an approved run. + Never delete or unprotect `release-approval` while activation is true. Unset + activation first and finish/cancel pending release runs before changing it. The owner may explicitly enable an interim route with both environments reviewed while historical runs age out; that route still has extra prompts. Previous `RELEASE_CHANNELS_CONFIGURED` is obsolete and should be removed. diff --git a/scripts/check-release-approvals.py b/scripts/check-release-approvals.py index c5f0b7f..6891396 100644 --- a/scripts/check-release-approvals.py +++ b/scripts/check-release-approvals.py @@ -112,7 +112,7 @@ def guard_check(jobs: dict, workflow_name: str) -> None: f"{workflow_name}: owner-confirmed setup is required before environment jobs") def approval_check(jobs: dict, workflow_name: str, reusable: bool = False) -> None: - approval = jobs["approve"] + approval = jobs.get("approve", {}) require(environment(approval) == "release-approval" and not secret_bearing(approval), f"{workflow_name}: review gate must be secretless in release-approval") require(needs(approval) == ["guard"] and not approval.get("continue-on-error"), @@ -180,11 +180,11 @@ def approval_check(jobs: dict, workflow_name: str, reusable: bool = False) -> No require("uses" not in job, f"{filename}: nested channels require approval policy review") require(count == 1, f"{filename}: expected one guarded channel job") for filename in STANDALONE_ONLY: - jobs = workflows[filename]["jobs"] - guard_check(jobs, filename) - approval_check(jobs, filename) - require(len(jobs) == 3, f"{filename}: classify additional jobs before adding them") - for job_id, job in jobs.items(): + standalone_jobs = workflows[filename]["jobs"] + guard_check(standalone_jobs, filename) + approval_check(standalone_jobs, filename) + require(len(standalone_jobs) == 3, f"{filename}: classify additional jobs before adding them") + for job_id, job in standalone_jobs.items(): if job_id not in ("guard", "approve"): require(environment(job) == "release", f"{filename}/{job_id}: standalone secrets require the sole release store") @@ -204,7 +204,9 @@ def approval_check(jobs: dict, workflow_name: str, reusable: bool = False) -> No if environment(job) == "release-approval"] return { "proof": "structural dry-run; live environment configuration is owner-confirmed", - "secret_names_by_environment": {"release": sorted(all_names), "release-approval": []}, + "secret_names_by_environment": {"release": sorted(all_names), "release-approval": sorted(set().union(*( + secret_names(workflows[filename]["jobs"]["approve"]) + for filename in ("ship.yml", *CHANNELS.values(), *STANDALONE_ONLY))))}, "activation": "RELEASE_APPROVALS_CONFIGURED=true only after owner moves reviewer role", "fallback": "Unset/false activation refuses new release workflows before any environment job", "ship": {"reviewed_jobs": reviewed_jobs, "approval_count": len(reviewed_jobs), From 64d13c1e7a92e9253a5d3a2a09667c66a1dba98f Mon Sep 17 00:00:00 2001 From: shiny-code-bot Date: Sat, 3 Oct 2026 11:35:24 -0400 Subject: [PATCH 4/8] Fix formatting in the owner migration instructions --- docs/release.md | 7 ++++--- 1 file changed, 4 insertions(+), 3 deletions(-) diff --git a/docs/release.md b/docs/release.md index a614d64..4b741f9 100644 --- a/docs/release.md +++ b/docs/release.md @@ -54,8 +54,9 @@ unless the repository variable `RELEASE_APPROVALS_CONFIGURED` is exactly `true`. Set it only after the owner finishes the role move below. Removing it pauses new release runs; it does not re-add an environment reviewer. Each pending channel guard reads the variable again, so removing it can also block a channel in an -approved run. Jobs whose guards already passed are unaffected. The previous `RELEASE_CHANNELS_CONFIGURED` variable and -`release-channels` environment are unused by this design. +approved run. Jobs whose guards already passed are unaffected. The previous +`RELEASE_CHANNELS_CONFIGURED` variable and `release-channels` environment are +unused by this design. Retry failed channel jobs within the original approved run to retain its review; a new Ship dispatch needs its own approval. Re-running the approval job itself @@ -85,7 +86,7 @@ Historical runs use their original workflow commit when re-run. For up to a pre-redesign run can still reference `release` without the new secretless gate. Before removing its reviewer, Chris decides the historical-run treatment in [direction#24](https://github.com/cbusillo/direction/issues/24) and records it on -#747. Waiting for that window to end preserves logs; deleting runs would require +issue #747. Waiting for that window to end preserves logs; deleting runs would require Chris's explicit choice and evidence preservation. The agent performs neither. Do not remove the reviewer or activate the final one-approval configuration before that decision. If Chris chooses a waiting period, he can first configure From d7c6fcd723ea49ad612951fdc95dd6d24531e11e Mon Sep 17 00:00:00 2001 From: shiny-code-bot Date: Sat, 3 Oct 2026 11:44:03 -0400 Subject: [PATCH 5/8] Verify required release review through read-only environment metadata --- .github/github.json | 2 +- .../app-store-connect-companion-upload.yml | 6 ++ .../workflows/app-store-connect-upload.yml | 6 ++ .github/workflows/ci.yml | 6 ++ .github/workflows/release.yml | 6 ++ .github/workflows/ship.yml | 6 ++ .github/workflows/submit-app-store-review.yml | 6 ++ .../testflight-beta-distribution.yml | 6 ++ .../upload-app-store-screenshots.yml | 6 ++ Tests/ScriptsTests/test_release_approvals.py | 42 ++++++++++++- docs/release.md | 7 ++- docs/repo-settings.md | 4 +- scripts/check-release-approval-environment.py | 62 +++++++++++++++++++ scripts/check-release-approvals.py | 7 +++ 14 files changed, 168 insertions(+), 4 deletions(-) create mode 100644 scripts/check-release-approval-environment.py diff --git a/.github/github.json b/.github/github.json index aafb126..57b076e 100644 --- a/.github/github.json +++ b/.github/github.json @@ -50,7 +50,7 @@ }, "validate": { "default": "scripts/commit-gate.sh", - "releaseApprovalPolicy": "uv run scripts/check-release-approvals.py (structural dry-run: one secretless Ship approval; standalone gates; one release secret store; owner role move and RELEASE_APPROVALS_CONFIGURED=true required)", + "releaseApprovalPolicy": "uv run scripts/check-release-approvals.py (structural dry-run: one secretless Ship approval; standalone gates; one release secret store; live review-metadata check; owner role move and RELEASE_APPROVALS_CONFIGURED=true required)", "ciChangeScope": "python3 scripts/ci-change-scope.py --base (pull requests only; skips swift build, swift test, and the companion builds when every changed path is documentation or validation tooling; unknown paths run everything; pushes to main and CodeQL always run in full)", "companionUploadFixtureTools": "CONTEXT_PANEL_UPLOAD_FIXTURE_TOOLS_DIR= scripts/upload-app-store-connect-companion-app.sh --export-only (test-only seam for fixture xcodebuild and codesign; refused for uploads)", "cloudKitCompanionSchema": "scripts/validate-cloudkit-companion-schema.sh", diff --git a/.github/workflows/app-store-connect-companion-upload.yml b/.github/workflows/app-store-connect-companion-upload.yml index 7b06774..f43fed2 100644 --- a/.github/workflows/app-store-connect-companion-upload.yml +++ b/.github/workflows/app-store-connect-companion-upload.yml @@ -75,6 +75,7 @@ jobs: runs-on: ubuntu-latest permissions: contents: read + actions: read steps: - name: Checkout uses: actions/checkout@v7 @@ -96,6 +97,11 @@ jobs: RELEASE_APPROVALS_CONFIGURED: ${{ vars.RELEASE_APPROVALS_CONFIGURED }} run: scripts/release-approval-config.sh + - name: Verify Required Review Environment + env: + GH_TOKEN: ${{ github.token }} + run: python3 scripts/check-release-approval-environment.py + approve: name: Approve Release Intent needs: guard diff --git a/.github/workflows/app-store-connect-upload.yml b/.github/workflows/app-store-connect-upload.yml index 06b3a75..74d6579 100644 --- a/.github/workflows/app-store-connect-upload.yml +++ b/.github/workflows/app-store-connect-upload.yml @@ -58,6 +58,7 @@ jobs: runs-on: ubuntu-latest permissions: contents: read + actions: read steps: - name: Checkout uses: actions/checkout@v7 @@ -79,6 +80,11 @@ jobs: RELEASE_APPROVALS_CONFIGURED: ${{ vars.RELEASE_APPROVALS_CONFIGURED }} run: scripts/release-approval-config.sh + - name: Verify Required Review Environment + env: + GH_TOKEN: ${{ github.token }} + run: python3 scripts/check-release-approval-environment.py + approve: name: Approve Release Intent needs: guard diff --git a/.github/workflows/ci.yml b/.github/workflows/ci.yml index eef3384..7e2275b 100644 --- a/.github/workflows/ci.yml +++ b/.github/workflows/ci.yml @@ -11,6 +11,7 @@ name: CI permissions: contents: read + actions: read concurrency: group: ci-${{ github.workflow }}-${{ github.event.pull_request.number || github.sha }} @@ -53,6 +54,11 @@ jobs: - name: Set up uv for release approval policy lint uses: astral-sh/setup-uv@v7 + - name: Verify environment metadata read capability + env: + GH_TOKEN: ${{ github.token }} + run: python3 scripts/check-release-approval-environment.py --probe release + - name: Check release approval graph run: uv run scripts/check-release-approvals.py diff --git a/.github/workflows/release.yml b/.github/workflows/release.yml index d815d86..168e4ca 100644 --- a/.github/workflows/release.yml +++ b/.github/workflows/release.yml @@ -66,6 +66,7 @@ jobs: runs-on: ubuntu-latest permissions: contents: read + actions: read steps: - name: Checkout uses: actions/checkout@v7 @@ -87,6 +88,11 @@ jobs: RELEASE_APPROVALS_CONFIGURED: ${{ vars.RELEASE_APPROVALS_CONFIGURED }} run: scripts/release-approval-config.sh + - name: Verify Required Review Environment + env: + GH_TOKEN: ${{ github.token }} + run: python3 scripts/check-release-approval-environment.py + approve: name: Approve Release Intent needs: guard diff --git a/.github/workflows/ship.yml b/.github/workflows/ship.yml index 8684997..be4d5ad 100644 --- a/.github/workflows/ship.yml +++ b/.github/workflows/ship.yml @@ -89,6 +89,7 @@ jobs: runs-on: ubuntu-latest permissions: contents: read + actions: read steps: - name: Checkout uses: actions/checkout@v7 @@ -110,6 +111,11 @@ jobs: RELEASE_APPROVALS_CONFIGURED: ${{ vars.RELEASE_APPROVALS_CONFIGURED }} run: scripts/release-approval-config.sh + - name: Verify Required Review Environment + env: + GH_TOKEN: ${{ github.token }} + run: python3 scripts/check-release-approval-environment.py + approve: name: Approve Release Intent needs: guard diff --git a/.github/workflows/submit-app-store-review.yml b/.github/workflows/submit-app-store-review.yml index 6c41bf2..4ff8d3d 100644 --- a/.github/workflows/submit-app-store-review.yml +++ b/.github/workflows/submit-app-store-review.yml @@ -269,6 +269,7 @@ jobs: runs-on: ubuntu-latest permissions: contents: read + actions: read steps: - name: Checkout uses: actions/checkout@v7 @@ -290,6 +291,11 @@ jobs: RELEASE_APPROVALS_CONFIGURED: ${{ vars.RELEASE_APPROVALS_CONFIGURED }} run: scripts/release-approval-config.sh + - name: Verify Required Review Environment + env: + GH_TOKEN: ${{ github.token }} + run: python3 scripts/check-release-approval-environment.py + approve: name: Approve Release Intent needs: guard diff --git a/.github/workflows/testflight-beta-distribution.yml b/.github/workflows/testflight-beta-distribution.yml index 0e21499..e2f33ab 100644 --- a/.github/workflows/testflight-beta-distribution.yml +++ b/.github/workflows/testflight-beta-distribution.yml @@ -85,6 +85,7 @@ jobs: runs-on: ubuntu-latest permissions: contents: read + actions: read steps: - name: Checkout uses: actions/checkout@v7 @@ -106,6 +107,11 @@ jobs: RELEASE_APPROVALS_CONFIGURED: ${{ vars.RELEASE_APPROVALS_CONFIGURED }} run: scripts/release-approval-config.sh + - name: Verify Required Review Environment + env: + GH_TOKEN: ${{ github.token }} + run: python3 scripts/check-release-approval-environment.py + approve: name: Approve Release Intent needs: guard diff --git a/.github/workflows/upload-app-store-screenshots.yml b/.github/workflows/upload-app-store-screenshots.yml index d5265bb..b606e95 100644 --- a/.github/workflows/upload-app-store-screenshots.yml +++ b/.github/workflows/upload-app-store-screenshots.yml @@ -50,6 +50,7 @@ jobs: runs-on: ubuntu-latest permissions: contents: read + actions: read steps: - name: Checkout uses: actions/checkout@v7 @@ -68,6 +69,11 @@ jobs: RELEASE_APPROVALS_CONFIGURED: ${{ vars.RELEASE_APPROVALS_CONFIGURED }} run: scripts/release-approval-config.sh + - name: Verify Required Review Environment + env: + GH_TOKEN: ${{ github.token }} + run: python3 scripts/check-release-approval-environment.py + approve: name: Approve Release Intent needs: guard diff --git a/Tests/ScriptsTests/test_release_approvals.py b/Tests/ScriptsTests/test_release_approvals.py index 5658dc2..7223809 100644 --- a/Tests/ScriptsTests/test_release_approvals.py +++ b/Tests/ScriptsTests/test_release_approvals.py @@ -4,6 +4,8 @@ import unittest import os import subprocess +from unittest.mock import patch +from types import SimpleNamespace from typing import Any spec = importlib.util.spec_from_file_location( @@ -16,11 +18,13 @@ def guard() -> dict[str, Any]: - return {"steps": [ + return {"permissions": {"actions": "read"}, "steps": [ {"uses": "actions/checkout@v7", "with": {"fetch-depth": 0}}, {"run": 'scripts/release-workflow-guard.sh --version "${INPUT_VERSION}" --build-number "${INPUT_BUILD_NUMBER}"'}, {"run": "scripts/release-approval-config.sh", "env": {"RELEASE_APPROVALS_CONFIGURED": "${{ vars.RELEASE_APPROVALS_CONFIGURED }}"}}, + {"run": "python3 scripts/check-release-approval-environment.py", + "env": {"GH_TOKEN": "${{ github.token }}"}}, ]} @@ -56,6 +60,12 @@ def test_full_ship_plan_requires_one_approval(self) -> None: def test_approval_bypasses_and_extra_prompts_fail(self) -> None: mutations = [ + lambda d: d["ship.yml"]["jobs"]["approve"].update(**{"if": "${{ false }}"}), + lambda d: d["release.yml"]["jobs"]["guard"]["steps"][3].update(**{"if": "${{ false }}"}), + lambda d: d["release.yml"]["jobs"]["guard"]["steps"][3]["env"].update(GH_TOKEN="${{ secrets.OPERATOR_TOKEN }}"), + lambda d: d.update({"rogue.yml": {"jobs": {"gate": {"environment": "release-approval"}}}}), + lambda d: d[policy.STANDALONE_ONLY[0]]["jobs"].update(extra={}), + lambda d: d[policy.STANDALONE_ONLY[0]]["jobs"]["submit"].update(needs="guard"), lambda d: d["release.yml"]["jobs"]["approve"].update(**{"continue-on-error": True}), lambda d: d["release.yml"]["jobs"]["approve"].update(needs=[]), lambda d: d["release.yml"]["jobs"].pop("approve"), @@ -121,3 +131,33 @@ def test_setup_confirmation_fails_closed(self) -> None: env["RELEASE_APPROVALS_CONFIGURED"] = value result = subprocess.run([str(script)], env=env, capture_output=True, text=True) self.assertEqual(result.returncode == 0, succeeds) + + def test_live_review_metadata_refuses_missing_or_bypassed_review(self) -> None: + metadata_spec = importlib.util.spec_from_file_location( + "approval_environment", Path(__file__).resolve().parents[2] + / "scripts/check-release-approval-environment.py" + ) + if metadata_spec is None or metadata_spec.loader is None: + raise RuntimeError("environment module could not be loaded") + metadata = importlib.util.module_from_spec(metadata_spec) + metadata_spec.loader.exec_module(metadata) + good = {"name": "release-approval", "can_admins_bypass": False, + "protection_rules": [{"type": "required_reviewers", "prevent_self_review": False, + "reviewers": [{"type": "User", "reviewer": {"login": "owner"}}]}]} + metadata.check(good, "owner") + mutations = [ + lambda d: d.update(protection_rules=[]), + lambda d: d.update(can_admins_bypass=True), + lambda d: d.pop("can_admins_bypass"), + lambda d: d["protection_rules"][0].update(reviewers=[]), + lambda d: d["protection_rules"][0]["reviewers"][0]["reviewer"].update(login="someone_else"), + lambda d: d["protection_rules"][0].update(prevent_self_review=True), + ] + for mutate in mutations: + document = copy.deepcopy(good) + mutate(document) + with self.assertRaises(ValueError): + metadata.check(document, "owner") + with patch.object(metadata.subprocess, "run", return_value=SimpleNamespace(returncode=1)): + with self.assertRaises(ValueError): + metadata.fetch_environment("owner/repo", "release-approval") diff --git a/docs/release.md b/docs/release.md index 4b741f9..d7ff0d1 100644 --- a/docs/release.md +++ b/docs/release.md @@ -51,7 +51,12 @@ Dispatch release workflows from `main`. Before any environment job, the secretless trust guard verifies the protected ref, checked-out commit, main ancestry, version, and build number. A second secretless check refuses new runs unless the repository variable `RELEASE_APPROVALS_CONFIGURED` is exactly `true`. -Set it only after the owner finishes the role move below. Removing it pauses new +Before any environment job, a read-only metadata check also requires +`release-approval` to exist, name the repository owner as sole required reviewer, +allow solo self-review and disable administrator bypass. Missing rules, a missing +environment or an unavailable API fail closed. CI probes this metadata read using +the built-in workflow token; no operator credential or secret endpoint is used. +Set activation only after the owner finishes the role move below. Removing it pauses new release runs; it does not re-add an environment reviewer. Each pending channel guard reads the variable again, so removing it can also block a channel in an approved run. Jobs whose guards already passed are unaffected. The previous diff --git a/docs/repo-settings.md b/docs/repo-settings.md index 3abfbb0..9513825 100644 --- a/docs/repo-settings.md +++ b/docs/repo-settings.md @@ -30,7 +30,9 @@ Expected GitHub settings: - Force pushes and branch deletion: blocked for `main`. - Review environment: `release-approval`, with `cbusillo` as required reviewer, self-review allowed, administrator bypass disabled, no wait timer and no - secrets. Selected deployment **Branch** rule: `main` only. + secrets. Selected deployment **Branch** rule: `main` only. Every release trust + guard verifies the required reviewer and disabled bypass through read-only + environment metadata before the reviewed job can start. - Secret environment: existing `release`, with no reviewer or wait timer and a selected deployment **Branch** rule for protected `main` only. Every GitHub release secret lives only here. Ship preflight and all six standalone/channel diff --git a/scripts/check-release-approval-environment.py b/scripts/check-release-approval-environment.py new file mode 100644 index 0000000..a4b9f20 --- /dev/null +++ b/scripts/check-release-approval-environment.py @@ -0,0 +1,62 @@ +#!/usr/bin/env python3 +"""Read environment protection metadata; never read environment secrets.""" +from __future__ import annotations + +import argparse +import json +import os +import subprocess +from urllib.parse import quote + + +def fetch_environment(repository: str, name: str) -> dict: + # gh uses the built-in workflow token supplied as GH_TOKEN by Actions. + result = subprocess.run( + ["gh", "api", "--method", "GET", f"repos/{repository}/environments/{quote(name, safe='')}"], + capture_output=True, text=True, check=False, + ) + if result.returncode: + raise ValueError("environment metadata is unavailable; refusing release") + value = json.loads(result.stdout) + if not isinstance(value, dict): + raise ValueError("invalid environment metadata") + return value + + +def check(environment: dict, owner: str) -> None: + if environment.get("name") != "release-approval": + raise ValueError("release-approval environment is missing") + rules = [rule for rule in environment.get("protection_rules", []) + if rule.get("type") == "required_reviewers"] + if len(rules) != 1: + raise ValueError("release-approval must have a required reviewer") + reviewers = rules[0].get("reviewers", []) + if len(reviewers) != 1 or reviewers[0].get("type") != "User" or ( + reviewers[0].get("reviewer", {}).get("login", "").casefold() != owner.casefold() + ): + raise ValueError("release-approval must require the repository owner's review") + if environment.get("can_admins_bypass") is not False: + raise ValueError("release-approval must disable administrator bypass") + if rules[0].get("prevent_self_review") is not False: + raise ValueError("release-approval must allow the solo owner to review") + + +def main() -> int: + parser = argparse.ArgumentParser(description=__doc__) + parser.add_argument("--probe", help="CI read-only capability probe of an existing environment") + args = parser.parse_args() + repository = os.environ.get("GITHUB_REPOSITORY", "") + if repository.count("/") != 1: + parser.exit(1, "GITHUB_REPOSITORY is required\n") + try: + value = fetch_environment(repository, args.probe or "release-approval") + if not args.probe: + check(value, repository.split("/", 1)[0]) + print("environment metadata read succeeded" if args.probe else "required owner review verified") + except (ValueError, TypeError, KeyError, OSError) as error: + parser.exit(1, f"release review metadata: {error}\n") + return 0 + + +if __name__ == "__main__": + raise SystemExit(main()) diff --git a/scripts/check-release-approvals.py b/scripts/check-release-approvals.py index 6891396..cd40c05 100644 --- a/scripts/check-release-approvals.py +++ b/scripts/check-release-approvals.py @@ -110,6 +110,13 @@ def guard_check(jobs: dict, workflow_name: str) -> None: and setup_steps[0].get("env", {}).get("RELEASE_APPROVALS_CONFIGURED") == "${{ vars.RELEASE_APPROVALS_CONFIGURED }}", f"{workflow_name}: owner-confirmed setup is required before environment jobs") + metadata_steps = [step for step in steps if step.get("run") + == "python3 scripts/check-release-approval-environment.py"] + require(len(metadata_steps) == 1 and not metadata_steps[0].get("if") + and not metadata_steps[0].get("continue-on-error") + and metadata_steps[0].get("env", {}).get("GH_TOKEN") == "${{ github.token }}" + and guard.get("permissions", {}).get("actions") == "read", + f"{workflow_name}: live required-review metadata must be checked") def approval_check(jobs: dict, workflow_name: str, reusable: bool = False) -> None: approval = jobs.get("approve", {}) From 7431ef8f53f73d428aba19532fc49a48edf9482a Mon Sep 17 00:00:00 2001 From: shiny-code-bot Date: Sat, 3 Oct 2026 11:48:13 -0400 Subject: [PATCH 6/8] Resolve inspection findings in release metadata validation --- Tests/ScriptsTests/test_release_approvals.py | 2 +- docs/release.md | 4 ++-- scripts/check-release-approval-environment.py | 2 +- 3 files changed, 4 insertions(+), 4 deletions(-) diff --git a/Tests/ScriptsTests/test_release_approvals.py b/Tests/ScriptsTests/test_release_approvals.py index 7223809..260a6fc 100644 --- a/Tests/ScriptsTests/test_release_approvals.py +++ b/Tests/ScriptsTests/test_release_approvals.py @@ -158,6 +158,6 @@ def test_live_review_metadata_refuses_missing_or_bypassed_review(self) -> None: mutate(document) with self.assertRaises(ValueError): metadata.check(document, "owner") - with patch.object(metadata.subprocess, "run", return_value=SimpleNamespace(returncode=1)): + with patch.object(subprocess, "run", return_value=SimpleNamespace(returncode=1)): with self.assertRaises(ValueError): metadata.fetch_environment("owner/repo", "release-approval") diff --git a/docs/release.md b/docs/release.md index d7ff0d1..2471513 100644 --- a/docs/release.md +++ b/docs/release.md @@ -56,8 +56,8 @@ Before any environment job, a read-only metadata check also requires allow solo self-review and disable administrator bypass. Missing rules, a missing environment or an unavailable API fail closed. CI probes this metadata read using the built-in workflow token; no operator credential or secret endpoint is used. -Set activation only after the owner finishes the role move below. Removing it pauses new -release runs; it does not re-add an environment reviewer. Each pending channel +Set activation only after the owner finishes the role move below. Removing it +pauses new release runs; it does not re-add an environment reviewer. Each pending channel guard reads the variable again, so removing it can also block a channel in an approved run. Jobs whose guards already passed are unaffected. The previous `RELEASE_CHANNELS_CONFIGURED` variable and `release-channels` environment are diff --git a/scripts/check-release-approval-environment.py b/scripts/check-release-approval-environment.py index a4b9f20..ae49f7a 100644 --- a/scripts/check-release-approval-environment.py +++ b/scripts/check-release-approval-environment.py @@ -13,7 +13,7 @@ def fetch_environment(repository: str, name: str) -> dict: # gh uses the built-in workflow token supplied as GH_TOKEN by Actions. result = subprocess.run( ["gh", "api", "--method", "GET", f"repos/{repository}/environments/{quote(name, safe='')}"], - capture_output=True, text=True, check=False, + capture_output=True, text=True, ) if result.returncode: raise ValueError("environment metadata is unavailable; refusing release") From 04ba9cf05084b15f8e6913adb21c95b175c517b1 Mon Sep 17 00:00:00 2001 From: shiny-code-bot Date: Sat, 3 Oct 2026 11:52:49 -0400 Subject: [PATCH 7/8] Grant reusable guards metadata reads and validate secret-store branch scope --- .github/workflows/ci.yml | 3 +- .github/workflows/ship.yml | 10 +++ Tests/ScriptsTests/test_release_approvals.py | 21 ++++- docs/release.md | 18 +++-- scripts/check-release-approval-environment.py | 78 +++++++++++++++---- scripts/check-release-approvals.py | 3 + 6 files changed, 107 insertions(+), 26 deletions(-) diff --git a/.github/workflows/ci.yml b/.github/workflows/ci.yml index 7e2275b..2ce0c29 100644 --- a/.github/workflows/ci.yml +++ b/.github/workflows/ci.yml @@ -55,9 +55,10 @@ jobs: uses: astral-sh/setup-uv@v7 - name: Verify environment metadata read capability + if: ${{ github.repository == 'cbusillo/context-panel' }} env: GH_TOKEN: ${{ github.token }} - run: python3 scripts/check-release-approval-environment.py --probe release + run: python3 scripts/check-release-approval-environment.py --probe - name: Check release approval graph run: uv run scripts/check-release-approvals.py diff --git a/.github/workflows/ship.yml b/.github/workflows/ship.yml index be4d5ad..bfc15c1 100644 --- a/.github/workflows/ship.yml +++ b/.github/workflows/ship.yml @@ -171,6 +171,7 @@ jobs: uses: ./.github/workflows/release.yml permissions: contents: write + actions: read with: version: ${{ inputs.version }} build_number: ${{ needs.validate.outputs.build_number }} @@ -183,6 +184,9 @@ jobs: needs: validate if: ${{ inputs.app_store_channel != 'skip' }} uses: ./.github/workflows/app-store-connect-upload.yml + permissions: + contents: read + actions: read with: version: ${{ inputs.version }} build_number: ${{ needs.validate.outputs.build_number }} @@ -194,6 +198,9 @@ jobs: needs: validate if: ${{ inputs.companion_app_store_channel != 'skip' }} uses: ./.github/workflows/app-store-connect-companion-upload.yml + permissions: + contents: read + actions: read with: version: ${{ inputs.version }} build_number: ${{ needs.validate.outputs.build_number }} @@ -220,6 +227,9 @@ jobs: ) }} uses: ./.github/workflows/testflight-beta-distribution.yml + permissions: + contents: read + actions: read with: version: ${{ inputs.version }} build_number: >- diff --git a/Tests/ScriptsTests/test_release_approvals.py b/Tests/ScriptsTests/test_release_approvals.py index 260a6fc..80a40e7 100644 --- a/Tests/ScriptsTests/test_release_approvals.py +++ b/Tests/ScriptsTests/test_release_approvals.py @@ -34,7 +34,8 @@ def fixture() -> dict[str, Any]: "validate": {"environment": "release", "needs": "approve"}} documents = {"ship.yml": {"jobs": ship}} for channel, filename in policy.CHANNELS.items(): - ship[channel] = {"uses": f"./.github/workflows/{filename}", "needs": "validate"} + ship[channel] = {"uses": f"./.github/workflows/{filename}", "needs": "validate", + "permissions": {"contents": "read", "actions": "read"}} documents[filename] = { "on": {"workflow_dispatch": {}, "workflow_call": {}}, "jobs": {"guard": guard(), "approve": dict(approval, **{"if": policy.STANDALONE_APPROVAL}), @@ -60,6 +61,7 @@ def test_full_ship_plan_requires_one_approval(self) -> None: def test_approval_bypasses_and_extra_prompts_fail(self) -> None: mutations = [ + lambda d: d["ship.yml"]["jobs"]["github-release"]["permissions"].pop("actions"), lambda d: d["ship.yml"]["jobs"]["approve"].update(**{"if": "${{ false }}"}), lambda d: d["release.yml"]["jobs"]["guard"]["steps"][3].update(**{"if": "${{ false }}"}), lambda d: d["release.yml"]["jobs"]["guard"]["steps"][3]["env"].update(GH_TOKEN="${{ secrets.OPERATOR_TOKEN }}"), @@ -158,6 +160,21 @@ def test_live_review_metadata_refuses_missing_or_bypassed_review(self) -> None: mutate(document) with self.assertRaises(ValueError): metadata.check(document, "owner") + secret_store = {"deployment_branch_policy": {"protected_branches": True, + "custom_branch_policies": False}} + metadata.check_branches(secret_store, [{"name": "main"}]) + for branches in [[], [{"name": "main"}, {"name": "task"}]]: + with self.assertRaises(ValueError): + metadata.check_branches(secret_store, branches) + with self.assertRaises(ValueError): + metadata.check_branches({}, [{"name": "main"}]) + metadata.check_branches({"deployment_branch_policy": {"protected_branches": False, + "custom_branch_policies": True}}, + [{"name": "main", "type": "branch"}]) + hidden_review = copy.deepcopy(good) + hidden_review["protection_rules"][0]["reviewers"][0]["reviewer"] = None + with self.assertRaises(ValueError): + metadata.check_fields(hidden_review) with patch.object(subprocess, "run", return_value=SimpleNamespace(returncode=1)): with self.assertRaises(ValueError): - metadata.fetch_environment("owner/repo", "release-approval") + metadata.fetch_metadata("repos/owner/repo/environments/release-approval") diff --git a/docs/release.md b/docs/release.md index 2471513..5100977 100644 --- a/docs/release.md +++ b/docs/release.md @@ -40,7 +40,8 @@ The four reusable channels identify Ship through `github.workflow_ref`, GitHub's caller workflow identity, rather than a dispatch input. Only this repository's `ship.yml` on `refs/heads/main` skips their standalone `approve` job; their caller jobs must depend on successful Ship validation. Every other -caller and direct dispatch requires its own secretless reviewed gate before +caller must grant `actions: read` and `contents: read` for the metadata and +checkout guards; direct dispatch requires its own secretless reviewed gate before using `release`. App Store Review submission and screenshots always require that gate, including reusable calls. Channel jobs explicitly reject failed or cancelled guards and failed standalone approvals; the skipped-gate path is @@ -54,12 +55,14 @@ unless the repository variable `RELEASE_APPROVALS_CONFIGURED` is exactly `true`. Before any environment job, a read-only metadata check also requires `release-approval` to exist, name the repository owner as sole required reviewer, allow solo self-review and disable administrator bypass. Missing rules, a missing -environment or an unavailable API fail closed. CI probes this metadata read using +environment or an unavailable API fail closed. The check also verifies that +`release` accepts only `main`, and the CI probe proves the required metadata +fields are visible. CI probes this metadata read using the built-in workflow token; no operator credential or secret endpoint is used. Set activation only after the owner finishes the role move below. Removing it -pauses new release runs; it does not re-add an environment reviewer. Each pending channel -guard reads the variable again, so removing it can also block a channel in an -approved run. Jobs whose guards already passed are unaffected. The previous +pauses new release runs; it does not re-add an environment reviewer. Each pending +channel guard reads the variable again, so removing it can also block a channel +in an approved run. Jobs whose guards already passed are unaffected. The previous `RELEASE_CHANNELS_CONFIGURED` variable and `release-channels` environment are unused by this design. @@ -130,7 +133,10 @@ The one-approval contract applies after the role move is complete. 5. In **Settings → Secrets and variables → Actions → Variables**, delete the obsolete `RELEASE_CHANNELS_CONFIGURED` variable if present. Click **New repository variable**, name it `RELEASE_APPROVALS_CONFIGURED`, enter `true`, - and click **Add variable** (or edit/save an existing variable). Do this last. + and click **Add variable** (or edit/save an existing variable). Do this last + for the final role move. On an owner-chosen interim route, enable it after + step 2 and the inventory/duplicate cleanup while keeping `release` reviewed; + complete step 3 only at the recorded migration time. 6. Confirm configuration and names on #747 from `cbusillo`, never values. No release dispatch is required or authorized for this setup task. diff --git a/scripts/check-release-approval-environment.py b/scripts/check-release-approval-environment.py index ae49f7a..694e41d 100644 --- a/scripts/check-release-approval-environment.py +++ b/scripts/check-release-approval-environment.py @@ -9,51 +9,95 @@ from urllib.parse import quote -def fetch_environment(repository: str, name: str) -> dict: +def fetch_metadata(endpoint: str, *, pages: bool = False) -> dict | list: # gh uses the built-in workflow token supplied as GH_TOKEN by Actions. - result = subprocess.run( - ["gh", "api", "--method", "GET", f"repos/{repository}/environments/{quote(name, safe='')}"], - capture_output=True, text=True, - ) + command = ["gh", "api", "--method", "GET", endpoint] + if pages: + command += ["--paginate", "--slurp"] + result = subprocess.run(command, capture_output=True, text=True) if result.returncode: raise ValueError("environment metadata is unavailable; refusing release") - value = json.loads(result.stdout) - if not isinstance(value, dict): - raise ValueError("invalid environment metadata") - return value + return json.loads(result.stdout) + + +def check_fields(environment: dict) -> None: + if not isinstance(environment.get("can_admins_bypass"), bool) or not isinstance( + environment.get("protection_rules"), list + ): + raise ValueError("environment protection metadata is incomplete") + for rule in environment["protection_rules"]: + if rule.get("type") == "required_reviewers": + reviewers = rule.get("reviewers") + if not isinstance(reviewers, list) or not reviewers or any( + not (reviewer.get("reviewer") or {}).get("login") for reviewer in reviewers + ): + raise ValueError("reviewer metadata is incomplete") def check(environment: dict, owner: str) -> None: + check_fields(environment) if environment.get("name") != "release-approval": raise ValueError("release-approval environment is missing") - rules = [rule for rule in environment.get("protection_rules", []) + rules = [rule for rule in environment["protection_rules"] if rule.get("type") == "required_reviewers"] if len(rules) != 1: raise ValueError("release-approval must have a required reviewer") reviewers = rules[0].get("reviewers", []) if len(reviewers) != 1 or reviewers[0].get("type") != "User" or ( - reviewers[0].get("reviewer", {}).get("login", "").casefold() != owner.casefold() + (reviewers[0].get("reviewer") or {}).get("login", "").casefold() != owner.casefold() ): raise ValueError("release-approval must require the repository owner's review") - if environment.get("can_admins_bypass") is not False: + if environment["can_admins_bypass"] is not False: raise ValueError("release-approval must disable administrator bypass") if rules[0].get("prevent_self_review") is not False: raise ValueError("release-approval must allow the solo owner to review") +def check_branches(environment: dict, branches: list[dict]) -> None: + policy = environment.get("deployment_branch_policy") or {} + if policy.get("protected_branches") is True and policy.get("custom_branch_policies") is False: + allowed = [{"name": branch.get("name"), "type": "branch"} for branch in branches] + elif policy.get("custom_branch_policies") is True and policy.get("protected_branches") is False: + allowed = [{"name": branch.get("name"), "type": branch.get("type")} for branch in branches] + else: + raise ValueError("release secrets need a restricted branch policy") + if allowed != [{"name": "main", "type": "branch"}]: + raise ValueError("release secrets must allow only Branch main") + + +def read_environment(repository: str, name: str) -> dict: + value = fetch_metadata(f"repos/{repository}/environments/{quote(name, safe='')}") + if not isinstance(value, dict) or value.get("name") != name: + raise ValueError("invalid environment metadata") + check_fields(value) + return value + + +def check_secret_store(repository: str) -> None: + environment = read_environment(repository, "release") + if (environment.get("deployment_branch_policy") or {}).get("protected_branches"): + pages = fetch_metadata(f"repos/{repository}/branches?protected=true&per_page=100", pages=True) + branches = [branch for page in pages for branch in page] + else: + pages = fetch_metadata(f"repos/{repository}/environments/release/deployment-branch-policies?per_page=100", + pages=True) + branches = [branch for page in pages for branch in page.get("branch_policies", [])] + check_branches(environment, branches) + + def main() -> int: parser = argparse.ArgumentParser(description=__doc__) - parser.add_argument("--probe", help="CI read-only capability probe of an existing environment") + parser.add_argument("--probe", action="store_true", help="CI probe of secret-store protection metadata") args = parser.parse_args() repository = os.environ.get("GITHUB_REPOSITORY", "") if repository.count("/") != 1: parser.exit(1, "GITHUB_REPOSITORY is required\n") try: - value = fetch_environment(repository, args.probe or "release-approval") + check_secret_store(repository) if not args.probe: - check(value, repository.split("/", 1)[0]) - print("environment metadata read succeeded" if args.probe else "required owner review verified") - except (ValueError, TypeError, KeyError, OSError) as error: + check(read_environment(repository, "release-approval"), repository.split("/", 1)[0]) + print("secret-store protection metadata verified" if args.probe else "required owner review verified") + except (ValueError, TypeError, KeyError, AttributeError, OSError) as error: parser.exit(1, f"release review metadata: {error}\n") return 0 diff --git a/scripts/check-release-approvals.py b/scripts/check-release-approvals.py index cd40c05..a9038bf 100644 --- a/scripts/check-release-approvals.py +++ b/scripts/check-release-approvals.py @@ -153,6 +153,9 @@ def approval_check(jobs: dict, workflow_name: str, reusable: bool = False) -> No f"{channel}: channel must use the same-commit local workflow") require("validate" in needs(call), f"{channel}: missing intent dependency") require(not call.get("continue-on-error"), f"{channel}: cannot tolerate failure") + require(call.get("permissions", {}).get("actions") == "read" + and call.get("permissions", {}).get("contents") in ("read", "write"), + f"{channel}: caller must grant metadata read and checkout permissions") condition = re.sub(r"\s+", " ", call.get("if", "")).strip() if re.search(r"\b(always|failure|cancelled)\s*\(", condition): # The TestFlight join intentionally runs with skipped upload channels. From 793093e2ef656c2c8d2912b35c77080dec1c5e38 Mon Sep 17 00:00:00 2001 From: shiny-code-bot Date: Sat, 3 Oct 2026 12:01:20 -0400 Subject: [PATCH 8/8] Clarify metadata probe evidence and reusable caller permissions --- Tests/ScriptsTests/test_release_approvals.py | 6 +++--- docs/release.md | 10 ++++++---- 2 files changed, 9 insertions(+), 7 deletions(-) diff --git a/Tests/ScriptsTests/test_release_approvals.py b/Tests/ScriptsTests/test_release_approvals.py index 80a40e7..a5ce0ae 100644 --- a/Tests/ScriptsTests/test_release_approvals.py +++ b/Tests/ScriptsTests/test_release_approvals.py @@ -1,12 +1,12 @@ import copy import importlib.util -from pathlib import Path -import unittest import os +from pathlib import Path import subprocess -from unittest.mock import patch from types import SimpleNamespace from typing import Any +import unittest +from unittest.mock import patch spec = importlib.util.spec_from_file_location( "release_approvals", Path(__file__).resolve().parents[2] / "scripts/check-release-approvals.py" diff --git a/docs/release.md b/docs/release.md index 5100977..f5a29ab 100644 --- a/docs/release.md +++ b/docs/release.md @@ -40,8 +40,9 @@ The four reusable channels identify Ship through `github.workflow_ref`, GitHub's caller workflow identity, rather than a dispatch input. Only this repository's `ship.yml` on `refs/heads/main` skips their standalone `approve` job; their caller jobs must depend on successful Ship validation. Every other -caller must grant `actions: read` and `contents: read` for the metadata and -checkout guards; direct dispatch requires its own secretless reviewed gate before +caller must grant `actions: read` plus at least `contents: read` for metadata +and checkout; GitHub Release publication requires `contents: write`. Direct +dispatch requires its own secretless reviewed gate before using `release`. App Store Review submission and screenshots always require that gate, including reusable calls. Channel jobs explicitly reject failed or cancelled guards and failed standalone approvals; the skipped-gate path is @@ -56,8 +57,9 @@ Before any environment job, a read-only metadata check also requires `release-approval` to exist, name the repository owner as sole required reviewer, allow solo self-review and disable administrator bypass. Missing rules, a missing environment or an unavailable API fail closed. The check also verifies that -`release` accepts only `main`, and the CI probe proves the required metadata -fields are visible. CI probes this metadata read using +`release` accepts only `main`. The CI probe verifies the secret store's current +branch policy and protection-field visibility; the release guard checks the +review gate's actual reviewer settings when it exists. Both use the built-in workflow token; no operator credential or secret endpoint is used. Set activation only after the owner finishes the role move below. Removing it pauses new release runs; it does not re-add an environment reviewer. Each pending