From db3a63fb34b7e1be5bc502b544dd466e8c846620 Mon Sep 17 00:00:00 2001 From: scttfrdmn <3011922+scttfrdmn@users.noreply.github.com> Date: Mon, 20 Jul 2026 14:28:19 -0700 Subject: [PATCH] feat(plugin): 'plugin validate --strict' permission/step consistency check (spore-plugins#8) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Increment 5 (final) of the registry supply-chain RFC. Adds ValidatePermissionConsistency + a --strict flag on 'spawn plugin validate' that cross-checks a declared permissions: block against the plugin's actual steps, so the declaration surfaced by 'plugin inspect' is enforced, not decorative: - instance.root=false ⇒ no remote step that runs as root (a run step needs as_user; fetch/extract always run as root) - instance.network=false ⇒ no fetch step - controller.network=false ⇒ no local fetch/URL step - a permissions: block is required under --strict Base Validate (always-on, lenient) is unchanged; --strict is opt-in and run by the official registry's lint CI. New ValidateSpecFileStrict wrapper. Tests cover each mismatch + the consistent cases + the missing-block case. --- CHANGELOG.md | 9 ++ cmd/plugin.go | 16 +++- docs-gen/plugin.md | 15 +++- pkg/plugin/consistency_test.go | 153 +++++++++++++++++++++++++++++++++ pkg/plugin/validate.go | 104 ++++++++++++++++++++++ 5 files changed, 293 insertions(+), 4 deletions(-) create mode 100644 pkg/plugin/consistency_test.go diff --git a/CHANGELOG.md b/CHANGELOG.md index 38fc9f0..98680fe 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -8,6 +8,15 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0 ## [Unreleased] ### Added +- **`spawn plugin validate --strict` enforces permission/step consistency** + (spore-plugins#8). The strict mode cross-checks a plugin's declared + `permissions:` block against its actual steps — e.g. `instance.root=false` must + have no remote step that runs as root (a `run` step needs `as_user`, and + fetch/extract always run as root), `instance.network=false` must have no fetch, + `controller.network=false` no local network step — and requires a `permissions:` + block to be present. The official registry's CI runs `--strict`, so a published + plugin's declared capability surface is enforced at publish time rather than + being merely decorative. - **Installed-plugin provenance is recorded on the instance** (spore-plugins#8). A successful `spawn plugin install` now writes a `spore:plugin:` EC2 tag (version, content-digest + commit prefixes, and the verification tier reached — diff --git a/cmd/plugin.go b/cmd/plugin.go index cd2228b..7ddedb4 100644 --- a/cmd/plugin.go +++ b/cmd/plugin.go @@ -726,15 +726,24 @@ instance. Checks schema, semver, known step/condition/config types, that the containing directory matches the plugin name, and that every {{ config.X }} template reference points at a declared config parameter. +With --strict, also enforce that the declared permissions: block is consistent +with the plugin's steps (e.g. instance.root=false must have no remote step that +runs as root). --strict requires a permissions: block. The official registry's +CI runs --strict so a published plugin's declared capability surface is enforced. + Examples: spawn plugin validate ./plugins/tailscale/plugin.yaml - spawn plugin validate ./plugins/*/plugin.yaml`, + spawn plugin validate --strict ./plugins/*/plugin.yaml`, Args: cobra.MinimumNArgs(1), RunE: func(cmd *cobra.Command, args []string) error { out := cmd.OutOrStdout() + validate := plugin.ValidateSpecFile + if pluginValidateStrict { + validate = plugin.ValidateSpecFileStrict + } var failed int for _, path := range args { - if err := plugin.ValidateSpecFile(path); err != nil { + if err := validate(path); err != nil { failed++ fmt.Fprintf(out, "✗ %s\n", path) for _, line := range strings.Split(err.Error(), "\n") { @@ -751,6 +760,8 @@ Examples: }, } +var pluginValidateStrict bool + var pluginManifestOut string var pluginManifestCmd = &cobra.Command{ @@ -1056,6 +1067,7 @@ func init() { pluginCmd.AddCommand(pluginStatusCmd) pluginCmd.AddCommand(pluginRemoveCmd) pluginCmd.AddCommand(pluginValidateCmd) + pluginValidateCmd.Flags().BoolVar(&pluginValidateStrict, "strict", false, "Also enforce permissions/step consistency (requires a permissions: block)") pluginCmd.AddCommand(pluginManifestCmd) pluginManifestCmd.Flags().StringVarP(&pluginManifestOut, "output", "o", "", "Write manifest to this file instead of stdout") diff --git a/docs-gen/plugin.md b/docs-gen/plugin.md index b80866e..c431ea9 100644 --- a/docs-gen/plugin.md +++ b/docs-gen/plugin.md @@ -154,11 +154,22 @@ instance. Checks schema, semver, known step/condition/config types, that the containing directory matches the plugin name, and that every {{ config.X }} template reference points at a declared config parameter. +With --strict, also enforce that the declared permissions: block is consistent +with the plugin's steps (e.g. instance.root=false must have no remote step that +runs as root). --strict requires a permissions: block. The official registry's +CI runs --strict so a published plugin's declared capability surface is enforced. + Examples: spawn plugin validate ./plugins/tailscale/plugin.yaml - spawn plugin validate ./plugins/*/plugin.yaml + spawn plugin validate --strict ./plugins/*/plugin.yaml ``` -spawn plugin validate ... +spawn plugin validate ... [flags] ``` +**Flags:** + +| Flag | Short | Type | Default | Description | +|------|-------|------|---------|-------------| +| `--strict` | | bool | | Also enforce permissions/step consistency (requires a permissions: block) | + diff --git a/pkg/plugin/consistency_test.go b/pkg/plugin/consistency_test.go new file mode 100644 index 0000000..554e025 --- /dev/null +++ b/pkg/plugin/consistency_test.go @@ -0,0 +1,153 @@ +package plugin_test + +import ( + "strings" + "testing" + + "github.com/spore-host/spawn/pkg/plugin" +) + +func TestValidatePermissionConsistency(t *testing.T) { + cases := []struct { + name string + spec string + wantErr string // "" = expect pass + }{ + { + name: "consistent: root=true with a root run step", + spec: `name: p +version: v1.0.0 +description: d +permissions: + instance: + root: true + network: true +remote: + install: + - type: fetch + url: https://example.com/x + dest: /tmp/x + - type: run + run: install /tmp/x /usr/local/bin/x +`, + wantErr: "", + }, + { + name: "root=false but a run step runs as root", + spec: `name: p +version: v1.0.0 +description: d +permissions: + instance: + root: false +remote: + install: + - type: run + run: systemctl enable x +`, + wantErr: "runs as root but permissions.instance.root=false", + }, + { + name: "root=false but a fetch step (always root)", + spec: `name: p +version: v1.0.0 +description: d +permissions: + instance: + root: false + network: true +remote: + install: + - type: fetch + url: https://example.com/x + dest: /tmp/x +`, + wantErr: "runs as root", + }, + { + name: "root=false consistent: only as_user run steps", + spec: `name: p +version: v1.0.0 +description: d +permissions: + instance: + root: false +remote: + install: + - type: run + run: whoami + as_user: true +`, + wantErr: "", + }, + { + name: "instance.network=false but a fetch downloads", + spec: `name: p +version: v1.0.0 +description: d +permissions: + instance: + root: true + network: false +remote: + install: + - type: fetch + url: https://example.com/x + dest: /tmp/x +`, + wantErr: "permissions.instance.network=false", + }, + { + name: "controller.network=false but a local fetch", + spec: `name: p +version: v1.0.0 +description: d +permissions: + controller: + network: false + instance: + root: true +local: + provision: + - type: fetch + url: https://example.com/x + dest: /tmp/x +`, + wantErr: "permissions.controller.network=false", + }, + { + name: "missing permissions block fails strict", + spec: `name: p +version: v1.0.0 +description: d +remote: + install: + - type: run + run: "true" +`, + wantErr: "permissions: block is required", + }, + } + + for _, tc := range cases { + t.Run(tc.name, func(t *testing.T) { + spec, err := plugin.ParseSpec([]byte(tc.spec)) + if err != nil { + t.Fatalf("ParseSpec: %v", err) + } + err = spec.ValidatePermissionConsistency() + if tc.wantErr == "" { + if err != nil { + t.Errorf("expected consistency to pass, got: %v", err) + } + return + } + if err == nil { + t.Fatalf("expected error containing %q, got nil", tc.wantErr) + } + if !strings.Contains(err.Error(), tc.wantErr) { + t.Errorf("error %q does not contain %q", err.Error(), tc.wantErr) + } + }) + } +} diff --git a/pkg/plugin/validate.go b/pkg/plugin/validate.go index 1466de7..b3e074b 100644 --- a/pkg/plugin/validate.go +++ b/pkg/plugin/validate.go @@ -275,6 +275,93 @@ func (s *PluginSpec) invalidTemplateRefs() []string { return keys } +// remoteRunsAsRoot reports whether a remote step executes as root on the +// instance. spored runs every remote step as root EXCEPT a "run" step that opts +// into as_user (fetch/extract always run as root). Mirrors remote_executor.go. +func remoteRunsAsRoot(st Step) bool { + if st.Type == "run" { + return !st.AsUser + } + return true // fetch / extract +} + +// ValidatePermissionConsistency performs STRICTER, publish-time checks that the +// author-declared permissions: block is consistent with the plugin's actual +// steps — so the declaration surfaced by `spawn plugin inspect` can be trusted, +// not just decorative. It is separate from Validate (which is always-on and +// intentionally lenient) because these checks only make sense at publish time +// for the official registry; a plugin without a permissions: block is skipped. +// +// Checks: +// - instance.root=false must have NO remote step that runs as root (every +// remote run step must be as_user, and there must be no fetch/extract step, +// which always run as root); +// - instance.network=false must have no fetch step (a transitive download); +// - controller.network=false must have no local step with a fetch/URL. +func (s *PluginSpec) ValidatePermissionConsistency() error { + if s.Permissions == nil { + return &ValidationError{Problems: []string{"permissions: block is required for publish-time consistency checks"}} + } + var probs []string + add := func(format string, args ...interface{}) { + probs = append(probs, fmt.Sprintf(format, args...)) + } + + remoteGroups := []struct { + label string + steps []Step + }{ + {"remote.install", s.Remote.Install}, + {"remote.configure", s.Remote.Configure}, + {"remote.start", s.Remote.Start}, + {"remote.stop", s.Remote.Stop}, + {"remote.health.steps", s.Remote.Health.Steps}, + } + + if !s.Permissions.Instance.Root { + for _, g := range remoteGroups { + for i, st := range g.steps { + if remoteRunsAsRoot(st) { + add("%s[%d]: runs as root but permissions.instance.root=false (a %q step needs as_user, and fetch/extract always run as root)", g.label, i, st.Type) + } + } + } + } + + if !s.Permissions.Instance.Network { + for _, g := range remoteGroups { + for i, st := range g.steps { + if st.Type == "fetch" { + add("%s[%d]: is a fetch (network download) but permissions.instance.network=false", g.label, i) + } + } + } + } + + if !s.Permissions.Controller.Network { + for _, g := range []struct { + label string + steps []Step + }{ + {"local.provision", s.Local.Provision}, + {"local.deprovision", s.Local.Deprovision}, + {"local.reconcile", s.Local.Reconcile}, + } { + for i, st := range g.steps { + if st.Type == "fetch" || st.URL != "" { + add("%s[%d]: performs a network fetch but permissions.controller.network=false", g.label, i) + } + } + } + } + + if len(probs) > 0 { + sort.Strings(probs) + return &ValidationError{Problems: probs} + } + return nil +} + // ValidateSpecFile parses and fully validates a plugin.yaml at path, inferring // the expected plugin name from the containing directory (the registry layout // plugins//plugin.yaml). @@ -286,3 +373,20 @@ func ValidateSpecFile(path string) error { dirName := filepath.Base(filepath.Dir(path)) return spec.Validate(dirName) } + +// ValidateSpecFileStrict runs ValidateSpecFile plus the publish-time permission/ +// step consistency checks (ValidatePermissionConsistency). Used by the registry's +// lint CI (`spawn plugin validate --strict`). A plugin with no permissions: block +// passes the base validation but fails strict, so official plugins must declare +// their capability surface. +func ValidateSpecFileStrict(path string) error { + spec, err := ParseSpecFile(path) + if err != nil { + return err + } + dirName := filepath.Base(filepath.Dir(path)) + if verr := spec.Validate(dirName); verr != nil { + return verr + } + return spec.ValidatePermissionConsistency() +}