HOMELAB-2194: feat(redact): mask secrets in tool output before context, session logs and subagent transcripts #65

Merged

aaron merged 3 commits from plane/HOMELAB-2194-redact into main 2026-10-23 18:01:48 +00:00

Conversation 3 Commits 3 Files changed 20 +5813 -4

aaron commented 2026-10-23 17:11:39 +00:00

Owner

Copy link

Tracking: HOMELAB-2218 (child of HOMELAB-2194, Pre-LD5 W1 Foundations).

Why

On 2026-10-23 a pi subagent probe echoed the in-cluster auditor's Claude OAuth token (sk-ant-oat01-…) into its transcript. It landed in /tmp/pi-subagents-1005/…/tasks/<id>.output and in the parent session's context, and it had to be rotated.

What

• redact/: a new extension. It adds a tool_result hook for every tool. pi runs that hook before it builds the toolResult message, emits tool_execution_end and appends to the session JSONL, so model context, the session file and compaction only ever see the masked form.
  • patterns.ts: the one pattern file. It covers Anthropic keys and OAuth/refresh tokens, OpenAI sk-, GitHub/GitLab PATs, Slack, Stripe, Google, npm, HF, Tailscale, Plane and Outline keys, URL credentials, Authorization: Bearer|Basic|token, JWTs, OpenBao/Vault hvs./legacy s. tokens, AWS key IDs and secrets, age keys, PEM private keys, and credential-named assignments (password=, "password": "…", PASS=, FORGEJO_API_KEY=<40hex>…).
  • structural.ts: k8s Secret data:/stringData: (YAML and JSON, kind: List, the last-applied annotation), kubectl get secret … -o jsonpath, and bao kv get (table, -field, -format=json).
  • The mask is [REDACTED:<type> …last4], with no tail for secrets under 16 chars. Quotes are preserved, so JSON stays valid.
  • Outputs only. Tool-call arguments and files on disk are never touched.
  • Each masked result gets one value-free line: [redact] N secrets redacted in this tool result (types). There is also a status-bar counter and a JSONL count log (~/.pi/agent/redact.log).
  • Fail-open. If the redactor throws, the original output passes through and a warning is raised. The tradeoff is documented in the README. PI_REDACT_DISABLED=1 is an operator kill switch.
• pi-swarm-suite, via redact-bridge.ts:
  • redact is a required child extension. It wins over extensions: allowlists and exclude_extensions:, and it is force-added by path for isolated: true children (most fleet charters are isolated).
  • Every .output transcript entry, including the initial prompt, is written through the redactor.
  • The two sides are coupled only through globalThis[Symbol.for("pi-extensions.redact")], so each works without the other.
• CI job for redact, plus README and docs.

Tests

• redact: 68 tests. They include a two-way corpus: real-looking secrets must be masked, and SHAs, digests, UUIDs, Plane ids, ConfigMaps, Deployments, ExternalSecrets, helm templates, sops ENC[], Go/TS/Python source and prose must come back byte-identical. The extension contract is driven through a fake pi (args untouched, no mutation, notice, log, fail-open, kill switch).
• pi-swarm-suite: the new redact-bridge.test.ts (required-extension decision, forced path, .output masking, fail-open). The full suite passes (1472).
• False-positive sweep over prod, infra-core and pi-swarm-suite (1.6k files plus 200 commits of git log -p). After tightening, the remaining hits are credential-shaped test fixtures and documented Secret examples.

Limitations (also in redact/README.md)

• Live tool_execution_update partials (the TUI tail, --mode json stdout) are not masked. pi has no hook to rewrite them.
• The bash full-output temp file (fullOutputPath) is not masked, because we never alter files.
• The operator's !cmd output, user prompts and assistant text are not rewritten.
• Some secrets have no shape and no key next to them: a bare password line, a lone 40-hex PAT, a short unquoted YAML password.
• Quoted credential literals in test fixtures are masked when read.
• A pi in another process is covered only if it loads this extension.

🤖 Generated with Claude Code

