fix(control-plane): bound golden-run name and tag writes into workflow_runs.metadata

[AbirAbbas]

Summary

POST /api/ui/v2/workflow-runs/:run_id/golden wrote the caller-supplied golden-run name and tag list into workflow_runs.metadata with no bounds at all. The name was only TrimSpace'd, and sanitizeStringList trimmed and de-duped but capped neither the entry count nor the entry length — and it sized its output slice and de-dupe map straight from the attacker-controlled input length. This caps tags at 20 entries of at most 64 runes, truncates the name at 200 runes, and stops the helper from preallocating from the input length.

The change is forward-only and silent: nothing re-validates on read, and oversized input is clamped rather than rejected.

Why

Refs #944.

The golden metadata row is re-read and re-serialised on every runs-list page that contains the run, so an unbounded write there is stored amplification rather than a one-off. A probe stored a 1 MiB name that accounted for ~98% of the resulting metadata row — which is why capping tags alone would have left the strictly larger hole open.

This is not Fixes #944: that issue is a broader proposal for first-class run display names, labels, links and external status. This PR only closes the unbounded-write hole on the existing golden route.

Changes

fix(control-plane): bound golden-run name and tag writes into run metadata

• Add maxGoldenTags (20) / maxGoldenTagRunes (64) / maxGoldenNameRunes (200) as package constants, so the follow-up run-metadata endpoint can reuse the same bounds and the same helper.
• sanitizeStringList takes maxCount, maxRunes; over-long entries are dropped, not truncated, and lengths are counted with utf8.RuneCountInString.
• Size the output slice and de-dupe map from min(len(values), maxCount) and break the loop once maxCount survivors are collected.
• New truncateRunes helper cuts on a rune boundary by walking rune start offsets, so a multi-megabyte name costs no extra allocation. Applied to req.Name; the run_id fallback is deliberately not truncated.

test(control-plane): cover golden-run metadata bounds and forward-only reads

• Unit coverage for sanitizeStringList and truncateRunes, an allocation-contract test, a handler round-trip, and a read-back test. The two pre-existing golden-route tests are left untouched as the behaviour-unchanged regression guard.

Validation contract

Behaviour	Covering test
Saving a golden run with 500 tags persists at most 20.	TestSanitizeStringListBounds/count_cap_preserves_first_entries; TestWorkflowRunHandlerSaveGoldenRunBoundsNameAndTags (51 tags posted → 20 stored)
A tag longer than 64 runes is DROPPED, not truncated — byte-slicing mid-rune yields invalid UTF-8 that json.Marshal rewrites to U+FFFD. Counted in runes, so a 64-rune CJK tag survives.	TestSanitizeStringListBounds/overlong_ASCII_dropped, /multibyte_rune_cap, plus the byte-identity assertion on a 64-rune CJK tag in TestSanitizeStringListBounds
The golden-run name is capped at ~200 runes on a rune boundary, and never truncated to empty (empty falls back to run_id and would change the saved label).	TestTruncateRunes (all 7 rows, incl. multibyte_boundary); TestWorkflowRunHandlerSaveGoldenRunBoundsNameAndTags asserts a 1 MiB name stores exactly 200 runes, non-empty, and that a blank name still falls back to run_id
Output slice is min(len(values), maxCount) and the loop breaks at maxCount, so a 5M-element array cannot allocate ~80 MB before any cap applies; the de-dupe map is bounded with it.	TestSanitizeStringListDoesNotPreallocateFromInputLength — 100k entries, asserts cap(out) <= 20 (a length-only assertion would still pass against the old preallocation)
Normal tags behave exactly as before: trim, de-dupe, order preserved.	TestSanitizeStringListBounds/legacy_sanitizing, plus the untouched pre-existing TestWorkflowRunHandlerSaveGoldenRun* tests
Silent clamping (not a 400) on this UI-private route, because rejecting would break the existing save-as-golden button.	TestWorkflowRunHandlerSaveGoldenRunBoundsNameAndTags asserts 200 OK with clamped storage; the pre-existing TestWorkflowRunHandlerSaveGoldenRunErrors still pins the genuine 400/404/409 cases
An existing row that already holds oversized golden metadata still reads back and renders unchanged — forward-only, no remediation of already-written rows.	TestWorkflowRunHandlerGoldenReadBackIsNotRevalidated — a pre-seeded row with 50 tags and a 500-rune name surfaces in full on both the list and detail responses
The caps are named constants, not inline literals, so run-metadata-v1 can reuse the helper.	Enforced by construction (the const block in workflow_runs.go); maxGoldenNameRunes is referenced directly by the handler test rather than a hard-coded 200

