pdurlej/platform

Fork 0

fix(platformctl): require break-glass ack #677

Merged

pdurlej merged 1 commit from codex/188-break-glass-ack into main

2026-06-01 19:17:42 +02:00

codex commented

2026-06-01 19:08:43 +02:00

Collaborator

Canary status: missing - fire canary 3+3 manually before merge

Summary

Hardens platformctl apply --break-glass so destructive plans require a second operator-controlled incident ack, not only free-text --reason.

Canary Context Pack

Product story

Break-glass should be explicit enough that an agent or operator cannot accidentally turn a destructive plan green by adding a vague reason string. The runtime mutation lane needs a second, PR/SHA-bound signal before destructive apply can proceed.

What changed

Adds PLATFORMCTL_BREAK_GLASS_ACK as the secondary break-glass approval signal.
Expected ack shape is deterministic and tied to the approved PR/SHA: break-glass:<pr>:<sha12>.
Destructive plans now fail closed unless --break-glass, non-empty --reason, and the ack env are all present.
Failed break-glass attempts emit structured warning logs with reason, PR, actor, and destructive count.
Successful break-glass dry-runs record break_glass_ack=verified in apply results/status artifacts.
Adds regression tests for missing break-glass, missing/mismatched ack, and valid ack dry-run.

Why it changed

Closes #188: platformctl apply --break-glass --reason ... previously used only free text as a secondary signal.

Files touched

control-plane/platformctl/apply.py
control-plane/platformctl/tests/test_apply_phase3.py

Relevant context

Issue #188
M06 apply-pipeline hardening lane
Previous actor binding PR #676 / issue #190

Runtime evidence

No runtime apply was run. This is code and tests only.

Known constraints

The normal approved-PR apply path is unchanged.
The CLI file is intentionally untouched because it has unrelated local dirty changes.
The ack is an incident-confirmation guard, not a secret. It is safe to show the expected value in failure output.

Explicit out-of-scope

No production mutation.
No live break-glass execution.
No signed-token or Forgejo-comment approval flow in this slice.
No broad approval redesign.

Requested decision

Review and merge if checks stay green.

Merge blockers

Destructive plans must fail closed without the secondary ack.
Failed break-glass attempts must be visible in logs/results.
Normal non-destructive apply behavior must not regress.

Spec sources read

Issue #188 - acceptance sketch and security-sensitive scope.
control-plane/platformctl/apply.py - break-glass gate and apply result/status behavior.
control-plane/platformctl/tests/test_apply_phase3.py - apply gate regression tests.

Validation

UV_CACHE_DIR=/private/tmp/codex-uv-cache uv run pytest platformctl/tests/test_apply_phase3.py platformctl/tests/test_apply_env_file.py platformctl/tests/test_pr_sanity.py platformctl/tests/test_forgejo_ci_scripts_contract.py platformctl/tests/test_health_phase3.py - 214 passed
PYTHONPATH=control-plane UV_CACHE_DIR=/private/tmp/codex-uv-cache uv run --project control-plane python -m platformctl.cli validate all --json - exitCode 0

Closes #188

