pdurlej/platform

Fork 0

fix(platformctl): verify apply PR base branch #665

Merged

pdurlej merged 1 commit from codex/189-apply-pr-base-guard into main

2026-06-01 14:45:26 +02:00

codex commented

2026-06-01 14:16:23 +02:00

Collaborator

Canary status: missing - platformctl apply approval hardening; run canary manually before merge if branch policy requires it.

Canary Context Pack

Product story

A trusted apply should only accept approval evidence from a PR merged into the expected deployment branch. A merged PR targeting some other branch must not unlock production apply.

What changed

verify_approved_sha now checks the approved PR base ref. It defaults to main, supports an explicit expected_base_ref parameter, and also honors sanitized PLATFORMCTL_EXPECTED_BASE_REF for controlled non-main deployments.

Why it changed

Issue #189 / PR-BIND-01 asked to bind approved apply PRs to the expected deployment branch, normally main.

Files touched

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

Relevant context

Existing approval check already binds SHA to explicit merged PR.
This adds base-branch binding before SHA acceptance.
#191 reason field is reused with base_ref_mismatch.

Runtime evidence

No runtime commands were run. Validation:

UV_CACHE_DIR=/private/tmp/codex-uv-cache uv run pytest platformctl/tests/test_apply_phase3.py -k 'verify_approved_sha or check_specific_pr or apply_plan_rejects_failed_approval'
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
PYTHONPATH=control-plane UV_CACHE_DIR=/private/tmp/codex-uv-cache uv run --project control-plane python -m platformctl.cli validate all --json

Results: targeted 16 passed; broader 135 passed; validate all exitCode 0.

Known constraints

This PR does not change direct runtime apply mechanics. It changes only approval validation.

Explicit out-of-scope

