pdurlej/platform

Fork 0

fix(platformctl): restrict apply status artifact permissions #654

Merged

pdurlej merged 1 commit from codex/203-status-artifact-perms into main

2026-06-01 14:44:48 +02:00

codex commented

2026-06-01 08:32:58 +02:00

Collaborator

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

Summary

Restricts platformctl apply status artifact creation so status JSON files are not world-readable under normal creation.

Closes #203

Canary Context Pack

Product story

Apply status artifacts can include operational status, remote error shape, and redacted command output. They should remain useful for agents without becoming broadly readable local artifacts.

What changed

_write_status_artifact now creates the temporary status file with mode 0600 before atomic replace.
Existing unique-temp-name coverage now observes os.open instead of Path.write_text.
Added an explicit test for final status artifact file mode.

Why it changed

Issue #203 requires restrictive permissions for apply status artifacts without weakening existing path traversal or redaction behavior.

Files touched

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

Relevant context

#203 STATUS-PERMS-01
Existing status artifact redaction and unsafe module-id tests in test_apply_phase3.py

Runtime evidence

No production/runtime mutation. Local unit tests and module validation only.

Known constraints

This intentionally does not add fsync durability; #204 owns that separate change.

Explicit out-of-scope

No runtime apply.
No status artifact schema change.
No #204 fsync behavior.
No Infisical/key-map changes.

Requested decision

Approve if the file permission behavior and tests satisfy #203.

Merge blockers

Any world-readable status artifact creation path remains.
Existing status artifact redaction/path traversal tests regress.

Spec sources read

https://git.pdurlej.com/pdurlej/platform/issues/203 — issue scope and acceptance criteria.
control-plane/platformctl/apply.py — status artifact persistence implementation.
control-plane/platformctl/tests/test_apply_phase3.py — existing status artifact tests.

Validation

UV_CACHE_DIR=/private/tmp/codex-uv-cache uv run pytest platformctl/tests/test_apply_phase3.py -k 'status_artifact' — 14 passed.
UV_CACHE_DIR=/private/tmp/codex-uv-cache uv run pytest platformctl/tests/test_apply_phase3.py platformctl/tests/test_pr_sanity.py — 121 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.

Canary status: missing — fire canary 3+3 manually before merge ## Summary Restricts `platformctl apply` status artifact creation so status JSON files are not world-readable under normal creation. Closes #203 ## Canary Context Pack ### Product story Apply status artifacts can include operational status, remote error shape, and redacted command output. They should remain useful for agents without becoming broadly readable local artifacts. ### What changed - `_write_status_artifact` now creates the temporary status file with mode `0600` before atomic replace. - Existing unique-temp-name coverage now observes `os.open` instead of `Path.write_text`. - Added an explicit test for final status artifact file mode. ### Why it changed Issue #203 requires restrictive permissions for apply status artifacts without weakening existing path traversal or redaction behavior. ### Files touched - `control-plane/platformctl/apply.py` - `control-plane/platformctl/tests/test_apply_phase3.py` ### Relevant context - #203 STATUS-PERMS-01 - Existing status artifact redaction and unsafe module-id tests in `test_apply_phase3.py` ### Runtime evidence No production/runtime mutation. Local unit tests and module validation only. ### Known constraints This intentionally does not add fsync durability; #204 owns that separate change. ### Explicit out-of-scope - No runtime apply. - No status artifact schema change. - No #204 fsync behavior. - No Infisical/key-map changes. ### Requested decision Approve if the file permission behavior and tests satisfy #203. ### Merge blockers - Any world-readable status artifact creation path remains. - Existing status artifact redaction/path traversal tests regress. ## Spec sources read - `https://git.pdurlej.com/pdurlej/platform/issues/203` — issue scope and acceptance criteria. - `control-plane/platformctl/apply.py` — status artifact persistence implementation. - `control-plane/platformctl/tests/test_apply_phase3.py` — existing status artifact tests. ## Validation - `UV_CACHE_DIR=/private/tmp/codex-uv-cache uv run pytest platformctl/tests/test_apply_phase3.py -k 'status_artifact'` — 14 passed. - `UV_CACHE_DIR=/private/tmp/codex-uv-cache uv run pytest platformctl/tests/test_apply_phase3.py platformctl/tests/test_pr_sanity.py` — 121 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.

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

2026-06-01 08:32:58 +02:00

codex added the

class/security-sensitive

label

2026-06-01 08:32:58 +02:00

codex added 1 commit

2026-06-01 08:32:58 +02:00

fix(platformctl): restrict apply status artifact permissions

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

Details

platformctl plan / auto-apply scope (pull_request) Successful in 1m16s

Details

pyfallow / Pyfallow gate (control-plane) (pull_request) Successful in 3m18s

Details

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

Details

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

Details

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

Details

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

Details

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

Details

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

Details

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

Details

patchwarden-pr-sanity / sanity (pull_request) Successful in 2m51s

Details

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

Details

6cd3e7ff89

codex commented

2026-06-01 08:44:00 +02:00

Author

Collaborator

Patchwarden PR sanity

Status: eligible_sanity_clean
PR: 654
Commit: 6cd3e7ff899607316c8d1c55019238476f7841e5
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
low File descriptor leak if os.fdopen fails after os.open succeeds
- Evidence: In control-plane/platformctl/apply.py line 623-624: fd = os.open(tmp, ...) followed by os.fdopen(fd, ...). If os.fdopen raises, fd is not closed since os.fdopen only takes ownership on success.
- Next: Consider wrapping os.fdopen in try/except with os.close(fd) in the handler, or accept this as unlikely edge case since os.fdopen rarely fails on a valid fd.

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

Status: ok
Verdict: OK
Findings: none

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

Status: ok
Verdict: OK
Findings: 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: `eligible_sanity_clean` - PR: `654` - Commit: `6cd3e7ff899607316c8d1c55019238476f7841e5` - 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` - **`low`** File descriptor leak if os.fdopen fails after os.open succeeds - Evidence: `In control-plane/platformctl/apply.py line 623-624: fd = os.open(tmp, ...) followed by os.fdopen(fd, ...). If os.fdopen raises, fd is not closed since os.fdopen only takes ownership on success.` - Next: Consider wrapping os.fdopen in try/except with os.close(fd) in the handler, or accept this as unlikely edge case since os.fdopen rarely fails on a valid fd. ### `global-deepseek` / `deepseek-v4-pro:cloud` - Status: `ok` - Verdict: `OK` - Findings: none ### `redteam` / `kimi-k2.6:cloud` - Status: `ok` - Verdict: `OK` - Findings: 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.