From cbd89d678aaca288509af5695b499d4c38db0c09 Mon Sep 17 00:00:00 2001 From: Commitrail Probe Date: Fri, 23 Oct 2026 14:15:13 +0100 Subject: [PATCH 1/2] docs(plan): bound CLI administrator bypass guard repair --- .../initiatives/WS-CLI-001/WS-CLI-001-47.md | 69 +++++++++++++++++++ 1 file changed, 69 insertions(+) create mode 100644 .commitrail/initiatives/WS-CLI-001/WS-CLI-001-47.md diff --git a/.commitrail/initiatives/WS-CLI-001/WS-CLI-001-47.md b/.commitrail/initiatives/WS-CLI-001/WS-CLI-001-47.md new file mode 100644 index 000000000..117c9c3e8 --- /dev/null +++ b/.commitrail/initiatives/WS-CLI-001/WS-CLI-001-47.md @@ -0,0 +1,69 @@ +# WS-CLI-001-47 — Reject release environments with administrator approval bypass + +- Initiative: WS-CLI-001 +- Durable disposition: Planned +- Intended merge outcome: The existing CLI release-control guard refuses an environment unless GitHub explicitly reports administrator protection-rule bypass disabled; no release is published by this change. + +## Intent + +The first CLI publication must wait for an independent human approval. GitHub permits administrators to bypass environment protection rules by default, but the merged guard checks required reviewers and self-review prevention without checking that bypass setting. A configuration that defeats the approval gate can therefore pass the source guard. This is a bounded fail-closed repair to issue #562, not a new release channel or permission model. + +## Current behavior + +`scripts/check_cli_release_controls.py::validate_environment` checks the `cli-release` name, a required-reviewer rule with `prevent_self_review`, and the sole `main` deployment branch policy. `scripts/test_cli_release_controls.py` has no administrator-bypass case. A valid-looking environment with `can_admins_bypass: true` currently passes validation. The `release_guard` and `publish` jobs both call this validator with the environment returned by GitHub's GET endpoint. GitHub's documented GET endpoint accepts Actions read permission; the repository's existing public environment response exposes `can_admins_bypass: false`, including without authentication. Missing or altered fields in a future response must fail closed. + +## Bounded change + +### Allowed + +- `scripts/check_cli_release_controls.py`: require the existing environment response to contain literal `can_admins_bypass: false`. +- `scripts/test_cli_release_controls.py`: preserve existing controls and add a discriminating pass/fail matrix for administrator bypass and unknown field values. +- `docs/engineering/cli-release.md`: tell the release administrator to disable the GitHub UI bypass option and verify the GET response before approval. +- This change record: intent, proof and remaining human boundary. + +### Not allowed + +No workflow, permissions, package, CLI, backend/API, roadmap, overview or index change; no GitHub setting, environment, tag, release, dispatch, secret or credential write. Do not infer a false bypass value from an omitted field or add an undocumented API write parameter. Do not contact the named independent approver; the product owner coordinates that approval. + +## Design and decisions + +Extend the existing `validate_environment` owner with an identity check (`is False`) before the other environment-policy checks. A missing key, null, true or string value is not proof that administrator bypass is disabled. The existing `github_json` path and job permission split stay unchanged. GitHub's documented environment PUT parameters do not include this setting; the operator runbook uses the documented UI control and a read-only GET verification. A separate manual-only check was rejected because it leaves the machine guard able to pass a bypass-enabled environment. No parallel control service or token is needed. + +## Acceptance criteria + +- [ ] An environment with literal `can_admins_bypass: false` and the existing valid reviewer/branch controls passes; all existing tag-ruleset checks remain unchanged. +- [ ] `true`, `null`, a missing field, string values and numeric `0` each fail before native release builds or publication; the regression fails against the merged pre-fix guard. +- [ ] The operator instructions identify the UI setting and read-only verification without claiming the workflow can configure it. +- [ ] Existing workflow permissions, native proof, archive assembly and public-release status remain unchanged. The absent live `cli-release` environment still prevents dispatch; no tag or release is created. + +## Risk and review routing + +- Risk class: L1 — bounded release security and CI control integrity. +- Required reviewers: focused plan review before code; security, CI integrity and documentation on a clean implementation target. QA/test-delta review is added if the negative proof needs specialist replay. +- Human review focus: literal-false fail-closed semantics, GET field visibility under the existing read token, unchanged job permission boundary, and the UI-only configuration plus independent-approval responsibility. + +## Evidence + +Verification plan: first extend the existing valid environment fixture with literal +`false`, then add `test_environment_administrator_bypass_must_be_explicitly_disabled` +and `test_environment_missing_administrator_bypass_fails`. Run those exact tests +against the unchanged owner and require failures at the new assertions before +adding the identity guard. Run the full release-control module after the repair, +the canonical Agent Gates commands, and a scoped diff check confirming no workflow +or permission changes. The pure validator needs no database or publication drill. + +| Claim | Command or proof | Result | Remaining uncertainty | +|---|---|---|---| +| Pre-fix defect reaches assertion | Call `validate_environment` with a valid fixture plus `can_admins_bypass: true` | Accepted by merged guard | Regression must reject it after repair | +| Field can be inspected read-only | Official GET permission contract; public GET of existing repository environment | Actions read documented; response includes literal `false` | New `cli-release` environment must be verified when configured | +| No distribution capability change | Compare exact changed paths with the merged release workflow and roadmap | Pending implementation diff | First protected publication remains separate | + +## Review findings + +Record focused findings and their resolution here only if material; exact-head review status belongs in the pull request. + +## Reconciliation + +- Current-source reconciliation: based on main `7f6a71a69daf563742b72c706e6d09aa742ac25e`; recheck if main advances. The roadmap already calls out the protected `cli-release` environment as outstanding, so this narrows its validation rather than changing capability, exposure or next dependency. +- Next usable boundary: configure and verify the protected environment and immutable-tag ruleset, then a user-approved exact tag and manual release after the catalogue integration gate. +- Remaining risks: administrators control repository policy; an independent person must verify the configured environment and ruleset immediately before approving publication. The dispatch path cannot be exercised until those external controls exist. From 0c8bb5999132b06bc0cad8b30925eb1153e35c1f Mon Sep 17 00:00:00 2001 From: Commitrail Probe Date: Fri, 23 Oct 2026 14:23:56 +0100 Subject: [PATCH 2/2] fix(cli-release): reject administrator environment bypass --- .../initiatives/WS-CLI-001/WS-CLI-001-47.md | 15 ++++++++------- docs/engineering/cli-release.md | 18 +++++++++++++++++- scripts/check_cli_release_controls.py | 2 ++ scripts/test_cli_release_controls.py | 17 +++++++++++++++++ 4 files changed, 44 insertions(+), 8 deletions(-) diff --git a/.commitrail/initiatives/WS-CLI-001/WS-CLI-001-47.md b/.commitrail/initiatives/WS-CLI-001/WS-CLI-001-47.md index 117c9c3e8..4327df7b5 100644 --- a/.commitrail/initiatives/WS-CLI-001/WS-CLI-001-47.md +++ b/.commitrail/initiatives/WS-CLI-001/WS-CLI-001-47.md @@ -1,7 +1,7 @@ # WS-CLI-001-47 — Reject release environments with administrator approval bypass - Initiative: WS-CLI-001 -- Durable disposition: Planned +- Durable disposition: Complete - Intended merge outcome: The existing CLI release-control guard refuses an environment unless GitHub explicitly reports administrator protection-rule bypass disabled; no release is published by this change. ## Intent @@ -31,10 +31,10 @@ Extend the existing `validate_environment` owner with an identity check (`is Fal ## Acceptance criteria -- [ ] An environment with literal `can_admins_bypass: false` and the existing valid reviewer/branch controls passes; all existing tag-ruleset checks remain unchanged. -- [ ] `true`, `null`, a missing field, string values and numeric `0` each fail before native release builds or publication; the regression fails against the merged pre-fix guard. -- [ ] The operator instructions identify the UI setting and read-only verification without claiming the workflow can configure it. -- [ ] Existing workflow permissions, native proof, archive assembly and public-release status remain unchanged. The absent live `cli-release` environment still prevents dispatch; no tag or release is created. +- [x] An environment with literal `can_admins_bypass: false` and the existing valid reviewer/branch controls passes; all existing tag-ruleset checks remain unchanged. +- [x] `true`, `null`, a missing field, string values and numeric `0` each fail before native release builds or publication; the regression fails against the merged pre-fix guard. +- [x] The operator instructions identify the UI setting and read-only verification without claiming the workflow can configure it. +- [x] Existing workflow permissions, native proof, archive assembly and public-release status remain unchanged. The absent live `cli-release` environment still blocks release preparation; no tag or release is created. ## Risk and review routing @@ -54,9 +54,10 @@ or permission changes. The pure validator needs no database or publication drill | Claim | Command or proof | Result | Remaining uncertainty | |---|---|---|---| -| Pre-fix defect reaches assertion | Call `validate_environment` with a valid fixture plus `can_admins_bypass: true` | Accepted by merged guard | Regression must reject it after repair | +| Pre-fix defect reaches assertion | Both named regressions against the unchanged guard, with all existing controls valid | Six assertion failures: true, null, two strings, numeric zero and missing field were accepted | None for this pure predicate | +| Literal-false contract | Full `scripts/test_cli_release_controls.py` after repair | Eight tests pass, including existing reviewer/branch/tag rejection proof | Hosted preview still supplies native packaging proof | | Field can be inspected read-only | Official GET permission contract; public GET of existing repository environment | Actions read documented; response includes literal `false` | New `cli-release` environment must be verified when configured | -| No distribution capability change | Compare exact changed paths with the merged release workflow and roadmap | Pending implementation diff | First protected publication remains separate | +| No distribution capability change | Compare exact changed paths with the merged release workflow and roadmap | Only owner, tests, runbook and this record change | First protected publication remains separate | ## Review findings diff --git a/docs/engineering/cli-release.md b/docs/engineering/cli-release.md index 8107e2c21..15ecebedc 100644 --- a/docs/engineering/cli-release.md +++ b/docs/engineering/cli-release.md @@ -18,7 +18,23 @@ integration suite or prove every deployed backend version. Before the first release, a repository administrator must configure the `cli-release` GitHub environment with required human reviewers and a deployment branch restriction to `main`, with self-review prevention enabled, then inspect -those settings. The workflow queries the environment API in both preparation +those settings. In Settings > Environments > `cli-release`, deselect **Allow +administrators to bypass configured protection rules** and save the protection +rules, following [GitHub's environment instructions](https://docs.github.com/en/actions/how-tos/deploy/configure-and-manage-deployments/manage-environments). +Before approval, verify the environment's read-only GET response: + +```bash +gh api repos/Flow-Research/workstream/environments/cli-release +``` + +It must contain literal `"can_admins_bypass": false`; missing, null, true, +string or numeric values are not proof that bypass is disabled. The +[GET endpoint](https://docs.github.com/en/rest/deployments/environments#get-an-environment) +uses the existing Actions read permission. GitHub's documented environment +update parameters do not include this setting; configure it in the UI, not +through a workflow or an assumed API write parameter. + +The workflow queries the environment API in both preparation and publication and fails closed if those controls are absent or unavailable. At planning time, the environment could not be verified (GitHub API returned 404, which could mean absent or inaccessible). It is an external prerequisite, diff --git a/scripts/check_cli_release_controls.py b/scripts/check_cli_release_controls.py index 61fd34ecc..0eeb8066b 100644 --- a/scripts/check_cli_release_controls.py +++ b/scripts/check_cli_release_controls.py @@ -16,6 +16,8 @@ def validate_environment(environment: dict, branches: dict) -> None: if environment.get("name") != "cli-release": raise ValueError("release environment identity mismatch") + if environment.get("can_admins_bypass") is not False: + raise ValueError("release environment must disable administrator bypass") policy = environment.get("deployment_branch_policy") or {} if policy.get("custom_branch_policies") is not True: raise ValueError("release environment must restrict deployment branches") diff --git a/scripts/test_cli_release_controls.py b/scripts/test_cli_release_controls.py index 50f95944b..6fafae129 100644 --- a/scripts/test_cli_release_controls.py +++ b/scripts/test_cli_release_controls.py @@ -15,6 +15,7 @@ REPOSITORY = "Flow-Research/workstream" ENVIRONMENT = { "name": "cli-release", + "can_admins_bypass": False, "deployment_branch_policy": {"custom_branch_policies": True}, "protection_rules": [ { @@ -44,6 +45,22 @@ def test_exact_controls_pass(self) -> None: validate_environment(ENVIRONMENT, BRANCHES) validate_ruleset(SUMMARY, RULESET, REPOSITORY) + def test_environment_administrator_bypass_must_be_explicitly_disabled(self) -> None: + for value in (True, None, "false", "true", 0): + with ( + self.subTest(value=value), + self.assertRaisesRegex(ValueError, "administrator bypass"), + ): + validate_environment( + {**ENVIRONMENT, "can_admins_bypass": value}, BRANCHES + ) + + def test_environment_missing_administrator_bypass_fails(self) -> None: + environment = deepcopy(ENVIRONMENT) + del environment["can_admins_bypass"] + with self.assertRaisesRegex(ValueError, "administrator bypass"): + validate_environment(environment, BRANCHES) + def test_hidden_bypass_field_is_not_treated_as_empty_proof(self) -> None: ruleset = deepcopy(RULESET) del ruleset["bypass_actors"]