No actor/merger identity binding (#190 remains separate).
No break-glass secondary approval (#188 remains separate).
No runtime apply or production restart.

Requested decision

Approve if main should remain the default deployment branch and the sanitized override is acceptable for controlled future lanes.

Merge blockers

Forgejo PR payloads lack base.ref in production, which would now fail closed.
A non-main deployment lane needs a different canonical env name before merge.

Spec sources read

Issue #189 - scope and acceptance sketch.
control-plane/platformctl/apply.py - approval validation path.
control-plane/platformctl/tests/test_apply_phase3.py - approval regression tests.

Closes #189

Canary status: missing - platformctl apply approval hardening; run canary manually before merge if branch policy requires it. ## Canary Context Pack ### Product story A trusted apply should only accept approval evidence from a PR merged into the expected deployment branch. A merged PR targeting some other branch must not unlock production apply. ### What changed `verify_approved_sha` now checks the approved PR base ref. It defaults to `main`, supports an explicit `expected_base_ref` parameter, and also honors sanitized `PLATFORMCTL_EXPECTED_BASE_REF` for controlled non-main deployments. ### Why it changed Issue #189 / PR-BIND-01 asked to bind approved apply PRs to the expected deployment branch, normally `main`. ### Files touched - `control-plane/platformctl/apply.py` - `control-plane/platformctl/tests/test_apply_phase3.py` ### Relevant context - Existing approval check already binds SHA to explicit merged PR. - This adds base-branch binding before SHA acceptance. - #191 reason field is reused with `base_ref_mismatch`. ### Runtime evidence No runtime commands were run. Validation: ```bash UV_CACHE_DIR=/private/tmp/codex-uv-cache uv run pytest platformctl/tests/test_apply_phase3.py -k 'verify_approved_sha or check_specific_pr or apply_plan_rejects_failed_approval' 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 PYTHONPATH=control-plane UV_CACHE_DIR=/private/tmp/codex-uv-cache uv run --project control-plane python -m platformctl.cli validate all --json ``` Results: targeted 16 passed; broader 135 passed; `validate all` exitCode 0. ### Known constraints This PR does not change direct runtime apply mechanics. It changes only approval validation. ### Explicit out-of-scope - No actor/merger identity binding (#190 remains separate). - No break-glass secondary approval (#188 remains separate). - No runtime apply or production restart. ### Requested decision Approve if `main` should remain the default deployment branch and the sanitized override is acceptable for controlled future lanes. ### Merge blockers - Forgejo PR payloads lack `base.ref` in production, which would now fail closed. - A non-main deployment lane needs a different canonical env name before merge. ## Spec sources read - Issue #189 - scope and acceptance sketch. - `control-plane/platformctl/apply.py` - approval validation path. - `control-plane/platformctl/tests/test_apply_phase3.py` - approval regression tests. Closes #189

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

2026-06-01 14:16:23 +02:00

codex added the

class/security-sensitive

label

2026-06-01 14:16:23 +02:00

codex added 1 commit

2026-06-01 14:16:23 +02:00

fix(platformctl): verify apply PR base branch

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

Details

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

Details

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

Details

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

Details

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

Details

python-ci / Python 3.11 (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

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

Details

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

Details

patchwarden-pr-sanity / sanity (pull_request) Successful in 4m34s

Details

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

Details

ab082cba0a

codex commented

2026-06-01 14:24:59 +02:00

Author

Collaborator

Patchwarden PR sanity

Status: advisory_findings
PR: 665
Commit: ab082cba0a69cf0a11ebb911c2173266c58c5e4e
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: NOT_OK
blocker Forgejo API compatibility: missing base.ref causes fail-closed rejection
- Evidence: In _pr_base_ref() (apply.py, lines ~272-278), if base is missing or base.ref is missing, the function returns None. In _check_specific_pr() (apply.py, lines ~297-303), None != expected_base_ref causes rejection with APPROVAL_REASON_BASE_REF
- Next: Resolve Forgejo API compatibility before merge. Either: (1) update Forgejo to provide base.ref, (2) add fallback logic for missing base.ref, or (3) document this as a known limitation and ensure production deployments have base.ref available.
medium Silent fallback in _expected_base_ref() masks configuration errors
- Evidence: In _expected_base_ref() (apply.py, lines ~258-263), if PLATFORMCTL_EXPECTED_BASE_REF env var is set to an invalid value, the function silently falls back to 'main' instead of raising an error or logging a warning. This could mask misconfigu
- Next: Consider logging a warning when falling back to 'main' due to an invalid env var value, or raising an error if the env var is explicitly set but invalid.

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

Status: ok
Verdict: NOT_OK
blocker Production Forgejo API lacks base.ref causing all approvals to fail closed
- Evidence: PR description states 'Forgejo PR payloads lack base.ref in production, which would now fail closed.' Code in apply.py _pr_base_refreturns None if base.ref missing, and_check_specific_pr compares None != expected_base_ref, resulting i
- Next: Verify Forgejo API response in production and either ensure base.ref is present or implement a fallback (e.g., skip check if base.ref missing and log warning) before merging.
blocker Non-main deployment lane requires canonical env name before merge
- Evidence: PR description states 'A non-main deployment lane needs a different canonical env name before merge.' This implies the current default 'main' may not be appropriate for all lanes, and the env var override may not be fully validated.
- Next: Define and document the canonical env name for non-main lanes, and ensure the override mechanism is tested in staging before merging.
high Missing canary validation for approval hardening
- Evidence: PR description says 'Canary status: missing'. The change modifies the approval path which is critical for production apply; without canary, regressions may go undetected.
- Next: Run canary in a staging environment that mimics production Forgejo API responses before merging.

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

Status: ok
Verdict: NOT_OK
blocker Production Forgejo payloads lack base.ref causing total apply denial
- Evidence: PR description merge blockers explicitly states: 'Forgejo PR payloads lack base.refin production, which would now fail closed.' In apply.py,_pr_base_refreturnsNonewhen the 'base' dict or 'ref' key is missing, and_check_specific`
- Next: Do not merge until production Forgejo API responses are confirmed to include base.ref, or add a fallback/shim that skips the base-ref check when the field is absent from the payload.

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: `665` - Commit: `ab082cba0a69cf0a11ebb911c2173266c58c5e4e` - 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: `NOT_OK` - **`blocker`** Forgejo API compatibility: missing base.ref causes fail-closed rejection - Evidence: `In _pr_base_ref() (apply.py, lines ~272-278), if base is missing or base.ref is missing, the function returns None. In _check_specific_pr() (apply.py, lines ~297-303), None != expected_base_ref causes rejection with APPROVAL_REASON_BASE_REF` - Next: Resolve Forgejo API compatibility before merge. Either: (1) update Forgejo to provide base.ref, (2) add fallback logic for missing base.ref, or (3) document this as a known limitation and ensure production deployments have base.ref available. - **`medium`** Silent fallback in _expected_base_ref() masks configuration errors - Evidence: `In _expected_base_ref() (apply.py, lines ~258-263), if PLATFORMCTL_EXPECTED_BASE_REF env var is set to an invalid value, the function silently falls back to 'main' instead of raising an error or logging a warning. This could mask misconfigu` - Next: Consider logging a warning when falling back to 'main' due to an invalid env var value, or raising an error if the env var is explicitly set but invalid. ### `global-deepseek` / `deepseek-v4-pro:cloud` - Status: `ok` - Verdict: `NOT_OK` - **`blocker`** Production Forgejo API lacks base.ref causing all approvals to fail closed - Evidence: `PR description states 'Forgejo PR payloads lack base.ref in production, which would now fail closed.' Code in apply.py `_pr_base_ref` returns None if base.ref missing, and `_check_specific_pr` compares None != expected_base_ref, resulting i` - Next: Verify Forgejo API response in production and either ensure base.ref is present or implement a fallback (e.g., skip check if base.ref missing and log warning) before merging. - **`blocker`** Non-main deployment lane requires canonical env name before merge - Evidence: `PR description states 'A non-main deployment lane needs a different canonical env name before merge.' This implies the current default 'main' may not be appropriate for all lanes, and the env var override may not be fully validated.` - Next: Define and document the canonical env name for non-main lanes, and ensure the override mechanism is tested in staging before merging. - **`high`** Missing canary validation for approval hardening - Evidence: `PR description says 'Canary status: missing'. The change modifies the approval path which is critical for production apply; without canary, regressions may go undetected.` - Next: Run canary in a staging environment that mimics production Forgejo API responses before merging. ### `redteam` / `kimi-k2.6:cloud` - Status: `ok` - Verdict: `NOT_OK` - **`blocker`** Production Forgejo payloads lack base.ref causing total apply denial - Evidence: `PR description merge blockers explicitly states: 'Forgejo PR payloads lack `base.ref` in production, which would now fail closed.' In apply.py, `_pr_base_ref` returns `None` when the 'base' dict or 'ref' key is missing, and `_check_specific` - Next: Do not merge until production Forgejo API responses are confirmed to include `base.ref`, or add a fallback/shim that skips the base-ref check when the field is absent from the payload. ## 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.