Cloonar/coding-lab

Fork 0

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

New issue

Closed

opened 2026-07-10 17:37:02 +02:00 by dominik.polakovics · 2 comments

dominik.polakovics commented

2026-07-10 17:37:02 +02:00

Owner

What to build

Server-side harness layer: every tracker write that arrives through a run token — PR/change-request titles and bodies, issue comments, issue create — is scanned against that repo's secret values using the matcher from #104 (exact + base64/hex/urlencoded forms). A hit is rejected with a 400 naming the matching secret (name only, never the value) so the agent can rewrite and retry. Nothing is silently redacted — a body that says [REDACTED] mid-sentence on the forge is worse than a clean retry.

Scope:

Applies to the agentapi (run-token) write paths only; operator PAT writes are not scanned (the operator is not the threat model).
Works identically for both tracker bindings (forge REST client and builtin change requests) — scan sits in front of the Tracker seam, not inside a backend.
Zero-secret repos skip the scan with no overhead worth mentioning.

Acceptance criteria

labctl pr create / issue comment / issue create with a secret value (any matched form) in title or body is rejected 400, error names the secret only
Clean writes are untouched, byte-identical
Both tracker bindings covered by the same scan site (test via the Tracker seam)
Operator (PAT) write paths are not scanned
The plaintext value appears in no error message, log line, or response

Blocked by

#104 (secrets + matcher must exist)

## What to build Server-side harness layer: every tracker write that arrives through a **run token** — PR/change-request titles and bodies, issue comments, issue create — is scanned against that repo's secret values using the matcher from #104 (exact + base64/hex/urlencoded forms). A hit is **rejected with a 400 naming the matching secret** (name only, never the value) so the agent can rewrite and retry. Nothing is silently redacted — a body that says `[REDACTED]` mid-sentence on the forge is worse than a clean retry. Scope: - Applies to the agentapi (run-token) write paths only; operator PAT writes are not scanned (the operator is not the threat model). - Works identically for both tracker bindings (forge REST client and builtin change requests) — scan sits in front of the Tracker seam, not inside a backend. - Zero-secret repos skip the scan with no overhead worth mentioning. ## Acceptance criteria - [ ] `labctl pr create` / `issue comment` / `issue create` with a secret value (any matched form) in title or body is rejected 400, error names the secret only - [ ] Clean writes are untouched, byte-identical - [ ] Both tracker bindings covered by the same scan site (test via the Tracker seam) - [ ] Operator (PAT) write paths are not scanned - [ ] The plaintext value appears in no error message, log line, or response ## Blocked by - #104 (secrets + matcher must exist)

dominik.polakovics added the

needs-triage

label

2026-07-10 17:37:02 +02:00

dominik.polakovics referenced this issue

2026-07-10 17:57:05 +02:00

feat(secrets): per-repo secrets — vault to labctl broker exec (tracer bullet) #104

dominik.polakovics referenced this issue

2026-07-10 18:33:51 +02:00

