diff --git a/cmd/modelith/main.go b/cmd/modelith/main.go index 5f27576..1fded25 100644 --- a/cmd/modelith/main.go +++ b/cmd/modelith/main.go @@ -6,15 +6,20 @@ import ( "encoding/json" "errors" "fmt" + "io" + "io/fs" "os" "path/filepath" "runtime/debug" "strings" + "time" "github.com/spf13/cobra" + "github.com/stacklok/modelith/internal/deps" "github.com/stacklok/modelith/internal/lint" "github.com/stacklok/modelith/internal/model" + "github.com/stacklok/modelith/internal/provenance" "github.com/stacklok/modelith/internal/render/markdown" "github.com/stacklok/modelith/internal/schema" ) @@ -82,10 +87,141 @@ func rootCmd() *cobra.Command { SilenceErrors: true, Version: buildVersion(), } - root.AddCommand(lintCmd(), renderCmd(), schemaCmd()) + root.AddCommand(lintCmd(), renderCmd(), schemaCmd(), depsCmd()) return root } +// ---- deps ---- + +func depsCmd() *cobra.Command { + cmd := &cobra.Command{ + Use: "deps", + Short: "Manage models vendored from other repositories", + Long: strings.TrimSpace(` +Manage models vendored from other repositories. + +A vendored model is a copy of a model whose home is elsewhere, committed here +and marked with a provenance header. Commands in this group are the only ones +that use the network; lint and render never do.`), + } + cmd.AddCommand(depsImportCmd()) + return cmd +} + +func depsImportCmd() *cobra.Command { + var ref string + cmd := &cobra.Command{ + Use: "import [dir]", + Short: "Vendor a model from another repository", + Long: strings.TrimSpace(` +Vendor a model from another repository into this one. + + is the address of the file as it appears in a browser on github.com. The +copy is written into [dir] (the working directory by default) with a provenance +header recording where it came from, and is verified against that header by +every later lint. + +Fetching is delegated to the gh CLI, which must be installed and authenticated. +The imports list of the model that will reference this copy is yours to edit; +this command prints the entry to add.`), + Example: strings.TrimSpace(` + modelith deps import https://github.com/acme/billing/blob/main/docs/payments.modelith.yaml + modelith deps import https://github.com/acme/billing/blob/main/docs/payments.modelith.yaml docs/ + modelith deps import --ref v2.1.0 https://github.com/acme/billing/blob/main/docs/payments.modelith.yaml`), + Args: cobra.RangeArgs(1, 2), + RunE: func(cmd *cobra.Command, args []string) error { + dir := "." + if len(args) == 2 { + dir = args[1] + } + info, err := os.Stat(dir) + if err != nil { + return fmt.Errorf("%s: %w", dir, err) + } + if !info.IsDir() { + return fmt.Errorf("%s is not a directory — the destination is a directory, and the filename comes from the origin", dir) + } + + res, err := deps.Import(cmd.Context(), deps.Options{ + URL: args[0], + Dir: dir, + Ref: ref, + Now: time.Now(), + }) + if err != nil { + return err + } + + // A failure to read the working directory only costs the entry + // its relative form, so it is not worth failing a completed + // import over. + wd, _ := os.Getwd() + printImportResult(cmd.OutOrStdout(), cmd.ErrOrStderr(), res, wd) + return nil + }, + } + cmd.Flags().StringVar(&ref, "ref", "", "ref to fetch, overriding the one in the URL (a tag pins the copy)") + return cmd +} + +// printImportResult reports a completed import: what was written, the entry to +// paste, whether the fetched model reaches for models of its own, and the trust +// warning ADR-0014 requires. +// +// The warning prints and does not prompt. An agent usually drives this command, +// so a prompt is either auto-answered theatre or a wedged non-interactive run. +// The fetched file is inert until it is named in an imports list, and this +// command deliberately does not do that — the manual step is the real gate. +func printImportResult(out, errOut io.Writer, res *deps.Result, wd string) { + verb := "wrote" + if res.Replaced { + verb = "replaced" + } + name := filepath.Base(res.Path) + fmt.Fprintf(out, "%s %s at %s\n", verb, res.Path, res.Header.Commit) + // The entry is printed relative to the working directory, because that is + // the only thing this command knows: an import path is relative to the model + // that declares it, and which model that will be is the user's to decide — + // the same reason the imports list is not edited here. + fmt.Fprintf(out, "\nAdd it to the model that references it, as a path relative to that model "+ + "(this one is relative to the current directory):\n\n"+ + " imports:\n - %s\n", importEntry(res.Path, wd)) + if n := len(res.TheirImports); n > 0 { + declares := fmt.Sprintf("declares %d imports of its own", n) + if n == 1 { + declares = "declares an import of its own" + } + fmt.Fprintf(out, "\nNote: %s %s (%s).\n"+ + "modelith vendors one file, not a dependency tree, and resolution is not\n"+ + "transitive — if you need items from those models, import them directly.\n", + name, declares, strings.Join(res.TheirImports, ", ")) + } + fmt.Fprint(errOut, "\nWarning: a vendored model is untrusted content that will be rendered\n"+ + "into your published Markdown. Only vendor from sources you trust.\n") +} + +// importEntry writes a path the way an `imports:` entry is written: relative, +// slash separated, and explicitly so, so it reads as a path rather than as a +// scope. +// +// An absolute destination is relativized against wd, because an absolute import +// is a lint error — imports resolve relative to the model that declares them so +// they work in any checkout. An agent driving this command usually passes an +// absolute directory, so printing one back would hand it a line the linter +// rejects. A "../" result is fine; only an absolute one is not. +func importEntry(p, wd string) string { + if filepath.IsAbs(p) && wd != "" { + if rel, err := filepath.Rel(wd, p); err == nil { + p = rel + } + } + p = filepath.ToSlash(p) + if strings.HasPrefix(p, "/") || strings.HasPrefix(p, "./") || strings.HasPrefix(p, "../") { + return p + } + return "./" + p +} + // ---- lint ---- func lintCmd() *cobra.Command { @@ -205,9 +341,63 @@ func renderCmd() *cobra.Command { if err != nil { return fmt.Errorf("%s: %w", in, err) } + target := out + if target == "" { + target = defaultOut(in) + } + + // A target under a directory that does not exist is a misconfigured + // -o, not a document waiting to be written. It is settled before + // anything below can exempt a file from the check, so no exemption + // can turn a typo into a pass. + if check && !isDir(filepath.Dir(target)) { + return fmt.Errorf("cannot check %s: %s is not a directory", target, filepath.Dir(target)) + } + + // A vendored model's rendered form belongs to its home repository, + // so it arrives with no committed .md and no obligation to carry + // one. --check runs over globs, and demanding one here would make + // this repository regenerate somebody else's document every time + // their model moved (ADR-0015). Naming the file to render it still + // renders it; that is how a deep link into a vendored model's .md + // gets something to point at — and once such an .md is committed, it + // goes stale like any other, so from then on --check does check it. + // + // What stays skipped is everything the origin owns rather than this + // repository: a copy this build cannot render at all — a newer schema + // version, a shape this binary does not know — is not a --check + // failure, because there is nothing here to fix. `modelith lint` + // reports it, loudly and once. + // + // The exemption is claimed by a *clean* header, not by + // provenance.Present. Present answers a deliberately broad question + // so that a broken header still classifies a file as somebody else's + // copy, which is right for lint: the header defects are reported and + // the completeness noise is suppressed. Skipping on that same answer + // would be silent, and would hand the exemption to a model this + // repository owns that merely carries a "# modelith-" comment. An + // exemption needs a valid claim, so a file whose header does not + // parse cleanly is checked like any other — its defects are already + // failing `modelith lint`. + header, headerProblems := provenance.Parse(data) + vendored := header != nil && len(headerProblems) == 0 + skipVendored := func(tail string) error { + fmt.Fprintf(cmd.OutOrStdout(), "%s is a vendored copy %s\n", in, tail) + return nil + } + unrenderable := fmt.Sprintf("this modelith cannot render — skipped (run `modelith lint %s` for details)", in) + if check && vendored { + if _, err := os.Stat(target); errors.Is(err, fs.ErrNotExist) { + return skipVendored("with no committed " + filepath.Base(target) + " — skipped") + } + } + // Validate against the schema first so a malformed file fails with a // friendly, located error rather than the raw strict-YAML parse error. if findings := lint.Structural(data); len(findings) > 0 { + if check && vendored { + return skipVendored(unrenderable) + } var b strings.Builder fmt.Fprintf(&b, "%s is not a valid domain model — run `modelith lint %s` for details:", in, in) for _, f := range findings { @@ -221,6 +411,9 @@ func renderCmd() *cobra.Command { } m, err := model.Parse(data) if err != nil { + if check && vendored { + return skipVendored(unrenderable) + } return err } @@ -238,10 +431,6 @@ func renderCmd() *cobra.Command { return err } - target := out - if target == "" { - target = defaultOut(in) - } outDir, err := filepath.Abs(filepath.Dir(target)) if err != nil { return fmt.Errorf("resolving %s: %w", target, err) @@ -276,6 +465,14 @@ func renderCmd() *cobra.Command { return cmd } +// isDir reports whether p exists and is a directory. Anything else — missing, +// a plain file, unreadable — is false, because every one of those means a +// target under p is not merely uncommitted. +func isDir(p string) bool { + info, err := os.Stat(p) + return err == nil && info.IsDir() +} + // defaultOut is where render writes when no -o is given: a model's .md beside // its .yaml source. It is the rendered location an import link assumes for the // model it names (see importLinkTarget in internal/render/markdown) — a link diff --git a/cmd/modelith/main_test.go b/cmd/modelith/main_test.go index 1510a1c..14666f2 100644 --- a/cmd/modelith/main_test.go +++ b/cmd/modelith/main_test.go @@ -8,6 +8,9 @@ import ( "path/filepath" "strings" "testing" + + "github.com/stacklok/modelith/internal/deps" + "github.com/stacklok/modelith/internal/provenance" ) // run executes the CLI with args, capturing stdout+stderr, and returns the @@ -270,6 +273,287 @@ func TestRenderCheckDetectsDrift(t *testing.T) { } } +// vendorHeader stamps content the way `deps import` does, so a test fixture is +// a vendored copy that verifies against its own digest. +func vendorHeader(content string) string { + return "# modelith-vendored: " + provenance.Banner + "\n" + + "# modelith-fetch: git\n" + + "# modelith-origin: https://github.com/stacklok/some-repo\n" + + "# modelith-path: docs/m.modelith.yaml\n" + + "# modelith-ref: main\n" + + "# modelith-commit: 4f2c1e9\n" + + "# modelith-imported: 2026-07-27\n" + + "# modelith-digest: " + provenance.Digest([]byte(content)) + "\n" +} + +// TestRenderCheckSkipsAVendoredCopy covers the half of ADR-0015 that lives in +// the CLI: a vendored model has no committed .md here, and --check runs over +// globs, so demanding one would fail a build over somebody else's document. +// Rendering it by name still works — that is what a deep link points at. +func TestRenderCheckSkipsAVendoredCopy(t *testing.T) { + dir := t.TempDir() + yamlPath := writeTemp(t, dir, "m.modelith.yaml", vendorHeader(minimalValid)+minimalValid) + + // No .md exists at all, which is the state a fresh `deps import` leaves. + out, err := run(t, "render", "--check", yamlPath) + if err != nil { + t.Fatalf("--check failed on a vendored copy: %v (%s)", err, out) + } + if !strings.Contains(out, "vendored copy") { + t.Errorf("output does not say why it was skipped: %q", out) + } + + if _, err := os.Stat(filepath.Join(dir, "m.modelith.md")); !os.IsNotExist(err) { + t.Error("--check wrote a file") + } + + if _, err := run(t, "render", yamlPath); err != nil { + t.Fatalf("rendering a vendored model by name failed: %v", err) + } + if _, err := os.Stat(filepath.Join(dir, "m.modelith.md")); err != nil { + t.Errorf("rendering by name wrote nothing: %v", err) + } +} + +// TestRenderCheckChecksACommittedVendoredMarkdown pins the other side of the +// skip: once this repository has committed a vendored model's .md — the way a +// deep link into it gets a target — that file goes stale like any other, and +// nothing else would say so. Skipping it unconditionally left a refreshed copy +// with a rendered document describing the version before it. +func TestRenderCheckChecksACommittedVendoredMarkdown(t *testing.T) { + dir := t.TempDir() + yamlPath := writeTemp(t, dir, "m.modelith.yaml", vendorHeader(minimalValid)+minimalValid) + + if _, err := run(t, "render", yamlPath); err != nil { + t.Fatalf("rendering a vendored model by name failed: %v", err) + } + out, err := run(t, "render", "--check", yamlPath) + if err != nil { + t.Fatalf("--check failed on a freshly rendered copy: %v (%s)", err, out) + } + if !strings.Contains(out, "up to date") { + t.Errorf("output does not report the check: %q", out) + } + + writeTemp(t, dir, "m.modelith.md", "stale content\n") + if _, err := run(t, "render", "--check", yamlPath); err == nil { + t.Fatal("a stale committed .md for a vendored copy passed --check") + } else if !strings.Contains(err.Error(), "out of date") { + t.Fatalf("expected an out-of-date error, got: %v", err) + } +} + +// TestRenderCheckRejectsAnOutPathUnderNoDirectory pins that no exemption +// swallows a misconfigured -o. A target under a directory that does not exist +// stats as ErrNotExist exactly like an uncommitted one, so reading it as +// "nothing committed yet" let a typo pass this gate. The unrenderable case is +// the one that survived the first attempt at this: its skip returned after the +// target check rather than before it. +func TestRenderCheckRejectsAnOutPathUnderNoDirectory(t *testing.T) { + unrenderable := strings.Replace(minimalValid, "version: v1", "version: v99", 1) + + for _, tc := range []struct{ name, src string }{ + {"a vendored copy", vendorHeader(minimalValid) + minimalValid}, + {"a model this repository owns", minimalValid}, + {"a vendored copy this build cannot render", vendorHeader(unrenderable) + unrenderable}, + } { + t.Run(tc.name, func(t *testing.T) { + dir := t.TempDir() + yamlPath := writeTemp(t, dir, "m.modelith.yaml", tc.src) + out, err := run(t, "render", "--check", "-o", filepath.Join(dir, "nosuchdir", "out.md"), yamlPath) + if err == nil { + t.Fatalf("a target under a missing directory passed --check: %s", out) + } + }) + } +} + +// TestRenderCheckDoesNotExemptAModelThisRepoOwns pins that the --check skip is +// claimed by a clean provenance header, not by provenance.Present. +// +// Present answers a deliberately broad question so a broken header still marks a +// file as somebody else's copy, which is what lint needs. Skipping on that same +// answer handed the exemption to a model this repository owns that merely +// carried a "# modelith-" comment: lint shouted five errors, but a CI job +// running only `render --check` silently lost its gate and exited 0. +func TestRenderCheckDoesNotExemptAModelThisRepoOwns(t *testing.T) { + dir := t.TempDir() + yamlPath := writeTemp(t, dir, "own.modelith.yaml", + minimalValid+"\n# modelith-note: this file is generated\n") + + out, err := run(t, "render", "--check", yamlPath) + if err == nil { + t.Fatalf("a model this repository owns was exempted from --check: %s", out) + } + if strings.Contains(out, "vendored copy") { + t.Errorf("it was reported as a vendored copy:\n%s", out) + } + + // A header with a real defect in it earns no exemption either: the file is + // checked like any other, and lint reports the defect. + displaced := writeTemp(t, dir, "displaced.modelith.yaml", + "---\n"+vendorHeader(minimalValid)+minimalValid) + if _, err := run(t, "render", "--check", displaced); err == nil { + t.Error("a vendored copy with a displaced header was exempted from --check") + } +} + +// TestRenderCheckStaysQuietOnAVendoredCopyItCannotRender pins that --check does +// not turn this repository's build red over a defect in a document it does not +// own. A copy fetched from a repository on a newer modelith can be unreadable +// here; `modelith lint` reports that, once, and --check has nothing to add +// because there is no .md this repository could regenerate. +func TestRenderCheckStaysQuietOnAVendoredCopyItCannotRender(t *testing.T) { + dir := t.TempDir() + future := strings.Replace(minimalValid, "version: v1", "version: v99", 1) + if future == minimalValid { + t.Fatal("fixture did not declare a version to replace") + } + yamlPath := writeTemp(t, dir, "m.modelith.yaml", vendorHeader(future)+future) + writeTemp(t, dir, "m.modelith.md", "a rendered form from a modelith that understood it\n") + + out, err := run(t, "render", "--check", yamlPath) + if err != nil { + t.Fatalf("--check failed on an unrenderable vendored copy: %v (%s)", err, out) + } + if !strings.Contains(out, "cannot render") { + t.Errorf("output does not say why it was skipped: %q", out) + } + + // The same file, owned rather than vendored, is still this repository's + // problem and still fails. + ownPath := writeTemp(t, dir, "own.modelith.yaml", future) + writeTemp(t, dir, "own.modelith.md", "whatever\n") + if _, err := run(t, "render", "--check", ownPath); err == nil { + t.Fatal("an unrenderable model this repository owns passed --check") + } +} + +// TestDepsImportOutput covers what a completed import tells the user. All three +// parts are load-bearing: the snippet is the second, manual step this command +// deliberately leaves to the user, the note is the whole answer to +// non-transitive vendoring, and the warning is what ADR-0014 requires in place +// of hardening the renderer further. +func TestDepsImportOutput(t *testing.T) { + res := &deps.Result{ + Path: filepath.Join("docs", "payments.modelith.yaml"), + Header: &provenance.Header{Commit: "4f2c1e9"}, + } + + t.Run("a leaf model", func(t *testing.T) { + var out, errOut bytes.Buffer + printImportResult(&out, &errOut, res, "") + // The entry names where the copy actually landed. Printing the bare + // basename told a user who passed a directory to paste a path that + // resolves to nothing. + for _, want := range []string{"wrote docs/payments.modelith.yaml at 4f2c1e9", "imports:", "- ./docs/payments.modelith.yaml"} { + if !strings.Contains(out.String(), want) { + t.Errorf("stdout does not contain %q:\n%s", want, out.String()) + } + } + if strings.Contains(out.String(), "dependency tree") { + t.Error("a leaf model got the transitive-imports note") + } + if !strings.Contains(errOut.String(), "Only vendor from sources you trust") { + t.Errorf("the trust warning is missing from stderr:\n%s", errOut.String()) + } + }) + + t.Run("a model with imports of its own", func(t *testing.T) { + var out, errOut bytes.Buffer + chained := *res + chained.TheirImports = []string{"./ledger.modelith.yaml"} + printImportResult(&out, &errOut, &chained, "") + for _, want := range []string{ + "declares an import of its own (./ledger.modelith.yaml)", + "resolution is not", + "import them directly", + } { + if !strings.Contains(out.String(), want) { + t.Errorf("stdout does not contain %q:\n%s", want, out.String()) + } + } + }) + + t.Run("a copy in the working directory", func(t *testing.T) { + var out, errOut bytes.Buffer + here := *res + here.Path = "payments.modelith.yaml" + printImportResult(&out, &errOut, &here, "") + if !strings.Contains(out.String(), "- ./payments.modelith.yaml") { + t.Errorf("stdout does not contain the entry:\n%s", out.String()) + } + }) + + // An agent driving this command usually passes an absolute destination. + // Printing one straight back produced an entry the linter rejects outright: + // imports are relative so they resolve in any checkout. + t.Run("an absolute destination is relativized", func(t *testing.T) { + var out, errOut bytes.Buffer + abs := *res + abs.Path = filepath.Join(string(filepath.Separator), "home", "me", "repo", "docs", "payments.modelith.yaml") + printImportResult(&out, &errOut, &abs, filepath.Join(string(filepath.Separator), "home", "me", "repo")) + if !strings.Contains(out.String(), "- ./docs/payments.modelith.yaml") { + t.Errorf("stdout does not contain a relative entry:\n%s", out.String()) + } + if strings.Contains(out.String(), "- /home/me") { + t.Errorf("stdout still contains an absolute import entry:\n%s", out.String()) + } + }) + + t.Run("a replaced copy says so", func(t *testing.T) { + var out, errOut bytes.Buffer + replaced := *res + replaced.Replaced = true + printImportResult(&out, &errOut, &replaced, "") + if !strings.HasPrefix(out.String(), "replaced ") { + t.Errorf("stdout does not report the replacement:\n%s", out.String()) + } + }) +} + +// TestDepsImportRejectsBadArguments covers the paths that fail before any fetch +// is attempted, so they need no network and no gh. +func TestDepsImportRejectsBadArguments(t *testing.T) { + dir := t.TempDir() + file := writeTemp(t, dir, "not-a-dir", "x") + + cases := []struct { + name string + args []string + want string + }{ + { + name: "a host modelith cannot fetch from", + args: []string{"deps", "import", "https://gitlab.com/acme/billing/-/blob/main/m.modelith.yaml", dir}, + want: "github.com/stacklok/modelith/issues", + }, + { + name: "a URL that names no file", + args: []string{"deps", "import", "https://github.com/acme/billing", dir}, + want: "not a GitHub file URL", + }, + { + name: "a destination that is a file", + args: []string{"deps", "import", "https://github.com/acme/billing/blob/main/m.modelith.yaml", file}, + want: "is not a directory", + }, + { + name: "a destination that does not exist", + args: []string{"deps", "import", "https://github.com/acme/billing/blob/main/m.modelith.yaml", filepath.Join(dir, "nope")}, + want: "no such file or directory", + }, + } + for _, tc := range cases { + t.Run(tc.name, func(t *testing.T) { + _, err := run(t, tc.args...) + if err == nil || !strings.Contains(err.Error(), tc.want) { + t.Fatalf("want an error containing %q, got %v", tc.want, err) + } + }) + } +} + func TestSchemaOutputsValidJSON(t *testing.T) { out, err := run(t, "schema") if err != nil { diff --git a/docs/06-schema-reference.md b/docs/06-schema-reference.md index 85ca977..b6853d2 100644 --- a/docs/06-schema-reference.md +++ b/docs/06-schema-reference.md @@ -335,6 +335,8 @@ Six rules are worth knowing before you use this: - **Nothing is fetched.** `imports` names files that are already in your repository; `lint` and `render` never touch the network ([ADR-0011](https://github.com/stacklok/modelith/blob/main/project-docs/adr/0011-network-boundary.md)). + Getting a model from *another* repository into yours is a separate, explicit + step — see [Vendoring](./10-vendoring.md). - **An import cannot leave the repository.** `..` is fine, but only as far as the repository holding the model — the nearest directory above it with a `.git` entry. Past that it is an error naming where the path resolved to and diff --git a/docs/07-cli.md b/docs/07-cli.md index 4c1eb13..282e1ea 100644 --- a/docs/07-cli.md +++ b/docs/07-cli.md @@ -87,3 +87,39 @@ into another validator. ```sh modelith schema > modelith.schema.json ``` + +## `modelith deps` + +Manages models **vendored** from other repositories — copies committed here and +marked with a provenance header. This is the only command group that uses the +network; `lint` and `render` never do. + +### `modelith deps import [dir]` + +Fetches a model and writes it into `dir` (the working directory by default) as +a vendored copy. + +```sh +modelith deps import https://github.com/acme/billing/blob/main/docs/payments.modelith.yaml docs/ +``` + +| Argument / flag | Meaning | +|---|---| +| `` | The address of the file as it appears in a browser on github.com. | +| `[dir]` | Destination **directory**, defaulting to `.`. The filename always comes from the origin. | +| `--ref` | Ref to fetch, overriding the one in the URL. A tag pins the copy; naming a branch whose name contains a slash is also how you tell modelith where the ref ends and the path begins. | + +A browse URL gives no way to tell a slashed ref from the path after it, so +modelith splits at the first segment. `--ref` fixes that split only when it +names the ref that is *in* the URL — it cannot both pin a different ref and +re-split the path. To pin, open the file on the ref you want and import that +URL. When a fetch fails, the error says how the URL was split. + +Fetching is delegated to [`gh`](https://cli.github.com), which must be installed +and authenticated. The command writes the file and prints the `imports:` entry +to add — it does not edit your model. It refuses to overwrite a file at the +destination that is not an earlier copy of the same model. + +See [Vendoring a model from another +repository](./10-vendoring.md) for what the header records, how a vendored file +is linted differently, and why vendoring fetches one file rather than a tree. diff --git a/docs/08-github-action.md b/docs/08-github-action.md index 3a67014..38913dd 100644 --- a/docs/08-github-action.md +++ b/docs/08-github-action.md @@ -52,6 +52,31 @@ and verifies it against the release's published checksums before running it. The pinning your `uses:` reference to a commit SHA, this keeps CI runs reproducible: a given action commit always installs the same `modelith` version. +## Vendored models in the glob + +If your glob matches a model [vendored from another +repository](./10-vendoring.md), the action treats it as somebody else's +document: + +- **Completeness gaps in it are not reported**, so `completeness: error` cannot + fail your build over a model you did not write. +- **`check-rendered` skips it when you have not committed a `.md` for it**, so + you are not asked to commit one whose home is another repository. If you *do* + commit one — the way a deep link into a vendored model's Markdown gets a + target — it is checked for staleness from then on. +- **`check-rendered` also skips a copy this modelith cannot render**, such as + one written against a newer schema version. `lint` reports that, and repeating + it here would fail your build twice over one problem you cannot fix in your + own repository. + +Both skips take a clean provenance header to claim. A file whose header has a +defect in it is checked like any model you wrote, so a stray `# modelith-` +comment cannot quietly switch the gate off. + +Everything else still applies. A vendored file that is not a valid domain +model, or that has been edited since it was imported, fails the build — both +are about *your* repository's copy, and both are yours to fix. + ## Regenerating the Markdown The action **gates**; it does not commit. When `check-rendered` fails, run diff --git a/docs/10-vendoring.md b/docs/10-vendoring.md new file mode 100644 index 0000000..844d60b --- /dev/null +++ b/docs/10-vendoring.md @@ -0,0 +1,205 @@ +--- +sidebar_position: 10 +title: Vendoring a model from another repository +description: Copy a model from another repository, keep track of where it came from, and reference its items. +--- + +# Vendoring a model from another repository + +[Imports](./06-schema-reference.md#imports) let one model reference an item +another one defines — but only across files that are already in your +repository. **Vendoring** is how a model from *somewhere else* gets there: you +fetch a copy, commit it, and modelith records where it came from. + +```sh +modelith deps import https://github.com/acme/billing/blob/main/docs/payments.modelith.yaml docs/ +``` + +That URL is the address of the file as it appears in your browser on +github.com — open the model on GitHub and copy the address bar. + +## What you get + +A copy of the file, byte for byte, with a **provenance header** added at the +top: + +```yaml +# yaml-language-server: $schema=https://modelith.sh/schema/domain-model/v1.json +# modelith-vendored: DO NOT EDIT — this file is a copy. Change it at its origin. +# modelith-fetch: git +# modelith-origin: https://github.com/acme/billing +# modelith-path: docs/payments.modelith.yaml +# modelith-ref: main +# modelith-commit: 4f2c1e9c8b3ad0e5f71b2c9a6d4e8f30ab5c7d21 +# modelith-imported: 2026-07-27 +# modelith-digest: sha256:9a1f… +``` + +The header is a comment, not part of the schema — a model you write yourself +never has one, and never meets any of this. + +| Key | What it records | +|---|---| +| `vendored` | That this file is a copy. Nothing enforces it; it is there so a person or an agent about to edit the file stops. | +| `fetch` | How to get it again. `git` today. | +| `origin`, `path`, `ref` | Where it came from and what to track. A tag in `ref` pins the copy; a branch follows it. | +| `commit` | The commit that last touched *this file* at that ref — so it does not move when unrelated commits land. | +| `imported` | When you fetched it. | +| `digest` | SHA-256 of the file with the header lines removed, so stamping the header does not change it. | + +## Then add it to your model + +`deps import` writes the file and stops. It does **not** edit your model's +`imports:` — it prints the line to add: + +```yaml +imports: + - ./docs/payments.modelith.yaml +``` + +The printed path is relative to the directory you ran the command in, because +that is the only thing `deps import` knows. An import path is relative to the +model that *declares* it, so if that model does not sit beside your working +directory, adjust it — exactly like any other import. Until you add that line, +the copy is an inert file that nothing reads. + +That second step is deliberate. A vendored model is content someone else wrote +that will be rendered into *your* published Markdown, so `deps import` warns you +and leaves the decision — and the diff — visible. + +:::warning[Only vendor from sources you trust] + +A vendored model's prose ends up in your rendered `.md`. modelith escapes HTML +in prose fields, but the file is still somebody else's text landing in your +docs. Vendoring is designed for projects that already trust each other. + +::: + +## How a vendored file is treated differently + +Once a file carries a provenance header, modelith knows it is not your work. +Two things change, and nothing else: + +- **Completeness findings are suppressed.** Missing invariants, entities no + scenario exercises, unused enums and glossary terms are gaps in a document + 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. + +**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 +fetching a different ref, or by talking to whoever owns it. + +`modelith render --check` skips a vendored file that has no committed `.md`: +its rendered Markdown belongs to its home repository, so you are not asked to +commit one. Rendering a vendored model by naming it still works, which is how a +deep link into it gets something to point at — and once you commit that `.md`, +`--check` treats it like any other and tells you when refreshing the copy has +left it stale. + +The one thing `--check` will not do is fail over a vendored model this modelith +cannot render at all — one written against a newer schema version, say. It says +it skipped it and moves on; `modelith lint` is where that is reported, once. + +Being skipped is an exemption, and it takes a **clean** provenance header to +claim one. A file whose header has a defect in it — a misplaced line, a missing +key — is checked like any other model. That way a mistyped comment can never +quietly switch a gate off: the header defect fails `lint`, and the rendered +output is still checked. + +## What it will not overwrite + +The filename comes from the origin, so a copy can land on a file you already +have. `deps import` refuses two cases rather than clobbering them: + +- **A model you wrote.** No provenance header means the file is yours, and no + re-fetch could bring it back. Import into a different directory, or move the + file aside first. +- **A copy of a *different* model with the same basename.** Two `payments.modelith.yaml` + files from two repositories cannot share a directory; give them separate ones. + +A copy from the *same* repository at a different path is refused too, because +modelith cannot tell a model that moved upstream from a second model whose file +happens to share a name. The message offers both remedies: delete the copy and +import again if it moved, or import into a different directory if they are two +different models. + +Re-importing over an existing copy of the same model at the same path is the +ordinary refresh, and that goes through — it reports `replaced` rather than +`wrote`. Only the origin's *casing* is ignored in that comparison, because +GitHub treats an owner and repository name case-insensitively. + +## Keeping the copy honest + +Every `modelith lint` re-checks a vendored file against the digest in its own +header. If someone edits the copy, lint says so: + +``` +error [semantic] (root): this vendored file no longer matches the digest its +provenance header records (recorded sha256:9a1f…, computed sha256:2c7b…) — it +has been edited since it was imported. Refresh it with `modelith deps import +https://github.com/acme/billing/blob/main/docs/payments.modelith.yaml`, or +delete the provenance header if the change is a deliberate fork, which makes +this repository the file's home. +``` + +Both remedies are real. Re-running `deps import` replaces the copy with what +upstream has now. Deleting the header makes the file an ordinary model of +yours — an honest description of having forked it, and it re-enables the +completeness checks, because now it *is* your document. + +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. + +## Vendoring is one file, not a dependency tree + +If the model you fetch imports models of its own, **those are not fetched**. +`deps import` tells you they exist: + +``` +Note: payments.modelith.yaml declares an import of its own (./ledger.modelith.yaml). +modelith vendors one file, not a dependency tree, and resolution is not +transitive — if you need items from those models, import them directly. +``` + +This matches how [resolution already +works](./06-schema-reference.md#imports): `payments.Thing` reaches only items +defined *directly* in the file bound to `payments`. If the item you want lives +one hop further away, vendor that model too and give it its own scope. The +linter says so at the reference site when it can tell: + +``` +attribute type "payments.Carrier" names no enum "Carrier" in +"./payments.modelith.yaml" — that model imports a model of its own, and +resolution is not transitive: if "Carrier" is defined in one of them, add that +model to this model's `imports:` too and reference it with its own scope. +``` + +Fetching a tree would mean a directory layout, rewritten paths, and an answer +for what happens when two models want different versions of the same third +model — a package manager, for a problem an explicit second `deps import` +already solves. + +## Requirements and limits + +- **`gh` must be installed and authenticated.** modelith implements no network + transport of its own; it delegates to the [GitHub + CLI](https://cli.github.com), which already solves authentication for private + and internal repositories. +- **GitHub only, for now.** A URL on another host is an error that asks you to + [open an issue](https://github.com/stacklok/modelith/issues). That is not a + brush-off: the header records *how* it was fetched, so adding another + transport is straightforward — what is missing is a real user to build it + for, and an issue is how you become one. +- **`lint` and `render` never touch the network**, whatever you pass them + ([ADR-0011](https://github.com/stacklok/modelith/blob/main/project-docs/adr/0011-network-boundary.md)). + Everything under `modelith deps` is opt-in, and nothing else fetches. + +The design and its trade-offs are +[ADR-0010](https://github.com/stacklok/modelith/blob/main/project-docs/adr/0010-cross-model-references-by-vendoring.md) +and +[ADR-0015](https://github.com/stacklok/modelith/blob/main/project-docs/adr/0015-vendoring-is-a-whole-file-copy.md). diff --git a/docs/index.md b/docs/index.md index b946d4b..911b13d 100644 --- a/docs/index.md +++ b/docs/index.md @@ -55,3 +55,4 @@ fails on drift (`modelith render --check`) — like a generated-code check. | [Schema](./06-schema-reference.md) | The JSON Schema that defines a valid model | | [`modelith` CLI](./07-cli.md) | The `lint` / `render` engine the agent and CI run | | [GitHub Action](./08-github-action.md) | The same checks in CI | +| [Vendoring](./10-vendoring.md) | Referencing a model that lives in another repository | diff --git a/internal/deps/deps.go b/internal/deps/deps.go new file mode 100644 index 0000000..439741c --- /dev/null +++ b/internal/deps/deps.go @@ -0,0 +1,358 @@ +// Package deps acquires a model from another repository and stamps it as a +// vendored copy. +// +// It implements no network transport of its own. Every fetch is delegated to +// the gh CLI, executed as an argv array and never through a shell, so this +// binary holds no TLS configuration and no credentials (ADR-0011). +package deps + +import ( + "context" + "errors" + "fmt" + "io/fs" + "net/url" + "os" + "os/exec" + "path" + "path/filepath" + "strings" + "time" + + "github.com/stacklok/modelith/internal/model" + "github.com/stacklok/modelith/internal/provenance" +) + +// Runner runs an external command and returns its standard output. It is the +// seam the gh calls go through, so Import is testable without a network. +type Runner interface { + Run(ctx context.Context, name string, args ...string) ([]byte, error) +} + +// ExecRunner runs commands with os/exec. +type ExecRunner struct{} + +// Run executes name with args and returns its standard output. Standard error +// is folded into the returned error, because gh reports why it refused there. +func (ExecRunner) Run(ctx context.Context, name string, args ...string) ([]byte, error) { + // nolint:gosec // G204 flags a variable command and arguments, which is + // what a transport seam is. The command is the literal "gh" at both call + // sites; the arguments are literals plus an endpoint assembled from a URL + // that ParseSource has already validated, with each path segment and query + // value escaped and traversal segments rejected outright (escaping a + // segment cannot neutralise a segment that *is* a traversal, so ParseSource + // refuses those rather than passing them on). Nothing here comes from a + // model file, and there is no + // shell: exec passes an argv array, so a metacharacter is a byte in an + // argument rather than syntax (ADR-0010). + cmd := exec.CommandContext(ctx, name, args...) + var stderr strings.Builder + cmd.Stderr = &stderr + out, err := cmd.Output() + if err != nil { + if errors.Is(err, exec.ErrNotFound) { + return nil, fmt.Errorf("%s is not installed — modelith delegates fetching to it; install it from https://cli.github.com and run `%s auth login`", name, name) + } + if msg := strings.TrimSpace(stderr.String()); msg != "" { + return nil, fmt.Errorf("%s %s: %s", name, strings.Join(args, " "), msg) + } + return nil, fmt.Errorf("%s %s: %w", name, strings.Join(args, " "), err) + } + return out, nil +} + +// Source is a model file in another repository, as an origin URL parsed into +// the parts a fetch and a later refresh both need. +type Source struct { + Origin string // https://github.com/owner/repo + Owner string + Repo string + Ref string + Path string // path within the repository +} + +// ParseSource reads a GitHub blob URL — the address of the file as it appears +// in a browser — into its parts. A non-empty ref overrides the one in the URL, +// and when it is the ref the URL names, it is also what disambiguates a branch +// whose name contains a slash from the path that follows it. It cannot do both +// at once: overriding with a *different* ref leaves the split to be taken at the +// first segment, which splitHint explains when the fetch that follows fails. +func ParseSource(raw, ref string) (Source, error) { + u, err := url.Parse(strings.TrimSpace(raw)) + if err != nil { + return Source{}, fmt.Errorf("%q is not a URL: %w", raw, err) + } + // A host is case-insensitive, and a browser hands back the "www." form as + // readily as the bare one; neither is a different site. + if host := strings.TrimPrefix(strings.ToLower(u.Host), "www."); host != "github.com" { + return Source{}, fmt.Errorf( + "modelith can currently fetch only from github.com, and %q is on %q. Support for other hosts is not written yet because nobody has needed it — if you do, please open an issue at %s saying where your models live", + raw, u.Host, issuesURL) + } + // A URL copied from the browser often carries ?plain=1 and a #L12 anchor. + // Neither is part of the file's address. + parts := strings.Split(strings.Trim(u.Path, "/"), "/") + if len(parts) < 5 || parts[2] != "blob" { + return Source{}, fmt.Errorf( + "%q is not a GitHub file URL — it should look like https://github.com///blob//, which is the address you get by opening the file on github.com", + raw) + } + // A dot segment or an empty one is not part of any address github.com + // serves, and url.Parse does not remove one. It has to be rejected here + // rather than escaped later: escapePath escapes the characters within a + // segment, which leaves a segment that *is* a traversal exactly as it was, + // and the endpoint fetchContent builds would then leave the repository's + // contents namespace. + for _, p := range parts { + if p == "" || p == "." || p == ".." { + return Source{}, fmt.Errorf( + "%q has a %q path segment, which is not part of a file's address on github.com — copy the address bar from the file's page rather than assembling the URL by hand", + raw, p) + } + } + src := Source{ + Owner: parts[0], + Repo: parts[1], + Origin: "https://github.com/" + parts[0] + "/" + parts[1], + } + rest := strings.Join(parts[3:], "/") + // A ref may contain slashes ("release/v2"), and the URL gives no way to tell + // where it ends. Splitting at the first segment is right for the common + // case; an explicit ref settles the rest. + switch { + case ref != "" && strings.HasPrefix(rest, ref+"/"): + src.Ref, src.Path = ref, strings.TrimPrefix(rest, ref+"/") + default: + src.Ref, src.Path, _ = strings.Cut(rest, "/") + if ref != "" { + src.Ref = ref + } + } + if src.Path == "" { + return Source{}, fmt.Errorf("%q names no file inside the repository", raw) + } + return src, nil +} + +const issuesURL = "https://github.com/stacklok/modelith/issues" + +// Options are the inputs to Import. +type Options struct { + // URL is the GitHub blob URL of the model to vendor. + URL string + // Dir is the directory to write into. Empty means the working directory. + Dir string + // Ref overrides the ref in URL. + Ref string + // Now stamps the header's imported date, in local time. + Now time.Time + // Run is the command seam; nil uses ExecRunner. + Run Runner +} + +// Result reports what Import did, so the caller can print it. +type Result struct { + // Path is where the vendored copy was written. + Path string + // Header is what was stamped into it. + Header *provenance.Header + // Replaced says an earlier copy was overwritten. + Replaced bool + // TheirImports are the fetched model's own imports. Vendoring fetches one + // file, so these were not followed; the caller says so. + TheirImports []string +} + +// Import fetches the model at opts.URL and writes it into opts.Dir as a +// vendored copy carrying a provenance header. +// +// It writes the file and nothing else. The importing model's `imports:` list is +// the caller's to edit: rewriting a user's YAML costs comment and formatting +// fidelity, and a repository with several models gives no way to guess which +// one meant to import this. That second, manual step is also the gate on the +// injection risk vendoring carries (ADR-0014) — the file is inert until it is +// named in an imports list. +func Import(ctx context.Context, opts Options) (*Result, error) { + src, err := ParseSource(opts.URL, opts.Ref) + if err != nil { + return nil, err + } + runner := opts.Run + if runner == nil { + runner = ExecRunner{} + } + + content, err := fetchContent(ctx, runner, src) + if err != nil { + return nil, fmt.Errorf("%w%s", err, splitHint(src, err)) + } + if provenance.Present(content) { + return nil, fmt.Errorf( + "%s carries a %s line, so modelith reads it as somebody else's copy rather than a model's home. If it is a copy, vendor it from the origin its header names instead, so this repository tracks the model's home. If it is not — the line is an ordinary comment that happens to use modelith's reserved prefix at column zero — it has to be indented or removed at the origin before this file can be vendored", + opts.URL, provenance.LinePrefix) + } + m, err := model.Parse(content) + if err != nil { + return nil, fmt.Errorf( + "%s did not parse as a domain model — check that the URL names a *.modelith.yaml file, and that this modelith is new enough to read it: %w", + opts.URL, err) + } + if m.Kind != "DomainModel" { + declares := fmt.Sprintf("declares kind %q", m.Kind) + if m.Kind == "" { + declares = "declares no kind" + } + return nil, fmt.Errorf("%s is not a domain model — it %s, not \"DomainModel\"", opts.URL, declares) + } + + commit, err := fetchCommit(ctx, runner, src) + if err != nil { + return nil, err + } + + h := &provenance.Header{ + Vendored: provenance.Banner, + Fetch: "git", + Origin: src.Origin, + Path: src.Path, + Ref: src.Ref, + Commit: commit, + Imported: opts.Now.Format("2006-01-02"), + Digest: provenance.Digest(content), + } + + target := filepath.Join(opts.Dir, path.Base(src.Path)) + replaced, err := guardTarget(target, src) + if err != nil { + return nil, err + } + if err := os.WriteFile(target, provenance.Stamp(content, h), 0o644); err != nil { + return nil, fmt.Errorf("writing %s: %w", target, err) + } + + res := &Result{Path: target, Header: h, Replaced: replaced} + for _, imp := range m.Imports { + res.TheirImports = append(res.TheirImports, imp.Path) + } + return res, nil +} + +// guardTarget decides whether writing to target is safe, and reports whether an +// existing copy is being replaced. +// +// The destination filename comes from the origin, so it can collide with a file +// this repository already has: a model of its own, or a copy of a different +// model that happens to share a basename. Import refuses to fetch a file that is +// already somebody else's copy; the same care is owed to what it writes over, +// because a model this repository wrote is not recoverable by fetching again. +func guardTarget(target string, src Source) (replaced bool, err error) { + existing, readErr := os.ReadFile(target) + if errors.Is(readErr, fs.ErrNotExist) { + return false, nil + } + if readErr != nil { + return false, fmt.Errorf("reading %s: %w", target, readErr) + } + if !provenance.Present(existing) { + return false, fmt.Errorf( + "%s already exists and carries no provenance header, so it is a model this repository owns rather than a copy of one — importing would overwrite it. Import into a different directory, or move that file aside first", + target) + } + // A header too malformed to name where it came from is still a vendored + // copy, and replacing it is how it gets repaired; only a header that names + // a *different* model blocks the write. + h, _ := provenance.Parse(existing) + if h.Origin == "" || h.Path == "" { + return true, nil + } + switch { + // GitHub treats an owner and a repository name case-insensitively, and + // Origin keeps whatever casing the URL was typed with, so comparing these + // byte-for-byte would refuse a refresh over nothing but capitalisation. + case !strings.EqualFold(h.Origin, src.Origin): + return false, fmt.Errorf( + "%s is a vendored copy of %s/%s, not of %s/%s — two different models share that filename. Import into a different directory so both can live here", + target, h.Origin, h.Path, src.Origin, src.Path) + case h.Path != src.Path: + // Same repository, different path: either the model moved upstream or + // this repository has two models from it sharing a basename. Only the + // user knows which, and the two remedies are different. + return false, fmt.Errorf( + "%s is a vendored copy of %s in %s, and you are importing %s from the same repository. If the model moved, delete %s and import again; if these are two different models, import into a different directory so both can live here", + target, h.Path, h.Origin, src.Path, target) + } + return true, nil +} + +// splitHint explains the one way ParseSource can be wrong about a URL it +// accepted. A browse URL gives no way to tell where a ref containing a slash +// ends and the path begins, so the split is taken at the first segment; a failed +// fetch is where that guess surfaces, as a 404 that looks like the file is +// simply not there. +// +// It is offered only for that failure. A missing gh, a rejected credential, an +// unreachable network — none of those say anything about the URL, and adding a +// paragraph about ref splitting to them would send the reader after the wrong +// problem. A single-segment path had nothing to lose to the ref, so it gets no +// hint either. +func splitHint(src Source, err error) string { + if !strings.Contains(src.Path, "/") || !isNotFound(err) { + return "" + } + return fmt.Sprintf( + "\n\nmodelith read %q as the ref and %q as the path. If the ref has a slash in it that split is wrong: pass the whole ref to --ref, or — if --ref is already pinning a different ref, which cannot re-split the path — open the file on the ref you want and import that URL instead", + src.Ref, src.Path) +} + +// isNotFound reports whether err is gh saying the endpoint does not exist. +// +// It matches gh's text because gh is a separate program: it reports the status +// on stderr and exits 1, so there is no typed error to unwrap. Reading it wrong +// costs a hint that should not have printed, or one that should have. +func isNotFound(err error) bool { + msg := err.Error() + return strings.Contains(msg, "404") || strings.Contains(msg, "Not Found") +} + +// fetchContent returns the file's bytes. The raw media type asks the API for +// the content itself rather than a JSON envelope carrying it base64-encoded, so +// nothing here has to decode. +func fetchContent(ctx context.Context, runner Runner, src Source) ([]byte, error) { + endpoint := fmt.Sprintf("repos/%s/%s/contents/%s?ref=%s", + src.Owner, src.Repo, escapePath(src.Path), url.QueryEscape(src.Ref)) + out, err := runner.Run(ctx, "gh", "api", "-H", "Accept: application/vnd.github.raw", endpoint) + if err != nil { + return nil, err + } + return out, nil +} + +// fetchCommit returns the commit that last touched the file at this ref. +// +// That, rather than the head of the ref, is what identifies the version of this +// file: it does not move when unrelated commits land on the branch, so a later +// freshness check reports the model changing rather than the repository being +// busy. +func fetchCommit(ctx context.Context, runner Runner, src Source) (string, error) { + endpoint := fmt.Sprintf("repos/%s/%s/commits?path=%s&sha=%s&per_page=1", + src.Owner, src.Repo, url.QueryEscape(src.Path), url.QueryEscape(src.Ref)) + out, err := runner.Run(ctx, "gh", "api", endpoint, "--jq", ".[0].sha") + if err != nil { + return "", err + } + sha := strings.TrimSpace(string(out)) + if sha == "" || sha == "null" { + return "", fmt.Errorf("%s/%s has no commit touching %q at %q", src.Owner, src.Repo, src.Path, src.Ref) + } + return sha, nil +} + +// escapePath escapes each segment of a repository path, leaving the separators +// alone so the API still sees a path. +func escapePath(p string) string { + segments := strings.Split(p, "/") + for i, s := range segments { + segments[i] = url.PathEscape(s) + } + return strings.Join(segments, "/") +} diff --git a/internal/deps/deps_test.go b/internal/deps/deps_test.go new file mode 100644 index 0000000..ce3a43d --- /dev/null +++ b/internal/deps/deps_test.go @@ -0,0 +1,540 @@ +package deps + +import ( + "context" + "fmt" + "os" + "path/filepath" + "strings" + "testing" + "time" + + "github.com/stacklok/modelith/internal/provenance" +) + +// fakeRunner answers the gh calls Import makes from a map keyed by the API +// endpoint. A hand-written fake rather than a mock: it behaves like gh, +// answering the two endpoints it knows and failing the way gh fails on +// anything else, so a test cannot accidentally assert a response the real +// command could never produce. +type fakeRunner struct { + content string + sha string + // calls records every argv, so a test can assert what was executed. + calls [][]string + // fail, when set, is returned for any call whose endpoint contains it. + fail string +} + +func (f *fakeRunner) Run(_ context.Context, name string, args ...string) ([]byte, error) { + f.calls = append(f.calls, append([]string{name}, args...)) + endpoint := args[len(args)-1] + for _, a := range args { + if strings.HasPrefix(a, "repos/") { + endpoint = a + } + } + if f.fail != "" && strings.Contains(endpoint, f.fail) { + return nil, fmt.Errorf("gh: HTTP 404: Not Found (%s)", endpoint) + } + switch { + case strings.Contains(endpoint, "/contents/"): + return []byte(f.content), nil + case strings.Contains(endpoint, "/commits"): + return []byte(f.sha + "\n"), nil + } + return nil, fmt.Errorf("gh: unexpected endpoint %q", endpoint) +} + +const upstream = `# yaml-language-server: $schema=https://modelith.sh/schema/domain-model/v1.json +kind: DomainModel +version: v1 +title: Payments +enums: + PaymentMethod: + values: + - name: card +` + +const blobURL = "https://github.com/acme/billing/blob/main/docs/payments.modelith.yaml" + +const sha = "4f2c1e9c8b3ad0e5f71b2c9a6d4e8f30ab5c7d21" + +func TestParseSource(t *testing.T) { + t.Parallel() + + cases := []struct { + name string + raw string + ref string + want Source + wantErr string + }{ + { + name: "a browser blob URL", + raw: blobURL, + want: Source{ + Origin: "https://github.com/acme/billing", Owner: "acme", Repo: "billing", + Ref: "main", Path: "docs/payments.modelith.yaml", + }, + }, + { + name: "a query and anchor are not part of the address", + raw: blobURL + "?plain=1#L12", + want: Source{ + Origin: "https://github.com/acme/billing", Owner: "acme", Repo: "billing", + Ref: "main", Path: "docs/payments.modelith.yaml", + }, + }, + { + name: "an explicit ref overrides the one in the URL", + raw: blobURL, + ref: "v2.1.0", + want: Source{ + Origin: "https://github.com/acme/billing", Owner: "acme", Repo: "billing", + Ref: "v2.1.0", Path: "docs/payments.modelith.yaml", + }, + }, + { + // The URL alone cannot say where a slashed ref ends and the path + // begins; naming the ref settles it. + name: "an explicit slashed ref splits the rest correctly", + raw: "https://github.com/acme/billing/blob/release/v2/docs/payments.modelith.yaml", + ref: "release/v2", + want: Source{ + Origin: "https://github.com/acme/billing", Owner: "acme", Repo: "billing", + Ref: "release/v2", Path: "docs/payments.modelith.yaml", + }, + }, + { + // A host is case-insensitive. Rejecting this one produced the + // self-contradictory "only from github.com, and … is on GitHub.com". + name: "the host is matched without regard to case or a www prefix", + raw: "https://WWW.GitHub.com/acme/billing/blob/main/docs/payments.modelith.yaml", + want: Source{ + Origin: "https://github.com/acme/billing", Owner: "acme", Repo: "billing", + Ref: "main", Path: "docs/payments.modelith.yaml", + }, + }, + { + // url.Parse does not remove dot segments, and escapePath cannot + // neutralise a segment that *is* a traversal — it would reach the + // API endpoint intact and leave the contents namespace. + name: "a traversal segment is not an address github.com serves", + raw: "https://github.com/acme/billing/blob/main/../../../user/repos", + wantErr: `has a ".." path segment`, + }, + { + name: "an empty segment is rejected too", + raw: "https://github.com/acme/billing/blob/main//payments.modelith.yaml", + wantErr: `has a "" path segment`, + }, + { + name: "another host asks for an issue rather than guessing", + raw: "https://gitlab.com/acme/billing/-/blob/main/payments.modelith.yaml", + wantErr: "github.com/stacklok/modelith/issues", + }, + { + name: "a repository URL names no file", + raw: "https://github.com/acme/billing", + wantErr: "not a GitHub file URL", + }, + { + name: "a tree URL is not a file URL", + raw: "https://github.com/acme/billing/tree/main/docs", + wantErr: "not a GitHub file URL", + }, + } + for _, tc := range cases { + t.Run(tc.name, func(t *testing.T) { + t.Parallel() + got, err := ParseSource(tc.raw, tc.ref) + if tc.wantErr != "" { + if err == nil || !strings.Contains(err.Error(), tc.wantErr) { + t.Fatalf("want an error containing %q, got %v", tc.wantErr, err) + } + return + } + if err != nil { + t.Fatal(err) + } + if got != tc.want { + t.Errorf("ParseSource() = %+v, want %+v", got, tc.want) + } + }) + } +} + +func importInto(t *testing.T, dir string, r *fakeRunner, url string) (*Result, error) { + t.Helper() + return Import(context.Background(), Options{ + URL: url, + Dir: dir, + Now: time.Date(2026, 7, 27, 12, 0, 0, 0, time.Local), + Run: r, + }) +} + +func TestImport_StampsAVerifiableCopy(t *testing.T) { + t.Parallel() + + dir := t.TempDir() + r := &fakeRunner{content: upstream, sha: sha} + res, err := importInto(t, dir, r, blobURL) + if err != nil { + t.Fatal(err) + } + + written, err := os.ReadFile(res.Path) + if err != nil { + t.Fatal(err) + } + if got, want := res.Path, filepath.Join(dir, "payments.modelith.yaml"); got != want { + t.Errorf("wrote %s, want %s", got, want) + } + if res.Replaced { + t.Error("reported replacing a file that did not exist") + } + + h, problems := provenance.Parse(written) + if len(problems) != 0 { + t.Fatalf("the stamped copy has header problems: %+v", problems) + } + want := provenance.Header{ + Vendored: provenance.Banner, + Fetch: "git", + Origin: "https://github.com/acme/billing", + Path: "docs/payments.modelith.yaml", + Ref: "main", + Commit: sha, + Imported: "2026-07-27", + Digest: provenance.Digest([]byte(upstream)), + } + if *h != want { + t.Errorf("stamped header = %+v, want %+v", *h, want) + } + if ok, got := h.Verify(written); !ok { + t.Errorf("the freshly written copy does not verify: computed %s", got) + } + + // The editor directive stays first; the model survives byte for byte. + if !strings.HasPrefix(string(written), "# yaml-language-server:") { + t.Error("the editor directive is no longer the first line") + } + if !strings.Contains(string(written), " PaymentMethod:\n") { + t.Error("the model content did not survive the stamp") + } +} + +// TestImport_CallsGhWithTheExpectedEndpoints asserts the whole argv of both +// calls, not that they merely contain something. The endpoints are the contract +// with gh — a ref silently dropped from the content fetch would return whatever +// the default branch holds, and the copy would be stamped with a ref it is not +// actually at. +// +// Nothing here goes through a shell; that is a property of ExecRunner passing +// an argv array to os/exec, which no assertion about argument text could show. +func TestImport_CallsGhWithTheExpectedEndpoints(t *testing.T) { + t.Parallel() + + r := &fakeRunner{content: upstream, sha: sha} + if _, err := importInto(t, t.TempDir(), r, blobURL); err != nil { + t.Fatal(err) + } + want := [][]string{ + {"gh", "api", "-H", "Accept: application/vnd.github.raw", + "repos/acme/billing/contents/docs/payments.modelith.yaml?ref=main"}, + {"gh", "api", + "repos/acme/billing/commits?path=docs%2Fpayments.modelith.yaml&sha=main&per_page=1", + "--jq", ".[0].sha"}, + } + if len(r.calls) != len(want) { + t.Fatalf("want %d gh calls, got %d: %+v", len(want), len(r.calls), r.calls) + } + for i, call := range r.calls { + if strings.Join(call, "\x00") != strings.Join(want[i], "\x00") { + t.Errorf("call %d:\n got %q\nwant %q", i, call, want[i]) + } + } +} + +// TestImport_EscapesAPathThatNeedsIt pins that a path segment reaches the API +// escaped rather than as a second query parameter or a truncated path. +func TestImport_EscapesAPathThatNeedsIt(t *testing.T) { + t.Parallel() + + r := &fakeRunner{content: upstream, sha: sha} + url := "https://github.com/acme/billing/blob/main/docs/a b&c/payments.modelith.yaml" + if _, err := importInto(t, t.TempDir(), r, url); err != nil { + t.Fatal(err) + } + const wantContents = "repos/acme/billing/contents/docs/a%20b&c/payments.modelith.yaml?ref=main" + if got := r.calls[0][len(r.calls[0])-1]; got != wantContents { + t.Errorf("content endpoint = %q, want %q", got, wantContents) + } +} + +// TestADR_0015_ImportFetchesOneFileNotATree pins that vendoring does not +// recurse. The fetched model's own imports are reported to the user and +// nothing more: no extra call goes out, so no file arrives that the user did +// not ask for, at a scope this repository never bound. +func TestADR_0015_ImportFetchesOneFileNotATree(t *testing.T) { + t.Parallel() + + chained := upstream + "imports:\n - ./ledger.modelith.yaml\n - ./tax.modelith.yaml\n" + r := &fakeRunner{content: chained, sha: sha} + res, err := importInto(t, t.TempDir(), r, blobURL) + if err != nil { + t.Fatal(err) + } + want := []string{"./ledger.modelith.yaml", "./tax.modelith.yaml"} + if strings.Join(res.TheirImports, ",") != strings.Join(want, ",") { + t.Errorf("TheirImports = %v, want %v", res.TheirImports, want) + } + // Two calls and no more: the imports were reported, not followed. + if len(r.calls) != 2 { + t.Errorf("want two gh calls, got %d: %+v", len(r.calls), r.calls) + } +} + +func TestImport_Rejections(t *testing.T) { + t.Parallel() + + cases := []struct { + name string + runner *fakeRunner + url string + wantErr string + }{ + { + name: "a file that is already a vendored copy", + runner: &fakeRunner{content: "# modelith-origin: https://github.com/other/repo\n" + upstream, sha: sha}, + url: blobURL, + wantErr: "reads it as somebody else's copy", + }, + { + name: "a file that is not a domain model", + runner: &fakeRunner{content: "kind: SomethingElse\nversion: v1\n", sha: sha}, + url: blobURL, + wantErr: `it declares kind "SomethingElse"`, + }, + { + name: "a file with no kind at all", + runner: &fakeRunner{content: "title: Payments\n", sha: sha}, + url: blobURL, + wantErr: "it declares no kind", + }, + { + // A model authored by a newer modelith parses strictly and fails. + // Collapsing that into "is not a domain model" sent the user to + // check the URL, which is not what went wrong. + name: "a model this build cannot read says what the parser said", + runner: &fakeRunner{content: upstream + "futureField: yes\n", sha: sha}, + url: blobURL, + wantErr: "futureField", + }, + { + name: "a path with no commits at that ref", + runner: &fakeRunner{content: upstream, sha: "null"}, + url: blobURL, + wantErr: "no commit touching", + }, + { + name: "gh refusing the fetch", + runner: &fakeRunner{content: upstream, sha: sha, fail: "/contents/"}, + url: blobURL, + wantErr: "HTTP 404", + }, + { + // The likeliest reason a fetch 404s on a URL that parsed is that + // the ref has a slash in it and the path was split at the first + // segment. Saying so is the difference between a dead end and a fix. + name: "a refused fetch says how the URL was split", + runner: &fakeRunner{content: upstream, sha: sha, fail: "/contents/"}, + url: blobURL, + wantErr: `read "main" as the ref and "docs/payments.modelith.yaml" as the path`, + }, + } + for _, tc := range cases { + t.Run(tc.name, func(t *testing.T) { + t.Parallel() + dir := t.TempDir() + _, err := importInto(t, dir, tc.runner, tc.url) + if err == nil || !strings.Contains(err.Error(), tc.wantErr) { + t.Fatalf("want an error containing %q, got %v", tc.wantErr, err) + } + entries, readErr := os.ReadDir(dir) + if readErr != nil { + t.Fatal(readErr) + } + if len(entries) != 0 { + t.Errorf("a rejected import left %d file(s) behind", len(entries)) + } + }) + } +} + +// TestImport_RefusesToOverwriteWhatItDidNotWrite pins the guard on the +// destination. The filename comes from the origin, so it can land on a file this +// repository already has — and overwriting a model this repository wrote loses +// work no re-fetch can recover, which the earlier os.Stat-only check did +// silently, reporting it as an ordinary "replaced". +func TestImport_RefusesToOverwriteWhatItDidNotWrite(t *testing.T) { + t.Parallel() + + const own = "kind: DomainModel\nversion: v1\ntitle: Ours\n" + + cases := []struct { + name string + existing string + wantErr string + }{ + { + name: "a model this repository owns", + existing: own, + wantErr: "carries no provenance header", + }, + { + name: "a copy of a different model with the same filename", + existing: "# modelith-vendored: " + provenance.Banner + "\n" + + "# modelith-fetch: git\n" + + "# modelith-origin: https://github.com/other/repo\n" + + "# modelith-path: docs/payments.modelith.yaml\n" + + "# modelith-ref: main\n# modelith-commit: abc\n" + + "# modelith-imported: 2026-07-27\n" + + "# modelith-digest: " + provenance.Digest([]byte(own)) + "\n" + own, + wantErr: "is a vendored copy of https://github.com/other/repo", + }, + } + for _, tc := range cases { + t.Run(tc.name, func(t *testing.T) { + t.Parallel() + dir := t.TempDir() + target := filepath.Join(dir, "payments.modelith.yaml") + if err := os.WriteFile(target, []byte(tc.existing), 0o644); err != nil { + t.Fatal(err) + } + _, err := importInto(t, dir, &fakeRunner{content: upstream, sha: sha}, blobURL) + if err == nil || !strings.Contains(err.Error(), tc.wantErr) { + t.Fatalf("want an error containing %q, got %v", tc.wantErr, err) + } + got, readErr := os.ReadFile(target) + if readErr != nil { + t.Fatal(readErr) + } + if string(got) != tc.existing { + t.Errorf("the refused import wrote over the file anyway:\n%s", got) + } + }) + } +} + +// TestSplitHint pins that the ref/path hint is offered for the failure it +// explains and no other. It was appended to every fetch error, so "gh is not +// installed" arrived with a paragraph about ref splitting attached — advice +// about a URL that was never in question. +func TestSplitHint(t *testing.T) { + t.Parallel() + + src := Source{Ref: "main", Path: "docs/payments.modelith.yaml"} + cases := []struct { + name string + src Source + err error + want bool + }{ + {"a not-found", src, fmt.Errorf("gh: HTTP 404: Not Found"), true}, + {"gh missing", src, fmt.Errorf("gh is not installed — modelith delegates fetching to it"), false}, + {"a rejected credential", src, fmt.Errorf("gh: HTTP 401: Bad credentials"), false}, + {"a forbidden repository", src, fmt.Errorf("gh: HTTP 403: Forbidden"), false}, + {"an unreachable network", src, fmt.Errorf("dial tcp: lookup api.github.com: no such host"), false}, + {"a single-segment path has nothing to lose to the ref", + Source{Ref: "main", Path: "payments.modelith.yaml"}, fmt.Errorf("gh: HTTP 404: Not Found"), false}, + } + for _, tc := range cases { + t.Run(tc.name, func(t *testing.T) { + t.Parallel() + if got := splitHint(tc.src, tc.err) != ""; got != tc.want { + t.Errorf("splitHint offered = %v, want %v", got, tc.want) + } + }) + } +} + +// TestImport_RefreshIsNotRefusedOverCasing pins that guardTarget compares an +// origin the way GitHub does. Owner and repository names are case-insensitive +// there, and Source.Origin keeps whatever casing the URL was typed with, so a +// byte-for-byte comparison refused a legitimate refresh — and explained it by +// claiming two different models shared the filename. +func TestImport_RefreshIsNotRefusedOverCasing(t *testing.T) { + t.Parallel() + + dir := t.TempDir() + if _, err := importInto(t, dir, &fakeRunner{content: upstream, sha: sha}, + "https://github.com/Acme/Billing/blob/main/docs/payments.modelith.yaml"); err != nil { + t.Fatal(err) + } + res, err := importInto(t, dir, &fakeRunner{content: upstream + " # newer\n", sha: "9" + sha[1:]}, blobURL) + if err != nil { + t.Fatalf("a refresh differing only in the origin's casing was refused: %v", err) + } + if !res.Replaced { + t.Error("did not report replacing the earlier copy") + } +} + +// TestImport_RefusesAMovedPathWithTheRightRemedy pins the other half: the same +// repository at a different path is ambiguous — the model moved, or two models +// there share a basename — and the two remedies differ. Offering only "import +// into a different directory" left the user of a moved model with no way +// forward the message named. +func TestImport_RefusesAMovedPathWithTheRightRemedy(t *testing.T) { + t.Parallel() + + dir := t.TempDir() + if _, err := importInto(t, dir, &fakeRunner{content: upstream, sha: sha}, blobURL); err != nil { + t.Fatal(err) + } + _, err := importInto(t, dir, &fakeRunner{content: upstream, sha: sha}, + "https://github.com/acme/billing/blob/main/models/payments.modelith.yaml") + if err == nil { + t.Fatal("a second model from the same repository silently replaced the first") + } + for _, want := range []string{"If the model moved, delete", "import into a different directory"} { + if !strings.Contains(err.Error(), want) { + t.Errorf("the error does not offer %q: %v", want, err) + } + } +} + +func TestImport_ReplacesAnEarlierCopy(t *testing.T) { + t.Parallel() + + dir := t.TempDir() + r := &fakeRunner{content: upstream, sha: sha} + if _, err := importInto(t, dir, r, blobURL); err != nil { + t.Fatal(err) + } + moved := &fakeRunner{content: upstream + " # a later revision\n", sha: "9" + sha[1:]} + res, err := importInto(t, dir, moved, blobURL) + if err != nil { + t.Fatal(err) + } + if !res.Replaced { + t.Error("did not report replacing the earlier copy") + } + written, err := os.ReadFile(res.Path) + if err != nil { + t.Fatal(err) + } + h, _ := provenance.Parse(written) + if h.Commit != moved.sha { + t.Errorf("commit is %s, want the newer %s", h.Commit, moved.sha) + } + if ok, _ := h.Verify(written); !ok { + t.Error("the replaced copy does not verify against its own digest") + } + if strings.Count(string(written), provenance.LinePrefix+"origin:") != 1 { + t.Error("the replaced copy carries more than one header") + } +} diff --git a/internal/lint/imports.go b/internal/lint/imports.go index a4f2d5e..247aac0 100644 --- a/internal/lint/imports.go +++ b/internal/lint/imports.go @@ -67,8 +67,11 @@ type importedModel struct { // in an entity position (relationship.entity, subtypeOf) — unsupported there, // but still a real reference: an import bound to one of them is not also // reported as unreferenced (see reportQualifiedEntityRefs). -func runImports(modelPath string, m *model.Model, files Files, res *Result, entityScopes map[string]bool) { - byScope, claimed := loadImports(modelPath, m, files, res) +// +// vendored says the model is a copy whose home is another repository, which +// silences the errors its imports list would raise here (see loadImports). +func runImports(modelPath string, m *model.Model, files Files, res *Result, entityScopes map[string]bool, vendored bool) { + byScope, claimed := loadImports(modelPath, m, files, res, vendored) used := checkQualifiedTypes(m, byScope, claimed, res) // An unreferenced import is a completeness finding, alongside the unused // enum and the unused glossary term: vocabulary the model declares and @@ -96,7 +99,15 @@ func runImports(modelPath string, m *model.Model, files Files, res *Result, enti // list claimed at all — including entries that then failed to load — mapped to // the path of the first entry that claimed it. Every rejection is reported as an // error: an import that cannot be resolved is a broken reference, not a gap. -func loadImports(modelPath string, m *model.Model, files Files, res *Result) (byScope map[string]importedModel, claimed map[string]string) { +// +// A vendored model is the exception, and it reports nothing here at all. Its +// imports name paths in the repository it came from, which do not exist in this +// one, so every entry would fail for a reason its authors cannot see and this +// repository is not expected to fix. Resolution is still attempted, because two +// vendored peers that import each other do resolve here; and every entry still +// claims its scope, which is what keeps a reference into an import that did not +// load from reading as a reference to nothing (see checkQualifiedTypes). +func loadImports(modelPath string, m *model.Model, files Files, res *Result, vendored bool) (byScope map[string]importedModel, claimed map[string]string) { byScope = map[string]importedModel{} claimed = map[string]string{} if len(m.Imports) == 0 { @@ -106,6 +117,9 @@ func loadImports(modelPath string, m *model.Model, files Files, res *Result) (by root, inRepo := files.ResolutionRoot(modelPath) for i, imp := range m.Imports { reject := func(format string, args ...any) { + if vendored { + return + } res.Findings = append(res.Findings, Finding{ Severity: SeverityError, Category: CategorySemantic, @@ -122,9 +136,11 @@ func loadImports(modelPath string, m *model.Model, files Files, res *Result) (by // a duplicate binding is then reported on its own terms instead of being // pre-empted by the second entry's unrelated trouble, and a broken path // does not also make every "scope.Name" that names it look like a - // reference to nothing. A written scope is held to the pattern by the - // schema; a derived one the schema never sees. - scopeOK := !imp.ScopeFromPath || scopeRE.MatchString(imp.Scope) + // reference to nothing. A written scope is held to this same pattern by + // the schema, which runs before this layer and gates it, so a scope that + // fails here was derived from the filename — which is what the advice + // below assumes. TestInvariant_ScopeSlugMatchesSchema guards the two. + scopeOK := scopeRE.MatchString(imp.Scope) if scopeOK { if prev, dup := claimed[imp.Scope]; dup { reject("import %q binds scope %q, which import %q already binds — give one of them an explicit, different scope so %s.Name resolves unambiguously", @@ -256,13 +272,38 @@ func checkQualifiedTypes(m *model.Model, byScope map[string]importedModel, claim broken("attribute type %q resolves to the entity %q in %q — only an enum can be referenced across models", attr.Type, item, imp.path) continue } - broken("attribute type %q names no enum %q in %q — an imported model's own imports are not reachable from here", attr.Type, item, imp.path) + broken("%s", unresolvedItemMessage(attr.Type, item, imp)) } } reportUnboundScopes(unbound, res) return used } +// unresolvedItemMessage explains a qualified type whose scope resolved but +// whose item is not there. +// +// The interesting case is an imported model that has imports of its own: the +// item may well be defined in one of them, and nothing about the reference site +// says why that does not resolve. Vendoring fetches one file and resolution +// reaches one hop (ADR-0010, ADR-0015), so the answer is always to import that +// model here as well and give it its own scope — never to expect this one to +// follow the chain. Saying so only when the imported model actually has imports +// keeps the advice off the plain case, where the mistake is a typo. +func unresolvedItemMessage(typ, item string, imp importedModel) string { + msg := fmt.Sprintf("attribute type %q names no enum %q in %q", typ, item, imp.path) + n := len(imp.model.Imports) + if n == 0 { + return msg + " — check the name, or whether you meant to import a different model" + } + theirs := fmt.Sprintf("%d models of its own", n) + if n == 1 { + theirs = "a model of its own" + } + return msg + fmt.Sprintf( + " — that model imports %s, and resolution is not transitive: if %q is defined in one of them, add that model to this model's `imports:` too and reference it with its own scope", + theirs, item) +} + // unboundScope accumulates the references to one scope no import binds, so a // single missing import is reported once rather than once per reference site. type unboundScope struct { diff --git a/internal/lint/imports_test.go b/internal/lint/imports_test.go index 4802b4f..8bc913e 100644 --- a/internal/lint/imports_test.go +++ b/internal/lint/imports_test.go @@ -67,6 +67,12 @@ entities: definition: A request for payment. ` +// chainedModel is an imported model that imports something itself — the case +// where an unresolved item might really be one hop further away. +const chainedModel = paymentsModel + `imports: + - ./payments.modelith.yaml +` + // importer builds a model that lists the given imports and types one attribute // with the given type. Each entry is written verbatim as a YAML sequence item, // so a case can use either the bare-path form or the explicit {scope, path} one. @@ -142,6 +148,8 @@ func TestImports_Resolution(t *testing.T) { "docs/shipping.modelith.yml": paymentsModel, "payments/payments.modelith.yaml": paymentsModel, "docs/legacy/pay-v2.modelith.yaml": paymentsModel, + "docs/PayMents.modelith.yaml": paymentsModel, + "docs/chained.modelith.yaml": chainedModel, "docs/not-a-model.yaml": "kind: SomethingElse\nversion: v1\n", } @@ -219,6 +227,27 @@ func TestImports_Resolution(t *testing.T) { `names no enum "Nonexistent"`, }}, }, + { + // The plain case advises a typo; the chained case below advises + // the import. Getting the wrong one is the whole failure mode. + name: "an unresolved item in a leaf model points at the name", + imports: []string{`"./payments.modelith.yaml"`}, + attrType: "payments.Nonexistent", + want: []wantFinding{{ + SeverityError, CategorySemantic, typePath, + "check the name, or whether you meant to import a different model", + }}, + }, + { + name: "an unresolved item in a model that imports others explains non-transitivity", + imports: []string{"{scope: payments, path: ./chained.modelith.yaml}"}, + attrType: "payments.Nonexistent", + want: []wantFinding{{ + SeverityError, CategorySemantic, typePath, + `that model imports a model of its own, and resolution is not transitive: ` + + `if "Nonexistent" is defined in one of them, add that model to this model's ` + "`imports:`" + ` too`, + }}, + }, { name: "qualified type resolving to an entity, not an enum", imports: []string{`"./payments.modelith.yaml"`}, @@ -233,6 +262,15 @@ func TestImports_Resolution(t *testing.T) { imports: []string{"{scope: billing, path: ./legacy/pay-v2.modelith.yaml}"}, attrType: "billing.PaymentMethod", }, + { + // The complement of "bare import whose filename is not a usable + // slug": the same file, rescued by the explicit form. A written + // scope is validated by the schema, so the slug the filename would + // have yielded is never consulted. + name: "explicit scope rescues a filename that is not a usable slug", + imports: []string{"{scope: payments, path: ./PayMents.modelith.yaml}"}, + attrType: "payments.PaymentMethod", + }, { name: "two imports binding the same scope", imports: []string{`"./payments.modelith.yaml"`, `"../payments/payments.modelith.yaml"`}, diff --git a/internal/lint/lint.go b/internal/lint/lint.go index 9b72935..f63e2d8 100644 --- a/internal/lint/lint.go +++ b/internal/lint/lint.go @@ -87,6 +87,11 @@ func Run(path string, src []byte, files Files) (*Result, error) { files = OSFiles{} } + // Whether this file is somebody else's copy is settled first: it decides + // which of the layers below are findings about a model this repository + // controls, and it holds even when the file does not parse. + vendored := runProvenance(src, res) + // Layer 1: structural validation against the JSON Schema. structuralOK, entityScopes := runStructural(src, res) @@ -105,6 +110,9 @@ func Run(path string, src []byte, files Files) (*Result, error) { Message: err.Error(), }) } + if vendored { + dropOwnedDiagnostics(res) + } sortFindings(res) return res, nil } @@ -117,7 +125,7 @@ func Run(path string, src []byte, files Files) (*Result, error) { // in an entity position is not one of those rejections (see runStructural), // so it does not take the imports layer down with it. if structuralOK { - runImports(path, m, files, res, entityScopes) + runImports(path, m, files, res, entityScopes, vendored) } runRelationshipShape(m, res) runSubtypes(m, res) @@ -125,6 +133,9 @@ func Run(path string, src []byte, files Files) (*Result, error) { runPairing(m, res) runCompleteness(m, res) + if vendored { + dropOwnedDiagnostics(res) + } sortFindings(res) return res, nil } diff --git a/internal/lint/provenance.go b/internal/lint/provenance.go new file mode 100644 index 0000000..12c0593 --- /dev/null +++ b/internal/lint/provenance.go @@ -0,0 +1,81 @@ +package lint + +import ( + "fmt" + + "github.com/stacklok/modelith/internal/provenance" +) + +// runProvenance reports defects in the file's provenance header and verifies +// the copy against the digest that header records. It returns whether the file +// is vendored — a copy of a model whose home is another repository. +// +// A vendored file is not this repo's work, so the diagnostics that are findings +// about a model its authors control do not apply to it. What that suppresses is +// exactly two things, and no more (ADR-0015): the completeness category, in +// dropOwnedDiagnostics, and the errors its own imports list would raise, in +// loadImports. Structural and semantic checks still run: a vendored file that is +// not a valid domain model breaks this repo's build and is this repo's problem. +func runProvenance(src []byte, res *Result) bool { + h, problems := provenance.Parse(src) + if h == nil { + return false + } + for _, p := range problems { + msg := p.Message + if p.Line > 0 { + msg = fmt.Sprintf("line %d: %s", p.Line, msg) + } + res.Findings = append(res.Findings, Finding{ + Severity: SeverityError, + Category: CategorySemantic, + Path: "", + Message: "provenance header: " + msg, + }) + } + // A digest that is not in the recorded shape is already reported above. + // Comparing against it would report the same mistake a second time, as a + // mismatch it cannot help but produce. + if provenance.ValidDigest(h.Digest) { + if ok, got := h.Verify(src); !ok { + res.Findings = append(res.Findings, Finding{ + Severity: SeverityError, + Category: CategorySemantic, + Path: "", + Message: fmt.Sprintf( + "this vendored file no longer matches the digest its provenance header records (recorded %s, computed %s) — it has been edited since it was imported. Refresh it with `modelith deps import %s`, or delete the provenance header if the change is a deliberate fork, which makes this repository the file's home", + h.Digest, got, refreshTarget(h)), + }) + } + } + return true +} + +// refreshTarget is what to hand `modelith deps import` to replace this copy. For +// a git origin that is the blob URL the header's parts describe; for anything +// else the origin alone, which is all such a method records. +func refreshTarget(h *provenance.Header) string { + if h.Fetch == "git" && h.Origin != "" && h.Ref != "" && h.Path != "" { + return fmt.Sprintf("%s/blob/%s/%s", h.Origin, h.Ref, h.Path) + } + return h.Origin +} + +// dropOwnedDiagnostics removes the findings that are about a model's own +// authoring rather than about this repository's use of it. They are exactly the +// completeness category: entities with no invariants, entities no scenario +// exercises, unused glossary terms, unused enums, and an import nothing +// references. +// +// Without this the Action this repo ships — which lints every globbed file with +// --completeness — turns a user's CI red for gaps in somebody else's document. +func dropOwnedDiagnostics(res *Result) { + kept := res.Findings[:0] + for _, f := range res.Findings { + if f.Category == CategoryCompleteness { + continue + } + kept = append(kept, f) + } + res.Findings = kept +} diff --git a/internal/lint/provenance_test.go b/internal/lint/provenance_test.go new file mode 100644 index 0000000..a9685ad --- /dev/null +++ b/internal/lint/provenance_test.go @@ -0,0 +1,245 @@ +package lint + +import ( + "strings" + "testing" + + "github.com/stacklok/modelith/internal/provenance" +) + +// stamp returns src as a vendored copy: a well-formed provenance header whose +// digest matches the content it is stamped into. +func stamp(t *testing.T, src string) string { + t.Helper() + h := &provenance.Header{ + Vendored: provenance.Banner, + Fetch: "git", + Origin: "https://github.com/stacklok/some-repo", + Path: "docs/payments.modelith.yaml", + Ref: "main", + Commit: "4f2c1e9c8b3ad0e5f71b2c9a6d4e8f30ab5c7d21", + Imported: "2026-07-27", + Digest: provenance.Digest([]byte(src)), + } + return string(provenance.Stamp([]byte(src), h)) +} + +// gappy is a model with one finding in each layer that a vendored file's status +// could plausibly touch: a semantic error (a relationship to an entity that is +// not defined), a semantic warning (a PascalCase type naming no enum), and +// completeness gaps (no invariants, and no scenario exercising the entity). +const gappy = `kind: DomainModel +version: v1 +entities: + Visit: + definition: One car's stay in the garage. + attributes: + - name: settledWith + type: PaymentMethod + relationships: + - entity: Nonexistent + cardinality: "n:1" + ownership: referenced +` + +func lintSource(t *testing.T, src string, files fakeFiles) *Result { + t.Helper() + res, err := Run(importerPath, []byte(src), files) + if err != nil { + t.Fatal(err) + } + return res +} + +// TestADR_0015_HeaderSuppressesCompletenessOnly pins ADR-0015's rule as a +// difference rather than as a hand-written expectation: the same model is linted +// twice, once as this repository's own work and once as a vendored copy, and +// every finding that disappears must be a completeness finding. +// +// Written this way the test cannot be satisfied by a suppression that is too +// broad. A rule that also dropped semantic warnings would pass a test that +// merely listed what a vendored file should report. +func TestADR_0015_HeaderSuppressesCompletenessOnly(t *testing.T) { + t.Parallel() + + files := fakeFiles{".git": ""} + own := lintSource(t, gappy, files) + vendored := lintSource(t, stamp(t, gappy), files) + + byKey := func(f Finding) string { return string(f.Category) + "|" + f.Path + "|" + f.Message } + + kept := map[string]bool{} + for _, f := range vendored.Findings { + kept[byKey(f)] = true + } + + var droppedCompleteness int + for _, f := range own.Findings { + if kept[byKey(f)] { + continue + } + if f.Category != CategoryCompleteness { + t.Errorf("a %s finding was suppressed on a vendored file: %s: %s", f.Category, f.Path, f.Message) + continue + } + droppedCompleteness++ + } + if droppedCompleteness == 0 { + t.Fatal("the fixture produced no completeness findings, so this proves nothing") + } + + // And nothing new appeared: a well-formed header is not itself a finding. + ownKeys := map[string]bool{} + for _, f := range own.Findings { + ownKeys[byKey(f)] = true + } + for _, f := range vendored.Findings { + if !ownKeys[byKey(f)] { + t.Errorf("vendoring added a finding: %s: %s", f.Path, f.Message) + } + } + + // The semantic error and warning both survive, which is the half of the + // rule that a broad suppression would break. + var errs, warns int + for _, f := range vendored.Findings { + switch f.Severity { + case SeverityError: + errs++ + case SeverityWarning: + warns++ + } + } + if errs == 0 || warns == 0 { + t.Errorf("vendored file reported %d error(s) and %d warning(s); both layers should still run", errs, warns) + } +} + +// TestVendored_ImportsRaiseNothing covers the second suppression and the +// downstream consequence that makes it worth anything: an unresolvable import +// is silent, and so is the reference that resolves through it. Without the +// second half, every vendored model that has imports of its own is broken here. +func TestVendored_ImportsRaiseNothing(t *testing.T) { + t.Parallel() + + files := fakeFiles{".git": ""} + src := importer([]string{`"./nowhere.modelith.yaml"`}, "nowhere.PaymentMethod") + + own := lintSource(t, src, files) + if len(importFindings(own.Findings)) == 0 { + t.Fatal("the unstamped fixture reported nothing, so this proves nothing") + } + + vendored := lintSource(t, stamp(t, src), files) + if got := importFindings(vendored.Findings); len(got) != 0 { + t.Errorf("a vendored file reported on its own imports: %+v", got) + } +} + +// TestVendored_ImportsStillResolveWhenTheyCan pins that suppressing the errors +// did not switch resolution off: two vendored peers that import each other are +// both present here, and a reference between them still has to resolve to a real +// enum — or be reported when it does not. +func TestVendored_ImportsStillResolveWhenTheyCan(t *testing.T) { + t.Parallel() + + files := fakeFiles{".git": "", "docs/payments.modelith.yaml": paymentsModel} + src := importer([]string{`"./payments.modelith.yaml"`}, "payments.Nonexistent") + + got := importFindings(lintSource(t, stamp(t, src), files).Findings) + if len(got) != 1 || !strings.Contains(got[0].Message, `names no enum "Nonexistent"`) { + t.Errorf("want the unresolved item reported, got %+v", got) + } +} + +func TestVendored_DigestMismatch(t *testing.T) { + t.Parallel() + + files := fakeFiles{".git": ""} + stamped := stamp(t, gappy) + + t.Run("an untouched copy verifies", func(t *testing.T) { + t.Parallel() + for _, f := range lintSource(t, stamped, files).Findings { + if strings.Contains(f.Message, "digest") { + t.Errorf("unexpected digest finding: %s", f.Message) + } + } + }) + + t.Run("an edited copy is an error naming both remedies", func(t *testing.T) { + t.Parallel() + edited := strings.Replace(stamped, "One car's stay in the garage.", "One car's stay.", 1) + if edited == stamped { + t.Fatal("the test did not edit the model") + } + res := lintSource(t, edited, files) + var found *Finding + for i := range res.Findings { + if strings.Contains(res.Findings[i].Message, "no longer matches the digest") { + found = &res.Findings[i] + break + } + } + if found == nil { + t.Fatal("no digest finding") + } + if found.Severity != SeverityError { + t.Errorf("digest mismatch is a %s, want an error", found.Severity) + } + for _, want := range []string{ + "modelith deps import https://github.com/stacklok/some-repo/blob/main/docs/payments.modelith.yaml", + "delete the provenance header", + } { + if !strings.Contains(found.Message, want) { + t.Errorf("digest message does not offer %q: %s", want, found.Message) + } + } + }) +} + +func TestVendored_HeaderProblemsAreReported(t *testing.T) { + t.Parallel() + + files := fakeFiles{".git": ""} + cases := []struct { + name string + header string + contains string + }{ + {"unknown key", "# modelith-surface: sha256:x\n", "unknown provenance key"}, + {"unknown fetch method", "# modelith-fetch: carrier-pigeon\n", `unknown fetch method "carrier-pigeon"`}, + {"missing required key", "# modelith-fetch: git\n", `missing "# modelith-commit"`}, + } + for _, tc := range cases { + t.Run(tc.name, func(t *testing.T) { + t.Parallel() + var found bool + for _, f := range lintSource(t, tc.header+gappy, files).Findings { + if strings.Contains(f.Message, tc.contains) { + if f.Severity != SeverityError { + t.Errorf("header problem is a %s, want an error", f.Severity) + } + found = true + } + } + if !found { + t.Errorf("no finding containing %q", tc.contains) + } + }) + } +} + +// TestVendored_MalformedHeaderStillSuppressesCompleteness pins that presence, +// not a clean parse, is what makes a file vendored. A typo in the header would +// otherwise be buried under gaps in a document this repository does not own. +func TestVendored_MalformedHeaderStillSuppressesCompleteness(t *testing.T) { + t.Parallel() + + files := fakeFiles{".git": ""} + for _, f := range lintSource(t, "# modelith-surface: sha256:x\n"+gappy, files).Findings { + if f.Category == CategoryCompleteness { + t.Errorf("completeness finding survived a malformed header: %s: %s", f.Path, f.Message) + } + } +} diff --git a/internal/model/model.go b/internal/model/model.go index 8efc5c1..62189af 100644 --- a/internal/model/model.go +++ b/internal/model/model.go @@ -57,11 +57,6 @@ const ScopeSlug = `[a-z][a-z0-9-]*` type Import struct { Scope string `json:"scope"` Path string `json:"path"` - // ScopeFromPath records that Scope came from Path's basename rather than - // being written out, so the linter can point at the explicit form when a - // filename yields an unusable slug. The schema validates an explicit scope - // itself; a derived one it never sees. - ScopeFromPath bool `json:"-"` } // UnmarshalJSON lets an import be written as a bare path ("./payments.modelith.yaml", @@ -77,7 +72,7 @@ func (i *Import) UnmarshalJSON(data []byte) error { if err := json.Unmarshal(trimmed, &s); err != nil { return err } - *i = Import{Scope: ScopeFromPath(s), Path: s, ScopeFromPath: true} + *i = Import{Scope: ScopeFromPath(s), Path: s} return nil } type rawImport Import diff --git a/internal/model/model_test.go b/internal/model/model_test.go index 5308147..9e51b12 100644 --- a/internal/model/model_test.go +++ b/internal/model/model_test.go @@ -185,7 +185,7 @@ entities: t.Fatalf("unexpected error: %v", err) } want := []Import{ - {Scope: "payments", Path: "./payments.modelith.yaml", ScopeFromPath: true}, + {Scope: "payments", Path: "./payments.modelith.yaml"}, {Scope: "billing", Path: "./legacy/pay-v2.modelith.yaml"}, } if len(m.Imports) != len(want) { diff --git a/internal/model/network_test.go b/internal/model/network_test.go index 104a111..4645a5d 100644 --- a/internal/model/network_test.go +++ b/internal/model/network_test.go @@ -28,6 +28,7 @@ func TestADR_0011_OfflinePackages(t *testing.T) { "github.com/stacklok/modelith/internal/render/mermaid", "github.com/stacklok/modelith/internal/schema", "github.com/stacklok/modelith/internal/model", + "github.com/stacklok/modelith/internal/provenance", } // net/url and net/netip are parsers that perform no I/O; the JSON Schema diff --git a/internal/provenance/provenance.go b/internal/provenance/provenance.go new file mode 100644 index 0000000..d707444 --- /dev/null +++ b/internal/provenance/provenance.go @@ -0,0 +1,296 @@ +// Package provenance reads and writes the comment header that marks a model +// file as a vendored copy of a model whose home is another repository. +package provenance + +import ( + "crypto/sha256" + "encoding/hex" + "fmt" + "regexp" + "slices" + "strings" +) + +// LinePrefix begins every provenance line. +// +// A line is *header* only when it starts at column zero and sits in the leading +// comment block. Parse reads exactly those lines and Strip removes exactly +// those lines, so the bytes Parse reads and the bytes Digest covers can never +// disagree about which lines are header — the digest is defined by the strip +// (ADR-0015). +// +// Present asks a different question and deliberately answers it more broadly; +// see its own comment. +const LinePrefix = "# modelith-" + +// lineRE matches one provenance line and splits it into key and value. The +// value is everything after the colon, trimmed; a free-text value such as the +// vendored banner is deliberately unconstrained. +var lineRE = regexp.MustCompile(`^# modelith-([a-z][a-z0-9-]*):(.*)$`) + +// digestRE is the recorded digest's shape. +var digestRE = regexp.MustCompile(`^sha256:[0-9a-f]{64}$`) + +// Banner is the value deps import writes for the vendored key. Nothing enforces +// it — a digest computed over the file cannot cover the header that records it — +// but an agent editing the file reads it and stops, which is the point. +const Banner = "DO NOT EDIT — this file is a copy. Change it at its origin." + +// Header is the provenance block of a vendored model file. +type Header struct { + Vendored string + Fetch string + Origin string + Path string + Ref string + Commit string + Imported string + Digest string +} + +// keyOrder is the order Format writes the keys in, and the set of keys that +// exist at all: a line naming anything else is a Problem. +var keyOrder = []string{"vendored", "fetch", "origin", "path", "ref", "commit", "imported", "digest"} + +// commonKeys are required whatever the fetch method is. methodKeys are the ones +// each method requires on top, so adding a method means declaring what it +// carries rather than inheriting git's key set by default. +var ( + commonKeys = []string{"vendored", "fetch", "imported", "digest"} + methodKeys = map[string][]string{ + "git": {"origin", "path", "ref", "commit"}, + } +) + +// Methods returns the fetch methods this build understands. +func Methods() []string { + // One entry today; sorted so a diagnostic listing them is deterministic + // when there are more. + out := make([]string, 0, len(methodKeys)) + for m := range methodKeys { + out = append(out, m) + } + slices.Sort(out) + return out +} + +func (h *Header) field(key string) *string { + switch key { + case "vendored": + return &h.Vendored + case "fetch": + return &h.Fetch + case "origin": + return &h.Origin + case "path": + return &h.Path + case "ref": + return &h.Ref + case "commit": + return &h.Commit + case "imported": + return &h.Imported + case "digest": + return &h.Digest + } + return nil +} + +// Problem is a defect in the header itself — an unknown key, a missing required +// one, a line outside the leading comment block. It carries the 1-based line so +// a diagnostic can point at it; Line is zero for a problem about the header as a +// whole rather than about one line. +type Problem struct { + Line int + Message string +} + +// Present reports whether src carries a provenance line anywhere, which is what +// makes a file vendored. +// +// It is deliberately broader than the header rule Parse and Strip share. A +// header with a typo in it — or one displaced below the leading comment block +// by an edit that prepended a line — still means the file is somebody else's +// copy. Asking the narrow question here would answer "not vendored" for a file +// that plainly is one, and every downstream consequence of that answer is +// silent: the digest stops being checked, completeness gaps about a document +// nobody here controls appear, and `deps import` refuses to repair the file on +// the grounds that it carries no header. Answering broadly instead makes the +// same input loud — Parse reports each displaced line, and the missing-key +// checks fire — which is the failure mode to prefer when the two must differ. +func Present(src []byte) bool { + for _, line := range strings.Split(string(src), "\n") { + if lineRE.MatchString(line) { + return true + } + } + return false +} + +// Parse reads the provenance header from src. It returns a nil Header when src +// carries no provenance line at all, which is the ordinary case for a model +// this repo owns. Otherwise it returns what it could read plus every defect it +// found; callers report the defects and may still use the header. +func Parse(src []byte) (*Header, []Problem) { + if !Present(src) { + return nil, nil + } + var ( + h Header + problems []Problem + seen = map[string]int{} + ) + lines := strings.Split(string(src), "\n") + lead := leadingCommentBlock(lines) + for i, line := range lines { + m := lineRE.FindStringSubmatch(line) + if m == nil { + continue + } + num, key, value := i+1, m[1], strings.TrimSpace(m[2]) + if i >= lead { + problems = append(problems, Problem{num, fmt.Sprintf( + "provenance line %q is below the leading comment block, so it was read as an ordinary comment rather than as header — the whole header belongs at the top of the file, before any model content", + LinePrefix+key)}) + continue + } + field := h.field(key) + if field == nil { + problems = append(problems, Problem{num, fmt.Sprintf( + "unknown provenance key %q — this modelith knows %s", + LinePrefix+key, quotedList(keyOrder))}) + continue + } + if prev, dup := seen[key]; dup { + problems = append(problems, Problem{num, fmt.Sprintf( + "provenance key %q appears twice, on lines %d and %d", LinePrefix+key, prev, num)}) + continue + } + seen[key] = num + *field = value + } + + problems = append(problems, h.validate(seen)...) + return &h, problems +} + +// validate checks the key set against the fetch method and the shape of the +// values that have one. seen maps a key to the line it was read from, so a +// problem about a value points at it. +func (h *Header) validate(seen map[string]int) []Problem { + var problems []Problem + + required := append([]string{}, commonKeys...) + method, known := methodKeys[h.Fetch] + switch { + case h.Fetch == "": + // The missing-key report below names it; naming a method that is not + // there as unknown too would report one mistake twice. + case !known: + problems = append(problems, Problem{seen["fetch"], fmt.Sprintf( + "unknown fetch method %q — this modelith supports %s. If you need another, please open an issue at https://github.com/stacklok/modelith/issues", + h.Fetch, quotedList(Methods()))}) + default: + required = append(required, method...) + } + + for _, key := range required { + if *h.field(key) == "" { + problems = append(problems, Problem{0, fmt.Sprintf( + "provenance header is missing %q", LinePrefix+key)}) + } + } + if h.Digest != "" && !digestRE.MatchString(h.Digest) { + problems = append(problems, Problem{seen["digest"], fmt.Sprintf( + "provenance digest %q is not in the form sha256:<64 hex digits>", h.Digest)}) + } + return problems +} + +// ValidDigest reports whether s is a digest in the form a header records. A +// caller that has already reported a malformed one uses this to skip the +// comparison, which such a digest cannot help but fail. +func ValidDigest(s string) bool { return digestRE.MatchString(s) } + +// Verify reports whether src still hashes to the digest its own header records, +// and returns the digest src actually has. +func (h *Header) Verify(src []byte) (ok bool, got string) { + got = Digest(src) + return got == h.Digest, got +} + +// Strip returns src with its header removed, which is the content the digest +// covers. Only the leading comment block is stripped: a "# modelith-" line +// below it is not header, and Parse reports it as misplaced rather than reading +// it (see LinePrefix). Removing it here would let an edit that added or removed +// one leave the digest untouched, which is the drift the digest exists to see. +func Strip(src []byte) []byte { + lines := strings.Split(string(src), "\n") + lead := leadingCommentBlock(lines) + kept := lines[:0] + for i, line := range lines { + if i < lead && lineRE.MatchString(line) { + continue + } + kept = append(kept, line) + } + return []byte(strings.Join(kept, "\n")) +} + +// Digest returns the digest of src as a header records it: SHA-256 over src +// with its provenance lines removed, so stamping a header does not change the +// digest of the file it is stamped into. +func Digest(src []byte) string { + sum := sha256.Sum256(Strip(src)) + return "sha256:" + hex.EncodeToString(sum[:]) +} + +// Format renders h as the header block, every line terminated. +func (h *Header) Format() string { + var b strings.Builder + for _, key := range keyOrder { + if v := *h.field(key); v != "" { + fmt.Fprintf(&b, "%s%s: %s\n", LinePrefix, key, v) + } + } + return b.String() +} + +// Stamp returns src with h's header inserted: after a leading +// "# yaml-language-server:" line when src opens with one, so the editor +// directive stays first, and at the top otherwise. +func Stamp(src []byte, h *Header) []byte { + lines := strings.Split(string(src), "\n") + at, lead := 0, leadingCommentBlock(lines) + for i := 0; i < lead; i++ { + if strings.HasPrefix(lines[i], "# yaml-language-server:") { + at = i + 1 + } + } + head := strings.Join(lines[:at], "\n") + if at > 0 { + head += "\n" + } + return []byte(head + h.Format() + strings.Join(lines[at:], "\n")) +} + +// leadingCommentBlock returns the number of lines in the run at the top of the +// file that are blank or comments — where the header belongs. +func leadingCommentBlock(lines []string) int { + for i, line := range lines { + trimmed := strings.TrimSpace(line) + if trimmed == "" || strings.HasPrefix(trimmed, "#") { + continue + } + return i + } + return len(lines) +} + +func quotedList(items []string) string { + quoted := make([]string, len(items)) + for i, s := range items { + quoted[i] = fmt.Sprintf("%q", s) + } + return strings.Join(quoted, ", ") +} diff --git a/internal/provenance/provenance_test.go b/internal/provenance/provenance_test.go new file mode 100644 index 0000000..794ba6e --- /dev/null +++ b/internal/provenance/provenance_test.go @@ -0,0 +1,408 @@ +package provenance + +import ( + "crypto/sha256" + "encoding/hex" + "strings" + "testing" +) + +// vendored is a stamped file: the editor directive, then the header, then a +// model. Tests that need the unstamped content use plain. +const vendored = `# yaml-language-server: $schema=https://modelith.sh/schema/domain-model/v1.json +# modelith-vendored: DO NOT EDIT — this file is a copy. Change it at its origin. +# modelith-fetch: git +# modelith-origin: https://github.com/stacklok/some-repo +# modelith-path: docs/payments.modelith.yaml +# modelith-ref: main +# modelith-commit: 4f2c1e9c8b3ad0e5f71b2c9a6d4e8f30ab5c7d21 +# modelith-imported: 2026-07-27 +# modelith-digest: sha256:0000000000000000000000000000000000000000000000000000000000000000 +kind: DomainModel +version: v1 +title: Payments +` + +const plain = `# yaml-language-server: $schema=https://modelith.sh/schema/domain-model/v1.json +kind: DomainModel +version: v1 +title: Payments +` + +func TestPresent(t *testing.T) { + t.Parallel() + + cases := []struct { + name string + src string + want bool + }{ + {"a stamped file", vendored, true}, + {"a model this repo owns", plain, false}, + {"an ordinary comment that only looks like one", "#modelith-fetch: git\nkind: DomainModel\n", false}, + {"an indented provenance line is an ordinary comment", " # modelith-fetch: git\nkind: DomainModel\n", false}, + {"a broken header is still a header", "# modelith-nonsense: x\nkind: DomainModel\n", true}, + {"a header displaced below the model content is still a header", plain + "# modelith-fetch: git\n", true}, + } + for _, tc := range cases { + t.Run(tc.name, func(t *testing.T) { + t.Parallel() + if got := Present([]byte(tc.src)); got != tc.want { + t.Errorf("Present() = %v, want %v", got, tc.want) + } + }) + } +} + +func TestParse_Valid(t *testing.T) { + t.Parallel() + + h, problems := Parse([]byte(vendored)) + if len(problems) != 0 { + t.Fatalf("unexpected problems: %+v", problems) + } + want := Header{ + Vendored: Banner, + Fetch: "git", + Origin: "https://github.com/stacklok/some-repo", + Path: "docs/payments.modelith.yaml", + Ref: "main", + Commit: "4f2c1e9c8b3ad0e5f71b2c9a6d4e8f30ab5c7d21", + Imported: "2026-07-27", + Digest: "sha256:" + strings.Repeat("0", 64), + } + if *h != want { + t.Errorf("Parse() = %+v, want %+v", *h, want) + } +} + +func TestParse_NoHeader(t *testing.T) { + t.Parallel() + + h, problems := Parse([]byte(plain)) + if h != nil || problems != nil { + t.Errorf("Parse() = %+v, %+v; want nil, nil", h, problems) + } +} + +func TestParse_Problems(t *testing.T) { + t.Parallel() + + // header builds a stamped file with the named lines replaced or dropped, so + // each case differs from a valid header in exactly one way. + header := func(edits map[string]string) string { + var b strings.Builder + b.WriteString("# yaml-language-server: $schema=x\n") + for _, key := range keyOrder { + value := map[string]string{ + "vendored": Banner, + "fetch": "git", + "origin": "https://github.com/stacklok/some-repo", + "path": "docs/payments.modelith.yaml", + "ref": "main", + "commit": "4f2c1e9", + "imported": "2026-07-27", + "digest": "sha256:" + strings.Repeat("0", 64), + }[key] + if edit, ok := edits[key]; ok { + value = edit + } + if value == "" { + continue + } + b.WriteString(LinePrefix + key + ": " + value + "\n") + } + b.WriteString("kind: DomainModel\n") + return b.String() + } + + cases := []struct { + name string + src string + wantLine int + contains string + }{ + { + name: "an unknown key names the ones that exist", + src: "# modelith-surface: sha256:x\n" + plain, + wantLine: 1, + contains: `unknown provenance key "# modelith-surface"`, + }, + { + name: "a duplicate key names both lines", + src: "# modelith-ref: main\n# modelith-ref: other\n" + plain, + wantLine: 2, + contains: `appears twice, on lines 1 and 2`, + }, + { + name: "a missing required key is reported without a line", + src: header(map[string]string{"commit": ""}), + wantLine: 0, + contains: `missing "# modelith-commit"`, + }, + { + name: "an unknown fetch method points at the issue tracker", + src: header(map[string]string{"fetch": "carrier-pigeon"}), + wantLine: 3, + contains: `unknown fetch method "carrier-pigeon"`, + }, + { + name: "a malformed digest names the shape", + src: header(map[string]string{"digest": "sha256:nope"}), + wantLine: 9, + contains: "sha256:<64 hex digits>", + }, + { + name: "a provenance line below the model content is misplaced", + src: "# modelith-fetch: git\n" + plain + "# modelith-origin: https://github.com/stacklok/some-repo\n", + wantLine: 6, + contains: "below the leading comment block", + }, + } + + for _, tc := range cases { + t.Run(tc.name, func(t *testing.T) { + t.Parallel() + _, problems := Parse([]byte(tc.src)) + var found *Problem + for i := range problems { + if strings.Contains(problems[i].Message, tc.contains) { + found = &problems[i] + break + } + } + if found == nil { + t.Fatalf("no problem containing %q; got %+v", tc.contains, problems) + } + if found.Line != tc.wantLine { + t.Errorf("problem on line %d, want %d: %s", found.Line, tc.wantLine, found.Message) + } + }) + } +} + +// TestADR_0015_DigestCoversTheFileWithoutItsHeader pins ADR-0015's digest definition. +// +// The expectation is built by hand — the header lines written out and removed +// by the test itself, then hashed with crypto/sha256 directly — rather than by +// calling Strip. A test that reuses the implementation's own helper would agree +// with it however wrong the helper was, which is exactly how two HIGH bugs got +// past CI in the renderer work. +func TestADR_0015_DigestCoversTheFileWithoutItsHeader(t *testing.T) { + t.Parallel() + + sum := sha256.Sum256([]byte(plain)) + want := "sha256:" + hex.EncodeToString(sum[:]) + + if got := Digest([]byte(plain)); got != want { + t.Errorf("Digest(plain) = %s, want %s", got, want) + } + if got := Digest([]byte(vendored)); got != want { + t.Errorf("Digest(vendored) = %s, want %s — the header must not change the digest of the file it is stamped into", got, want) + } +} + +// TestADR_0015_UnknownFetchMethodIsAnError pins that the fetch method is a +// closed set. A header naming a method this build does not implement is +// reported rather than being read as if it were git — which would verify a +// digest against keys that mean something else — and the diagnostic asks for an +// issue, because a real user wanting another transport is what justifies +// writing one. +func TestADR_0015_UnknownFetchMethodIsAnError(t *testing.T) { + t.Parallel() + + src := strings.Replace(vendored, LinePrefix+"fetch: git", LinePrefix+"fetch: carrier-pigeon", 1) + _, problems := Parse([]byte(src)) + + var found bool + for _, p := range problems { + if strings.Contains(p.Message, `unknown fetch method "carrier-pigeon"`) { + found = true + if !strings.Contains(p.Message, issuesHint) { + t.Errorf("the diagnostic does not ask for an issue: %s", p.Message) + } + if !strings.Contains(p.Message, `"git"`) { + t.Errorf("the diagnostic does not name the methods that work: %s", p.Message) + } + } + } + if !found { + t.Fatalf("no unknown-method problem; got %+v", problems) + } +} + +const issuesHint = "https://github.com/stacklok/modelith/issues" + +func TestDigest_ChangesWithTheContent(t *testing.T) { + t.Parallel() + + edited := strings.Replace(vendored, "title: Payments", "title: Payment", 1) + if Digest([]byte(edited)) == Digest([]byte(vendored)) { + t.Error("editing the model left the digest unchanged") + } +} + +// TestDigest_IgnoresEveryHeaderChange pins that the digest is blind to the +// header and to nothing else: rewriting each key in turn must not move it, +// because the header records a value derived from the rest of the file. +func TestDigest_IgnoresEveryHeaderChange(t *testing.T) { + t.Parallel() + + base := Digest([]byte(vendored)) + for _, key := range keyOrder { + src := strings.Replace(vendored, LinePrefix+key+": ", LinePrefix+key+": changed-", 1) + if src == vendored { + t.Fatalf("test did not rewrite %q", key) + } + if got := Digest([]byte(src)); got != base { + t.Errorf("rewriting %q moved the digest to %s", key, got) + } + } +} + +// TestDigest_CoversALineBelowTheHeader pins the other half of the strip rule: +// only the leading comment block is header, so a "# modelith-" line anywhere +// below it is content the digest covers. Stripping it there would make an edit +// that adds or removes one invisible to drift detection — and would make a model +// this repository owns, which happened to carry such a comment, read as +// somebody else's copy. +func TestDigest_CoversALineBelowTheHeader(t *testing.T) { + t.Parallel() + + added := vendored + "# modelith-fetch: git\n" + if Digest([]byte(added)) == Digest([]byte(vendored)) { + t.Error("a provenance-looking line added below the model left the digest unchanged") + } + if !strings.Contains(string(Strip([]byte(added))), "# modelith-fetch: git") { + t.Error("Strip removed a line below the leading comment block") + } +} + +// TestPresent_ADisplacedHeaderStaysLoud pins the reason Present asks a broader +// question than Parse and Strip do. Prepending one non-comment line to a +// vendored file puts its whole header below the leading comment block. Answering +// the narrow question there would call the file this repository's own work, and +// every consequence of that answer is silent: no digest check, completeness +// gaps about somebody else's document, and a `deps import` that refuses to +// repair the file because it "carries no provenance header". +func TestPresent_ADisplacedHeaderStaysLoud(t *testing.T) { + t.Parallel() + + displaced := "---\n" + vendored + if !Present([]byte(displaced)) { + t.Fatal("a vendored file with its header displaced read as this repository's own work") + } + h, problems := Parse([]byte(displaced)) + if h == nil { + t.Fatal("Parse returned no header for a displaced one") + } + var misplaced, missing int + for _, p := range problems { + switch { + case strings.Contains(p.Message, "below the leading comment block"): + misplaced++ + case strings.Contains(p.Message, "is missing"): + missing++ + } + } + if misplaced != len(keyOrder) { + t.Errorf("reported %d misplaced lines, want one per key (%d)", misplaced, len(keyOrder)) + } + if missing == 0 { + t.Error("no key was reported missing, so the header read as complete") + } + // Nothing was stripped, so the digest cannot match what the header records. + if string(Strip([]byte(displaced))) != displaced { + t.Error("Strip removed a line from below the leading comment block") + } +} + +func TestVerify(t *testing.T) { + t.Parallel() + + h := &Header{Digest: Digest([]byte(plain))} + if ok, _ := h.Verify([]byte(vendored)); !ok { + t.Error("a stamped copy of the same content did not verify") + } + edited := strings.Replace(vendored, "title: Payments", "title: Payment", 1) + ok, got := h.Verify([]byte(edited)) + if ok { + t.Error("an edited copy verified") + } + if got == h.Digest { + t.Error("Verify reported the recorded digest as the one it computed") + } +} + +func TestStamp(t *testing.T) { + t.Parallel() + + h := &Header{ + Vendored: Banner, + Fetch: "git", + Origin: "https://github.com/stacklok/some-repo", + Path: "docs/payments.modelith.yaml", + Ref: "main", + Commit: "4f2c1e9c8b3ad0e5f71b2c9a6d4e8f30ab5c7d21", + Imported: "2026-07-27", + Digest: Digest([]byte(plain)), + } + + t.Run("below a yaml-language-server line", func(t *testing.T) { + t.Parallel() + got := string(Stamp([]byte(plain), h)) + lines := strings.Split(got, "\n") + if !strings.HasPrefix(lines[0], "# yaml-language-server:") { + t.Errorf("first line is %q, want the editor directive", lines[0]) + } + if !strings.HasPrefix(lines[1], LinePrefix+"vendored:") { + t.Errorf("second line is %q, want the vendored banner", lines[1]) + } + if !strings.Contains(got, "kind: DomainModel") { + t.Error("the model content did not survive stamping") + } + }) + + t.Run("at the top when there is no directive", func(t *testing.T) { + t.Parallel() + src := "kind: DomainModel\nversion: v1\n" + got := string(Stamp([]byte(src), h)) + if !strings.HasPrefix(got, LinePrefix+"vendored:") { + t.Errorf("stamped file starts %q", got[:40]) + } + if !strings.HasSuffix(got, src) { + t.Error("the model content did not survive stamping unchanged") + } + }) + + t.Run("a stamped file parses back to the header", func(t *testing.T) { + t.Parallel() + back, problems := Parse(Stamp([]byte(plain), h)) + if len(problems) != 0 { + t.Fatalf("stamping produced a header with problems: %+v", problems) + } + if *back != *h { + t.Errorf("round trip gave %+v, want %+v", *back, *h) + } + }) + + t.Run("the stamped file verifies against its own digest", func(t *testing.T) { + t.Parallel() + stamped := Stamp([]byte(plain), h) + parsed, _ := Parse(stamped) + if ok, got := parsed.Verify(stamped); !ok { + t.Errorf("a freshly stamped file does not verify: recorded %s, computed %s", parsed.Digest, got) + } + }) +} + +func TestStrip(t *testing.T) { + t.Parallel() + + if got := string(Strip([]byte(vendored))); got != plain { + t.Errorf("Strip() = %q, want %q", got, plain) + } + if got := string(Strip([]byte(plain))); got != plain { + t.Errorf("Strip() changed a file with no header: %q", got) + } +} diff --git a/internal/render/markdown/markdown_test.go b/internal/render/markdown/markdown_test.go index 8f4ebb2..a2a2bd0 100644 --- a/internal/render/markdown/markdown_test.go +++ b/internal/render/markdown/markdown_test.go @@ -337,7 +337,7 @@ func TestRenderImports_SectionAndLinkedTypes(t *testing.T) { m := &model.Model{ Imports: []model.Import{ - {Scope: "payments", Path: "../payments/payments.modelith.yaml", ScopeFromPath: true}, + {Scope: "payments", Path: "../payments/payments.modelith.yaml"}, {Scope: "billing", Path: "./legacy/pay-v2.modelith.yaml"}, }, Entities: map[string]model.Entity{ @@ -392,7 +392,7 @@ func TestRenderImports_LinkRelativeToOutputDir(t *testing.T) { m := &model.Model{ Imports: []model.Import{ - {Scope: "payments", Path: "../payments/payments.modelith.yaml", ScopeFromPath: true}, + {Scope: "payments", Path: "../payments/payments.modelith.yaml"}, {Scope: "billing", Path: "./legacy/pay-v2.modelith.yaml"}, }, Entities: map[string]model.Entity{ @@ -507,7 +507,7 @@ func TestRenderImports_HostilePathCannotEscapeItsMarkup(t *testing.T) { {Scope: "spaced", Path: "./a b.modelith.yaml"}, // A newline would inject a heading into the Imports list; the derived // scope carries it too, since it comes from the same string. - {Scope: "line\nbreak", Path: "./a\n## Injected\n\nb.modelith.yaml", ScopeFromPath: true}, + {Scope: "line\nbreak", Path: "./a\n## Injected\n\nb.modelith.yaml"}, {Scope: "ticked", Path: "./a`b``c.modelith.yaml"}, }, Entities: map[string]model.Entity{ diff --git a/project-docs/adr/0015-vendoring-is-a-whole-file-copy.md b/project-docs/adr/0015-vendoring-is-a-whole-file-copy.md new file mode 100644 index 0000000..bb0ffe9 --- /dev/null +++ b/project-docs/adr/0015-vendoring-is-a-whole-file-copy.md @@ -0,0 +1,145 @@ +# Vendoring is a whole-file copy, verified by a content digest + +A vendored model is fetched whole, stamped with a provenance header comment, +and verified offline against a SHA-256 of its own bytes. Vendoring does not +recurse, the fetch is `gh`-only, and the trust warning prints rather than +blocks. Supersedes ADR-0010's **Digest** section; the rest of ADR-0010 stands. + +## Context + +ADR-0010 specified vendoring in full before any of it was built. Implementing +it, three of its answers turned out to cost more than the problem they solve, +measured against the threat model ADR-0014 settled: a model author already has +commit access and could edit the generated `.md` directly, so there is no +privilege boundary, and vendoring is expected to happen between projects that +already trust each other. The remaining gaps are correctness bugs, not +vulnerabilities, and the agreed answer to the injection risk is to *say so* at +the point of import rather than to armour the renderer further. + +## Decision + +**A content digest, not a surface digest.** The header carries a SHA-256 over +the file's bytes with every `# modelith-: ` line removed. ADR-0010's +surface digest — structure and normative content, excluding `description`, +`definition`, `note`, `derivation`, scenario steps, `title`, and key order — +required a canonicalisation pass and imposed a permanent tax: every future +schema field would have to be classified surface-or-documentation, forever, or +the digest silently drifts. Checked against its two consumers, that buys very +little. For verify-on-lint, which is the job that must work offline with no +network, a whole-file digest is *strictly better*: the realistic accidental +edit is a teammate fixing a typo in someone else's model, which a surface +digest deliberately ignores. It loses only to `deps check`, where a +documentation-only upstream change will now report as a change — and `check` +has both files in hand and diffs them to report what moved, so a human judges. +A `surface:` key can be added later, with a real complaint driving it. + +The digest is not redundant with `commit`. `commit` says what upstream had; +the digest says whether the bytes here still match what was fetched, which is +a question `lint` has to answer without a network. + +**The header.** One namespaced key per comment line, mirroring the +`# yaml-language-server:` line already in every model: + +```yaml +# modelith-vendored: DO NOT EDIT — this file is a copy. Change it at its origin. +# modelith-fetch: git +# modelith-origin: https://github.com/stacklok/some-repo +# modelith-path: docs/payments.modelith.yaml +# modelith-ref: main +# modelith-commit: 4f2c1e9c8b3ad0e5f71b2c9a6d4e8f30ab5c7d21 +# modelith-imported: 2026-07-27 +# modelith-digest: sha256:9a1f... +``` + +Parsing and stripping are the *same* rule — lines matching +`^# modelith-: ` — so the two cannot disagree, which matters because the +digest is defined by the strip. Such a line below the leading comment block is +an error. The `vendored` key carries the DO NOT EDIT banner as its value: +nothing enforces it, but an agent reading the file will pause on it, and +making the banner a key rather than free text keeps the single strip rule. + +`fetch:` declares the method and scopes which other keys apply — `git` uses +`origin`, `path`, `ref`, and `commit`; a future `https` method would carry +`origin` alone and lean on the digest as its only integrity anchor. An unknown +`modelith-*` key or an unknown `fetch:` value is an error rather than being +ignored, which is the right posture pre-release even though it means an older +binary rejects a newer header. + +**What the header suppresses.** A provenance header suppresses the +completeness category and import-resolution errors, and nothing else. +Structural and semantic checks still run: a vendored file that is not valid +modelith breaks *this* repo's build and is this repo's problem. ADR-0010's +list of ownership-scoped diagnostics — no invariants, no scenario exercises an +entity, unused glossary term, unused enum — is exactly `CategoryCompleteness`, +so the rule needs no new classification. Semantic *warnings* about someone +else's authoring stay: they never fail a build, and suppressing them would +require a second, fuzzier judgement about which semantic checks are +ownership-scoped. + +Suppressing an unresolvable import must suppress its downstream consequence +too. A vendored file's own `imports:` point at paths in its home repo; if the +references into those scopes still errored, every vendored file that has +imports would be broken here and the suppression would be worthless. + +A digest mismatch is an error, not a warning: the file lies about its own +provenance, and both remedies are cheap and honest — re-run `deps update`, or +delete the header, which converts it to a canonical local file and is an +accurate description of having forked it. The diagnostic names both. + +**Vendoring does not recurse.** ADR-0010 already made *resolution* +non-transitive: `scope.Name` reaches only items defined directly in the file +bound to that scope. Fetching stays non-transitive to match. Referencing an +item that a vendored model merely uses rather than defines is a lint error +whose remedy is to vendor that model directly and bind it to its own scope, +and `deps import` says at fetch time when the file it fetched declares imports +of its own. + +The closure was considered and rejected on cost. It requires a vendor +directory layout, rewriting import paths on fetch — which breaks the +unmodified-copy invariant that both the digest and a clean `deps update` +overwrite rest on — a dedupe policy for diamond dependencies, and an answer to +a conflict ADR-0012 makes unavoidable: the *importer* binds the scope, so a +transitively vendored file can arrive bound to one slug by its parent and +another by this repo. Resolving that needs a lock file and an alias layer, +both of which ADR-0010 rejected. It is also consistent with the reason +vendoring is whole-file: referencing something new from an already-imported +model is a local edit, and referencing something from a *different* model is +another explicit import. + +**`gh` is the only transport.** `gh` already solves authentication for the +private and internal repositories the motivating models require, and it +resolves content and commit in two API calls. A `git` clone path would work +with any remote but is a second code path with sparse-checkout handling for a +case with no live instance, which is the bar ADR-0007 sets. A non-GitHub +origin is an error that asks the user to file an issue, so the first real user +becomes the signal to build it. `fetch: git` names what the origin *is* — a +file at a path in a git repo at a ref — not which binary fetched it, so a +`git` fallback or an `https` method slots in without a header migration. + +**The warning prints; it does not block.** ADR-0014 requires `deps import` and +`deps update` to warn that a vendored model is untrusted content that will be +rendered into published Markdown. An interactive confirmation is the wrong +shape: an agent usually drives these commands, so a prompt is either +auto-answered theatre or a wedged non-interactive CI run. Warn-and-proceed +means the fetch has happened by the time the warning is read, which is +acceptable because the file is inert until it is added to `imports:` — and +`deps import` deliberately does not edit `imports:`, so that second, manual +step is the real gate. + +## Consequences + +- `deps import` writes a file and prints the `imports:` snippet to paste. + Rewriting a user's YAML in place would cost comment and formatting + fidelity and would force the command to answer which model it is importing + *into* when a repo holds several. +- No schema change. `imports:` already exists and a header comment is not + schema, so `TestSchemaStructSync` and the schema-URL consistency tests are + not in play. +- Pinned by `TestADR_0015_*`: the digest covers file bytes minus the header + lines, a header suppresses completeness and nothing else, an unknown + `fetch:` value is an error, and `deps import` fetches no transitive imports. +- An unbalanced code fence in a vendored model's prose still corrupts the + importing repo's rendered Markdown (issue #39). Vendoring strengthens the + case for that rule, since it is where someone else's prose lands in your + published document, but it shares no code with this change and is a visibly + broken document rather than a privilege boundary.