Tracking: **HOMELAB-2218** (child of HOMELAB-2194, Pre-LD5 W1 Foundations). ## Why On 2026-10-23 a pi subagent probe echoed the in-cluster auditor's Claude OAuth token (`sk-ant-oat01-…`) into its transcript. It landed in `/tmp/pi-subagents-1005/…/tasks/<id>.output` and in the parent session's context, and it had to be rotated. ## What - **`redact/`**: a new extension. It adds a `tool_result` hook for every tool. pi runs that hook before it builds the toolResult message, emits `tool_execution_end` and appends to the session JSONL, so model context, the session file and compaction only ever see the masked form. - `patterns.ts`: **the one pattern file.** It covers Anthropic keys and OAuth/refresh tokens, OpenAI `sk-`, GitHub/GitLab PATs, Slack, Stripe, Google, npm, HF, Tailscale, Plane and Outline keys, URL credentials, `Authorization: Bearer|Basic|token`, JWTs, OpenBao/Vault `hvs.`/legacy `s.` tokens, AWS key IDs and secrets, age keys, PEM private keys, and credential-named assignments (`password=`, `"password": "…"`, `PASS=`, `FORGEJO_API_KEY=<40hex>`…). - `structural.ts`: k8s Secret `data:`/`stringData:` (YAML and JSON, `kind: List`, the last-applied annotation), `kubectl get secret … -o jsonpath`, and `bao kv get` (table, `-field`, `-format=json`). - The mask is `[REDACTED:<type> …last4]`, with no tail for secrets under 16 chars. Quotes are preserved, so JSON stays valid. - **Outputs only.** Tool-call arguments and files on disk are never touched. - Each masked result gets one value-free line: `[redact] N secrets redacted in this tool result (types)`. There is also a status-bar counter and a JSONL count log (`~/.pi/agent/redact.log`). - **Fail-open.** If the redactor throws, the original output passes through and a warning is raised. The tradeoff is documented in the README. `PI_REDACT_DISABLED=1` is an operator kill switch. - **`pi-swarm-suite`**, via `redact-bridge.ts`: - `redact` is a required child extension. It wins over `extensions:` allowlists and `exclude_extensions:`, and it is force-added by path for `isolated: true` children (most fleet charters are isolated). - Every `.output` transcript entry, including the initial prompt, is written through the redactor. - The two sides are coupled only through `globalThis[Symbol.for("pi-extensions.redact")]`, so each works without the other. - CI job for `redact`, plus README and docs. ## Tests - `redact`: 68 tests. They include a two-way corpus: real-looking secrets must be masked, and SHAs, digests, UUIDs, Plane ids, ConfigMaps, Deployments, ExternalSecrets, helm templates, sops `ENC[]`, Go/TS/Python source and prose must come back byte-identical. The extension contract is driven through a fake pi (args untouched, no mutation, notice, log, fail-open, kill switch). - `pi-swarm-suite`: the new `redact-bridge.test.ts` (required-extension decision, forced path, `.output` masking, fail-open). The full suite passes (1472). - False-positive sweep over prod, infra-core and pi-swarm-suite (1.6k files plus 200 commits of `git log -p`). After tightening, the remaining hits are credential-shaped test fixtures and documented Secret examples. ## Limitations (also in redact/README.md) - Live `tool_execution_update` partials (the TUI tail, `--mode json` stdout) are not masked. pi has no hook to rewrite them. - The bash full-output temp file (`fullOutputPath`) is not masked, because we never alter files. - The operator's `!cmd` output, user prompts and assistant text are not rewritten. - Some secrets have no shape and no key next to them: a bare password line, a lone 40-hex PAT, a short unquoted YAML password. - Quoted credential literals in test fixtures are masked when read. - A pi in another process is covered only if it loads this extension. 🤖 Generated with [Claude Code](https://claude.com/claude-code)

aaron added 1 commit 2026-10-23 17:11:39 +00:00

HOMELAB-2194: feat(redact): mask secrets in tool output before context, session logs and subagent transcripts ... 3dc09e8eac

On 2026-10-23 a subagent probe echoed the in-cluster auditor's Claude OAuth
token into its transcript (subagent .output file + parent context); it had to
be rotated. This adds a `redact` extension that masks secrets in every tool
result via pi's `tool_result` hook, which runs before pi builds the
toolResult message, emits tool_execution_end and appends to the session file.

- redact/patterns.ts: the single, extensible pattern list (Anthropic keys and
  OAuth tokens, OpenAI sk-, GitHub/GitLab PATs, Slack/Stripe/Google/npm/HF/
  Tailscale/Plane/Outline keys, URL credentials, Authorization headers, JWTs,
  OpenBao/Vault tokens, AWS key id + secret, age keys, PEM private keys,
  credential-named assignments incl. 40-hex Forgejo PATs in context).
- redact/structural.ts: k8s Secret data (YAML/JSON/List/last-applied),
  kubectl secret jsonpath output, bao/vault kv get (table/-field/json).
- Mask format [REDACTED:<type> …last4]; outputs only (args and files never
  touched); one value-free "[redact] N secrets redacted" line per result,
  status counter, JSONL count log. Fail-open with a warning (documented).
- Two-way test corpus: real-looking secrets must be masked; SHAs, digests,
  UUIDs, Plane ids, manifests, source code and prose must stay byte-identical.
- pi-swarm-suite: redact is a required child extension (wins over allowlists
  and exclude_extensions; force-added by path for isolated children), and the
  .output transcript is written through the redactor (redact-bridge.ts).
- CI job for redact; README/docs.

Tracking: HOMELAB-2218 (child of HOMELAB-2194).

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>

aaron referenced this pull request from a commit 2026-10-23 17:28:14 +00:00

HOMELAB-2194: fix(redact): linear-time patterns + review round 1 gaps

aaron added 1 commit 2026-10-23 17:28:14 +00:00

HOMELAB-2194: fix(redact): linear-time patterns + review round 1 gaps ... d32b490aa3

