pdurlej/platform

Fork 0

fix(platformctl): whitelist smoke subprocess environment #656

Merged

pdurlej merged 1 commit from codex/210-smoke-env-whitelist into main

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

codex commented

2026-06-01 08:41:28 +02:00

Collaborator

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

Summary

Stops platformctl health smoke checks from passing the full Codex/runner process environment into tests/smoke.sh.

Closes #210

Canary Context Pack

Product story

Smoke checks need enough environment to reach the expected Tailnet hosts, but they should not inherit unrelated provider tokens or agent credentials.

What changed

_smoke_env() now builds an explicit allowlist: PATH, HOME, USER, PLATFORMCTL_SMOKE_SSH_USER, PLATFORMCTL_SMOKE_REMOTE_MODE, and required host env vars.
Default PLATFORMCTL_SMOKE_REMOTE_MODE=skip behavior is preserved.
Tests assert token/API-key-looking env vars are excluded.

Why it changed

Issue #210 requires restricting smoke subprocess environment inheritance so arbitrary secret-like environment variables are not passed to tests/smoke.sh.

Files touched

control-plane/platformctl/health.py
control-plane/platformctl/tests/test_health_phase3.py

Relevant context

#210 SMOKE-ENV-WHITELIST-01
Existing run_smoke health command tests.

Runtime evidence

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

Known constraints

This preserves the current required host/env validation and does not change tests/smoke.sh behavior beyond env input narrowing.

Explicit out-of-scope

No live smoke execution.
No SSH/runtime changes.
No changes to provider/token resolution.

Requested decision

Approve if smoke subprocesses now receive only the intended environment fields.

Merge blockers

Token/provider env vars can still be inherited by smoke subprocesses.
Required SSH host/user env validation regresses.

Spec sources read

https://git.pdurlej.com/pdurlej/platform/issues/210 — issue scope and acceptance criteria.
control-plane/platformctl/health.py — run_smoke and _smoke_env implementation.
control-plane/platformctl/tests/test_health_phase3.py — existing health/smoke tests.
tests/smoke.sh — smoke env var usage.

Validation

UV_CACHE_DIR=/private/tmp/codex-uv-cache uv run pytest platformctl/tests/test_health_phase3.py -k 'smoke_env or run_smoke' — 5 passed.
UV_CACHE_DIR=/private/tmp/codex-uv-cache uv run pytest platformctl/tests/test_health_phase3.py platformctl/tests/test_pr_sanity.py — 49 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 Stops `platformctl health` smoke checks from passing the full Codex/runner process environment into `tests/smoke.sh`. Closes #210 ## Canary Context Pack ### Product story Smoke checks need enough environment to reach the expected Tailnet hosts, but they should not inherit unrelated provider tokens or agent credentials. ### What changed - `_smoke_env()` now builds an explicit allowlist: `PATH`, `HOME`, `USER`, `PLATFORMCTL_SMOKE_SSH_USER`, `PLATFORMCTL_SMOKE_REMOTE_MODE`, and required host env vars. - Default `PLATFORMCTL_SMOKE_REMOTE_MODE=skip` behavior is preserved. - Tests assert token/API-key-looking env vars are excluded. ### Why it changed Issue #210 requires restricting smoke subprocess environment inheritance so arbitrary secret-like environment variables are not passed to `tests/smoke.sh`. ### Files touched - `control-plane/platformctl/health.py` - `control-plane/platformctl/tests/test_health_phase3.py` ### Relevant context - #210 SMOKE-ENV-WHITELIST-01 - Existing `run_smoke` health command tests. ### Runtime evidence No production/runtime mutation. Local unit tests and module validation only. ### Known constraints This preserves the current required host/env validation and does not change `tests/smoke.sh` behavior beyond env input narrowing. ### Explicit out-of-scope - No live smoke execution. - No SSH/runtime changes. - No changes to provider/token resolution. ### Requested decision Approve if smoke subprocesses now receive only the intended environment fields. ### Merge blockers - Token/provider env vars can still be inherited by smoke subprocesses. - Required SSH host/user env validation regresses. ## Spec sources read - `https://git.pdurlej.com/pdurlej/platform/issues/210` — issue scope and acceptance criteria. - `control-plane/platformctl/health.py` — `run_smoke` and `_smoke_env` implementation. - `control-plane/platformctl/tests/test_health_phase3.py` — existing health/smoke tests. - `tests/smoke.sh` — smoke env var usage. ## Validation - `UV_CACHE_DIR=/private/tmp/codex-uv-cache uv run pytest platformctl/tests/test_health_phase3.py -k 'smoke_env or run_smoke'` — 5 passed. - `UV_CACHE_DIR=/private/tmp/codex-uv-cache uv run pytest platformctl/tests/test_health_phase3.py platformctl/tests/test_pr_sanity.py` — 49 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:41:28 +02:00

codex added the

class/security-sensitive

label

2026-06-01 08:41:28 +02:00

codex added 1 commit

2026-06-01 08:41:28 +02:00

