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

control-plane/internal/handlers/ui/coverage_execution_workflow_handlers_test.go

@@ -7,9 +7,11 @@ import (
 	"net/http"
 	"net/http/httptest"
 	"path/filepath"
+	"strconv"
 	"strings"
 	"testing"
 	"time"
+	"unicode/utf8"
 
 	"github.com/Agent-Field/agentfield/control-plane/internal/storage"
 	"github.com/Agent-Field/agentfield/control-plane/pkg/types"
@@ -504,6 +506,198 @@ func TestWorkflowRunHandlerSaveGoldenRunPreservesLineageMetadata(t *testing.T) {
 	require.Contains(t, metadata, "golden")
 }
 
+func TestSanitizeStringListBounds(t *testing.T) {
+	distinct := make([]string, 100)
+	for i := range distinct {
+		distinct[i] = "tag-" + strconv.Itoa(i)
+	}
+	longASCII := strings.Repeat("a", 65)
+	maxASCII := strings.Repeat("a", 64)
+	maxCJK := strings.Repeat("漢", 64)
+	longCJK := strings.Repeat("漢", 65)
+	veryLong := strings.Repeat("z", 1000)
+
+	tests := []struct {
+		name      string
+		values    []string
+		maxCount  int
+		maxRunes  int
+		want      []string
+		wantEmpty bool
+	}{
+		{name: "count cap preserves first entries", values: distinct, maxCount: 20, maxRunes: 64, want: distinct[:20]},
+		{name: "overlong ASCII dropped", values: []string{longASCII, maxASCII}, maxCount: 20, maxRunes: 64, want: []string{maxASCII}},
+		{name: "multibyte rune cap", values: []string{maxCJK, longCJK}, maxCount: 20, maxRunes: 64, want: []string{maxCJK}},
+		{name: "legacy sanitizing", values: []string{" smoke ", "smoke", "", "   ", "restart"}, maxCount: 20, maxRunes: 64, want: []string{"smoke", "restart"}},
+		{name: "nil input", values: nil, maxCount: 20, maxRunes: 64, wantEmpty: true},
+		{name: "zero count", values: []string{"tag"}, maxCount: 0, maxRunes: 64, wantEmpty: true},
+		{name: "zero runes disables length cap", values: []string{veryLong}, maxCount: 20, maxRunes: 0, want: []string{veryLong}},
+	}
+
+	for _, tc := range tests {
+		t.Run(tc.name, func(t *testing.T) {
+			got := sanitizeStringList(tc.values, tc.maxCount, tc.maxRunes)
+			if tc.wantEmpty {
+				require.Empty(t, got)
+				return
+			}
+			require.Equal(t, tc.want, got)
+		})
+	}
+
+	require.Equal(t, 64, utf8.RuneCountInString(maxCJK))
+	require.Equal(t, maxCJK, sanitizeStringList([]string{maxCJK}, 20, 64)[0])
+}
+
+func TestSanitizeStringListDoesNotPreallocateFromInputLength(t *testing.T) {
+	values := make([]string, 100000)
+	for i := range values {
+		values[i] = "tag-" + strconv.Itoa(i)
+	}
+
+	out := sanitizeStringList(values, 20, 64)
+	require.Len(t, out, 20)
+	require.LessOrEqual(t, cap(out), 20)
+}
+
+func TestTruncateRunes(t *testing.T) {
+	tests := []struct {
+		name     string
+		value    string
+		maxRunes int
+		want     string
+	}{
+		{name: "shorter than cap", value: "short", maxRunes: 10, want: "short"},
+		{name: "exactly at cap", value: "exact", maxRunes: 5, want: "exact"},
+		{name: "longer ASCII", value: "abcdef", maxRunes: 5, want: "abcde"},
+		{name: "multibyte boundary", value: "漢字仮名", maxRunes: 3, want: "漢字仮"},
+		{name: "zero cap", value: "value", maxRunes: 0, want: ""},
+		{name: "negative cap", value: "value", maxRunes: -1, want: ""},
+		{name: "empty input", value: "", maxRunes: 5, want: ""},
+	}
+
+	for _, tc := range tests {
+		t.Run(tc.name, func(t *testing.T) {
+			got := truncateRunes(tc.value, tc.maxRunes)
+			require.Equal(t, tc.want, got)
+			require.True(t, utf8.ValidString(got))
+			require.NotContains(t, got, "�")
+			if utf8.RuneCountInString(tc.value) > tc.maxRunes && tc.maxRunes > 0 {
+				require.Equal(t, tc.maxRunes, utf8.RuneCountInString(got))
+			}
+		})
+	}
+}
+
+func TestWorkflowRunHandlerSaveGoldenRunBoundsNameAndTags(t *testing.T) {
+	gin.SetMode(gin.TestMode)
+	ls, ctx := setupUIHandlerStorage(t)
+	runID := "run-golden-bounds"
+	executionID := "exec-golden-bounds"
+	now := time.Date(2026, 4, 10, 12, 0, 0, 0, time.UTC)
+	completed := now.Add(time.Second)
+	require.NoError(t, ls.CreateExecutionRecord(ctx, &types.Execution{
+		ExecutionID: executionID, RunID: runID, AgentNodeID: "agent-alpha", NodeID: "agent-alpha",
+		ReasonerID: "planner", Status: types.ExecutionStatusSucceeded, InputPayload: json.RawMessage(`{"input":{}}`),
+		StartedAt: now, CompletedAt: &completed, CreatedAt: now, UpdatedAt: completed,
+	}))
+
+	handler := NewWorkflowRunHandler(ls)
+	router := gin.New()
+	router.POST("/api/ui/v1/workflow-runs/:run_id/golden", handler.SaveGoldenRunHandler)
+	tags := make([]string, 0, 51)
+	for i := 0; i < 50; i++ {
+		tags = append(tags, "tag-"+strconv.Itoa(i))
+	}
+	overlongTag := strings.Repeat("漢", 65)
+	tags = append([]string{overlongTag}, tags...)
+	payload, err := json.Marshal(saveGoldenRunRequest{Name: strings.Repeat("a", 1<<20), Tags: tags})
+	require.NoError(t, err)
+
+	req := httptest.NewRequest(http.MethodPost, "/api/ui/v1/workflow-runs/"+runID+"/golden", strings.NewReader(string(payload)))
+	req.Header.Set("Content-Type", "application/json")
+	resp := httptest.NewRecorder()
+	router.ServeHTTP(resp, req)
+	require.Equal(t, http.StatusOK, resp.Code)
+	var summary WorkflowRunSummary
+	require.NoError(t, json.Unmarshal(resp.Body.Bytes(), &summary))
+	require.NotNil(t, summary.Golden)
+	require.Len(t, summary.Golden.Tags, 20)
+	require.NotContains(t, summary.Golden.Tags, overlongTag)
+
+	run, err := ls.GetWorkflowRun(ctx, runID)
+	require.NoError(t, err)
+	require.Less(t, len(run.Metadata), 4096)
+	metadata := decodeWorkflowRunMetadata(run.Metadata)
+	encodedGolden, err := json.Marshal(metadata["golden"])
+	require.NoError(t, err)
+	var golden GoldenRunMetadata
+	require.NoError(t, json.Unmarshal(encodedGolden, &golden))
+	require.Len(t, golden.Tags, 20)
+	require.NotEmpty(t, golden.Name)
+	require.Equal(t, maxGoldenNameRunes, utf8.RuneCountInString(golden.Name))
+
+	emptyNameReq := httptest.NewRequest(http.MethodPost, "/api/ui/v1/workflow-runs/"+runID+"/golden", strings.NewReader(`{"name":"   "}`))
+	emptyNameReq.Header.Set("Content-Type", "application/json")
+	emptyNameResp := httptest.NewRecorder()
+	router.ServeHTTP(emptyNameResp, emptyNameReq)
+	require.Equal(t, http.StatusOK, emptyNameResp.Code)
+	parsed := handler.loadRunMetadata(ctx, runID)
+	require.NotNil(t, parsed)
+	require.NotNil(t, parsed.Golden)
+	require.Equal(t, runID, parsed.Golden.Name)
+}
+
+func TestWorkflowRunHandlerGoldenReadBackIsNotRevalidated(t *testing.T) {
+	gin.SetMode(gin.TestMode)
+	ls, ctx := setupUIHandlerStorage(t)
+	runID := "run-golden-existing-oversized"
+	executionID := "exec-golden-existing-oversized"
+	now := time.Date(2026, 4, 11, 12, 0, 0, 0, time.UTC)
+	completed := now.Add(time.Second)
+	tags := make([]string, 50)
+	for i := range tags {
+		tags[i] = "legacy-tag-" + strconv.Itoa(i)
+	}
+	name := strings.Repeat("漢", 500)
+	metadata, err := json.Marshal(map[string]interface{}{"golden": GoldenRunMetadata{Name: name, Tags: tags}})
+	require.NoError(t, err)
+	require.NoError(t, ls.StoreWorkflowRun(ctx, &types.WorkflowRun{
+		RunID: runID, RootWorkflowID: runID, RootExecutionID: &executionID,
+		Status: string(types.ExecutionStatusSucceeded), TotalSteps: 1, CompletedSteps: 1,
+		Metadata: metadata, CreatedAt: now, UpdatedAt: completed,
+	}))
+	require.NoError(t, ls.CreateExecutionRecord(ctx, &types.Execution{
+		ExecutionID: executionID, RunID: runID, AgentNodeID: "agent-alpha", NodeID: "agent-alpha",
+		ReasonerID: "planner", Status: types.ExecutionStatusSucceeded, InputPayload: json.RawMessage(`{"input":{}}`),
+		StartedAt: now, CompletedAt: &completed, CreatedAt: now, UpdatedAt: completed,
+	}))
+
+	handler := NewWorkflowRunHandler(ls)
+	router := gin.New()
+	router.GET("/api/ui/v2/workflow-runs", handler.ListWorkflowRunsHandler)
+	router.GET("/api/ui/v2/workflow-runs/:run_id", handler.GetWorkflowRunDetailHandler)
+
+	listResp := httptest.NewRecorder()
+	router.ServeHTTP(listResp, httptest.NewRequest(http.MethodGet, "/api/ui/v2/workflow-runs?page_size=200", nil))
+	require.Equal(t, http.StatusOK, listResp.Code)
+	var list WorkflowRunListResponse
+	require.NoError(t, json.Unmarshal(listResp.Body.Bytes(), &list))
+	require.Len(t, list.Runs, 1)
+	require.NotNil(t, list.Runs[0].Golden)
+	require.Equal(t, name, list.Runs[0].Golden.Name)
+	require.Len(t, list.Runs[0].Golden.Tags, 50)
+
+	detailResp := httptest.NewRecorder()
+	router.ServeHTTP(detailResp, httptest.NewRequest(http.MethodGet, "/api/ui/v2/workflow-runs/"+runID, nil))
+	require.Equal(t, http.StatusOK, detailResp.Code)
+	var detail WorkflowRunDetailResponse
+	require.NoError(t, json.Unmarshal(detailResp.Body.Bytes(), &detail))
+	require.NotNil(t, detail.Run.Golden)
+	require.Equal(t, name, detail.Run.Golden.Name)
+	require.Len(t, detail.Run.Golden.Tags, 50)
+}
+
 func TestWorkflowRunHandlerSaveGoldenRunErrors(t *testing.T) {
 	gin.SetMode(gin.TestMode)
 

control-plane/internal/handlers/ui/workflow_runs.go

@@ -9,6 +9,7 @@ import (
 	"strconv"
 	"strings"
 	"time"
+	"unicode/utf8"
 
 	"github.com/Agent-Field/agentfield/control-plane/internal/handlers"
 	"github.com/Agent-Field/agentfield/control-plane/internal/logger"
@@ -80,6 +81,19 @@ type GoldenRunMetadata struct {
 	SavedAt string   `json:"saved_at,omitempty"`
 }
 
+// Bounds on the golden-run metadata a client may write into
+// workflow_runs.metadata. This row is re-read and re-serialised on every
+// runs-list page that contains the run, so an unbounded name or tag list is
+// a stored amplification vector (issue #944). The UI's save-as-golden button
+// sends one hard-coded tag, so these caps are unreachable in practice today.
+// They are package-level constants rather than inline literals so the
+// run-metadata endpoint can reuse the same bounds.
+const (
+	maxGoldenTags      = 20
+	maxGoldenTagRunes  = 64
+	maxGoldenNameRunes = 200
+)
+
 type WorkflowRunListResponse struct {
 	Runs       []WorkflowRunSummary `json:"runs"`
 	TotalCount int                  `json:"total_count"`
@@ -261,8 +275,9 @@ func (h *WorkflowRunHandler) SaveGoldenRunHandler(c *gin.Context) {
 	}
 
 	now := time.Now().UTC()
-	name := strings.TrimSpace(req.Name)
+	name := truncateRunes(strings.TrimSpace(req.Name), maxGoldenNameRunes)
 	if name == "" {
+		// Do not truncate the run ID fallback; only the caller-supplied name is bounded.
 		name = runID
 	}
 	metadata := map[string]interface{}{}
@@ -271,7 +286,7 @@ func (h *WorkflowRunHandler) SaveGoldenRunHandler(c *gin.Context) {
 	}
 	metadata["golden"] = GoldenRunMetadata{
 		Name:    name,
-		Tags:    sanitizeStringList(req.Tags),
+		Tags:    sanitizeStringList(req.Tags, maxGoldenTags, maxGoldenTagRunes),
 		SavedBy: "user",
 		SavedAt: now.Format(time.RFC3339),
 	}
@@ -645,14 +660,37 @@ func decodeWorkflowRunMetadata(raw json.RawMessage) map[string]interface{} {
 	return metadata
 }
 
-func sanitizeStringList(values []string) []string {
-	out := make([]string, 0, len(values))
-	seen := map[string]struct{}{}
+// sanitizeStringList trims, de-duplicates and bounds a caller-supplied list of
+// short strings, preserving input order.
+//
+// Entries longer than maxRunes runes are DROPPED rather than truncated: cutting
+// a string at a byte offset can land mid-rune, and json.Marshal silently
+// rewrites the resulting invalid UTF-8 to U+FFFD. Rune counting (not len)
+// keeps a maxRunes-rune multi-byte tag legal.
+//
+// At most maxCount entries are returned. Both the output slice and the de-dupe
+// map are sized from min(len(values), maxCount) and the loop stops once
+// maxCount survivors are collected, so a huge attacker-supplied array cannot
+// force a large allocation before the cap applies. maxRunes <= 0 disables the
+// length cap; maxCount <= 0 yields an empty result.
+func sanitizeStringList(values []string, maxCount, maxRunes int) []string {
+	if maxCount <= 0 {
+		return nil
+	}
+	size := min(len(values), maxCount)
+	out := make([]string, 0, size)
+	seen := make(map[string]struct{}, size)
 	for _, value := range values {
+		if len(out) >= maxCount {
+			break
+		}
 		trimmed := strings.TrimSpace(value)
 		if trimmed == "" {
 			continue
 		}
+		if maxRunes > 0 && utf8.RuneCountInString(trimmed) > maxRunes {
+			continue
+		}
 		if _, ok := seen[trimmed]; ok {
 			continue
 		}
@@ -662,6 +700,24 @@ func sanitizeStringList(values []string) []string {
 	return out
 }
 
+// truncateRunes returns value limited to at most maxRunes runes, cutting on a
+// rune boundary so the result is always valid UTF-8. It walks the string by
+// rune start offsets instead of materialising a []rune, so a multi-megabyte
+// input costs no extra allocation.
+func truncateRunes(value string, maxRunes int) string {
+	if maxRunes <= 0 {
+		return ""
+	}
+	count := 0
+	for i := range value {
+		if count == maxRunes {
+			return value[:i]
+		}
+		count++
+	}
+	return value
+}
+
 func deriveOverallStatusForUI(executions []*types.Execution) string {
 	counts := make(map[string]int)
 	active := 0