From d3e2bc65a2656c7936e8ac2e3e473eb4958870fe Mon Sep 17 00:00:00 2001 From: Alex Dunmow Date: Mon, 10 Aug 2026 23:44:40 +0800 Subject: [PATCH] feat(plugin): validate keyed load-once manifests --- ...keyed-load-once-manifests-are-validated.md | 20 ++++++ go.mod | 2 +- go.sum | 4 +- internal/bnp/build.go | 6 ++ internal/bnp/codeless_test.go | 9 +-- internal/bnp/load_once.go | 17 +++++ internal/bnp/load_once_test.go | 65 +++++++++++++++++++ internal/bnp/verify.go | 5 ++ 8 files changed, 121 insertions(+), 7 deletions(-) create mode 100644 docs/adr/0002-keyed-load-once-manifests-are-validated.md create mode 100644 internal/bnp/load_once.go create mode 100644 internal/bnp/load_once_test.go diff --git a/docs/adr/0002-keyed-load-once-manifests-are-validated.md b/docs/adr/0002-keyed-load-once-manifests-are-validated.md new file mode 100644 index 0000000..2b13676 --- /dev/null +++ b/docs/adr/0002-keyed-load-once-manifests-are-validated.md @@ -0,0 +1,20 @@ +# Keyed load-once manifests are validated before packing + +The plugin SDK and CMS host support keyed `LoadOnce` callbacks whose durable +identity is the `(plugin name, key)` pair. A malformed or duplicate key would +make execution ambiguous and should not survive until production installation. + +Decision: both `ninja plugin build` and `ninja plugin verify` validate every +`load_once_keys` manifest entry with the SDK's canonical rules. Keys are 1–128 +bytes, start alphanumeric, and contain only lower-case letters, digits, `.`, +`_`, or `-`; duplicates are rejected. Codeless artifacts reject load-once keys +because they have no guest callback to execute. + +Consequences: + +- Build and verification fail before an invalid artifact can be published. +- CLI and host use the same SDK validator instead of drifting grammars. +- Changing a valid key remains the deliberate way to define a new one-time + operation. + +Keywords: plugin CLI, LoadOnce, load_once_keys, BNP, validation, codeless diff --git a/go.mod b/go.mod index 89b30b3..66db554 100644 --- a/go.mod +++ b/go.mod @@ -5,7 +5,7 @@ go 1.26.4 require ( connectrpc.com/connect v1.20.0 git.dev.alexdunmow.com/block/core v0.18.2 - git.dev.alexdunmow.com/block/pluginsdk v0.3.2 + git.dev.alexdunmow.com/block/pluginsdk v0.3.3 github.com/chromedp/cdproto v0.0.0-20260321001828-e3e3800016bc github.com/chromedp/chromedp v0.15.1 github.com/klauspost/compress v1.18.6 diff --git a/go.sum b/go.sum index ea1a254..52956f9 100644 --- a/go.sum +++ b/go.sum @@ -2,8 +2,8 @@ connectrpc.com/connect v1.20.0 h1:6TNDAB+WeNd2uolWNlYczB5E0KNNaVMNUEx8JEUsPmQ= connectrpc.com/connect v1.20.0/go.mod h1:A2ygJrukXwWy32vkCAAHNVguZrqZ+jeZ9rGRnGR4dN4= git.dev.alexdunmow.com/block/core v0.18.2 h1:+3OfZ424yoc1k1CucXYARk8TBkh8Z693HRkhf5Ci2wU= git.dev.alexdunmow.com/block/core v0.18.2/go.mod h1:GGuUu826AoJepC/hKLGJ7BX3PQaDss9ueCT0se6Ao2w= -git.dev.alexdunmow.com/block/pluginsdk v0.3.2 h1:aLGkLzmJ0+docoke1e9sZTtugNugPaD3PNJEonzurgI= -git.dev.alexdunmow.com/block/pluginsdk v0.3.2/go.mod h1:Z+eG+WZxAP0jfreLqlGcc0kkWKt8RWevzWyWn8d+dhM= +git.dev.alexdunmow.com/block/pluginsdk v0.3.3 h1:SEvKqCcvYmqy6ToA9j9oaSORmQFplOeQFbqAZfrJeUE= +git.dev.alexdunmow.com/block/pluginsdk v0.3.3/go.mod h1:Z+eG+WZxAP0jfreLqlGcc0kkWKt8RWevzWyWn8d+dhM= github.com/BurntSushi/toml v1.6.0 h1:dRaEfpa2VI55EwlIW72hMRHdWouJeRF7TPYhI+AUQjk= github.com/BurntSushi/toml v1.6.0/go.mod h1:ukJfTF/6rtPPRCnwkur4qwRxa8vTRFBF0uk2lLoLwho= github.com/chromedp/cdproto v0.0.0-20260321001828-e3e3800016bc h1:wkN/LMi5vc60pBRWx6qpbk/aEvq3/ZVNpnMvsw8PVVU= diff --git a/internal/bnp/build.go b/internal/bnp/build.go index 630cffc..913afa9 100644 --- a/internal/bnp/build.go +++ b/internal/bnp/build.go @@ -165,6 +165,9 @@ func Build(ctx context.Context, opts BuildOptions) (*BuildResult, error) { if err := applyManifestYAML(dir, manifest); err != nil { return nil, err } + if err := ValidateLoadOnceManifest(manifest); err != nil { + return nil, err + } if err := ValidateMCPManifest(manifest, mod.Plugin.Scope, mod.Plugin.Name); err != nil { return nil, err } @@ -320,6 +323,9 @@ func hooksPresent(m *abiv1.PluginManifest) []string { if m.GetHasLoadHook() { hooks = append(hooks, "load") } + if len(m.GetLoadOnceKeys()) > 0 { + hooks = append(hooks, "load-once") + } if m.GetHasUnloadHook() { hooks = append(hooks, "unload") } diff --git a/internal/bnp/codeless_test.go b/internal/bnp/codeless_test.go index 2755835..136daed 100644 --- a/internal/bnp/codeless_test.go +++ b/internal/bnp/codeless_test.go @@ -37,8 +37,8 @@ func codelessFixture(t *testing.T) string { "templates/fixture/landing.ninjatpl": `
{{ content }}
`, "templates/overrides/fixture/heading.ninjatpl": `

