From 68cd345cdf9d5e8dc5dd4f8f1536a1a66b1aeac8 Mon Sep 17 00:00:00 2001 From: Joe Beda Date: Wed, 23 Sep 2026 14:38:39 -0700 Subject: [PATCH 1/2] feat(lint): verify reachable vendored imports Signed-off-by: Joe Beda --- cmd/modelith/main.go | 24 ++--- cmd/modelith/main_test.go | 65 ++++++++++++ docs/10-vendoring.md | 6 ++ internal/lint/imports.go | 95 ++++++++++++----- internal/lint/imports_test.go | 19 ++++ internal/lint/plan.go | 97 +++++++++++++++++ internal/lint/plan_test.go | 191 ++++++++++++++++++++++++++++++++++ 7 files changed, 460 insertions(+), 37 deletions(-) create mode 100644 internal/lint/plan.go create mode 100644 internal/lint/plan_test.go diff --git a/cmd/modelith/main.go b/cmd/modelith/main.go index eda126d..43ee877 100644 --- a/cmd/modelith/main.go +++ b/cmd/modelith/main.go @@ -489,24 +489,22 @@ func lintCmd() *cobra.Command { } completenessAsError := completeness == "error" - type fileResult struct { - File string `json:"file"` - Findings []lint.Finding `json:"findings"` - } - var all []fileResult - blocking := false - + inputs := make([]lint.Input, 0, len(args)) for _, path := range args { data, err := os.ReadFile(path) if err != nil { return fmt.Errorf("%s: %w", path, err) } - res, err := lint.Run(path, data, lint.OSFiles{}) - if err != nil { - return fmt.Errorf("%s: %w", path, err) - } - all = append(all, fileResult{File: path, Findings: res.Findings}) - if res.HasBlocking(completenessAsError) { + inputs = append(inputs, lint.Input{Path: path, Source: data}) + } + all, err := lint.Plan(inputs, lint.OSFiles{}) + if err != nil { + return err + } + + blocking := false + for _, fr := range all { + if (&lint.Result{Findings: fr.Findings}).HasBlocking(completenessAsError) { blocking = true } } diff --git a/cmd/modelith/main_test.go b/cmd/modelith/main_test.go index 48a60ac..67ff3ba 100644 --- a/cmd/modelith/main_test.go +++ b/cmd/modelith/main_test.go @@ -10,6 +10,7 @@ import ( "testing" "github.com/stacklok/modelith/internal/deps" + "github.com/stacklok/modelith/internal/lint" "github.com/stacklok/modelith/internal/provenance" ) @@ -125,6 +126,70 @@ func TestLintMissingFileErrors(t *testing.T) { } } +func TestLintReportsDiscoveredVendoredChildUnderChildPath(t *testing.T) { + dir := t.TempDir() + childPath := filepath.Join(dir, "child.modelith.yaml") + child := strings.Replace(minimalValid, "A thing that exists in the model.", "A changed thing.", 1) + writeTemp(t, dir, "child.modelith.yaml", vendorHeader(minimalValid)+child) + rootPath := writeTemp(t, dir, "root.modelith.yaml", `kind: DomainModel +version: v1 +imports: + - ./child.modelith.yaml +entities: + Root: + definition: The root model. +`) + + out, err := run(t, "lint", rootPath) + if !errors.Is(err, errBlocking) { + t.Fatalf("expected errBlocking, got %v\noutput:\n%s", err, out) + } + if !strings.Contains(out, childPath+":\n error [semantic] (root): this vendored file no longer matches the digest") { + t.Fatalf("discovered provenance finding was not grouped under %s:\n%s", childPath, out) + } +} + +func TestLintDoesNotDuplicateExplicitDiscoveredChild(t *testing.T) { + dir := t.TempDir() + childPath := filepath.Join(dir, "child.modelith.yaml") + child := strings.Replace(minimalValid, "A thing that exists in the model.", "A changed thing.", 1) + writeTemp(t, dir, "child.modelith.yaml", vendorHeader(minimalValid)+child) + rootPath := writeTemp(t, dir, "root.modelith.yaml", `kind: DomainModel +version: v1 +imports: + - ./child.modelith.yaml +entities: + Root: + definition: The root model. +`) + + out, err := run(t, "lint", "--format", "json", rootPath, childPath) + if !errors.Is(err, errBlocking) { + t.Fatalf("expected errBlocking, got %v\noutput:\n%s", err, out) + } + var payload struct { + Files []struct { + File string `json:"file"` + Findings []lint.Finding `json:"findings"` + } `json:"files"` + } + if err := json.Unmarshal([]byte(out), &payload); err != nil { + t.Fatalf("invalid JSON: %v\noutput:\n%s", err, out) + } + var children []struct { + File string `json:"file"` + Findings []lint.Finding `json:"findings"` + } + for _, file := range payload.Files { + if file.File == childPath { + children = append(children, file) + } + } + if len(children) != 1 || len(children[0].Findings) != 1 { + t.Fatalf("child results = %+v, want one provenance finding", children) + } +} + func TestRenderWritesFileBesideSource(t *testing.T) { dir := t.TempDir() yamlPath := writeTemp(t, dir, "m.modelith.yaml", minimalValid) diff --git a/docs/10-vendoring.md b/docs/10-vendoring.md index a1afbac..250b153 100644 --- a/docs/10-vendoring.md +++ b/docs/10-vendoring.md @@ -155,6 +155,12 @@ This is drift detection, not a security boundary: anyone editing the file can recompute the header. It catches the well-meaning typo fix, which is the thing that actually happens. +When lint starts from an importing model, it follows locally readable imports +and verifies every vendored copy it reaches. A mismatch is reported against the +copy that needs repair, not its importer. This stays offline and does not add +new diagnostics for a nested import that cannot be read; lint does not become a +recursive semantic validator. + ## Keeping the copy current The section above is about your copy. This one is about the model it came from, diff --git a/internal/lint/imports.go b/internal/lint/imports.go index 247aac0..346422c 100644 --- a/internal/lint/imports.go +++ b/internal/lint/imports.go @@ -56,7 +56,34 @@ var ( type importedModel struct { index int // position in the importing model's imports list, for the finding path path string // the path as written in imports - model *model.Model + loadedImport +} + +// loadedImport is a contained import that was read successfully. model is set only +// when the contents parsed as a supported domain model. +type loadedImport struct { + resolvedPath string + source []byte + model *model.Model +} + +type importLoadFailureKind uint8 + +const ( + importOutsideRepository importLoadFailureKind = iota + 1 + importOutsideModelDirectory + importUnreadable + importNotDomainModel + importUnsupportedSchema +) + +// importLoadFailure retains the data loadImports needs to preserve its +// import-specific diagnostics. +type importLoadFailure struct { + kind importLoadFailureKind + resolvedPath string + err error + version string } // runImports resolves the model's imports, checks every qualified attribute @@ -113,7 +140,6 @@ func loadImports(modelPath string, m *model.Model, files Files, res *Result, ven if len(m.Imports) == 0 { return byScope, claimed } - dir := filepath.Dir(modelPath) root, inRepo := files.ResolutionRoot(modelPath) for i, imp := range m.Imports { reject := func(format string, args ...any) { @@ -173,37 +199,58 @@ func loadImports(modelPath string, m *model.Model, files Files, res *Result, ven // holds no model are four distinct diagnostics, and together they let a // model from an untrusted source probe the filesystem of whatever runner // lints it (ADR-0013). - joined := filepath.Join(dir, imp.Path) - if resolved := files.Resolve(joined); !withinRoot(root, resolved) { - if inRepo { + loaded, failure := loadImport(modelPath, root, inRepo, imp.Path, files) + if failure != nil { + switch failure.kind { + case importOutsideRepository: reject("import %q resolves to %q, outside %q — that directory is the repository holding this model (the nearest ancestor with a .git entry), and an import may not name a file beyond it", - imp.Path, resolved, root) - } else { + imp.Path, failure.resolvedPath, root) + case importOutsideModelDirectory: reject("import %q resolves to %q, outside %q — this model is in no repository, so resolution is confined to the directory holding it; move the imported model into that directory or below it", - imp.Path, resolved, root) + imp.Path, failure.resolvedPath, root) + case importUnreadable: + reject("import %q cannot be read: %v", imp.Path, failure.err) + case importNotDomainModel: + reject("import %q is not a domain model — lint it on its own with `modelith lint` to see why", imp.Path) + case importUnsupportedSchema: + reject("import %q declares schema version %q, which this modelith does not support: %s (upgrade modelith, or move that model to a supported version)", + imp.Path, failure.version, strings.Join(schema.SupportedVersions(), ", ")) } continue } - data, err := files.ReadFile(joined) - if err != nil { - reject("import %q cannot be read: %v", imp.Path, err) - continue - } - im, err := model.Parse(data) - if err != nil || im.Kind != "DomainModel" { - reject("import %q is not a domain model — lint it on its own with `modelith lint` to see why", imp.Path) - continue - } - if !schema.Supported(im.Version) { - reject("import %q declares schema version %q, which this modelith does not support: %s (upgrade modelith, or move that model to a supported version)", - imp.Path, im.Version, strings.Join(schema.SupportedVersions(), ", ")) - continue - } - byScope[imp.Scope] = importedModel{index: i, path: imp.Path, model: im} + byScope[imp.Scope] = importedModel{index: i, path: imp.Path, loadedImport: loaded} } return byScope, claimed } +// loadImport reads and validates an imported model after its path syntax and +// scope have been checked by loadImports. +func loadImport(modelPath, root string, inRepo bool, importPath string, files Files) (loadedImport, *importLoadFailure) { + joined := filepath.Join(filepath.Dir(modelPath), importPath) + resolvedPath := files.Resolve(joined) + if !withinRoot(root, resolvedPath) { + kind := importOutsideModelDirectory + if inRepo { + kind = importOutsideRepository + } + return loadedImport{}, &importLoadFailure{kind: kind, resolvedPath: resolvedPath} + } + data, err := files.ReadFile(joined) + if err != nil { + return loadedImport{}, &importLoadFailure{kind: importUnreadable, err: err} + } + loaded := loadedImport{resolvedPath: resolvedPath, source: data} + imported, err := model.Parse(data) + if err != nil || imported.Kind != "DomainModel" { + return loaded, &importLoadFailure{kind: importNotDomainModel} + } + if !schema.Supported(imported.Version) { + return loaded, &importLoadFailure{kind: importUnsupportedSchema, version: imported.Version} + } + loaded.model = imported + return loaded, nil +} + // checkQualifiedTypes resolves every qualified attribute type against the // imports and returns the set of scopes that were referenced. // diff --git a/internal/lint/imports_test.go b/internal/lint/imports_test.go index 8bc913e..92aaeb7 100644 --- a/internal/lint/imports_test.go +++ b/internal/lint/imports_test.go @@ -136,6 +136,25 @@ func assertFindings(t *testing.T, got []Finding, want []wantFinding) { } } +func TestLoadImport_SuccessRetainsLoadedData(t *testing.T) { + t.Parallel() + + files := fakeFiles{"docs/payments.modelith.yaml": paymentsModel} + loaded, failure := loadImport(importerPath, "docs", false, "./payments.modelith.yaml", files) + if failure != nil { + t.Fatalf("loadImport failed: %+v", failure) + } + if loaded.resolvedPath != "docs/payments.modelith.yaml" { + t.Errorf("resolved path = %q, want %q", loaded.resolvedPath, "docs/payments.modelith.yaml") + } + if string(loaded.source) != paymentsModel { + t.Errorf("source = %q, want %q", loaded.source, paymentsModel) + } + if loaded.model == nil || loaded.model.Kind != "DomainModel" || loaded.model.Version != "v1" { + t.Errorf("model = %+v, want parsed v1 domain model", loaded.model) + } +} + func TestImports_Resolution(t *testing.T) { t.Parallel() diff --git a/internal/lint/plan.go b/internal/lint/plan.go new file mode 100644 index 0000000..41aaa8f --- /dev/null +++ b/internal/lint/plan.go @@ -0,0 +1,97 @@ +package lint + +import ( + "path/filepath" + "strings" + "unicode" + + "github.com/stacklok/modelith/internal/model" + "github.com/stacklok/modelith/internal/schema" +) + +// Input is one explicitly requested model and the bytes read for it. +type Input struct { + Path string + Source []byte +} + +// FileResult is the output for one model. Explicit inputs retain the path the +// caller supplied; discovered imports use their resolved paths. +type FileResult struct { + File string `json:"file"` + Findings []Finding `json:"findings"` +} + +// Plan runs full lint for explicit inputs and provenance verification for their +// locally reachable imported models. +func Plan(inputs []Input, files Files) ([]FileResult, error) { + if files == nil { + files = OSFiles{} + } + + results := make([]FileResult, 0, len(inputs)) + visited := make(map[string]bool, len(inputs)) + queue := make([]loadedImport, 0, len(inputs)) + + for _, input := range inputs { + path := files.Resolve(input.Path) + if visited[path] { + continue + } + visited[path] = true + + result, err := Run(input.Path, input.Source, files) + if err != nil { + return nil, err + } + results = append(results, FileResult{File: input.Path, Findings: result.Findings}) + + if imported := traversableModel(input.Source); imported != nil { + queue = append(queue, loadedImport{resolvedPath: path, source: input.Source, model: imported}) + } + } + + for len(queue) > 0 { + current := queue[0] + queue = queue[1:] + root, inRepo := files.ResolutionRoot(current.resolvedPath) + for _, imp := range current.model.Imports { + if !traversableImport(imp.Path) { + continue + } + loaded, failure := loadImport(current.resolvedPath, root, inRepo, imp.Path, files) + if visited[loaded.resolvedPath] || (failure != nil && loaded.source == nil) { + continue + } + visited[loaded.resolvedPath] = true + + res := &Result{} + runProvenance(loaded.resolvedPath, loaded.source, res) + sortFindings(res) + if len(res.Findings) > 0 { + results = append(results, FileResult{File: loaded.resolvedPath, Findings: res.Findings}) + } + if failure == nil { + queue = append(queue, loaded) + } + } + } + + return results, nil +} + +// traversableModel accepts parsed domain models that this build supports. +func traversableModel(source []byte) *model.Model { + m, err := model.Parse(source) + if err != nil || m.Kind != "DomainModel" || !schema.Supported(m.Version) { + return nil + } + return m +} + +func traversableImport(path string) bool { + return path != "" && + !filepath.IsAbs(path) && + !strings.HasPrefix(path, "/") && + strings.IndexFunc(path, unicode.IsControl) < 0 +} diff --git a/internal/lint/plan_test.go b/internal/lint/plan_test.go new file mode 100644 index 0000000..99170a2 --- /dev/null +++ b/internal/lint/plan_test.go @@ -0,0 +1,191 @@ +package lint + +import ( + "strings" + "testing" +) + +func planned(t *testing.T, inputs []Input, files fakeFiles) []FileResult { + t.Helper() + results, err := Plan(inputs, files) + if err != nil { + t.Fatal(err) + } + return results +} + +func editedVendored(t *testing.T) string { + t.Helper() + copy := stamp(t, gappy) + edited := strings.Replace(copy, "One car's stay in the garage.", "One car's stay.", 1) + if edited == copy { + t.Fatal("fixture did not change the vendored copy") + } + return edited +} + +func TestPlan_DirectVendoredChild(t *testing.T) { + t.Parallel() + + const root = "models/root.modelith.yaml" + const child = "models/child.modelith.yaml" + childSource := editedVendored(t) + files := fakeFiles{".git": "", root: importer([]string{`"./child.modelith.yaml"`}, "child.PaymentMethod"), child: childSource} + + results := planned(t, []Input{{Path: root, Source: []byte(files[root])}}, files) + if len(results) != 2 || results[1].File != child { + t.Fatalf("results = %+v, want root followed by %q", results, child) + } + if len(results[1].Findings) != 1 || results[1].Findings[0].Path != "" || !strings.Contains(results[1].Findings[0].Message, "deps update "+child) { + t.Errorf("child provenance result = %+v", results[1]) + } +} + +func TestPlan_OwnedIntermediaryReachesVendoredGrandchild(t *testing.T) { + t.Parallel() + + const root = "models/root.modelith.yaml" + const middle = "models/middle.modelith.yaml" + const child = "models/child.modelith.yaml" + childSource := editedVendored(t) + files := fakeFiles{ + ".git": "", + root: importer([]string{`{scope: middle, path: "./middle.modelith.yaml"}`}, "middle.PaymentMethod"), + middle: importer([]string{`{scope: child, path: "./child.modelith.yaml"}`}, "child.PaymentMethod"), + child: childSource, + } + + results := planned(t, []Input{{Path: root, Source: []byte(files[root])}}, files) + if len(results) != 2 || results[1].File != child || len(results[1].Findings) != 1 { + t.Fatalf("results = %+v, want root plus grandchild provenance finding", results) + } + if !strings.Contains(results[1].Findings[0].Message, "deps update "+child) { + t.Errorf("grandchild = %+v, want remedy for %q", results[1], child) + } +} + +func TestPlan_SharedChildIsVerifiedOnce(t *testing.T) { + t.Parallel() + + const root = "models/root.modelith.yaml" + const child = "models/child.modelith.yaml" + files := fakeFiles{ + ".git": "", + root: importer([]string{ + `{scope: first, path: "./child.modelith.yaml"}`, + `{scope: second, path: "./child.modelith.yaml"}`, + }, "first.PaymentMethod"), + child: editedVendored(t), + } + + results := planned(t, []Input{{Path: root, Source: []byte(files[root])}}, files) + if len(results) != 2 || results[1].File != child || len(results[1].Findings) != 1 { + t.Errorf("results = %+v, want one provenance result for shared child", results) + } +} + +func TestPlan_CycleTerminates(t *testing.T) { + t.Parallel() + + const first = "models/first.modelith.yaml" + const second = "models/second.modelith.yaml" + secondSource := stamp(t, importer([]string{`{scope: first, path: "./first.modelith.yaml"}`}, "first.PaymentMethod")) + secondSource = strings.Replace(secondSource, "PaymentMethod", "EditedMethod", 1) + files := fakeFiles{ + ".git": "", + first: importer([]string{`{scope: second, path: "./second.modelith.yaml"}`}, "second.PaymentMethod"), + second: secondSource, + } + + results := planned(t, []Input{{Path: first, Source: []byte(files[first])}}, files) + if len(results) != 2 || results[0].File != first || results[1].File != second || len(results[1].Findings) != 1 { + t.Errorf("results = %+v, want one root and one visited cycle member", results) + } +} + +func TestPlan_BrokenNestedEdgeIsSilent(t *testing.T) { + t.Parallel() + + const root = "models/root.modelith.yaml" + const middle = "models/middle.modelith.yaml" + files := fakeFiles{ + ".git": "", + root: importer([]string{`{scope: middle, path: "./middle.modelith.yaml"}`}, "middle.PaymentMethod"), + middle: importer([]string{`{scope: missing, path: "./missing.modelith.yaml"}`}, "missing.PaymentMethod"), + } + + results := planned(t, []Input{{Path: root, Source: []byte(files[root])}}, files) + if len(results) != 1 || results[0].File != root { + t.Errorf("results = %+v, want only root and no nested missing-import finding", results) + } +} + +func TestPlan_ExplicitRootDiscoveredThroughImportRunsFullLintOnce(t *testing.T) { + t.Parallel() + + const root = "models/root.modelith.yaml" + const child = "models/child.modelith.yaml" + files := fakeFiles{ + ".git": "", + root: importer([]string{`{scope: child, path: "./child.modelith.yaml"}`}, "child.PaymentMethod"), + child: gappy, + } + + results := planned(t, []Input{{Path: root, Source: []byte(files[root])}, {Path: child, Source: []byte(files[child])}}, files) + if len(results) != 2 || results[1].File != child { + t.Fatalf("results = %+v, want explicit child once", results) + } + var completeness bool + for _, finding := range results[1].Findings { + if finding.Category == CategoryCompleteness { + completeness = true + } + } + if !completeness { + t.Errorf("explicit child did not receive full lint: %+v", results[1]) + } +} + +func TestPlan_ReportsReadableUnsupportedVendoredChild(t *testing.T) { + t.Parallel() + + const root = "models/root.modelith.yaml" + const child = "models/child.modelith.yaml" + childSource := strings.Replace(stamp(t, gappy), "version: v1", "version: v99", 1) + files := fakeFiles{".git": "", root: importer([]string{`"./child.modelith.yaml"`}, "child.PaymentMethod"), child: childSource} + + results := planned(t, []Input{{Path: root, Source: []byte(files[root])}}, files) + if len(results) != 2 || results[1].File != child || len(results[1].Findings) != 1 { + t.Errorf("results = %+v, want a provenance finding for the unsupported child", results) + } +} + +func TestPlan_PreservesExplicitInputPath(t *testing.T) { + t.Parallel() + + const path = "./models/root.modelith.yaml" + results := planned(t, []Input{{Path: path, Source: []byte(gappy)}}, fakeFiles{".git": ""}) + if len(results) != 1 || results[0].File != path { + t.Errorf("results = %+v, want explicit path %q", results, path) + } +} + +func TestPlan_UsesResolvedPathForDiscoveredFinding(t *testing.T) { + t.Parallel() + + const root = "models/root.modelith.yaml" + const child = "models/vendor/child.modelith.yaml" + files := fakeFiles{ + ".git": "", + root: importer([]string{`{scope: child, path: "./vendor/../vendor/child.modelith.yaml"}`}, "child.PaymentMethod"), + child: editedVendored(t), + } + + results := planned(t, []Input{{Path: root, Source: []byte(files[root])}}, files) + if len(results) != 2 || results[1].File != child { + t.Fatalf("results = %+v, want child attributed to resolved path %q", results, child) + } + if got := results[1].Findings; len(got) != 1 || !strings.Contains(got[0].Message, "deps update "+child) { + t.Errorf("finding = %+v, want remedy for resolved path %q", got, child) + } +} From 569058dba032b0d8b86849f2d9f2039869ee4adb Mon Sep 17 00:00:00 2001 From: Joe Beda Date: Wed, 23 Sep 2026 14:50:58 -0700 Subject: [PATCH 2/2] fix(lint): preserve recursive provenance semantics Signed-off-by: Joe Beda --- docs/10-vendoring.md | 8 +- internal/lint/plan.go | 41 ++++++---- internal/lint/plan_test.go | 79 +++++++++++++++++++ .../0017-recursive-provenance-verification.md | 20 +++++ 4 files changed, 131 insertions(+), 17 deletions(-) create mode 100644 project-docs/adr/0017-recursive-provenance-verification.md diff --git a/docs/10-vendoring.md b/docs/10-vendoring.md index 250b153..e61755c 100644 --- a/docs/10-vendoring.md +++ b/docs/10-vendoring.md @@ -85,9 +85,11 @@ Two things change, and nothing else: its own authors control. Without this, the [GitHub Action](./08-github-action.md) — which lints every matched file — would fail your build over someone else's model. -- **Its own `imports:` raise nothing.** A vendored model's imports name paths - in *its* repository, which do not exist in yours. Those are skipped, along - with the references that resolve through them. +- **Its own `imports:` do not receive semantic diagnostics.** A vendored + model's imports commonly name paths in *its* repository, which do not exist in + yours. Missing or broken nested edges stay silent, along with references that + resolve through them; readable local edges still participate in provenance + verification. **Structural and semantic checks still run.** A vendored file that is not a valid domain model breaks your build, and that is your problem to solve — by diff --git a/internal/lint/plan.go b/internal/lint/plan.go index 41aaa8f..dfcded3 100644 --- a/internal/lint/plan.go +++ b/internal/lint/plan.go @@ -22,32 +22,37 @@ type FileResult struct { Findings []Finding `json:"findings"` } -// Plan runs full lint for explicit inputs and provenance verification for their -// locally reachable imported models. +// Plan runs full lint for every explicit input and provenance verification for +// their locally reachable imported models. Explicit inputs retain both their +// argument order and multiplicity; resolved paths deduplicate only discovery. func Plan(inputs []Input, files Files) ([]FileResult, error) { if files == nil { files = OSFiles{} } results := make([]FileResult, 0, len(inputs)) - visited := make(map[string]bool, len(inputs)) + explicit := make(map[string]bool, len(inputs)) + for _, input := range inputs { + explicit[files.Resolve(input.Path)] = true + } + discovered := make(map[string]bool) queue := make([]loadedImport, 0, len(inputs)) for _, input := range inputs { - path := files.Resolve(input.Path) - if visited[path] { - continue - } - visited[path] = true - result, err := Run(input.Path, input.Source, files) if err != nil { return nil, err } results = append(results, FileResult{File: input.Path, Findings: result.Findings}) + // A structurally invalid root has already received its full result above, + // but its imports cannot safely seed the integrity-only crawl. if imported := traversableModel(input.Source); imported != nil { - queue = append(queue, loadedImport{resolvedPath: path, source: input.Source, model: imported}) + queue = append(queue, loadedImport{ + resolvedPath: files.Resolve(input.Path), + source: input.Source, + model: imported, + }) } } @@ -60,10 +65,10 @@ func Plan(inputs []Input, files Files) ([]FileResult, error) { continue } loaded, failure := loadImport(current.resolvedPath, root, inRepo, imp.Path, files) - if visited[loaded.resolvedPath] || (failure != nil && loaded.source == nil) { + if explicit[loaded.resolvedPath] || discovered[loaded.resolvedPath] || (failure != nil && loaded.source == nil) { continue } - visited[loaded.resolvedPath] = true + discovered[loaded.resolvedPath] = true res := &Result{} runProvenance(loaded.resolvedPath, loaded.source, res) @@ -72,7 +77,12 @@ func Plan(inputs []Input, files Files) ([]FileResult, error) { results = append(results, FileResult{File: loaded.resolvedPath, Findings: res.Findings}) } if failure == nil { - queue = append(queue, loaded) + // A broken nested edge stays silent, but a readable, structurally + // valid model continues the integrity-only crawl. + if imported := traversableModel(loaded.source); imported != nil { + loaded.model = imported + queue = append(queue, loaded) + } } } } @@ -80,8 +90,11 @@ func Plan(inputs []Input, files Files) ([]FileResult, error) { return results, nil } -// traversableModel accepts parsed domain models that this build supports. +// traversableModel accepts structurally valid domain models that this build supports. func traversableModel(source []byte) *model.Model { + if len(Structural(source)) > 0 { + return nil + } m, err := model.Parse(source) if err != nil || m.Kind != "DomainModel" || !schema.Supported(m.Version) { return nil diff --git a/internal/lint/plan_test.go b/internal/lint/plan_test.go index 99170a2..0dbd64b 100644 --- a/internal/lint/plan_test.go +++ b/internal/lint/plan_test.go @@ -146,6 +146,85 @@ func TestPlan_ExplicitRootDiscoveredThroughImportRunsFullLintOnce(t *testing.T) } } +func TestPlan_ExplicitAliasesRunFullLintInArgumentOrder(t *testing.T) { + t.Parallel() + + const first = "./models/root.modelith.yaml" + const second = "models/root.modelith.yaml" + results := planned(t, []Input{ + {Path: first, Source: []byte(gappy)}, + {Path: second, Source: []byte(gappy)}, + }, fakeFiles{".git": ""}) + + if len(results) != 2 || results[0].File != first || results[1].File != second { + t.Fatalf("results = %+v, want both explicit aliases in argument order", results) + } + for _, result := range results { + var completeness bool + for _, finding := range result.Findings { + if finding.Category == CategoryCompleteness { + completeness = true + } + } + if !completeness { + t.Errorf("%s did not receive full lint: %+v", result.File, result.Findings) + } + } +} + +func TestPlan_StructurallyInvalidExplicitRootDoesNotSeedTraversal(t *testing.T) { + t.Parallel() + + const root = "models/root.modelith.yaml" + const child = "models/child.modelith.yaml" + rootSource := importer([]string{`{scope: child, path: "./child.modelith.yaml"}`}, "child.PaymentMethod") + "unexpected: true\n" + files := fakeFiles{ + ".git": "", + root: rootSource, + child: editedVendored(t), + } + + results := planned(t, []Input{{Path: root, Source: []byte(rootSource)}}, files) + if len(results) != 1 || results[0].File != root { + t.Fatalf("results = %+v, want only the explicit root", results) + } + var structural bool + for _, finding := range results[0].Findings { + if finding.Category == CategoryStructural { + structural = true + } + } + if !structural { + t.Errorf("root did not retain its full structural result: %+v", results[0].Findings) + } +} + +// TestADR_0017_VendoredIntermediaryReachesMismatchedVendoredGrandchild pins the +// exception to ADR-0015's import suppression: locally readable imports of a +// vendored copy participate in the integrity-only crawl, without receiving +// transitive semantic lint. +func TestADR_0017_VendoredIntermediaryReachesMismatchedVendoredGrandchild(t *testing.T) { + t.Parallel() + + const root = "models/root.modelith.yaml" + const middle = "models/middle.modelith.yaml" + const child = "models/child.modelith.yaml" + files := fakeFiles{ + ".git": "", + root: importer([]string{`{scope: middle, path: "./middle.modelith.yaml"}`}, "middle.PaymentMethod"), + middle: stamp(t, importer([]string{`{scope: child, path: "./child.modelith.yaml"}`}, "child.PaymentMethod")), + child: editedVendored(t), + } + + results := planned(t, []Input{{Path: root, Source: []byte(files[root])}}, files) + if len(results) != 2 || results[0].File != root || results[1].File != child || len(results[1].Findings) != 1 { + t.Fatalf("results = %+v, want root plus the mismatched vendored grandchild", results) + } + if !strings.Contains(results[1].Findings[0].Message, "deps update "+child) { + t.Errorf("grandchild = %+v, want remedy for %q", results[1], child) + } +} + func TestPlan_ReportsReadableUnsupportedVendoredChild(t *testing.T) { t.Parallel() diff --git a/project-docs/adr/0017-recursive-provenance-verification.md b/project-docs/adr/0017-recursive-provenance-verification.md new file mode 100644 index 0000000..0224850 --- /dev/null +++ b/project-docs/adr/0017-recursive-provenance-verification.md @@ -0,0 +1,20 @@ +# Recursive provenance verification follows locally readable vendored imports + +`modelith lint` follows locally readable import edges while verifying provenance, +including edges declared by vendored models. This narrowly supersedes only +ADR-0015's consequence that suppressing a vendored model's import diagnostics +prevents traversal of those imports; all other ADR-0015 decisions stand. + +## Decision + +The crawl stays offline and integrity-only. It verifies every readable vendored +copy it reaches against the digest in that copy's header, reporting an actual +copied-file digest mismatch against that file. Missing, broken, unreadable, or +otherwise unusable nested edges remain silent, and the crawl does not perform +transitive semantic lint or network access. Normal import resolution remains +non-transitive: an importer still binds only the model it names directly. + +The additional local walk catches a hand-edited vendored grandchild that was +otherwise hidden behind a vendored intermediary, without taking ownership of +that intermediary's model semantics or fetching a dependency tree. It is pinned +by `TestADR_0017_VendoredIntermediaryReachesMismatchedVendoredGrandchild`.