How it was tested

CI-literal gates for the control-plane surface, re-run after rebasing onto origin/main (b458f9c3, v0.1.138-rc.2):

• go build ./... — PASS
• gofmt -l <touched files> — PASS (no output)
• go vet ./internal/handlers/ui (plus the packages the incoming main commits touched) — PASS
• go test -tags sqlite_fts5 -count=1 -timeout 40m $(go list ./... | grep -v internal/packages) — PASS

Patch-coverage gate (the required CI check, threshold 80% on lines touched vs origin/main): 100.00% across 25 touched lines on control-plane. Run before the rebase; the rebase was conflict-free and the incoming main commits only added tests in internal/events / internal/handlers plus a Go template bump, so the touched-line set is unchanged.

No live control plane was started — every assertion runs against the in-process handler plus the test storage fixture.

Notes / follow-ups

Findings from review that were deliberately left for later rather than folded in here:

• The drop-vs-truncate distinction is not fully mutation-proof. Verified by mutation: replacing the continue with a rune-safe truncate still passes every current assertion, because a truncated 65-漢 prefix collides with the 64-rune entry already in the list and the de-dupe map swallows it. The dangerous variant the contract actually warns about — naive byte-slicing trimmed[:64] — is caught (it makes multibyte_rune_cap fail on invalid UTF-8), so the security-relevant property is genuinely pinned; only the benign distinction is unguarded. One extra table row with a non-colliding value would close it.
• The transient allocation is still reachable. The helper's own allocations are now bounded, but the golden route is registered bare in the /api/ui/v2 group and the server's maxRequestBodyHandler gates only /api/v1/execute* and /api/v1/nodes/register*. So ShouldBindJSON still decodes the whole body before sanitizeStringList is entered. This PR closes the stored amplification hole that Proposal: first-class run display names, labels, links, and external status #944 is about; bounding the request body (via http.MaxBytesReader, as execution_logs.go already does) belongs with the stacked run-metadata cluster, which touches this file and already does reject-with-400 validation. The helper's doc comment overstates this slightly and should be reworded there.
• truncateRunes now exists twice under internal/handlers/ with the same name and shape but different semantics — the pre-existing one in internal/handlers/agentic/reasoners.go appends an ellipsis and trims trailing whitespace. They are unexported siblings so they do not collide today, but whoever hoists one into a shared package should rename rather than merge them.
• Name truncation can leave trailing whitespace, since TrimSpace runs before the cut. Purely cosmetic, and it cannot produce an empty name, so the contract still holds.

This must land before the stacked run-metadata cluster, which edits the same file and is specified to reuse the capped helper.

🤖 Generated with Claude Code

[github-actions]

📊 Coverage gate

Thresholds from .coverage-gate.toml: per-surface ≥ 84%, aggregate ≥ 85%, max per-surface regression ≤ 1.0 pp, max aggregate regression ≤ 0.50 pp.

Surface	Current	Baseline	Δ
control-plane	87.60%	87.40%	↑ +0.20 pp	🟡
sdk-go	93.10%	92.00%	↑ +1.10 pp	🟢
sdk-python	94.31%	93.73%	↑ +0.58 pp	🟢
sdk-typescript	91.71%	90.42%	↑ +1.29 pp	🟢
web-ui	84.76%	84.79%	↓ -0.03 pp	🟡
aggregate	85.81%	85.75%	↑ +0.06 pp	🟡

✅ Gate passed

No surface regressed past the allowed threshold and the aggregate stayed above the floor.

[github-actions]

📐 Patch coverage gate

Threshold: 80% on lines this PR touches vs origin/main (from .coverage-gate.toml:thresholds.min_patch).

Surface	Touched lines	Patch coverage	Status
control-plane	25	100.00%	✅
sdk-go	0	—	➖ no changes
sdk-python	0	—	➖ no changes
sdk-typescript	0	—	➖ no changes
web-ui	0	—	➖ no changes

✅ Patch gate passed

Every surface whose lines were touched by this PR has patch coverage at or above the threshold.