{{ text }}

`, "templates/email/fixture.ninjatpl": `
{{ body|safe }}
`, - "presets.json": `{"presets":[{"key":"default"}]}`, - "master_pages.json": `[{"key":"landing","title":"Landing","blocks":[{"block_key":"html","title":"Hero","content":{"x":1},"slot":"main","sort_order":1}]}]`, + "presets.json": `{"presets":[{"key":"default"}]}`, + "master_pages.json": `[{"key":"landing","title":"Landing","blocks":[{"block_key":"html","title":"Hero","content":{"x":1},"slot":"main","sort_order":1}]}]`, "blocks/blocks.yaml": "blocks:\n - key: hero\n title: Hero\n category: content\n" + " schema: hero.schema.json\n template: hero.ninjatpl\n providers: [site]\n", "blocks/hero.schema.json": `{"type":"object"}`, @@ -147,8 +147,8 @@ func TestCodelessBuildRejectsBadDeclarations(t *testing.T) { "manifest.yaml": "template_overrides:\n - template: x\n block: heading\n", }}, {"override missing block key", map[string]string{ - "plugin.mod": "[plugin]\nname = \"x\"\nversion = \"0.1.0\"\n", - "manifest.yaml": "template_overrides:\n - template: x\n", + "plugin.mod": "[plugin]\nname = \"x\"\nversion = \"0.1.0\"\n", + "manifest.yaml": "template_overrides:\n - template: x\n", "templates/overrides/x/heading.ninjatpl": `

{{ text }}