Canary status: missing - fire canary 3+3 manually before merge ## Summary Hardens `platformctl apply --break-glass` so destructive plans require a second operator-controlled incident ack, not only free-text `--reason`. ## Canary Context Pack ### Product story Break-glass should be explicit enough that an agent or operator cannot accidentally turn a destructive plan green by adding a vague reason string. The runtime mutation lane needs a second, PR/SHA-bound signal before destructive apply can proceed. ### What changed - Adds `PLATFORMCTL_BREAK_GLASS_ACK` as the secondary break-glass approval signal. - Expected ack shape is deterministic and tied to the approved PR/SHA: `break-glass:<pr>:<sha12>`. - Destructive plans now fail closed unless `--break-glass`, non-empty `--reason`, and the ack env are all present. - Failed break-glass attempts emit structured warning logs with reason, PR, actor, and destructive count. - Successful break-glass dry-runs record `break_glass_ack=verified` in apply results/status artifacts. - Adds regression tests for missing break-glass, missing/mismatched ack, and valid ack dry-run. ### Why it changed Closes #188: `platformctl apply --break-glass --reason ...` previously used only free text as a secondary signal. ### Files touched - `control-plane/platformctl/apply.py` - `control-plane/platformctl/tests/test_apply_phase3.py` ### Relevant context - Issue #188 - M06 apply-pipeline hardening lane - Previous actor binding PR #676 / issue #190 ### Runtime evidence No runtime apply was run. This is code and tests only. ### Known constraints - The normal approved-PR apply path is unchanged. - The CLI file is intentionally untouched because it has unrelated local dirty changes. - The ack is an incident-confirmation guard, not a secret. It is safe to show the expected value in failure output. ### Explicit out-of-scope - No production mutation. - No live break-glass execution. - No signed-token or Forgejo-comment approval flow in this slice. - No broad approval redesign. ### Requested decision Review and merge if checks stay green. ### Merge blockers - Destructive plans must fail closed without the secondary ack. - Failed break-glass attempts must be visible in logs/results. - Normal non-destructive apply behavior must not regress. ## Spec sources read - Issue #188 - acceptance sketch and security-sensitive scope. - `control-plane/platformctl/apply.py` - break-glass gate and apply result/status behavior. - `control-plane/platformctl/tests/test_apply_phase3.py` - apply gate regression tests. ## Validation - `UV_CACHE_DIR=/private/tmp/codex-uv-cache uv run pytest platformctl/tests/test_apply_phase3.py platformctl/tests/test_apply_env_file.py platformctl/tests/test_pr_sanity.py platformctl/tests/test_forgejo_ci_scripts_contract.py platformctl/tests/test_health_phase3.py` - 214 passed - `PYTHONPATH=control-plane UV_CACHE_DIR=/private/tmp/codex-uv-cache uv run --project control-plane python -m platformctl.cli validate all --json` - exitCode 0 Closes #188

codex added this to the 06 - Agent execution and CI governance milestone

2026-06-01 19:08:43 +02:00

codex added the

risk/process

class/security-sensitive

tier/full

agent/codex

labels

2026-06-01 19:08:43 +02:00

codex added 1 commit

2026-06-01 19:08:43 +02:00

fix(platformctl): require break-glass ack

canary-required / collect-diff (pull_request) Successful in 4s

Details

platformctl plan / auto-apply scope (pull_request) Successful in 17s

Details

pyfallow / Pyfallow gate (control-plane) (pull_request) Successful in 15s

Details

python-ci / Python 3.11 (pull_request) Successful in 35s

Details

python-ci / Python 3.12 (pull_request) Successful in 36s

Details

python-ci / Python 3.13 (pull_request) Successful in 36s

Details

base-is-main / guard (pull_request) Successful in 1s

Details

patchwarden-client-dry-run / collect-diff (pull_request) Successful in 3s

Details

canary-required / canary (pull_request) Successful in 12s

Details

patchwarden-client-dry-run / dry-run (pull_request) Successful in 17s

Details

patchwarden-pr-sanity / sanity (pull_request) Successful in 3m29s

Details

patchwarden-pr-sanity / collect-diff (pull_request) Successful in 3s

Details

1812584ccd

codex commented

2026-06-01 19:16:50 +02:00

Author

Collaborator

Patchwarden PR sanity

Status: advisory_findings
PR: 677
Commit: 1812584ccdb5f9577f5c0e3ac36096cf2fc18b87
Security-sensitive label: present
Authority: advisory model review plus deterministic blockers only
3+3 canary: still alive; this does not replace it

Deterministic findings

info sensitive-path-touched Sensitive path touched — control-plane/platformctl/apply.py
- Evidence: control-plane/platformctl/apply.py
- Next: Route through the existing 3+3/risk-tier process; model review remains advisory.

Model reviewers