Independent review (PR #65) findings:
- BLOCKER: quadratic scan on 50 KB `x-x-x…`/`a.a.a…` lines (5 s/result).
  Every open-ended class is now bounded and a rejected match retries from
  the secret start, not index+1. tests/perf.test.ts guards 11 pathological
  inputs (<1.5 s budget; all now <110 ms).
- .netrc / space-separated `password <value>` is masked.
- `kubectl get secret … -o json|yaml | jq/yq/base64/grep…` output is masked
  whole (the reduced value no longer looks like a Secret).
- `bao|vault kv get|read -format=yaml` data leaves are masked (KV v1/v2).
- Flow-style Secret data (`data: {k: v}`) is masked.
- details-only masks count in the status bar and log.
- README: transcript-vs-args wording, non-plain details and session-file
  tool-call args documented as limitations.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>

aaron commented 2026-10-23 17:28:25 +00:00

Author

Owner

Copy link

Independent review — round 1 (fresh pi -p --no-session, omniroute/kimi-pool; brief = PR diff + ticket requirements only)

VERDICT: REQUEST_CHANGES

1. [BLOCKER] Quadratic redaction scan on common large-output shapes — redact/patterns.ts:269,309,314,321 (unbounded classes) + redact/redactor.ts:104 (re.lastIndex = m.index + 1 retry). A 50 KB single-line dash/dot run — realistic in minified JS, sourcemaps, long slug/log lines — takes 5.3 s ("x-".repeat(25000)) and 2.9 s ("a.".repeat(25000)) per tool result at the committed HEAD (measured; the whole 11-case perf matrix takes 10.6 s vs 0.4 s after bounding). Cause: the KEY=value key class [A-Za-z0-9_.-]* and the url-credential scheme class [a-z0-9+.-]* restart matching at every position, and a rejected match retries from index+1, giving O(n²) on text with no separator at all. Built-in tools cap at 50 KB (truncate.js), so every affected tool result stalls seconds; extension/MCP-bridged results can be larger (minutes). The fail-open try/catch cannot mitigate a hang — the tool pipeline just blocks. The worktree already contains the correct fix (bounded {1,1024}/{0,63} classes, lastIndex = Math.max(m.index+1, start), plus tests/perf.test.ts) but it is uncommitted and not in the PR — commit it into this branch; without it this is an availability defect on a covered path.

2. [CONCERN] Space-separated password assignments are never masked — redact/patterns.ts:314-326. The assignment rules require = or : separators, so credential files leak in full: repro machine forgejo.example\nlogin aaron\npassword Sup3rSecretValue!x → count=0, raw value reaches model context and session JSONL. .netrc is exactly the kind of file an agent cats; the required category is "password= / "password": "…" / PASS= assignments" and password <value> is the canonical assignment form in these files. Add a password <value> (space-sep, start-of-line/after-whitespace, non-prose-gated) rule.

3. [CONCERN] kubectl get secret … -o json | jq -r .data.x | base64 -d bypasses command-aware masking — redact/structural.ts:292-296. KUBECTL_SECRET_BARE only matches -o jsonpath|go-template|template, so the extracted bare Secret value passes through unmasked (repro → count=0 for an opaque value with no vendor prefix). The requirement explicitly covers "k8s Secret data: values when kubectl get secret -o yaml/json output is shown" — here the -o json output is what is shown, transformed by jq. Broaden detection to -o json|yaml segments that feed a downstream extractor, or mask whole output for kubectl … secret … -o json|yaml piped into jq/yq/base64 -d when the final segment emits a bare value.

4. [CONCERN] bao/vault kv get -format=yaml leaks secrets under non-credential key names — redact/structural.ts:299-302. Only table, -field and -format=json are handled; repro tls_cert: Zm9v…== with command bao kv get -format=yaml kv/ld/db → count=0. Vault data keys are arbitrary (tls_cert, conn_str), so the generic key-named rules don't catch them. -format=yaml is a first-class CLI form. The README honestly lists only json, but this leaves a plausible, easily-hit leak on a path the structural module purports to cover.

5. [SUGGESTION] Flow-style YAML Secret data with short values slips through: data: {password: c2Vj} (4-char value) → count=0 (structural.ts block regex needs data: on its own line; the generic rule's 8-char floor rejects short values). Low severity (real base64 data values are longer), but a documented corner.

6. [SUGGESTION] Details-only redactions are invisible: in redact/index.ts the [redact] N secrets redacted notice and the status-bar counter only fire on count (content masks); a result masked solely inside details (detailsCount>0) returns new details with no notice and no counter increment. Operators and the model get no signal on that path.

7. [SUGGESTION] README claim tension: redact/README.md ("Outputs only. Tool-call arguments are never touched") vs pi-swarm-suite/src/core/redact-bridge.ts:53 which deep-masks arguments inside .output transcript entries (e.g. a raw token inside a bash command line in the transcript becomes [REDACTED:…]). The transcript masking is desirable and consistent with the ticket ("any session/transcript log") — but tighten the README wording so it doesn't read as a contradiction.

8. [SUGGESTION] redactDeep (redact/redactor.ts:135-138) silently skips non-plain objects (class instances, Maps) in tool details; pi's built-ins use plain objects (verified bash/read/grep details are metadata-only), but an extension/MCP tool returning a class-instance detail with a secret string would land unmasked in the session JSONL. Consider documenting this boundary next to the other limitations.

Verified while reviewing (no findings): pi hook ordering claims hold against installed 0.87.1 sources (finalizeExecutedToolCall runs afterToolCall before emitToolExecutionEnd and before the toolResult message is built/appended; runner catches handler exceptions and merges {content, details} exactly as the extension assumes); DefaultResourceLoader does load additionalExtensionPaths under noExtensions and dedupes by canonical path, so the forced child-load path in agent-runner.ts:660-666 works as designed; required categories (Anthropic sk-ant-*, OpenAI sk-≥32, GH/GL/Forgejo PATs in context, URL creds, Authorization, JWT, hvs./s., AKIA+aws_secret, age, PEM, password=/"password": forms, k8s Secret data incl. List/last-applied/jsonpath, bao table/-field/-format=json) all masked with reproducers; MUST-KEEP corpus (SHAs, digests, UUIDs, Plane ids, ConfigMaps, ExternalSecrets, helm templates, sops ENC, Go/TS/Python source, prose) byte-identical, plus a spot sweep of repo source files clean; fail-open tested both in the extension and the bridge; PI_REDACT_DISABLED is process-env only (a tool call cannot set it); args never masked on the live tool path and inputs never mutated; CI job valid, package-lock committed, ^0.80.2 pins match repo convention; full pi-swarm-suite suite passes 1472/1472 at HEAD, matching the PR body; redact suite 68/68 at HEAD.

Primary fix needed before merge: finding 1 (the bounded-pattern + lastIndex change and tests/perf.test.ts already exist uncommitted in the worktree — commit them). Findings 2-4 are secret-leak gaps on plausible covered paths and should be addressed or explicitly documented as limitations in the same PR.

---

Resolution (d32b490): 1 fixed (bounded classes + retry-from-secret-start; redact/tests/perf.test.ts). 2 fixed (netrc rule). 3 fixed (kubectl -o json|yaml → extractor pipelines masked whole). 4 fixed (-format=yaml). 5 fixed (flow-style data). 6 fixed (details-only counted in status/log). 7, 8 documented in redact/README.md. Delta re-review follows.

🤖 Generated with Claude Code

## Independent review — round 1 (fresh `pi -p --no-session`, omniroute/kimi-pool; brief = PR diff + ticket requirements only) VERDICT: REQUEST_CHANGES 1. [BLOCKER] Quadratic redaction scan on common large-output shapes — `redact/patterns.ts:269,309,314,321` (unbounded classes) + `redact/redactor.ts:104` (`re.lastIndex = m.index + 1` retry). A 50 KB single-line dash/dot run — realistic in minified JS, sourcemaps, long slug/log lines — takes **5.3 s (`"x-".repeat(25000)`) and 2.9 s (`"a.".repeat(25000)`) per tool result** at the committed HEAD (measured; the whole 11-case perf matrix takes 10.6 s vs 0.4 s after bounding). Cause: the `KEY=value` key class `[A-Za-z0-9_.-]*` and the url-credential scheme class `[a-z0-9+.-]*` restart matching at every position, and a rejected match retries from `index+1`, giving O(n²) on text with no separator at all. Built-in tools cap at 50 KB (`truncate.js`), so every affected tool result stalls seconds; extension/MCP-bridged results can be larger (minutes). The fail-open try/catch cannot mitigate a hang — the tool pipeline just blocks. The worktree already contains the correct fix (bounded `{1,1024}`/`{0,63}` classes, `lastIndex = Math.max(m.index+1, start)`, plus `tests/perf.test.ts`) but it is **uncommitted and not in the PR** — commit it into this branch; without it this is an availability defect on a covered path. 2. [CONCERN] Space-separated password assignments are never masked — `redact/patterns.ts:314-326`. The assignment rules require `=` or `: ` separators, so credential files leak in full: repro `machine forgejo.example\nlogin aaron\npassword Sup3rSecretValue!x` → count=0, raw value reaches model context and session JSONL. `.netrc` is exactly the kind of file an agent `cat`s; the required category is "password= / "password": "…" / PASS= assignments" and `password <value>` is the canonical assignment form in these files. Add a `password <value>` (space-sep, start-of-line/after-whitespace, non-prose-gated) rule. 3. [CONCERN] `kubectl get secret … -o json | jq -r .data.x | base64 -d` bypasses command-aware masking — `redact/structural.ts:292-296`. `KUBECTL_SECRET_BARE` only matches `-o jsonpath|go-template|template`, so the extracted bare Secret value passes through unmasked (repro → count=0 for an opaque value with no vendor prefix). The requirement explicitly covers "k8s Secret data: values when kubectl get secret -o yaml/json output is shown" — here the `-o json` output is what is shown, transformed by jq. Broaden detection to `-o json|yaml` segments that feed a downstream extractor, or mask whole output for `kubectl … secret … -o json|yaml` piped into `jq/yq/base64 -d` when the final segment emits a bare value. 4. [CONCERN] `bao/vault kv get -format=yaml` leaks secrets under non-credential key names — `redact/structural.ts:299-302`. Only table, `-field` and `-format=json` are handled; repro `tls_cert: Zm9v…==` with command `bao kv get -format=yaml kv/ld/db` → count=0. Vault data keys are arbitrary (`tls_cert`, `conn_str`), so the generic key-named rules don't catch them. `-format=yaml` is a first-class CLI form. The README honestly lists only json, but this leaves a plausible, easily-hit leak on a path the structural module purports to cover. 5. [SUGGESTION] Flow-style YAML Secret data with short values slips through: `data: {password: c2Vj}` (4-char value) → count=0 (`structural.ts` block regex needs `data:` on its own line; the generic rule's 8-char floor rejects short values). Low severity (real base64 data values are longer), but a documented corner. 6. [SUGGESTION] Details-only redactions are invisible: in `redact/index.ts` the `[redact] N secrets redacted` notice and the status-bar counter only fire on `count` (content masks); a result masked solely inside `details` (detailsCount>0) returns new details with no notice and no counter increment. Operators and the model get no signal on that path. 7. [SUGGESTION] README claim tension: `redact/README.md` ("Outputs only. Tool-call *arguments* are never touched") vs `pi-swarm-suite/src/core/redact-bridge.ts:53` which deep-masks arguments inside `.output` transcript entries (e.g. a raw token inside a bash command line in the transcript becomes `[REDACTED:…]`). The transcript masking is desirable and consistent with the ticket ("any session/transcript log") — but tighten the README wording so it doesn't read as a contradiction. 8. [SUGGESTION] `redactDeep` (`redact/redactor.ts:135-138`) silently skips non-plain objects (class instances, Maps) in tool `details`; pi's built-ins use plain objects (verified bash/read/grep details are metadata-only), but an extension/MCP tool returning a class-instance detail with a secret string would land unmasked in the session JSONL. Consider documenting this boundary next to the other limitations. Verified while reviewing (no findings): pi hook ordering claims hold against installed 0.87.1 sources (`finalizeExecutedToolCall` runs `afterToolCall` before `emitToolExecutionEnd` and before the toolResult message is built/appended; runner catches handler exceptions and merges `{content, details}` exactly as the extension assumes); `DefaultResourceLoader` does load `additionalExtensionPaths` under `noExtensions` and dedupes by canonical path, so the forced child-load path in `agent-runner.ts:660-666` works as designed; required categories (Anthropic sk-ant-*, OpenAI sk-≥32, GH/GL/Forgejo PATs in context, URL creds, Authorization, JWT, hvs./s., AKIA+aws_secret, age, PEM, password=/"password": forms, k8s Secret data incl. List/last-applied/jsonpath, bao table/-field/-format=json) all masked with reproducers; MUST-KEEP corpus (SHAs, digests, UUIDs, Plane ids, ConfigMaps, ExternalSecrets, helm templates, sops ENC, Go/TS/Python source, prose) byte-identical, plus a spot sweep of repo source files clean; fail-open tested both in the extension and the bridge; `PI_REDACT_DISABLED` is process-env only (a tool call cannot set it); args never masked on the live tool path and inputs never mutated; CI job valid, package-lock committed, `^0.80.2` pins match repo convention; full pi-swarm-suite suite passes 1472/1472 at HEAD, matching the PR body; redact suite 68/68 at HEAD. Primary fix needed before merge: finding 1 (the bounded-pattern + lastIndex change and `tests/perf.test.ts` already exist uncommitted in the worktree — commit them). Findings 2-4 are secret-leak gaps on plausible covered paths and should be addressed or explicitly documented as limitations in the same PR. --- **Resolution (d32b490):** 1 fixed (bounded classes + retry-from-secret-start; `redact/tests/perf.test.ts`). 2 fixed (netrc rule). 3 fixed (kubectl `-o json|yaml` → extractor pipelines masked whole). 4 fixed (`-format=yaml`). 5 fixed (flow-style data). 6 fixed (details-only counted in status/log). 7, 8 documented in `redact/README.md`. Delta re-review follows. 🤖 Generated with [Claude Code](https://claude.com/claude-code)

aaron added 1 commit 2026-10-23 17:35:01 +00:00

HOMELAB-2194: fix(redact): mask bao/vault output piped into extractors; no double count ... dfd0022cca

Delta re-review (round 2) concerns:
- bao/vault kv get|read reduced by jq/yq/grep/... is masked whole, like the
  kubectl pipeline case.
- The status-bar total counts details masks only when content was clean, so
  a details copy of the output no longer doubles the count.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>

aaron commented 2026-10-23 17:35:12 +00:00

Author

Owner

Copy link

Independent review — round 2 (delta re-review, fresh pi -p --no-session, omniroute/kimi-pool)

VERDICT: APPROVE

Round-1 findings:
#1 RESOLVED — bounded classes committed (patterns.ts:271,278,285,290,306,320,329,337), lastIndex = Math.max(m.index+1, start) (redactor.ts:107), tests/perf.test.ts added. Independent timing at HEAD: "x-".repeat(25000) 20.8 ms, "a.".repeat(25000) 19.7 ms, worst case ("a=".repeat(25000)) 105.9 ms — vs 5.3 s/2.9 s at round-1 HEAD. Full 11-case matrix linear. 87/87 tests pass.
#2 RESOLVED — new password space rule (patterns.ts:296-303). Repro machine forgejo.example\nlogin aaron\npassword Sup3rSecretValue!x → count=1, masked; one-line form also masked. Prose probes (password is stored…, password security is important, passwd: password updated successfully) unchanged; tests added.
#3 RESOLVED — kubectlSecretPipeline + KUBECTL_SECRET_DUMP (structural.ts:331-339, wired at 434). Repros kubectl get secret x -o json | jq -r .data.token | base64 -d, -o yaml | grep, --output=json | yq all masked whole (count=1). FP control kubectl get secrets | grep auditor untouched (test added).
#4 RESOLVED — redactVaultYaml + VAULT_YAML branch (structural.ts:344-365,455-460). Repro KV v2 (data.data leaves masked, metadata.version/lease_duration intact) and KV v1 (meta keys skipped) both masked; tests added.
#5 RESOLVED — DATA_FLOW_LINE + maskFlowMapping (structural.ts:104,139-157,207-218). Repro data: {password: c2Vj, token: 'dG9r'} → count=2, values masked; metadata: {name: x} untouched; flow-style ConfigMap in MUST_KEEP corpus.
#6 RESOLVED — index.ts:159 now adds r.count + r.detailsCount; new test asserts details-only mask yields redacted: 1 and a "detailsCount":1 log line.
#7 RESOLVED — README reworded ("arguments a tool call actually executes with", transcript-log exception explained); no longer contradicts redact-bridge.ts.
#8 DOCUMENTED — README "Non-plain details objects" limitation added (plus session-JSONL arguments limitation).

New findings:
[CONCERN] redact/structural.ts:455-460 — the extractor-pipeline bypass fixed for kubectl remains for vault. Reproducers: bao kv get -format=yaml kv/ld/db | grep tls_cert → output tls_cert: Zm9vYmFy… count=0 (no top-level data: line survives the extractor, and no generic rule keys on tls_cert); ... | yq '.data.data.tls_cert' → bare value count=0; bao kv get -format=json ... | jq -r .data.data.tls_cert → bare value count=0 (JSON.parse fails on the bare scalar, catch returns count=0). Same class as round-1 #3, on a path the README now advertises (-format=yaml). Not a regression (json form pre-dated the delta), but the same kubectlSecretPipeline-style guard should apply to VAULT_CMD segments feeding EXTRACTOR, or the whole-output mask for vault … -field|json|yaml | jq/yq/….
[CONCERN] redact/index.ts:161 — status-bar total double-counts: bash mirrors output into details.note, so one token masked in content yields sessionTotal += 2; the extension test now asserts redacted: 2 for a single secret. The log line and per-result notice are accurate (count/detailsCount split); only the operator-facing total inflates. Consider r.count + (r.count ? 0 : r.detailsCount).
[SUGGESTION] redact/structural.ts:139-157 — nested flow values are skipped: data: {config: {nested: c2VjcmV0}} → count=0 (value class [^,{}] stops at {, inner flow not walked). Valid k8s Secret data values are always strings, so input is invalid YAML anyway — document as a corner or walk one level.
[SUGGESTION] redact/patterns.ts:271 — bounding gap, theoretical only: URL userinfo >256 chars or password >1024 chars no longer matches. No realistic credential approaches this; no action needed unless the bounding is meant as a hard guarantee.

---

Resolution (dfd0022): CONCERN 1 fixed (bao/vault → extractor pipelines masked whole; test added). CONCERN 2 fixed (sessionTotal += count || detailsCount). Suggestions 3–4 accepted as documented corners (nested flow values are invalid Secret data; >1 KiB unquoted secrets are out of realistic range).

🤖 Generated with Claude Code

## Independent review — round 2 (delta re-review, fresh `pi -p --no-session`, omniroute/kimi-pool) VERDICT: APPROVE Round-1 findings: #1 RESOLVED — bounded classes committed (`patterns.ts:271,278,285,290,306,320,329,337`), `lastIndex = Math.max(m.index+1, start)` (`redactor.ts:107`), `tests/perf.test.ts` added. Independent timing at HEAD: `"x-".repeat(25000)` 20.8 ms, `"a.".repeat(25000)` 19.7 ms, worst case (`"a=".repeat(25000)`) 105.9 ms — vs 5.3 s/2.9 s at round-1 HEAD. Full 11-case matrix linear. 87/87 tests pass. #2 RESOLVED — new `password` space rule (`patterns.ts:296-303`). Repro `machine forgejo.example\nlogin aaron\npassword Sup3rSecretValue!x` → count=1, masked; one-line form also masked. Prose probes (`password is stored…`, `password security is important`, `passwd: password updated successfully`) unchanged; tests added. #3 RESOLVED — `kubectlSecretPipeline` + `KUBECTL_SECRET_DUMP` (`structural.ts:331-339`, wired at `434`). Repros `kubectl get secret x -o json | jq -r .data.token | base64 -d`, `-o yaml | grep`, `--output=json | yq` all masked whole (count=1). FP control `kubectl get secrets | grep auditor` untouched (test added). #4 RESOLVED — `redactVaultYaml` + `VAULT_YAML` branch (`structural.ts:344-365,455-460`). Repro KV v2 (`data.data` leaves masked, `metadata.version`/`lease_duration` intact) and KV v1 (meta keys skipped) both masked; tests added. #5 RESOLVED — `DATA_FLOW_LINE` + `maskFlowMapping` (`structural.ts:104,139-157,207-218`). Repro `data: {password: c2Vj, token: 'dG9r'}` → count=2, values masked; `metadata: {name: x}` untouched; flow-style ConfigMap in MUST_KEEP corpus. #6 RESOLVED — `index.ts:159` now adds `r.count + r.detailsCount`; new test asserts details-only mask yields `redacted: 1` and a `"detailsCount":1` log line. #7 RESOLVED — README reworded ("arguments a tool call actually executes with", transcript-log exception explained); no longer contradicts `redact-bridge.ts`. #8 DOCUMENTED — README "Non-plain `details` objects" limitation added (plus session-JSONL arguments limitation). New findings: [CONCERN] `redact/structural.ts:455-460` — the extractor-pipeline bypass fixed for kubectl remains for vault. Reproducers: `bao kv get -format=yaml kv/ld/db | grep tls_cert` → output ` tls_cert: Zm9vYmFy…` count=0 (no top-level `data:` line survives the extractor, and no generic rule keys on `tls_cert`); `... | yq '.data.data.tls_cert'` → bare value count=0; `bao kv get -format=json ... | jq -r .data.data.tls_cert` → bare value count=0 (JSON.parse fails on the bare scalar, catch returns count=0). Same class as round-1 #3, on a path the README now advertises (`-format=yaml`). Not a regression (json form pre-dated the delta), but the same `kubectlSecretPipeline`-style guard should apply to `VAULT_CMD` segments feeding `EXTRACTOR`, or the whole-output mask for `vault … -field|json|yaml | jq/yq/…`. [CONCERN] `redact/index.ts:161` — status-bar total double-counts: bash mirrors output into `details.note`, so one token masked in content yields `sessionTotal += 2`; the extension test now asserts `redacted: 2` for a single secret. The log line and per-result notice are accurate (count/detailsCount split); only the operator-facing total inflates. Consider `r.count + (r.count ? 0 : r.detailsCount)`. [SUGGESTION] `redact/structural.ts:139-157` — nested flow values are skipped: `data: {config: {nested: c2VjcmV0}}` → count=0 (value class `[^,{}]` stops at `{`, inner flow not walked). Valid k8s Secret data values are always strings, so input is invalid YAML anyway — document as a corner or walk one level. [SUGGESTION] `redact/patterns.ts:271` — bounding gap, theoretical only: URL userinfo >256 chars or password >1024 chars no longer matches. No realistic credential approaches this; no action needed unless the bounding is meant as a hard guarantee. --- **Resolution (dfd0022):** CONCERN 1 fixed (bao/vault → extractor pipelines masked whole; test added). CONCERN 2 fixed (`sessionTotal += count || detailsCount`). Suggestions 3–4 accepted as documented corners (nested flow values are invalid Secret data; >1 KiB unquoted secrets are out of realistic range). 🤖 Generated with [Claude Code](https://claude.com/claude-code)

aaron commented 2026-10-23 18:01:44 +00:00

Author

Owner

Copy link

Independent review — round 3 (delta re-check of dfd0022, fresh pi -p --no-session, omniroute/kimi-pool)

All verification complete. Both round-2 CONCERNs independently re-verified:

CONCERN 1 (vault extractor pipelines) — RESOLVED. dumpPipedToExtractor generalized (structural.ts:328-337) and wired for vault at structural.ts:442-446. All three reproducers now mask whole with no leak (slice-prefix check included): bao kv get -format=yaml | grep, | yq '.data.data.tls_cert' (bare value), | jq -r .data.data.tls_cert (bare scalar). Regression probes pass: realistic table dump (==== Data ==== format), full -format=yaml KV v2/v1, and kubectl grep FP control all intact.

CONCERN 2 (status-bar double-count) — RESOLVED. redact/index.ts:161 is now sessionTotal += r.count || r.detailsCount: content-masked result counts once even when bash mirrors into details.note; details-only masks still count. Extension test asserts redacted: 1 for one token; log line keeps the accurate "count":1/"detailsCount" split.

No new FP/leak found. Over-mask probes on the broadened vault branch all pass through unmasked (count=0): vault status | grep Sealed, vault list secret/ | grep, vault kv metadata get | grep — the pipeline guard only fires when a vault segment is followed by an extractor, and whole-output masking is fail-closed for genuine secret dumps. Kubectl path is pure parameterization, no behavior change. Full suite: 88/88 pass (was 87 + 1 new round-2 test).

VERDICT: APPROVE

Findings:

• [SUGGESTION] redact/structural.ts:442-446 — vault kv get kv/x | head -1 now masks whole output (header line only). Fail-closed and consistent with the kubectl precedent; acceptable, but if operator UX matters the dump predicate could later distinguish -format/get/read from list/status/metadata.
• [SUGGESTION] (carried from round 2, unchanged by delta) redact/structural.ts:139-157 nested flow values skipped; theoretical, invalid-input corner — README note would suffice.

---

No open blockers; CI 14/14 green on dfd0022. Suggestions accepted as-is (fail-closed | head masking is intended; nested flow values are invalid Secret data). Merging.

🤖 Generated with Claude Code

## Independent review — round 3 (delta re-check of dfd0022, fresh `pi -p --no-session`, omniroute/kimi-pool) All verification complete. Both round-2 CONCERNs independently re-verified: **CONCERN 1 (vault extractor pipelines)** — RESOLVED. `dumpPipedToExtractor` generalized (`structural.ts:328-337`) and wired for vault at `structural.ts:442-446`. All three reproducers now mask whole with no leak (slice-prefix check included): `bao kv get -format=yaml | grep`, `| yq '.data.data.tls_cert'` (bare value), `| jq -r .data.data.tls_cert` (bare scalar). Regression probes pass: realistic table dump (`==== Data ====` format), full `-format=yaml` KV v2/v1, and kubectl grep FP control all intact. **CONCERN 2 (status-bar double-count)** — RESOLVED. `redact/index.ts:161` is now `sessionTotal += r.count || r.detailsCount`: content-masked result counts once even when bash mirrors into `details.note`; details-only masks still count. Extension test asserts `redacted: 1` for one token; log line keeps the accurate `"count":1`/`"detailsCount"` split. **No new FP/leak found.** Over-mask probes on the broadened vault branch all pass through unmasked (count=0): `vault status | grep Sealed`, `vault list secret/ | grep`, `vault kv metadata get | grep` — the pipeline guard only fires when a vault segment is *followed by* an extractor, and whole-output masking is fail-closed for genuine secret dumps. Kubectl path is pure parameterization, no behavior change. Full suite: 88/88 pass (was 87 + 1 new round-2 test). VERDICT: APPROVE Findings: - [SUGGESTION] `redact/structural.ts:442-446` — `vault kv get kv/x | head -1` now masks whole output (header line only). Fail-closed and consistent with the kubectl precedent; acceptable, but if operator UX matters the dump predicate could later distinguish `-format`/`get`/`read` from `list`/`status`/`metadata`. - [SUGGESTION] (carried from round 2, unchanged by delta) `redact/structural.ts:139-157` nested flow values skipped; theoretical, invalid-input corner — README note would suffice. --- No open blockers; CI 14/14 green on dfd0022. Suggestions accepted as-is (fail-closed `| head` masking is intended; nested flow values are invalid Secret data). Merging. 🤖 Generated with [Claude Code](https://claude.com/claude-code)

aaron merged commit efdec6ca12 into main 2026-10-23 18:01:48 +00:00

aaron deleted branch plane/HOMELAB-2194-redact 2026-10-23 18:01:49 +00:00

aaron referenced this pull request from a commit 2026-10-23 18:01:50 +00:00

Merge pull request 'HOMELAB-2194: feat(redact): mask secrets in tool output before context, session logs and subagent transcripts' (#65) from plane/HOMELAB-2194-redact into main

aaron referenced this pull request from a commit 2026-10-23 18:23:28 +00:00

HOMELAB-2218: fix(redact): keep optional-chaining expressions and header docs unmasked

aaron referenced this pull request 2026-10-23 18:23:36 +00:00

HOMELAB-2218: fix(redact): keep optional-chaining expressions and header docs unmasked #66

Sign in to join this conversation.

Reviewers

No reviewers

Labels

Clear labels

No items

No labels

Milestone

Clear milestone

No items

No milestone

Projects

Clear projects

No items

No project

Assignees

Clear assignees

No assignees

1 participant

Notifications

Subscribe

Due date

The due date is invalid or out of range. Please use the format "yyyy-mm-dd".

No due date set.

Dependencies

No dependencies set.

Reference

Homelab/pi-extensions!65

No description provided.