fix(platformctl): whitelist smoke subprocess environment

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

Details

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

Details

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

Details

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

Details

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

Details

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

Details

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

Details

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

Details

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

Details

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

Details

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

Details

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

Details

cd8f71fefd

codex commented

2026-06-01 09:22:08 +02:00

Author

Collaborator

Patchwarden PR sanity

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

Deterministic findings

No deterministic findings.

Model reviewers

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

Status: ok
Verdict: OK
medium Smoke subprocess may need env vars not in allowlist
- Evidence: control-plane/platformctl/health.py lines 35-126: _SMOKE_BASE_ENV_KEYS only includes PATH, HOME, USER plus PLATFORMCTL_SMOKE_* and PLATFORMCTL_*_SSH_HOST vars. PR states 'No live smoke execution' tested and canary status is 'missing'
- Next: Run canary 3+3 as indicated in PR description before merge to verify smoke.sh doesn't require additional env vars beyond the allowlist

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

Status: ok
Verdict: NOT_OK
high SSH agent socket not passed to smoke subprocess may break authentication
- Evidence: In control-plane/platformctl/health.py, _smoke_env() builds an allowlist containing only PATH, HOME, USER, PLATFORMCTL_SMOKE_SSH_USER, PLATFORMCTL_SMOKE_REMOTE_MODE, and the two required SSH host vars. The diff shows no inclusion of SSH_AUT
- Next: Verify whether tests/smoke.sh uses SSH agent forwarding. If so, add SSH_AUTH_SOCK to _SMOKE_BASE_ENV_KEYS or document that the smoke environment must be configured with key-based authentication that does not require an agent.
medium Missing canary evidence before merge
- Evidence: PR description states 'Canary status: missing — fire canary 3+3 manually before merge'. The change modifies the environment passed to a subprocess that performs health checks, which could cause runtime failures not caught by unit tests. Mer
- Next: Execute canary deployment as described before merging, or provide explicit justification for skipping.
low PATH and HOME may be absent in restricted environment
- Evidence: In control-plane/platformctl/health.py, the new _smoke_env() uses a conditional dict comprehension: {key: value for key in _SMOKE_BASE_ENV_KEYS if (value := os.environ.get(key))}. If PATH or HOME are not set in the parent environment, they
- Next: Consider always setting PATH and HOME to sensible defaults if missing, or ensure the parent environment always provides them.

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

Status: error
Verdict: -
Note: ReadTimeout: The read operation timed out
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: `advisory_findings` - PR: `656` - Commit: `cd8f71fefd11f559a50c48c47e45262efea33251` - Security-sensitive label: `present` - Authority: advisory model review plus deterministic blockers only - 3+3 canary: still alive; this does not replace it ## Deterministic findings No deterministic findings. ## Model reviewers ### `global-glm` / `glm-5.1:cloud` - Status: `ok` - Verdict: `OK` - **`medium`** Smoke subprocess may need env vars not in allowlist - Evidence: `control-plane/platformctl/health.py lines 35-126: _SMOKE_BASE_ENV_KEYS only includes PATH, HOME, USER plus PLATFORMCTL_SMOKE_* and PLATFORMCTL_*_SSH_HOST vars. PR states 'No live smoke execution' tested and canary status is 'missing'` - Next: Run canary 3+3 as indicated in PR description before merge to verify smoke.sh doesn't require additional env vars beyond the allowlist ### `global-deepseek` / `deepseek-v4-pro:cloud` - Status: `ok` - Verdict: `NOT_OK` - **`high`** SSH agent socket not passed to smoke subprocess may break authentication - Evidence: `In control-plane/platformctl/health.py, _smoke_env() builds an allowlist containing only PATH, HOME, USER, PLATFORMCTL_SMOKE_SSH_USER, PLATFORMCTL_SMOKE_REMOTE_MODE, and the two required SSH host vars. The diff shows no inclusion of SSH_AUT` - Next: Verify whether tests/smoke.sh uses SSH agent forwarding. If so, add SSH_AUTH_SOCK to _SMOKE_BASE_ENV_KEYS or document that the smoke environment must be configured with key-based authentication that does not require an agent. - **`medium`** Missing canary evidence before merge - Evidence: `PR description states 'Canary status: missing — fire canary 3+3 manually before merge'. The change modifies the environment passed to a subprocess that performs health checks, which could cause runtime failures not caught by unit tests. Mer` - Next: Execute canary deployment as described before merging, or provide explicit justification for skipping. - **`low`** PATH and HOME may be absent in restricted environment - Evidence: `In control-plane/platformctl/health.py, the new _smoke_env() uses a conditional dict comprehension: {key: value for key in _SMOKE_BASE_ENV_KEYS if (value := os.environ.get(key))}. If PATH or HOME are not set in the parent environment, they ` - Next: Consider always setting PATH and HOME to sensible defaults if missing, or ensure the parent environment always provides them. ### `redteam` / `kimi-k2.6:cloud` - Status: `error` - Verdict: `-` - Note: ReadTimeout: The read operation timed out - 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.