feat(secrets): per-repo secrets — vault to labctl broker exec (tracer bullet) (#104) #112

dominik.polakovics referenced this issue

2026-07-10 18:48:10 +02:00

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

dominik.polakovics referenced this issue

2026-07-10 18:48:32 +02:00

feat(secrets): git leak guard — pre-push scan blocks pushes containing secret values #106

dominik.polakovics commented

2026-07-10 18:48:52 +02:00

Author

Owner

This was generated by AI during triage.

Agent Brief

Category: enhancement
Summary: Every tracker write arriving through a run token — PR/change-request create, issue create, issue comment — is scanned server-side against the repo's secret values; a hit is rejected with a 400 naming the matching secret (never the value) so the agent rewrites and retries.

Current behavior:
Run-token tracker writes flow through the agentapi into the Tracker seam, which fans out to the two bindings (forge REST client and builtin change requests). No content scanning happens: a title or body containing a secret value would land verbatim on the forge. The shared streaming matcher and decrypted-value access exist since #104.

Desired behavior:
A scan sits in front of the Tracker seam — one site covering both bindings identically — checking title and body of every run-token write against the repo's secret values in all four matched forms (exact, base64, hex, URL-encoded). A hit rejects the write with a 400 whose error names the secret only; nothing is silently redacted — a body reading [REDACTED] mid-sentence on the forge is worse than a clean retry. Operator PAT write paths are not scanned (the operator is not the threat model). Zero-secret repos skip the scan with negligible overhead.

Key interfaces:

The agentapi run-token write handlers / the boundary in front of the Tracker seam — the scan must be a single site, not per-backend logic
secrets.NewMatcher(values) over the write's title + body; values decrypted via the store's repo-secrets access from #104
Error shape: HTTP 400 naming the secret name; plaintext value in no error message, log line, or response

Acceptance criteria:

labctl pr create / issue create / issue comment with a secret value (any matched form) in title or body is rejected 400; the error names the secret only
Clean writes are untouched, byte-identical
Both tracker bindings are covered by the same scan site (tested via the Tracker seam)
Operator (PAT) write paths are not scanned
The plaintext value appears in no error message, log line, or response

Out of scope:

Silent redaction of tracker bodies (explicit grill decision: reject, don't rewrite)
Scanning operator writes or retroactively scanning existing forge content
Label, state, or other non-content tracker mutations
Broker redaction (#105), git push guard (#106), exposure detection (#108)

> *This was generated by AI during triage.* ## Agent Brief **Category:** enhancement **Summary:** Every tracker write arriving through a run token — PR/change-request create, issue create, issue comment — is scanned server-side against the repo's secret values; a hit is rejected with a 400 naming the matching secret (never the value) so the agent rewrites and retries. **Current behavior:** Run-token tracker writes flow through the agentapi into the Tracker seam, which fans out to the two bindings (forge REST client and builtin change requests). No content scanning happens: a title or body containing a secret value would land verbatim on the forge. The shared streaming matcher and decrypted-value access exist since #104. **Desired behavior:** A scan sits in front of the Tracker seam — one site covering both bindings identically — checking title and body of every run-token write against the repo's secret values in all four matched forms (exact, base64, hex, URL-encoded). A hit rejects the write with a 400 whose error names the secret only; nothing is silently redacted — a body reading `[REDACTED]` mid-sentence on the forge is worse than a clean retry. Operator PAT write paths are not scanned (the operator is not the threat model). Zero-secret repos skip the scan with negligible overhead. **Key interfaces:** - The agentapi run-token write handlers / the boundary in front of the Tracker seam — the scan must be a single site, not per-backend logic - `secrets.NewMatcher(values)` over the write's title + body; values decrypted via the store's repo-secrets access from #104 - Error shape: HTTP 400 naming the secret name; plaintext value in no error message, log line, or response **Acceptance criteria:** - [ ] `labctl pr create` / `issue create` / `issue comment` with a secret value (any matched form) in title or body is rejected 400; the error names the secret only - [ ] Clean writes are untouched, byte-identical - [ ] Both tracker bindings are covered by the same scan site (tested via the Tracker seam) - [ ] Operator (PAT) write paths are not scanned - [ ] The plaintext value appears in no error message, log line, or response **Out of scope:** - Silent redaction of tracker bodies (explicit grill decision: reject, don't rewrite) - Scanning operator writes or retroactively scanning existing forge content - Label, state, or other non-content tracker mutations - Broker redaction (#105), git push guard (#106), exposure detection (#108)

dominik.polakovics added

enhancement

ready-for-agent

and removed

needs-triage

labels

2026-07-10 18:49:39 +02:00

dominik.polakovics referenced this issue from a commit

2026-07-10 19:26:46 +02:00

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

dominik.polakovics referenced this issue from a pull request that will close it,

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

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

dominik.polakovics commented

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

Author

Owner

This was generated by AI while landing a PR.

Landing audit — PR #114 → verdict: PASS

Verification signal: repo CI (ci / native) is red, but the only failing package is internal/tmuxx, whose tmux integration tests fail identically on a clean origin/main (sessions exit immediately — a sandbox/infra limitation, not this PR). I reproduced locally: PR branch and clean main both fail tmuxx and only tmuxx. This forge does not gate merges on that signal (the prior 18 PRs merged against the same red).

What I ran (PR branch afk/107, CGO off):

go build ./... — clean
go test ./internal/tracker/secretscan/... ./internal/agentapi/... ./internal/httpapi/... — all ok
gofmt -l on changed files — clean; go vet — clean

Review findings:

Wiring is the whole run-token-only property: only agentapi's resolver is wrapped (secretscan.NewResolver); operator API and AFK engine keep the bare registry. Correct seam — above the forge/builtin fan-out, so one decorator covers both bindings.
writeTrackerError maps *secretscan.BlockedError → 400 ahead of the 502/500 folds, so a rejected leak is never miscoded as an upstream fault.
Fail-closed verified: store/decrypt errors return a plain (non-Blocked) error and never delegate; zero-secret repos cost one metadata query then delegate untouched.
BlockedError.Error() renders secret name/field/form only — no value in any encoding; package contains no logging.
ForRun re-wraps the rescoped tracker, preserving builtin agent attribution through the decorator.
Ride-along migration fix: main had two 0008 migrations (repo_secrets #112 + run_title #113) → goose duplicate-version boot failure. repo_secrets keeps 0008; run_title renumbered to 0009 in both sqlite+postgres dialects; ADR-0040 updated. Confirmed no duplicate versions remain.

Conventions: title is Conventional Commits (feat(secrets): …); head afk/107 carries a working Closes #107.

Clean to merge — awaiting free-text go-ahead from the human.

> *This was generated by AI while landing a PR.* **Landing audit — PR #114 → verdict: PASS** **Verification signal:** repo CI (`ci / native`) is **red**, but the only failing package is `internal/tmuxx`, whose tmux integration tests fail identically on a clean `origin/main` (sessions exit immediately — a sandbox/infra limitation, not this PR). I reproduced locally: PR branch and clean main both fail `tmuxx` and *only* `tmuxx`. This forge does not gate merges on that signal (the prior 18 PRs merged against the same red). **What I ran (PR branch `afk/107`, CGO off):** - `go build ./...` — clean - `go test ./internal/tracker/secretscan/... ./internal/agentapi/... ./internal/httpapi/...` — all **ok** - `gofmt -l` on changed files — clean; `go vet` — clean **Review findings:** - Wiring is the whole run-token-only property: only agentapi's resolver is wrapped (`secretscan.NewResolver`); operator API and AFK engine keep the bare registry. Correct seam — above the forge/builtin fan-out, so one decorator covers both bindings. - `writeTrackerError` maps `*secretscan.BlockedError` → 400 **ahead** of the 502/500 folds, so a rejected leak is never miscoded as an upstream fault. - Fail-closed verified: store/decrypt errors return a plain (non-Blocked) error and never delegate; zero-secret repos cost one metadata query then delegate untouched. - `BlockedError.Error()` renders secret **name/field/form only** — no value in any encoding; package contains no logging. - `ForRun` re-wraps the rescoped tracker, preserving builtin agent attribution through the decorator. - Ride-along migration fix: `main` had two `0008` migrations (`repo_secrets` #112 + `run_title` #113) → goose duplicate-version boot failure. `repo_secrets` keeps `0008`; `run_title` renumbered to `0009` in both sqlite+postgres dialects; ADR-0040 updated. Confirmed no duplicate versions remain. **Conventions:** title is Conventional Commits (`feat(secrets): …`); head `afk/107` carries a working `Closes #107`. Clean to merge — awaiting free-text go-ahead from the human.

dominik.polakovics closed this issue

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

dominik.polakovics referenced this issue 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

dominik.polakovics referenced this issue from a commit

2026-07-10 20:46:52 +02:00

fix(secretscan): satisfy golangci-lint on #107 test (errcheck + QF1008)