feat(secrets): reject run-token tracker writes carrying secret values (#107) #114

Merged

dominik.polakovics merged 2 commits from afk/107 into main

2026-07-10 19:43:44 +02:00

dominik.polakovics commented

2026-07-10 19:27:10 +02:00

Owner

Server-side leak guard on the tracker write path: every content-bearing write arriving through a run token — labctl pr create, issue create, issue comment — is scanned against the repo's secret values (exact, base64 all alphabets, hex both cases, URL-encoded; the #104 matcher) and rejected with a 400 naming the secret, field, and form only so the agent can rewrite and retry. Nothing is silently redacted.

Design

One scan site, both bindings: new package internal/tracker/secretscan — a Tracker decorator installed by wrapping the resolver handed to agentapi.New in cmd/lab/main.go. It sits above the registry's forge/builtin fan-out, so both bindings get identical behavior from a single implementation.
Run-token-only is a wiring property: only agentapi's resolver is wrapped; the operator API (Config{Tracker: trackerReg}) and the AFK engine keep the bare registry, so PAT writes are never scanned (pinned by TestOperatorWriteNotSecretScanned).
ForRun is forwarded per the instrument.go decorator pattern, so builtin agent attribution survives the wrap.
Fail closed: a store/decrypt failure refuses the write (opaque 5xx), never delegates unscanned. Zero-secret repos cost one metadata query and delegate untouched, byte-identical.
writeTrackerError maps *secretscan.BlockedError → 400 ahead of the forge-502/builtin-500 folds; the branch does not log. The package itself never logs, so no value can reach a log line. Unit + integration tests assert the plaintext and every derived encoding appear in no response.

Acceptance criteria → proof

Secret in title/body (any matched form) → 400 naming the secret only: TestSecretScan_PRExactValueBlocked, _IssueBase64TitleBlocked, _CommentURLEncodedBlocked (end-to-end through the HTTP handlers)
Clean writes byte-identical: TestSecretScan_CleanWritesSucceed (incl. the PR path's Closes #N append), TestZeroSecretRepoDelegates, TestCleanWritesDelegate
Both bindings via the same scan site: TestSecretScan_BuiltinBindingBlocked (real store-backed tracker, no row written) + forge fake tests above
Operator writes not scanned: TestOperatorWriteNotSecretScanned
No value in any error/log/response: TestErrorMessageLeaksNoValue, per-400 assertNoLeak over all encodings; the scan path contains no logging

Ride-along fix (first commit)

main carried two migrations numbered 0008 (repo_secrets from #112, run_title from #113), so goose failed at boot and every store-backed test failed with found duplicate migration version 8. repo_secrets merged first and keeps 0008; run_title is renumbered to 0009 in both dialects (ADR-0040 reference updated). Without this, nothing on this branch — or main — can be tested or booted.

go build ./..., go vet, gofmt clean; full go test ./... -count=1 green except the pre-existing sandbox-only internal/tmuxx tmux-environment failures (fail identically on a clean tree).

Closes #107

🤖 Generated with Claude Code

https://claude.ai/code/session_01Skb2pePwKkHmgCzTjysdBJ

Server-side leak guard on the tracker write path: every content-bearing write arriving through a run token — `labctl pr create`, `issue create`, `issue comment` — is scanned against the repo's secret values (exact, base64 all alphabets, hex both cases, URL-encoded; the #104 matcher) and rejected with a **400 naming the secret, field, and form only** so the agent can rewrite and retry. Nothing is silently redacted. ## Design - **One scan site, both bindings**: new package `internal/tracker/secretscan` — a `Tracker` decorator installed by wrapping the resolver handed to `agentapi.New` in `cmd/lab/main.go`. It sits above the registry's forge/builtin fan-out, so both bindings get identical behavior from a single implementation. - **Run-token-only is a wiring property**: only agentapi's resolver is wrapped; the operator API (`Config{Tracker: trackerReg}`) and the AFK engine keep the bare registry, so PAT writes are never scanned (pinned by `TestOperatorWriteNotSecretScanned`). - `ForRun` is forwarded per the `instrument.go` decorator pattern, so builtin agent attribution survives the wrap. - **Fail closed**: a store/decrypt failure refuses the write (opaque 5xx), never delegates unscanned. Zero-secret repos cost one metadata query and delegate untouched, byte-identical. - `writeTrackerError` maps `*secretscan.BlockedError` → 400 ahead of the forge-502/builtin-500 folds; the branch does not log. The package itself never logs, so no value can reach a log line. Unit + integration tests assert the plaintext and every derived encoding appear in no response. ## Acceptance criteria → proof - Secret in title/body (any matched form) → 400 naming the secret only: `TestSecretScan_PRExactValueBlocked`, `_IssueBase64TitleBlocked`, `_CommentURLEncodedBlocked` (end-to-end through the HTTP handlers) - Clean writes byte-identical: `TestSecretScan_CleanWritesSucceed` (incl. the PR path's `Closes #N` append), `TestZeroSecretRepoDelegates`, `TestCleanWritesDelegate` - Both bindings via the same scan site: `TestSecretScan_BuiltinBindingBlocked` (real store-backed tracker, no row written) + forge fake tests above - Operator writes not scanned: `TestOperatorWriteNotSecretScanned` - No value in any error/log/response: `TestErrorMessageLeaksNoValue`, per-400 `assertNoLeak` over all encodings; the scan path contains no logging ## Ride-along fix (first commit) `main` carried two migrations numbered `0008` (`repo_secrets` from #112, `run_title` from #113), so goose failed at boot and every store-backed test failed with `found duplicate migration version 8`. `repo_secrets` merged first and keeps `0008`; `run_title` is renumbered to `0009` in both dialects (ADR-0040 reference updated). Without this, nothing on this branch — or main — can be tested or booted. `go build ./...`, `go vet`, `gofmt` clean; full `go test ./... -count=1` green except the pre-existing sandbox-only `internal/tmuxx` tmux-environment failures (fail identically on a clean tree). Closes #107 🤖 Generated with [Claude Code](https://claude.com/claude-code) https://claude.ai/code/session_01Skb2pePwKkHmgCzTjysdBJ

dominik.polakovics added 2 commits

2026-07-10 19:27:10 +02:00

fix(migrations): renumber run_title to 0009 after duplicate version 8 fb4d5f49ed

PRs #112 (repo_secrets) and #113 (run_title) both landed a migration
numbered 0008 on main; goose refuses duplicate versions, so server boot
and every store-backed test failed with "found duplicate migration
version 8". repo_secrets merged first and keeps 0008; run_title moves
to 0009 in both dialects. ADR-0040's file reference updated.

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01Skb2pePwKkHmgCzTjysdBJ

feat(secrets): reject run-token tracker writes carrying secret values (#107 )

ci / native (pull_request) Failing after 5m11s

Details

900814b622

Every content-bearing tracker write arriving through a run token — PR/
change-request create, issue create, issue comment — is now scanned
against the repo's secret values (exact, base64, hex, URL-encoded forms
via the #104 matcher) before it reaches the backend. A hit is rejected
with a 400 naming the matching secret, field, and form only — never the
value in any encoding — so the agent rewrites and retries; nothing is
silently redacted.

The scan is a Tracker decorator (internal/tracker/secretscan) wrapped
around the resolver handed to agentapi in cmd/lab — one site above the
registry's forge/builtin fan-out, so both bindings are covered
identically, and run-token-only is a wiring property: the operator API
and AFK engine keep the bare registry, so PAT writes are never scanned.
The decorator forwards RunScoper.ForRun (instrument.go pattern) so
builtin agent attribution survives the wrap. Zero-secret repos cost one
metadata query and delegate untouched; store/decrypt failures fail
closed. agentapi maps *secretscan.BlockedError to 400 ahead of the
forge-502/builtin-500 folds and never logs it.

Closes #107

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01Skb2pePwKkHmgCzTjysdBJ

dominik.polakovics referenced this pull request

2026-07-10 19:38:22 +02:00

feat(secrets): sanitize run-token tracker writes against secret values #107

dominik.polakovics merged commit a390b8ec50 into main

2026-07-10 19:43:44 +02:00

dominik.polakovics referenced this pull request from a commit

2026-07-10 19:43:44 +02:00

Merge pull request 'feat(secrets): reject run-token tracker writes carrying secret values (#107)' (#114) from afk/107 into main