Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
70 changes: 70 additions & 0 deletions .commitrail/initiatives/WS-CLI-001/WS-CLI-001-47.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,70 @@
# WS-CLI-001-47 — Reject release environments with administrator approval bypass

- Initiative: WS-CLI-001
- 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

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

- [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

- 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 | 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 | Only owner, tests, runbook and this record change | 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 `a64ebf4257b972140ca0cf7f54a1e3fdb90103e4`, including the consolidated checker-only setup repairs. Those backend changes do not overlap this release-guard owner, tests or runbook. The roadmap still 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.
18 changes: 17 additions & 1 deletion docs/engineering/cli-release.md
Original file line number Diff line number Diff line change
Expand Up @@ -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,
Expand Down
2 changes: 2 additions & 0 deletions scripts/check_cli_release_controls.py
Original file line number Diff line number Diff line change
Expand Up @@ -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")
Expand Down
17 changes: 17 additions & 0 deletions scripts/test_cli_release_controls.py
Original file line number Diff line number Diff line change
Expand Up @@ -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": [
{
Expand Down Expand Up @@ -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"]
Expand Down
Loading