`global-glm` / `glm-5.1:cloud`

Status: ok
Verdict: OK
medium Missing test for mismatched ack value
- Evidence: control-plane/platformctl/tests/test_apply_phase3.py adds tests for missing ack (line 747, monkeypatch.delenv) and valid ack (line 779), but no test verifies rejection when PLATFORMCTL_BREAK_GLASS_ACK is set to a wrong value (e.g., 'break-g
- Next: Add a test case where BREAK_GLASS_ACK_ENV is set to an incorrect value (wrong PR or wrong SHA) and verify exit_code == EXIT_DESTRUCTIVE_NEEDS_APPROVAL and denial_reason == 'missing_or_mismatched_ack'.

`global-deepseek` / `deepseek-v4-pro:cloud`

Status: ok
Verdict: OK
Findings: none

`redteam` / `kimi-k2.6:cloud`

Status: ok
Verdict: NOT_OK
high break_glass_reason=None causes unhandled AttributeError in security gate
- Evidence: control-plane/platformctl/apply.py: _check_break_glass_approval calls reason.strip()unconditionally at line ~336. The callerapply_planpassesbreak_glass_reasondirectly without null guard. Old code treatedbreak_glass_reason as o
- Next: Guard against None in _check_break_glass_approval: change if not reason.strip(): to if not reason or not reason.strip():, and add a regression test for break_glass=True, break_glass_reason=None.

Policy notes

GLM 5.1 + DeepSeek V4 Pro are the operator-required model mix for this bot.
Optional red-team model is enabled only when PLATFORMCTL_PR_SANITY_REDTEAM_MODEL is configured.
Auto-merge is not enabled here.

# Patchwarden PR sanity - Status: `advisory_findings` - PR: `677` - Commit: `1812584ccdb5f9577f5c0e3ac36096cf2fc18b87` - Security-sensitive label: `present` - Authority: advisory model review plus deterministic blockers only - 3+3 canary: still alive; this does not replace it ## Deterministic findings - **`info` `sensitive-path-touched`** Sensitive path touched — `control-plane/platformctl/apply.py` - Evidence: `control-plane/platformctl/apply.py` - Next: Route through the existing 3+3/risk-tier process; model review remains advisory. ## Model reviewers ### `global-glm` / `glm-5.1:cloud` - Status: `ok` - Verdict: `OK` - **`medium`** Missing test for mismatched ack value - Evidence: `control-plane/platformctl/tests/test_apply_phase3.py adds tests for missing ack (line 747, monkeypatch.delenv) and valid ack (line 779), but no test verifies rejection when PLATFORMCTL_BREAK_GLASS_ACK is set to a wrong value (e.g., 'break-g` - Next: Add a test case where BREAK_GLASS_ACK_ENV is set to an incorrect value (wrong PR or wrong SHA) and verify exit_code == EXIT_DESTRUCTIVE_NEEDS_APPROVAL and denial_reason == 'missing_or_mismatched_ack'. ### `global-deepseek` / `deepseek-v4-pro:cloud` - Status: `ok` - Verdict: `OK` - Findings: none ### `redteam` / `kimi-k2.6:cloud` - Status: `ok` - Verdict: `NOT_OK` - **`high`** break_glass_reason=None causes unhandled AttributeError in security gate - Evidence: `control-plane/platformctl/apply.py: _check_break_glass_approval calls `reason.strip()` unconditionally at line ~336. The caller `apply_plan` passes `break_glass_reason` directly without null guard. Old code treated `break_glass_reason` as o` - Next: Guard against None in _check_break_glass_approval: change `if not reason.strip():` to `if not reason or not reason.strip():`, and add a regression test for `break_glass=True, break_glass_reason=None`. ## Policy notes - GLM 5.1 + DeepSeek V4 Pro are the operator-required model mix for this bot. - Optional red-team model is enabled only when `PLATFORMCTL_PR_SANITY_REDTEAM_MODEL` is configured. - Auto-merge is not enabled here.