`, }}, {"email wrapper missing template file", map[string]string{ @@ -177,6 +177,7 @@ func TestCodelessHookViolation(t *testing.T) { } bad := []*abiv1.PluginManifest{ {Codeless: true, HasHttpHandler: true}, + {Codeless: true, LoadOnceKeys: []string{"defaults.v1"}}, {Codeless: true, JobTypes: []string{"j"}}, {Codeless: true, DeclaredTags: []string{"t"}}, {Codeless: true, Blocks: []*abiv1.BlockMeta{{Key: "k"}}}, diff --git a/internal/bnp/load_once.go b/internal/bnp/load_once.go new file mode 100644 index 0000000..b9d8a5c --- /dev/null +++ b/internal/bnp/load_once.go @@ -0,0 +1,17 @@ +package bnp + +import ( + "fmt" + + abiv1 "git.dev.alexdunmow.com/block/pluginsdk/abi/v1" + "git.dev.alexdunmow.com/block/pluginsdk/plugin" +) + +// ValidateLoadOnceManifest enforces the stable keyed lifecycle contract before +// an artifact can be packed or accepted by ninja plugin verify. +func ValidateLoadOnceManifest(manifest *abiv1.PluginManifest) error { + if err := plugin.ValidateLoadOnceKeys(manifest.GetLoadOnceKeys()); err != nil { + return fmt.Errorf("bnp: %w", err) + } + return nil +} diff --git a/internal/bnp/load_once_test.go b/internal/bnp/load_once_test.go new file mode 100644 index 0000000..c447ef3 --- /dev/null +++ b/internal/bnp/load_once_test.go @@ -0,0 +1,65 @@ +package bnp + +import ( + "path/filepath" + "slices" + "strings" + "testing" + + abiv1 "git.dev.alexdunmow.com/block/pluginsdk/abi/v1" + "google.golang.org/protobuf/proto" +) + +func TestHooksPresentIncludesLoadOnce(t *testing.T) { + hooks := hooksPresent(&abiv1.PluginManifest{LoadOnceKeys: []string{"defaults.v1"}}) + if !slices.Contains(hooks, "load-once") { + t.Fatalf("hooksPresent() = %v, want load-once", hooks) + } +} + +func TestValidateLoadOnceManifest(t *testing.T) { + tests := []struct { + name string + keys []string + ok bool + }{ + {name: "valid", keys: []string{"documentation-main-menu.v1"}, ok: true}, + {name: "uppercase", keys: []string{"DocumentationMenu.v1"}}, + {name: "duplicate", keys: []string{"menu.v1", "menu.v1"}}, + } + for _, test := range tests { + t.Run(test.name, func(t *testing.T) { + err := ValidateLoadOnceManifest(&abiv1.PluginManifest{LoadOnceKeys: test.keys}) + if test.ok && err != nil { + t.Fatalf("ValidateLoadOnceManifest() error = %v", err) + } + if !test.ok && err == nil { + t.Fatal("ValidateLoadOnceManifest() error = nil") + } + }) + } +} + +func TestVerifyRejectsDuplicateLoadOnceKeys(t *testing.T) { + manifestBytes, err := proto.Marshal(&abiv1.PluginManifest{ + AbiVersion: 1, + Name: "fixture", + Version: "1.0.0", + LoadOnceKeys: []string{"defaults.v1", "defaults.v1"}, + }) + if err != nil { + t.Fatalf("marshal manifest: %v", err) + } + out := filepath.Join(t.TempDir(), "fixture.bnp") + _, err = packArtifact(out, []packEntry{ + {ArtifactPath: fileWasm, Data: []byte("wasm")}, + {ArtifactPath: fileMod, Data: []byte("[plugin]\nname = \"fixture\"\nversion = \"1.0.0\"\n")}, + {ArtifactPath: fileManifest, Data: manifestBytes}, + }) + if err != nil { + t.Fatalf("pack artifact: %v", err) + } + if _, err := Verify(out); err == nil || !strings.Contains(err.Error(), "duplicate load-once key") { + t.Fatalf("Verify() error = %v, want duplicate key rejection", err) + } +} diff --git a/internal/bnp/verify.go b/internal/bnp/verify.go index 0c8b128..a0e4be5 100644 --- a/internal/bnp/verify.go +++ b/internal/bnp/verify.go @@ -121,6 +121,9 @@ func Verify(bnpPath string) (*VerifyResult, error) { if modName != name { return nil, fmt.Errorf("bnp: manifest name %q != plugin.mod name %q", name, modName) } + if err := ValidateLoadOnceManifest(manifest); err != nil { + return nil, err + } if err := ValidateMCPManifest(manifest, parseModString(modBytes, "scope"), name); err != nil { return nil, err } @@ -176,6 +179,8 @@ func CodelessHookViolation(m *abiv1.PluginManifest) error { return viol("media hooks") case m.GetHasProvisioner(): return viol("a provisioner hook") + case len(m.GetLoadOnceKeys()) > 0: + return viol("load-once hooks") case len(m.GetJobTypes()) > 0: return viol("job handlers") case len(m.GetRagContentFetcherTypes()) > 0: