feat(secrets): broker output redaction in labctl secret exec (#105) #116

Merged
dominik.polakovics merged 4 commits from afk/105 into main 2026-07-10 20:53:28 +02:00

The broker now scans the wrapped command's stdout and stderr for the values it injected for THIS exec and rewrites every occurrence — exact, base64 (4 alphabets), hex (2 cases), or URL-encoded — to [REDACTED:NAME] before the bytes reach the terminal, and therefore before they can reach the transcript or chat.

What's in here

internal/secretsMatch gains a chunking-invariant Start offset; new Redactor, an io.Writer over the Matcher kernel. It holds back a tail window (longest-pattern−1 bytes) so a value split across write chunks is rewritten before emission, coalesces overlapping matches (no byte covered by any match is ever emitted — safety over fidelity), flushes the held tail safely on stream end, and poisons itself on underlying-writer errors. The type doc proves the window arithmetic and why Flush cannot leak.

internal/labctlrunSecretExec wraps env.Stdout/env.Stderr in one matcher+redactor each (a Matcher is not concurrency-safe; os/exec copies each stream from its own goroutine), and flushes both unconditionally before deciding the return code, so a child that prints a secret and exits non-zero still gets its tail redacted. Child exit code passes through verbatim; stdout stays stdout, stderr stays stderr; non-secret bytes are byte-identical. The child now runs on pipes, never a TTY — documented as deliberate (agents don't run interactive commands through the broker). Contained hits are quiet: no alert, no log, no plaintext anywhere (grill decision).

Acceptance criteria → proof

  • any of the four forms printed → [REDACTED:NAME]: TestRedactor_EachForm (all 9 derived encodings), e2e TestSecretExecRedactsPrintedValue / TestSecretExecRedactsEncodedForms
  • value split across write chunks: TestRedactor_ChunkingInvariance — identical output for one Write, every 2-way split position, byte-at-a-time, and uneven cycles (pipes can't force chunk boundaries reliably, so this criterion is pinned at the unit layer)
  • exit code / stream separation / byte-identical passthrough: TestSecretExecRedactionPreservesExitCode (prints secret, exits 5), TestSecretExecStderrRedactedAndSeparate, TestSecretExecArgvIsExactCommand
  • secret exec NAME -- printenv NAME → placeholder only: TestSecretExecPrintenvShowsPlaceholder (real coreutils printenv)
  • no redaction event logged/surfaced with plaintext: redaction is wired with zero logging; every e2e test asserts the plaintext appears nowhere in stdout+stderr

Also verified against the real binary: a stub agent API + bin/labctl secret exec — plain echo, printenv, a genuine base64 pipe, stderr+exit 7, and clean passthrough all behave per contract.

Also: fix(store) migration collision (#115)

Verifying this issue required a green suite, which surfaced that main is currently broken: 0008_repo_secrets.sql (#104) and 0008_run_title.sql (#111) both claim goose version 8, so store.Open fails everywhere (server cannot boot, store-backed tests red). First commit renumbers run_title0009 in both dialects — the safe direction, since no DB can have applied run_title as 8 (a tree with both files fails before applying anything) while the main lineage applied 8 = repo_secrets. Filed and explained in #115.

Verification: go test ./... green (sole exception: internal/tmuxx integration tests, which fail in this sandbox on pristine main too — package has zero in-repo deps, unrelated); go test -race green for internal/secrets + internal/labctl; golangci-lint run 0 issues; gofmt clean; CGO_ENABLED=0 go build ./... clean.

Closes #105
Closes #115

🤖 Generated with Claude Code

https://claude.ai/code/session_01Lwh1BiNqYbLRWmTwCZBhtY

The broker now scans the wrapped command's stdout and stderr for the values it injected for THIS exec and rewrites every occurrence — exact, base64 (4 alphabets), hex (2 cases), or URL-encoded — to `[REDACTED:NAME]` before the bytes reach the terminal, and therefore before they can reach the transcript or chat. ## What's in here **`internal/secrets`** — `Match` gains a chunking-invariant `Start` offset; new `Redactor`, an `io.Writer` over the Matcher kernel. It holds back a tail window (longest-pattern−1 bytes) so a value split across write chunks is rewritten before emission, coalesces overlapping matches (no byte covered by any match is ever emitted — safety over fidelity), flushes the held tail safely on stream end, and poisons itself on underlying-writer errors. The type doc proves the window arithmetic and why Flush cannot leak. **`internal/labctl`** — `runSecretExec` wraps `env.Stdout`/`env.Stderr` in one matcher+redactor each (a Matcher is not concurrency-safe; os/exec copies each stream from its own goroutine), and flushes both unconditionally before deciding the return code, so a child that prints a secret and exits non-zero still gets its tail redacted. Child exit code passes through verbatim; stdout stays stdout, stderr stays stderr; non-secret bytes are byte-identical. The child now runs on pipes, never a TTY — documented as deliberate (agents don't run interactive commands through the broker). Contained hits are quiet: no alert, no log, no plaintext anywhere (grill decision). ## Acceptance criteria → proof - any of the four forms printed → `[REDACTED:NAME]`: `TestRedactor_EachForm` (all 9 derived encodings), e2e `TestSecretExecRedactsPrintedValue` / `TestSecretExecRedactsEncodedForms` - value split across write chunks: `TestRedactor_ChunkingInvariance` — identical output for one Write, every 2-way split position, byte-at-a-time, and uneven cycles (pipes can't force chunk boundaries reliably, so this criterion is pinned at the unit layer) - exit code / stream separation / byte-identical passthrough: `TestSecretExecRedactionPreservesExitCode` (prints secret, exits 5), `TestSecretExecStderrRedactedAndSeparate`, `TestSecretExecArgvIsExactCommand` - `secret exec NAME -- printenv NAME` → placeholder only: `TestSecretExecPrintenvShowsPlaceholder` (real coreutils printenv) - no redaction event logged/surfaced with plaintext: redaction is wired with zero logging; every e2e test asserts the plaintext appears nowhere in stdout+stderr Also verified against the real binary: a stub agent API + `bin/labctl secret exec` — plain echo, `printenv`, a genuine `base64` pipe, stderr+`exit 7`, and clean passthrough all behave per contract. ## Also: fix(store) migration collision (#115) Verifying this issue required a green suite, which surfaced that main is currently broken: `0008_repo_secrets.sql` (#104) and `0008_run_title.sql` (#111) both claim goose version 8, so `store.Open` fails everywhere (server cannot boot, store-backed tests red). First commit renumbers `run_title` → `0009` in both dialects — the safe direction, since no DB can have applied run_title as 8 (a tree with both files fails before applying anything) while the main lineage applied 8 = repo_secrets. Filed and explained in #115. Verification: `go test ./...` green (sole exception: `internal/tmuxx` integration tests, which fail in this sandbox on pristine main too — package has zero in-repo deps, unrelated); `go test -race` green for `internal/secrets` + `internal/labctl`; `golangci-lint run` 0 issues; `gofmt` clean; `CGO_ENABLED=0 go build ./...` clean. Closes #105 Closes #115 🤖 Generated with [Claude Code](https://claude.com/claude-code) https://claude.ai/code/session_01Lwh1BiNqYbLRWmTwCZBhtY
migrations/{sqlite,postgres}/0008_run_title.sql (#111) collided with
0008_repo_secrets.sql (#104): the #111 branch was cut before #104 merged,
so both landed on main claiming goose version 8. goose rejects duplicate
versions at provider construction, so store.Open failed for both dialects
— the server could not boot on main and every store-backed test was red
(TestMigrationParity catches it).

The later-merged migration yields: run_title becomes 0009. Safe for every
real lineage — goose records applied versions by number, any DB migrated
before the collision has 8 = repo_secrets (#104 merged first), and no DB
can have applied run_title as 8 because a tree with both files fails
before applying anything.

Closes #115

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01Lwh1BiNqYbLRWmTwCZBhtY
feat(secrets): broker output redaction in labctl secret exec (#105)
All checks were successful
ci / native (pull_request) Successful in 5m30s
deb01b44dd
`labctl secret exec` now pipes the child's stdout and stderr through
per-stream redactors: any occurrence of a value injected for THIS exec —
exact, base64 (all four alphabets), hex (both cases), or URL-encoded —
reaches the terminal only as [REDACTED:NAME], killing the two biggest
accidental-leak paths (an API echoing the key back, a casual printenv)
before the bytes can reach the transcript or chat.

- internal/secrets: Match gains a Start offset (chunking-invariant, like
  End); new Redactor, an io.Writer over the Matcher kernel with a
  hold-back tail window so a value split across write chunks is still
  rewritten before emission, overlap-coalescing (no byte covered by any
  match is ever emitted), safe Flush on stream end, and poisoning on
  underlying-writer errors.
- internal/labctl: runSecretExec wraps env.Stdout/env.Stderr in one
  redactor each (a Matcher is not concurrency-safe; os/exec copies the
  streams from separate goroutines), flushes both unconditionally before
  deciding the exit code — the child's code still passes through
  verbatim, stdout stays stdout, stderr stays stderr, non-secret bytes
  byte-identical. The child now runs on pipes, never a TTY (documented:
  agents don't run interactive commands through the broker). Contained
  redaction hits stay quiet — no alert, no log, no plaintext anywhere
  (grill decision).

Closes #105

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01Lwh1BiNqYbLRWmTwCZBhtY
Merge remote-tracking branch 'origin/main' into land-afk-105
Some checks failed
ci / native (pull_request) Failing after 5m1s
bbffec871d
# Conflicts:
#	internal/labctl/labctl.go
#	internal/secrets/redactor.go
#	internal/secrets/redactor_test.go
fix(secretscan): satisfy golangci-lint on #107 test (errcheck + QF1008)
All checks were successful
ci / native (pull_request) Successful in 5m38s
3477c63c7e
The untagged golangci-lint gate is red on main: #107's secretscan_test.go
discards two *BlockedError returns (errcheck) and selects through the
embedded recTracker field (staticcheck QF1008). #107 merged because the
lint gate ran conditionally and its PR did not trip it; this branch's merge
does. Four no-op-behaviour fixes so the resolved tree is genuinely green.
Sign in to join this conversation.
No reviewers
No milestone
No project
No assignees
1 participant
Notifications
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
Cloonar/coding-lab!116
No description provided.