spec: multiline + command-keyed redaction (#10027)#10225
spec: multiline + command-keyed redaction (#10027)#10225lonexreb wants to merge 4 commits intowarpdotdev:masterfrom
Conversation
github-actions
Bot
added
the
external-contributor
![oz-for-oss[bot]](https://avatars.githubusercontent.com/in/3450139?s=60&v=4){kind=small}
There was a problem hiding this comment.
Overview
This spec defines command-keyed output redaction and multiline delimiter redaction for agent context. The direction is aligned with the stated secret-redaction gaps, but several requirements are ambiguous enough to produce incomplete or unsafe implementations.
Concerns
- Command matching is specified too narrowly for common
kubectl get secretforms and does not define normalization or argument-order coverage for default rules. - Multiline redaction scope conflicts with the acceptance criterion for pasted private-key blocks versus the explicit output-side-only V1 scope.
- Default override semantics are under-specified, including precedence, identity, and disable behavior.
Security
- The buffer-cap behavior must explicitly keep redaction active after the cap is reached until the end delimiter or block flush; otherwise oversized secret blocks can leak lines after the capped buffer is replaced.
Verdict
Found: 0 critical, 4 important, 1 suggestions
Request changes
Comment /oz-review on this pull request to retrigger a review (up to 3 times on the same pull request).
Powered by Oz
|
|
||
| ```toml | ||
| [[redaction.command_output_rules]] | ||
| command_pattern = "^kubectl\\s+get\\s+secret\\b" |
There was a problem hiding this comment.
kubectl get secret; specify normalization or default patterns for common secret reads like kubectl get secrets, kubectl -n prod get secret, and context/namespace flags before get.
| start_pattern = "-----BEGIN [A-Z ]+PRIVATE KEY-----" | ||
| end_pattern = "-----END [A-Z ]+PRIVATE KEY-----" | ||
| replacement = "[redacted: PEM key block]" | ||
| inclusive = true # include the BEGIN/END lines in the redaction |
There was a problem hiding this comment.
💡 [SUGGESTION] The config exposes inclusive, but the behavior only defines inclusive = true; either define exclusive-boundary behavior or omit the option from V1.
| ### B5 — Performance bound | ||
|
|
||
| Multiline buffering is capped at `WARP_REDACTION_MAX_BUFFER_BYTES` | ||
| (default 1 MiB). If the buffer fills before `end_pattern` matches, |
There was a problem hiding this comment.
end_pattern or block flush; replacing only the buffered bytes can leak subsequent lines from an oversized secret block.
| - A1. With default rules: running `kubectl get secret foo -o yaml` | ||
| shows real output in the terminal but the agent context shows | ||
| the replacement string. | ||
| - A2. With default rules: pasting a private-key block redacts the |
There was a problem hiding this comment.
pasting a key is input-path behavior unless the spec defines a rendered-output path; align the scope and acceptance test.
| the replacement string. | ||
| - A2. With default rules: pasting a private-key block redacts the | ||
| entire BEGIN..END range, not just the first line. | ||
| - A3. User-defined rule in TOML overrides defaults at the same |
There was a problem hiding this comment.
|
Round-1 fixes for oz-for-oss review: (1) added Command Matching Normalization with argv-set matching and 12 default tool rules; (2) reconciled V1 scope to include PEM-style private-key delimiter redaction in both output AND pasted input; (3) added Default Override Semantics defining id-based merge, disable flag, and precedence; (4) fixed Buffer Cap Behavior to keep redaction state active post-cap until end delimiter or flush. |
|
/oz-review |
|
I'm re-reviewing this spec-only pull request in response to a review request. You can view the conversation on Warp. I completed the review and no human review was requested for this pull request. Comment Powered by Oz |
There was a problem hiding this comment.
Overview
This spec defines command-keyed output redaction and multiline delimiter redaction for agent-bound context. The overall shape covers the main behavior, defaults, override semantics, buffer caps, and tests.
Concerns
- The sample/default rule ids are inconsistent, which would break the documented override and disable workflow for copied configs.
- The default command table uses glob-style wildcards even though command matching is defined as regex-based, leaving several default matches ambiguous.
- Exposed config semantics are incomplete for
inclusive, invalid user rules, and duplicate user ids.
Security
- The spec should explicitly require the new redactors at the centralized agent-context boundary for every path that can send command output or user text to the agent, not only the active block/prompt examples.
- Malformed user-supplied rules must fail closed per rule without disabling built-in protections.
Verdict
Found: 0 critical, 4 important, 1 suggestions
Request changes
Comment /oz-review on this pull request to retrigger a review (up to 3 times on the same pull request).
Powered by Oz
|
|
||
| ```toml | ||
| [[redaction.command_output_rules]] | ||
| id = "default-kubectl-secret" |
There was a problem hiding this comment.
default-kubectl-secret conflicts with the stable default id default-kubectl-get-secret used later for override/disable semantics; align the example so users copy the id that actually replaces or disables the default.
| ``` | ||
|
|
||
| When the active block's command (as detected by Warp's existing | ||
| command parser) matches `command_pattern`, the entire block output |
There was a problem hiding this comment.
|
|
||
| | id | subcommand path matched | | ||
| | ----------------------------------- | -------------------------------------------------------- | | ||
| | `default-kubectl-get-secret` | `kubectl get secret*` (incl. `secrets`, `secret/<name>`) | |
There was a problem hiding this comment.
secret* is glob syntax, but B1.1 defines command_pattern as regex-matched; in regex this does not cover secrets or secret/<name>. Specify exact regexes or state that the table uses glob semantics for all starred defaults so implementation and tests agree.
| start_pattern = "-----BEGIN [A-Z ]+PRIVATE KEY-----" | ||
| end_pattern = "-----END [A-Z ]+PRIVATE KEY-----" | ||
| replacement = "[redacted: PEM key block]" | ||
| inclusive = true # include the BEGIN/END lines in the redaction |
There was a problem hiding this comment.
💡 [SUGGESTION] The config exposes inclusive, but the behavior contract only defines the inclusive case; add explicit semantics for inclusive = false, including whether delimiter lines are emitted, redacted separately, or unsupported for defaults.
|
|
||
| #### B3.1 — Override semantics | ||
|
|
||
| User-supplied redaction rules merge with defaults by rule `id` |
There was a problem hiding this comment.
|
Round-2 fixes: (1) all rule ids harmonized to |
|
/oz-review |
|
I'm re-reviewing this spec-only pull request in response to a review request. You can view the conversation on Warp. I completed the review and no human review was requested for this pull request. Comment Powered by Oz |
There was a problem hiding this comment.
Overview
This spec adds command-keyed output redaction and delimiter-based multiline redaction for agent-bound context. The overall structure covers goals, defaults, override semantics, fail-closed config handling, telemetry constraints, acceptance criteria, and tests.
Concerns
- The built-in PEM regex does not match the bare
-----BEGIN PRIVATE KEY-----form called out by the problem statement, so a core default would still leak a common private-key block. - The command matching contract is internally inconsistent about whether
command_patternis matched against only the normalized subcommand path or against a string that includes trailing resource-name positionals.
Security
- The PEM default gap is security-relevant because implementing the spec as written would leave a common private-key format unredacted in both output and pasted input paths.
Verdict
Found: 1 critical, 1 important, 0 suggestions
Request changes
Comment /oz-review on this pull request to retrigger a review (up to 3 times on the same pull request).
Powered by Oz
| start_pattern = "-----BEGIN [A-Z ]+PRIVATE KEY-----" | ||
| end_pattern = "-----END [A-Z ]+PRIVATE KEY-----" |
There was a problem hiding this comment.
🚨 [CRITICAL] [SECURITY] This default misses bare PKCS#8 blocks like -----BEGIN PRIVATE KEY-----, which the problem statement explicitly names, so those keys would still reach the agent.
| start_pattern = "-----BEGIN [A-Z ]+PRIVATE KEY-----" | |
| end_pattern = "-----END [A-Z ]+PRIVATE KEY-----" | |
| start_pattern = "-----BEGIN (?:[A-Z ]+ )?PRIVATE KEY-----" | |
| end_pattern = "-----END (?:[A-Z ]+ )?PRIVATE KEY-----" |
| V1 ships defaults covering the most common secret-bearing CLIs. | ||
| Each is argument-order independent per B1.1. Patterns are matched | ||
| against the normalized subcommand path. The `(\s|$)` trailing | ||
| boundary admits trailing positionals like resource names without |
There was a problem hiding this comment.
command_pattern is matched only against the normalized subcommand path and resource names live in normalized_argv_set, so this cannot also rely on command_pattern trailing boundaries to admit resource-name positionals. Specify one matching input contract or route resource-name matching through argv_contains/argv_excludes.
|
Addressed both concerns from latest oz-for-oss review. The CRITICAL is fully resolved. CRITICAL — PEM bare-PKCS#8 unredacted (closed) Fix: changed quantifier to New test T16 is the regression guard with six BEGIN..END fixtures (one per form), each tested on BOTH the output stream and the pasted-input path. The test MUST fail under Important — Command-matching contract internal inconsistency (closed)
Added explicit clarification that the trailing New test T_command_match_contract asserts the contract end-to-end with a recording test double on the matcher: it fails if the matcher EVER sees a string containing a trailing resource positional. Covers all 12 default rules in B1.2. |
|
/oz-review |
|
I'm re-reviewing this spec-only pull request in response to a review request. You can view the conversation on Warp. I completed the review and no human review was requested for this pull request. Comment Powered by Oz |
There was a problem hiding this comment.
Overview
This spec defines command-keyed output redaction and multiline delimiter redaction for agent-bound context. The overall scope is clear, but several contract details still need to be resolved before implementation.
Concerns
- The command-matching section still contradicts itself about whether resource-name positionals are part of the string matched by
command_pattern. - Duplicate user-rule handling conflicts with the stated default override/disable semantics.
Security
- Command-keyed redaction does not define how common shell prefixes and compound invocations are handled, which could leave secret-bearing CLI output outside the default protections unless the behavior is specified and tested.
Verdict
Found: 0 critical, 3 important, 0 suggestions
Request changes
Comment /oz-review on this pull request to retrigger a review (up to 3 times on the same pull request).
Powered by Oz
| string `command_pattern` matches against — they are separated into | ||
| the argv set. | ||
|
|
||
| 1. **Argv parse:** Split using shell-aware tokenization (the same |
There was a problem hiding this comment.
AWS_PROFILE=prod aws secretsmanager ..., env ... aws ..., or sudo kubectl ... are normalized or out of scope; without this, common secret-bearing commands may bypass the default redaction rules. Define the behavior and add acceptance/tests for it.
| V1 ships defaults covering the most common secret-bearing CLIs. | ||
| Each is argument-order independent per B1.1. Patterns are matched | ||
| against the normalized subcommand path. The `(\s|$)` trailing | ||
| boundary admits trailing positionals like resource names without |
There was a problem hiding this comment.
command_pattern. Rewrite it so the trailing boundary only refers to subcommand-path tokens, not resource names.
| | -------------------------------------------- | -------------------------------------------------------------------------------------------------------------------- | | ||
| | Invalid regex in any pattern field | Rule is DROPPED. Other user rules and ALL defaults remain active. Warning logged with rule id and parser error. Surfaced in Settings UI as `Rule '<id>' has invalid pattern: <error>`. | | ||
| | Missing required field (`id`, `pattern`, …) | Rule is DROPPED. Other rules unaffected. Warning surfaced in Settings UI. | | ||
| | Duplicate user-supplied id (same id twice in user config) | First occurrence is kept; subsequent duplicates are DROPPED with a warning. Defaults are never overridden by a duplicate-id user rule. | |
There was a problem hiding this comment.
1 participant
Spec for #10027. Two new redaction mechanisms: command-keyed output rules (e.g.
kubectl get secretredacts entire block output) and multiline-bracketed rules (e.g. PEM BEGIN/END blocks). Defaults for common DevOps cases shipped in V1.Closes (spec-only) #10027.