pdurlej/platform

Fork 0

fix(platformctl): extend apply output redaction #637

Merged

pdurlej merged 1 commit from codex/m06-extend-apply-redaction into main

2026-05-30 14:17:45 +02:00

codex commented

2026-05-30 14:02:54 +02:00

Collaborator

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

Canary Context Pack

Product story

Apply output is consumed by operators and agents after remote execution. It must preserve enough diagnostics to debug while never exposing common secret spellings from remote stdout/stderr.

What changed

Extended _redact_remote_output to redact additional secret-looking fields: api_key, API-key, passwd, JSON secret/token fields, and URL query token parameters.

Why it changed

Issue #200 follows the redaction-first M06 chain. #199 removed raw remote commands from emitted apply results; this PR tightens stdout/stderr redaction before provenance/adversarial work builds on it.

Files touched

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

Relevant context

Issue #200: REDACT-EXTEND-01
Issue #194 grind order: redaction before provenance/adversarial matrix

Runtime evidence

No runtime mutation. Local validation only.

Known constraints

Remote transport still receives raw process output from the actual command; this PR only changes persisted/emitted apply output redaction.

Explicit out-of-scope

No command scrub changes (#206 next)
No provenance/adversarial matrix changes (#192/#193/#194 later)
No runtime apply or service restart

Requested decision

Approve merge if tests and Patchwarden checks are green.

Merge blockers

Secret-like values appear in emitted apply result/status artifacts
Existing apply tests regress

Spec sources read

Forgejo issue #200
docs/forgejo-agent-operations.md
control-plane/platformctl/apply.py
control-plane/platformctl/tests/test_apply_phase3.py
control-plane/platformctl/tests/test_apply_env_file.py

Validation

PYTHONPATH=control-plane control-plane/.venv/bin/python -m pytest control-plane/platformctl/tests/test_apply_phase3.py control-plane/platformctl/tests/test_apply_env_file.py — 67 passed
PYTHONPATH=control-plane control-plane/.venv/bin/python -m platformctl.cli validate all --json — passed

Closes #200

Canary status: missing — fire canary 3+3 manually before merge ## Canary Context Pack ### Product story Apply output is consumed by operators and agents after remote execution. It must preserve enough diagnostics to debug while never exposing common secret spellings from remote stdout/stderr. ### What changed Extended `_redact_remote_output` to redact additional secret-looking fields: `api_key`, `API-key`, `passwd`, JSON secret/token fields, and URL query token parameters. ### Why it changed Issue #200 follows the redaction-first M06 chain. #199 removed raw remote commands from emitted apply results; this PR tightens stdout/stderr redaction before provenance/adversarial work builds on it. ### Files touched - `control-plane/platformctl/apply.py` - `control-plane/platformctl/tests/test_apply_phase3.py` ### Relevant context - Issue #200: REDACT-EXTEND-01 - Issue #194 grind order: redaction before provenance/adversarial matrix ### Runtime evidence No runtime mutation. Local validation only. ### Known constraints Remote transport still receives raw process output from the actual command; this PR only changes persisted/emitted apply output redaction. ### Explicit out-of-scope - No command scrub changes (#206 next) - No provenance/adversarial matrix changes (#192/#193/#194 later) - No runtime apply or service restart ### Requested decision Approve merge if tests and Patchwarden checks are green. ### Merge blockers - Secret-like values appear in emitted apply result/status artifacts - Existing apply tests regress ## Spec sources read - Forgejo issue #200 - `docs/forgejo-agent-operations.md` - `control-plane/platformctl/apply.py` - `control-plane/platformctl/tests/test_apply_phase3.py` - `control-plane/platformctl/tests/test_apply_env_file.py` ## Validation - `PYTHONPATH=control-plane control-plane/.venv/bin/python -m pytest control-plane/platformctl/tests/test_apply_phase3.py control-plane/platformctl/tests/test_apply_env_file.py` — 67 passed - `PYTHONPATH=control-plane control-plane/.venv/bin/python -m platformctl.cli validate all --json` — passed Closes #200

codex added 1 commit

2026-05-30 14:02:54 +02:00

fix(platformctl): extend apply output redaction

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

Details

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

Details

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

Details

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

Details

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

Details

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

Details

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

Details

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

Details

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

Details

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

Details

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

Details

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

Details

51c9b7e6b1

codex added the

class/security-sensitive

label

2026-05-30 14:02:55 +02:00

codex commented

2026-05-30 14:10:06 +02:00

Author

Collaborator

Patchwarden PR sanity

Status: advisory_findings
PR: 637
Commit: 51c9b7e6b19d5615dbf4befe5fa2ac7d79fe4035
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 Nested JSON objects not properly redacted
- Evidence: In control-plane/platformctl/apply.py, the JSON regex rf'(?i)("{_REMOTE_SECRET_FIELD_RE}"\s*:\s*)("(?:[^"\\]|\\.)*"|[^,\s}}]+)' matches either quoted strings or unquoted values, but not nested objects. For input like {"secret":{"key":"value
- Next: Consider adding a fallback regex that matches any value after a secret field name (including nested objects), or document this limitation explicitly. A greedy match like [^}]+ or [^,}]+ after the colon could catch nested structures.
medium Test coverage gaps for edge cases
- Evidence: In control-plane/platformctl/tests/test_apply_phase3.py, the parametrized test cases cover simple key=value pairs, quoted JSON strings, and URL parameters, but not nested JSON objects, values with spaces (e.g., password=hunter 2 would becom
- Next: Add test cases for: nested JSON objects like {"secret":{"nested":"value"}}, values with spaces like password=hunter 2, and unquoted JSON values like {"secret":123} to validate redaction behavior comprehensively.

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

Status: ok
Verdict: NOT_OK
blocker General key-value redaction pattern leaks partial secrets due to restrictive value character class
- Evidence: In control-plane/platformctl/apply.py, the new general redaction regex [^\s&#,}}]+replaces the previous\S+. This stops at &, #, ,, }, causing incomplete redaction for values containing these characters. For example, token=a`
- Next: Change the value-matching part of the general pattern to be at least as inclusive as the original \S+, or define a safe boundary that does not leak partial secrets. Add test cases for values containing &, #, ,, }, and other common delimiters.
medium JSON redaction does not handle nested objects or arrays as secret values
- Evidence: In control-plane/platformctl/apply.py, the JSON redaction regex only matches simple scalar values (strings, numbers, booleans, null). If a secret field contains a nested object or array, e.g., {"token": {"nested": "secret"}}, the value
- Next: Either extend the JSON pattern to redact the entire value when it is an object or array (e.g., by matching balanced braces), or document this limitation explicitly and ensure no such structured secrets appear in remote output.

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

Status: ok
Verdict: NOT_OK
high Redaction bypass for prefixed password/api_key/passwd/authorization fields
- Evidence: control-plane/platformctl/apply.py introduces _REMOTE_SECRET_FIELD_RE with a negative lookbehind (?<![A-Za-z0-9_-]) on the key-value regex, but only the token/secret branch allows a [A-Za-z0-9]+[_-] prefix. The password, passwd, authorizati
- Next: Apply the prefix pattern (?:[A-Za-z0-9]+[_-])? uniformly to all keywords in _REMOTE_SECRET_FIELD_RE, or replace the lookbehind with a boundary assertion that does not exclude common separators.

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: `637` - Commit: `51c9b7e6b19d5615dbf4befe5fa2ac7d79fe4035` - 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`** Nested JSON objects not properly redacted - Evidence: `In control-plane/platformctl/apply.py, the JSON regex rf'(?i)("{_REMOTE_SECRET_FIELD_RE}"\s*:\s*)("(?:[^"\\]|\\.)*"|[^,\s}}]+)' matches either quoted strings or unquoted values, but not nested objects. For input like {"secret":{"key":"value` - Next: Consider adding a fallback regex that matches any value after a secret field name (including nested objects), or document this limitation explicitly. A greedy match like [^}]+ or [^,}]+ after the colon could catch nested structures. - **`medium`** Test coverage gaps for edge cases - Evidence: `In control-plane/platformctl/tests/test_apply_phase3.py, the parametrized test cases cover simple key=value pairs, quoted JSON strings, and URL parameters, but not nested JSON objects, values with spaces (e.g., password=hunter 2 would becom` - Next: Add test cases for: nested JSON objects like {"secret":{"nested":"value"}}, values with spaces like password=hunter 2, and unquoted JSON values like {"secret":123} to validate redaction behavior comprehensively. ### `global-deepseek` / `deepseek-v4-pro:cloud` - Status: `ok` - Verdict: `NOT_OK` - **`blocker`** General key-value redaction pattern leaks partial secrets due to restrictive value character class - Evidence: `In `control-plane/platformctl/apply.py`, the new general redaction regex `[^\s&#,}}]+` replaces the previous `\S+`. This stops at `&`, `#`, `,`, `}`, causing incomplete redaction for values containing these characters. For example, `token=a` - Next: Change the value-matching part of the general pattern to be at least as inclusive as the original `\S+`, or define a safe boundary that does not leak partial secrets. Add test cases for values containing `&`, `#`, `,`, `}`, and other common delimiters. - **`medium`** JSON redaction does not handle nested objects or arrays as secret values - Evidence: `In `control-plane/platformctl/apply.py`, the JSON redaction regex only matches simple scalar values (strings, numbers, booleans, null). If a secret field contains a nested object or array, e.g., `{"token": {"nested": "secret"}}`, the value ` - Next: Either extend the JSON pattern to redact the entire value when it is an object or array (e.g., by matching balanced braces), or document this limitation explicitly and ensure no such structured secrets appear in remote output. ### `redteam` / `kimi-k2.6:cloud` - Status: `ok` - Verdict: `NOT_OK` - **`high`** Redaction bypass for prefixed password/api_key/passwd/authorization fields - Evidence: `control-plane/platformctl/apply.py introduces _REMOTE_SECRET_FIELD_RE with a negative lookbehind (?<![A-Za-z0-9_-]) on the key-value regex, but only the token/secret branch allows a [A-Za-z0-9]+[_-] prefix. The password, passwd, authorizati` - Next: Apply the prefix pattern (?:[A-Za-z0-9]+[_-])? uniformly to all keywords in _REMOTE_SECRET_FIELD_RE, or replace the lookbehind with a boundary assertion that does not exclude common separators. ## 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.