From 80ec939383e16c653db2a4db3863b0349d50cc6e Mon Sep 17 00:00:00 2001 From: Eric Rakestraw Date: Sun, 26 Jul 2026 02:43:28 +0000 Subject: [PATCH] Validate normalize retry diagnostics --- docs/internal/pipeline.md | 8 ++ .../framework/contracts/typed_pipeline.go | 5 + .../framework/pipeline/normalize_retry.go | 34 ++++++ .../pipeline/runner_normalize_retry_test.go | 112 ++++++++++++++++-- internal/framework/pipeline/runner_typed.go | 5 +- 5 files changed, 154 insertions(+), 10 deletions(-) create mode 100644 internal/framework/pipeline/normalize_retry.go diff --git a/docs/internal/pipeline.md b/docs/internal/pipeline.md index 4f6f248..94b3382 100644 --- a/docs/internal/pipeline.md +++ b/docs/internal/pipeline.md @@ -331,6 +331,14 @@ normalize retry budget: `retries` permits that many additional attempts after the initial attempt. It neither creates a normalizer-local retry loop nor records an accepted checkpoint for the discarded attempt. +Before adding a normalize retry directive to attempt debug data, the runner +requires a nonblank, valid UTF-8 reason code of at most 128 bytes and a +nonblank, valid UTF-8 message of at most 4,096 bytes. These are encoded-byte +limits. The framework rejects an invalid directive without truncating or +rewriting either field. It validates only this mechanical contract; normalizers +remain responsible for ensuring their otherwise valid diagnostics do not expose +source material, credentials, paths, names, or other sensitive content. + When a later normalize attempt succeeds, its candidate alone proceeds through the usual validation and checkpoint path. When the final attempt still returns a directive, the runner validates its supplied safe fallback through that same diff --git a/internal/framework/contracts/typed_pipeline.go b/internal/framework/contracts/typed_pipeline.go index 7c049e1..9aa8b16 100644 --- a/internal/framework/contracts/typed_pipeline.go +++ b/internal/framework/contracts/typed_pipeline.go @@ -95,6 +95,11 @@ type TypedNormalizeResult[T any] struct { // NormalizeRetry asks the framework to retry normalization while retaining a // safe candidate for acceptance if the retry budget is exhausted. +const ( + MaxNormalizeRetryReasonCodeBytes = 128 + MaxNormalizeRetryMessageBytes = 4096 +) + type NormalizeRetry struct { ReasonCode string Message string diff --git a/internal/framework/pipeline/normalize_retry.go b/internal/framework/pipeline/normalize_retry.go new file mode 100644 index 0000000..47416fc --- /dev/null +++ b/internal/framework/pipeline/normalize_retry.go @@ -0,0 +1,34 @@ +package pipeline + +import ( + "errors" + "strings" + "unicode/utf8" + + "gitea.maximumdirect.net/eric/notarius/internal/framework/contracts" +) + +func validateNormalizeRetry(retry *contracts.NormalizeRetry) error { + if retry == nil { + return errors.New("normalize retry directive is missing") + } + if !utf8.ValidString(retry.ReasonCode) { + return errors.New("normalize retry directive reason code has invalid UTF-8") + } + if strings.TrimSpace(retry.ReasonCode) == "" { + return errors.New("normalize retry directive reason code is blank") + } + if len(retry.ReasonCode) > contracts.MaxNormalizeRetryReasonCodeBytes { + return errors.New("normalize retry directive reason code exceeds maximum length") + } + if !utf8.ValidString(retry.Message) { + return errors.New("normalize retry directive message has invalid UTF-8") + } + if strings.TrimSpace(retry.Message) == "" { + return errors.New("normalize retry directive message is blank") + } + if len(retry.Message) > contracts.MaxNormalizeRetryMessageBytes { + return errors.New("normalize retry directive message exceeds maximum length") + } + return nil +} diff --git a/internal/framework/pipeline/runner_normalize_retry_test.go b/internal/framework/pipeline/runner_normalize_retry_test.go index c1f4807..5bb7b09 100644 --- a/internal/framework/pipeline/runner_normalize_retry_test.go +++ b/internal/framework/pipeline/runner_normalize_retry_test.go @@ -150,14 +150,112 @@ func TestRunnerHandlesRetryableNormalizeFallbacks(t *testing.T) { } } -func TestRunnerRejectsBlankNormalizeRetryDiagnostic(t *testing.T) { - prepared := preparedAttemptDebugPipeline(t) - prepared.Steps[0].lanes[0].typed.normalize = func(context.Context, any, contracts.TypedNormalizeRequest[any]) (erasedTypedResult, error) { - return erasedTypedResult{Value: codecNotes{Items: []string{"safe"}}, Retry: &contracts.NormalizeRetry{ReasonCode: " ", Message: "missing reason"}}, nil +func TestRunnerValidatesNormalizeRetryDiagnostics(t *testing.T) { + const ( + reasonSentinel = "reason-diagnostic-sentinel" + messageSentinel = "message-diagnostic-sentinel" + ) + reasonOverLimit := strings.Repeat("r", contracts.MaxNormalizeRetryReasonCodeBytes-len(reasonSentinel)) + reasonSentinel + "x" + messageOverLimit := strings.Repeat("m", contracts.MaxNormalizeRetryMessageBytes-len(messageSentinel)) + messageSentinel + "x" + tests := []struct { + name string + retry contracts.NormalizeRetry + wantError string + hiddenValues []string + }{ + { + name: "accepts byte limits", + retry: contracts.NormalizeRetry{ + ReasonCode: strings.Repeat("r", contracts.MaxNormalizeRetryReasonCodeBytes), + Message: strings.Repeat("m", contracts.MaxNormalizeRetryMessageBytes), + }, + }, + { + name: "rejects oversized reason code", + retry: contracts.NormalizeRetry{ + ReasonCode: reasonOverLimit, + Message: messageSentinel, + }, + wantError: "reason code exceeds maximum length", + hiddenValues: []string{reasonSentinel, messageSentinel}, + }, + { + name: "rejects oversized message", + retry: contracts.NormalizeRetry{ + ReasonCode: reasonSentinel, + Message: messageOverLimit, + }, + wantError: "message exceeds maximum length", + hiddenValues: []string{reasonSentinel, messageSentinel}, + }, + { + name: "rejects invalid reason code UTF-8", + retry: contracts.NormalizeRetry{ + ReasonCode: reasonSentinel + string([]byte{0xff}), + Message: messageSentinel, + }, + wantError: "reason code has invalid UTF-8", + hiddenValues: []string{reasonSentinel, messageSentinel}, + }, + { + name: "rejects invalid message UTF-8", + retry: contracts.NormalizeRetry{ + ReasonCode: reasonSentinel, + Message: messageSentinel + string([]byte{0xff}), + }, + wantError: "message has invalid UTF-8", + hiddenValues: []string{reasonSentinel, messageSentinel}, + }, + { + name: "rejects blank reason code", + retry: contracts.NormalizeRetry{ + ReasonCode: " \t\n ", + Message: messageSentinel, + }, + wantError: "reason code is blank", + hiddenValues: []string{messageSentinel}, + }, + { + name: "rejects blank message", + retry: contracts.NormalizeRetry{ + ReasonCode: reasonSentinel, + Message: " \t\n ", + }, + wantError: "message is blank", + hiddenValues: []string{reasonSentinel}, + }, } - _, err := New().Run(context.Background(), RunInput{Prepared: prepared, RawInput: []byte("input"), Debug: newCapturedDebugRecorder()}) - if err == nil || !strings.Contains(err.Error(), "blank reason code or message") { - t.Fatalf("Run() error = %v, want retry diagnostic contract error", err) + + for _, tc := range tests { + t.Run(tc.name, func(t *testing.T) { + prepared := preparedAttemptDebugPipeline(t) + prepared.Steps[0].lanes[0].typed.normalize = func(context.Context, any, contracts.TypedNormalizeRequest[any]) (erasedTypedResult, error) { + return erasedTypedResult{Value: codecNotes{Items: []string{"safe"}}, Retry: &tc.retry}, nil + } + debug := newCapturedDebugRecorder() + output, err := New().Run(context.Background(), RunInput{Prepared: prepared, RawInput: []byte("input"), Debug: debug}) + if tc.wantError == "" { + if err != nil { + t.Fatalf("Run() error = %v, want accepted retry fallback", err) + } + if len(output.NormalizeOutputs) != 1 { + t.Fatalf("normalize outputs = %#v, want one accepted fallback", output.NormalizeOutputs) + } + return + } + if err == nil || !strings.Contains(err.Error(), tc.wantError) { + t.Fatalf("Run() error = %v, want fixed %q error", err, tc.wantError) + } + var debugOutput strings.Builder + for _, name := range debug.names() { + debugOutput.Write(debug.json[name]) + } + for _, value := range tc.hiddenValues { + if strings.Contains(err.Error(), value) || strings.Contains(debugOutput.String(), value) { + t.Fatalf("retry diagnostic leaked %q", value) + } + } + }) } } diff --git a/internal/framework/pipeline/runner_typed.go b/internal/framework/pipeline/runner_typed.go index ea0f559..e13397a 100644 --- a/internal/framework/pipeline/runner_typed.go +++ b/internal/framework/pipeline/runner_typed.go @@ -8,7 +8,6 @@ import ( "errors" "fmt" "path" - "strings" "time" "gitea.maximumdirect.net/eric/notarius/internal/core/source" @@ -365,8 +364,8 @@ func (r *Runner) runNormalizeStage(ctx context.Context, input RunInput, checkpoi } var retryPayload map[string]any if result.Retry != nil { - if strings.TrimSpace(result.Retry.ReasonCode) == "" || strings.TrimSpace(result.Retry.Message) == "" { - return false, nil, terminal.record(map[string]any{"output": debugCheckpointArtifact(serializedCandidate), "warnings": debugWarningEnvelopes(attemptWarnings)}, fmt.Errorf("normalize lane %q returned retry directive with blank reason code or message", lane.ID)) + if err := validateNormalizeRetry(result.Retry); err != nil { + return false, nil, terminal.record(map[string]any{"output": debugCheckpointArtifact(serializedCandidate), "warnings": debugWarningEnvelopes(attemptWarnings)}, fmt.Errorf("normalize lane %q returned invalid retry directive: %w", lane.ID, err)) } retryRemaining := attempt <= lane.Normalize.Retries retryPayload = map[string]any{