From 04da64fe3200bd04203a5cd1057b42e554eae406 Mon Sep 17 00:00:00 2001 From: Eric Rakestraw Date: Mon, 14 Sep 2026 00:39:47 +0000 Subject: [PATCH] Validate explicit configuration branches before conversion --- docs/roadmap/remediation.md | 3 + internal/core/config/pipeline_document.go | 7 +- .../core/config/pipeline_document_test.go | 66 +++++++++++++++++++ 3 files changed, 74 insertions(+), 2 deletions(-) diff --git a/docs/roadmap/remediation.md b/docs/roadmap/remediation.md index 0b85f7ba..870371ad 100644 --- a/docs/roadmap/remediation.md +++ b/docs/roadmap/remediation.md @@ -63,6 +63,9 @@ Add focused load-boundary regression tests without duplicating upstream tests. ## Finding 2 — Preserve Explicit Presence In Document Validation +Status: Addressed. Plain loading rejects unsupported empty validator overrides +and invalid reference branches; supported empty overrides retain their meaning. + Priority: P2. Plain validation and runtime conversion disagree about invalid configuration. diff --git a/internal/core/config/pipeline_document.go b/internal/core/config/pipeline_document.go index bbfa1d5c..a4584453 100644 --- a/internal/core/config/pipeline_document.go +++ b/internal/core/config/pipeline_document.go @@ -406,7 +406,7 @@ func validateDocumentBinding(binding BindingDocument, path configkit.Path, slot if referencesAllowed { validateDocumentReferences(binding.References, path.Key("references"), true, validation) } - if len(binding.Validators) > 0 && !validatorsAllowed { + if binding.Validators != nil && !validatorsAllowed { validation.Error(path.Key("validators"), "are not supported for this binding") } for index, validator := range binding.Validators { @@ -429,13 +429,16 @@ func validateDocumentReferences(references map[string]ReferenceDocument, path co for _, name := range sortedReferenceDocumentKeys(references) { source := references[name] sourcePath := path.Key(name) - hasPath := source.Path != nil && strings.TrimSpace(*source.Path) != "" + hasPath := source.Path != nil hasArtifact := source.Artifact != nil if hasPath == hasArtifact { validation.Error(sourcePath, "must select exactly one reference source") continue } if hasPath { + if strings.TrimSpace(*source.Path) == "" { + validation.Error(sourcePath.Key("path"), "must not be empty") + } continue } if !generatedAllowed { diff --git a/internal/core/config/pipeline_document_test.go b/internal/core/config/pipeline_document_test.go index b69828fe..fbcab983 100644 --- a/internal/core/config/pipeline_document_test.go +++ b/internal/core/config/pipeline_document_test.go @@ -2,11 +2,77 @@ package config import ( "errors" + "fmt" "testing" configkit "gitea.maximumdirect.net/eric/ConfigKit" ) +func TestDocumentRejectsExplicitInvalidBranches(t *testing.T) { + t.Parallel() + for _, slot := range []string{"input", "output"} { + t.Run(slot+" empty validators", func(t *testing.T) { + body := "input: {module: input, validators: []}" + if slot == "output" { + body = "input: {module: input}\n output: {module: output, validators: []}" + } + _, err := loadConfigSource(configkit.Bytes("document.yml", []byte("version: 5\npipelines:\n session:\n "+body+"\n")), LoadOptions{}) + assertDocumentDiagnosticAt(t, err, "/pipelines/session/"+slot+"/validators") + }) + } + for _, value := range []string{ + "{path: ''}", "{path: ' '}", + "{path: '', artifact: {step: previous, lane: registry}}", + "{path: ' ', artifact: {step: previous, lane: registry}}", + "{path: null, artifact: {step: previous, lane: registry}}", + } { + t.Run(value, func(t *testing.T) { + content := fmt.Sprintf("version: 5\npipelines:\n session:\n input: {module: input}\n artifacts:\n lane:\n extract:\n module: extract\n references:\n party: %s\n", value) + _, err := loadConfigSource(configkit.Bytes("document.yml", []byte(content)), LoadOptions{}) + path := "/pipelines/session/artifacts/lane/extract/references/party" + if value == "{path: ''}" || value == "{path: ' '}" || value == "{path: null, artifact: {step: previous, lane: registry}}" { + path += "/path" + } + assertDocumentDiagnosticAt(t, err, path) + }) + } +} + +func TestDocumentPreservesSupportedEmptyValidatorsAndReferenceForms(t *testing.T) { + t.Parallel() + loaded, err := loadConfigSource(configkit.Bytes("document.yml", []byte(`version: 5 +pipelines: + session: + input: {module: input} + chunk: {module: chunk, validators: []} + artifacts: + lane: + validators: [] + extract: + module: extract + validators: [] + references: + external: {path: /data/party.txt} + generated: {artifact: {step: previous, lane: registry}} + merge: {module: merge, validators: []} + normalize: {module: normalize, validators: []} +`)), LoadOptions{}) + if err != nil { + t.Fatal(err) + } + profile := loaded.Config.Pipelines["session"] + lane := profile.Artifacts["lane"] + for _, override := range []bool{profile.Chunk.Validators.Set, lane.Extract.Validators.Set, lane.Merge.Validators.Set, lane.Normalize.Validators.Set} { + if !override { + t.Fatal("supported empty validator override lost explicitness") + } + } + if len(lane.Validators) != 0 || lane.Extract.References["external"].Path != "/data/party.txt" || + lane.Extract.References["generated"].Artifact.Lane != "registry" { + t.Fatalf("lane = %#v", lane) + } +} + func TestDocumentPipelineNormalizesCanonicalIdentifiers(t *testing.T) { t.Parallel()