From 9ceccbb0e45c1b66e063c25afda370d0680b7fb5 Mon Sep 17 00:00:00 2001 From: Samuele Verzi Date: Mon, 7 Sep 2026 16:12:11 +0200 Subject: [PATCH] Verify plugin installs against a public key MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Plugins had no key surface at all: a cosign key-pair-signed artifact could only fail install-time verification, and `--allow-unsigned` could not rescue it because the artifact is genuinely signed. Skills gained the install-side key path in #6447; this transliterates it onto plugins, so a key-signed plugin can be installed project-scoped by supplying the matching public key, which is then pinned in the lock entry and reused. The key is the only trust anchor a key-pair bundle can have — no certificate, no transparency log — so it has to come from outside the artifact, and dispatch is lock-first: a key-pinned entry verifies against the key the lock records, and every disagreement with a supplied key is refused rather than resolved by precedence. Scope is install-only, matching skills v1: no `--public-key` on upgrade or sync, and no in-place re-anchor. Part of #6442 Co-Authored-By: Claude Opus 5 (1M context) Signed-off-by: Samuele Verzi --- cmd/thv/app/ai_plugin_info.go | 38 ++- cmd/thv/app/ai_plugin_install.go | 18 ++ cmd/thv/app/ai_plugin_trust_test.go | 46 ++- docs/cli/thv_ai-plugin_install.md | 1 + docs/server/docs.go | 4 + docs/server/swagger.json | 4 + docs/server/swagger.yaml | 7 + pkg/api/v1/plugins.go | 1 + pkg/api/v1/plugins_test.go | 41 +++ pkg/api/v1/plugins_types.go | 5 + pkg/plugins/client/client.go | 1 + pkg/plugins/client/client_test.go | 28 ++ pkg/plugins/client/dto.go | 4 + pkg/plugins/options.go | 9 + pkg/plugins/pluginsvc/install.go | 10 + pkg/plugins/pluginsvc/verify.go | 264 ++++++++++++++- pkg/plugins/pluginsvc/verify_test.go | 460 ++++++++++++++++++++++++++- 17 files changed, 917 insertions(+), 24 deletions(-) diff --git a/cmd/thv/app/ai_plugin_info.go b/cmd/thv/app/ai_plugin_info.go index 90bad43ed0..5c3823dbce 100644 --- a/cmd/thv/app/ai_plugin_info.go +++ b/cmd/thv/app/ai_plugin_info.go @@ -6,6 +6,7 @@ package app import ( "encoding/json" "fmt" + "io" "maps" "os" "slices" @@ -81,18 +82,7 @@ func printAIPluginInfoText(info *plugins.PluginInfo) { _, _ = fmt.Fprintf(w, "Name:\t%s\n", info.Metadata.Name) _, _ = fmt.Fprintf(w, "Version:\t%s\n", info.Metadata.Version) - switch { - case info.Provenance != nil && info.Provenance.Provisional: - _, _ = fmt.Fprintf(w, "Signed by:\t%s (provisional)\n", info.Provenance.SignerIdentity) - _, _ = fmt.Fprintf(w, "Cert issuer:\t%s\n", info.Provenance.CertIssuer) - case info.Provenance != nil: - _, _ = fmt.Fprintf(w, "Signed by:\t%s\n", info.Provenance.SignerIdentity) - _, _ = fmt.Fprintf(w, "Cert issuer:\t%s\n", info.Provenance.CertIssuer) - case info.Unsigned: - _, _ = fmt.Fprintf(w, "Signed by:\t(unsigned — explicit exception)\n") - case info.TrustUnrecorded: - _, _ = fmt.Fprintf(w, "Signed by:\t(trust unrecorded — run 'thv ai-plugin sync')\n") - } + printAIPluginTrustState(w, info) _, _ = fmt.Fprintf(w, "Description:\t%s\n", info.Metadata.Description) if s := info.InstalledPlugin; s != nil { @@ -135,6 +125,30 @@ func printAIPluginInfoText(info *plugins.PluginInfo) { _ = w.Flush() } +// printAIPluginTrustState renders the trust anchor the lock file records for +// the plugin. Every state gets a line of its own: a state that rendered as no +// trust block would be indistinguishable from a plugin nothing is pinning. +func printAIPluginTrustState(w io.Writer, info *plugins.PluginInfo) { + switch { + // Checked before the identity cases: a key-pinned entry has no signer + // identity and no cert issuer, so those would render as empty values and + // read exactly like an untracked install. + case info.Provenance != nil && info.Provenance.PublicKey != "": + _, _ = fmt.Fprintf(w, "Signed by:\t(cosign key pair)\n") + _, _ = fmt.Fprintf(w, "Public key:\t%s\n", info.Provenance.PublicKey) + case info.Provenance != nil && info.Provenance.Provisional: + _, _ = fmt.Fprintf(w, "Signed by:\t%s (provisional)\n", info.Provenance.SignerIdentity) + _, _ = fmt.Fprintf(w, "Cert issuer:\t%s\n", info.Provenance.CertIssuer) + case info.Provenance != nil: + _, _ = fmt.Fprintf(w, "Signed by:\t%s\n", info.Provenance.SignerIdentity) + _, _ = fmt.Fprintf(w, "Cert issuer:\t%s\n", info.Provenance.CertIssuer) + case info.Unsigned: + _, _ = fmt.Fprintf(w, "Signed by:\t(unsigned — explicit exception)\n") + case info.TrustUnrecorded: + _, _ = fmt.Fprintf(w, "Signed by:\t(trust unrecorded — run 'thv ai-plugin sync')\n") + } +} + // formatComponentInventory renders a ComponentInventory (map[string]int) as a // sorted, space-separated "key=count" sequence for deterministic output. func formatComponentInventory(inv plugins.ComponentInventory) string { diff --git a/cmd/thv/app/ai_plugin_install.go b/cmd/thv/app/ai_plugin_install.go index 8c16c6c0dd..2373919ca8 100644 --- a/cmd/thv/app/ai_plugin_install.go +++ b/cmd/thv/app/ai_plugin_install.go @@ -18,6 +18,7 @@ var ( aiPluginInstallProjectRoot string aiPluginInstallGroup string aiPluginInstallAllowUnsigned bool + aiPluginInstallPublicKey string ) var aiPluginInstallCmd = &cobra.Command{ @@ -49,6 +50,10 @@ func init() { aiPluginInstallCmd.Flags().StringVar(&aiPluginInstallGroup, "group", "", "Group to add the plugin to after installation") aiPluginInstallCmd.Flags().BoolVar(&aiPluginInstallAllowUnsigned, "allow-unsigned", false, "Allow installing a project-scoped plugin without a verified signature (recorded in the lock file)") + aiPluginInstallCmd.Flags().StringVar(&aiPluginInstallPublicKey, "public-key", "", + "Path to the cosign public key (cosign.pub) a key-pair-signed plugin must verify against."+ + " Required the first time such a plugin is installed project-scoped; the key is then pinned"+ + " in the lock file and reused automatically") } func aiPluginInstallCmdFunc(cmd *cobra.Command, args []string) error { @@ -59,6 +64,14 @@ func aiPluginInstallCmdFunc(cmd *cobra.Command, args []string) error { return err } + // Shared with `thv skill install`: the flag names a file, but the API + // carries the key material, because the server may be another process on + // another host where that path names nothing — or something else. + publicKey, err := readInstallPublicKey(aiPluginInstallPublicKey) + if err != nil { + return err + } + result, err := c.Install(cmd.Context(), plugins.InstallOptions{ Name: args[0], Scope: plugins.Scope(aiPluginInstallScope), @@ -67,6 +80,7 @@ func aiPluginInstallCmdFunc(cmd *cobra.Command, args []string) error { ProjectRoot: projectRoot, Group: aiPluginInstallGroup, AllowUnsigned: aiPluginInstallAllowUnsigned, + PublicKey: publicKey, }) if err != nil { return formatAIPluginError("install plugin", err) @@ -90,6 +104,10 @@ func printPluginInstallTrust(result *plugins.InstallResult) { } name := result.Plugin.Metadata.Name switch { + // Before the identity cases: a key-pinned install has no signer identity + // to name, and "signed by " with nothing after it is worse than silence. + case result.Provenance != nil && result.Provenance.PublicKey != "": + fmt.Printf("Installed %s (signed by a cosign key pair; the pinned public key is in the lock file)\n", name) case result.Provenance != nil && result.Provenance.Provisional: fmt.Printf("Installed %s (signed by %s; verification provisional — see lock file)\n", name, result.Provenance.SignerIdentity) diff --git a/cmd/thv/app/ai_plugin_trust_test.go b/cmd/thv/app/ai_plugin_trust_test.go index ede9476896..d241ed1ddb 100644 --- a/cmd/thv/app/ai_plugin_trust_test.go +++ b/cmd/thv/app/ai_plugin_trust_test.go @@ -13,12 +13,17 @@ import ( "github.com/stacklok/toolhive/pkg/plugins" ) +// testCLIPublicKeyB64 stands in for the base64 DER SPKI a key-pinned lock +// entry records; the CLI renders it verbatim and parses nothing. +const testCLIPublicKeyB64 = "MFkwEwYHKoZIzj0CAQYIKoZIzj0DAQcDQgAExlVDpbnOEv2fH3gS8n7UCHS9Gs0wKxIPR5EAcl8F1jSxlxAV/pll0NsSiuAK95Ws4Fpkn+5QkdVKNXy7LHgb2A==" + // TestAIPluginPushSigningFlags pins the signed-by-default publish surface: // the keyless flags must exist, the opt-out must not be preset (a defaulted // --no-sign would publish unsigned artifacts silently), and --key must NOT be // offered — ToolHive cannot verify key-signed artifacts at install time, so -// the flag would only produce uninstallable plugins (#6442). Re-add it in the -// change that makes key verification work. +// the flag would only produce uninstallable plugins (#6442). Install-time key +// verification now exists; re-add the flag in the change that restores plugin +// push signing, which is what closes #6442. func TestAIPluginPushSigningFlags(t *testing.T) { t.Parallel() @@ -27,12 +32,24 @@ func TestAIPluginPushSigningFlags(t *testing.T) { require.NotNil(t, flag, "thv ai-plugin push must expose --%s", name) } assert.Nil(t, aiPluginPushCmd.Flags().Lookup("key"), - "plugin signing is keyless-only; --key must not be advertised until install can verify it") + "plugin push is still keyless-only; --key returns with the push half of #6442") assert.Equal(t, "false", aiPluginPushCmd.Flags().Lookup("no-sign").DefValue, "pushing unsigned must always be an explicit choice") assert.Empty(t, aiPluginPushCmd.Flags().Lookup("identity-token").DefValue) } +// TestAIPluginInstallKeyFlag pins the consuming half of key signing: without +// --public-key on install there is no way to supply the trust anchor a +// key-signed artifact needs, since the key is recoverable from neither the +// artifact nor its bundle. +func TestAIPluginInstallKeyFlag(t *testing.T) { + t.Parallel() + + flag := aiPluginInstallCmd.Flags().Lookup("public-key") + require.NotNil(t, flag, "thv ai-plugin install must expose --public-key") + assert.Empty(t, flag.DefValue, "there is no default trust anchor to assume") +} + // TestPrintAIPluginInfoTextTrustStates covers each trust state the info // command renders. RFC THV-0080 wants the pinned identity visible at read // time, so a state that silently renders as "no trust block" is a bug. @@ -59,6 +76,20 @@ func TestPrintAIPluginInfoTextTrustStates(t *testing.T) { }, wantAbsent: []string{"provisional", "unsigned"}, }, + { + // A key-pinned entry has no signer identity and no cert issuer, so + // the identity rendering would print empty values and read exactly + // like an untracked install. + name: "key pinned", + info: plugins.PluginInfo{Provenance: &plugins.ProvenanceInfo{ + PublicKey: testCLIPublicKeyB64, + }}, + wantLines: []string{ + "Signed by: (cosign key pair)", + "Public key: " + testCLIPublicKeyB64, + }, + wantAbsent: []string{"Cert issuer", "provisional", "unsigned"}, + }, { name: "provisional", info: plugins.PluginInfo{Provenance: &plugins.ProvenanceInfo{ @@ -130,6 +161,15 @@ func TestPrintPluginInstallTrust(t *testing.T) { }, want: "Installed my-plugin (signed by /.github/workflows/release.yml)\n", }, + { + name: "key pinned", + result: &plugins.InstallResult{ + Plugin: plugins.InstalledPlugin{Metadata: plugins.PluginMetadata{Name: "my-plugin"}}, + Provenance: &plugins.ProvenanceInfo{PublicKey: testCLIPublicKeyB64}, + }, + want: "Installed my-plugin (signed by a cosign key pair; " + + "the pinned public key is in the lock file)\n", + }, { name: "provisional", result: &plugins.InstallResult{ diff --git a/docs/cli/thv_ai-plugin_install.md b/docs/cli/thv_ai-plugin_install.md index bb5636c179..fd2f770dfc 100644 --- a/docs/cli/thv_ai-plugin_install.md +++ b/docs/cli/thv_ai-plugin_install.md @@ -31,6 +31,7 @@ thv ai-plugin install [plugin-name] [flags] --group string Group to add the plugin to after installation -h, --help help for install --project-root string Project root path for project-scoped installs + --public-key string Path to the cosign public key (cosign.pub) a key-pair-signed plugin must verify against. Required the first time such a plugin is installed project-scoped; the key is then pinned in the lock file and reused automatically --scope string Installation scope (user, project) (default "user") ``` diff --git a/docs/server/docs.go b/docs/server/docs.go index 5bc3214c06..878fc6acb9 100644 --- a/docs/server/docs.go +++ b/docs/server/docs.go @@ -3563,6 +3563,10 @@ const docTemplate = `{ "description": "ProjectRoot is the project root path for project-scoped installs", "type": "string" }, + "public_key": { + "description": "PublicKey is the base64-encoded DER SPKI cosign public key the artifact\nmust verify against, for artifacts signed with a cosign key pair rather\nthan keylessly. Required the first time such an artifact is installed\nproject-scoped, and pinned in the lock file from then on.", + "type": "string" + }, "scope": { "$ref": "#/components/schemas/github_com_stacklok_toolhive_pkg_plugins.Scope" }, diff --git a/docs/server/swagger.json b/docs/server/swagger.json index 5dce3e3555..d0e8a2ef1f 100644 --- a/docs/server/swagger.json +++ b/docs/server/swagger.json @@ -3556,6 +3556,10 @@ "description": "ProjectRoot is the project root path for project-scoped installs", "type": "string" }, + "public_key": { + "description": "PublicKey is the base64-encoded DER SPKI cosign public key the artifact\nmust verify against, for artifacts signed with a cosign key pair rather\nthan keylessly. Required the first time such an artifact is installed\nproject-scoped, and pinned in the lock file from then on.", + "type": "string" + }, "scope": { "$ref": "#/components/schemas/github_com_stacklok_toolhive_pkg_plugins.Scope" }, diff --git a/docs/server/swagger.yaml b/docs/server/swagger.yaml index a95ad2b30e..7ad50b8294 100644 --- a/docs/server/swagger.yaml +++ b/docs/server/swagger.yaml @@ -3352,6 +3352,13 @@ components: project_root: description: ProjectRoot is the project root path for project-scoped installs type: string + public_key: + description: |- + PublicKey is the base64-encoded DER SPKI cosign public key the artifact + must verify against, for artifacts signed with a cosign key pair rather + than keylessly. Required the first time such an artifact is installed + project-scoped, and pinned in the lock file from then on. + type: string scope: $ref: '#/components/schemas/github_com_stacklok_toolhive_pkg_plugins.Scope' version: diff --git a/pkg/api/v1/plugins.go b/pkg/api/v1/plugins.go index f4c7a7f736..328c7dc270 100644 --- a/pkg/api/v1/plugins.go +++ b/pkg/api/v1/plugins.go @@ -134,6 +134,7 @@ func (s *PluginsRoutes) installPlugin(w http.ResponseWriter, r *http.Request) er Force: req.Force, Group: req.Group, AllowUnsigned: req.AllowUnsigned, + PublicKey: req.PublicKey, }) if err != nil { return err diff --git a/pkg/api/v1/plugins_test.go b/pkg/api/v1/plugins_test.go index bf19586f38..438d26ea37 100644 --- a/pkg/api/v1/plugins_test.go +++ b/pkg/api/v1/plugins_test.go @@ -767,6 +767,47 @@ func TestPluginsInstallLocationHeader(t *testing.T) { assert.Equal(t, "/api/v1beta/plugins/my-plugin", rec.Header().Get("Location")) } +// TestPluginsInstallCarriesPublicKey pins the API-side half of the key path: a +// public_key that dies at the handler would leave a key-signed artifact +// failing verification while the caller is told to supply the key they did. +func TestPluginsInstallCarriesPublicKey(t *testing.T) { + t.Parallel() + + const encodedKey = "MFkwEwYHKoZIzj0CAQYIKoZIzj0DAQcDQgAExlVDpbnOEv2fH3gS8n7UCHS9Gs0wKxIPR5" + + "EAcl8F1jSxlxAV/pll0NsSiuAK95Ws4Fpkn+5QkdVKNXy7LHgb2A==" + + ctrl := gomock.NewController(t) + mockSvc := plugmocks.NewMockPluginService(ctrl) + + mockSvc.EXPECT().Install(gomock.Any(), plugins.InstallOptions{ + Name: "my-plugin", + Scope: plugins.ScopeProject, + ProjectRoot: "/tmp/project", + PublicKey: encodedKey, + }).Return(&plugins.InstallResult{ + Plugin: plugins.InstalledPlugin{ + Metadata: plugins.PluginMetadata{Name: "my-plugin"}, + Scope: plugins.ScopeProject, + Status: plugins.InstallStatusInstalled, + }, + Provenance: &plugins.ProvenanceInfo{PublicKey: encodedKey}, + }, nil) + + router := chi.NewRouter() + router.Mount("/", PluginsRouter(mockSvc)) + + body := `{"name":"my-plugin","scope":"project","project_root":"/tmp/project",` + + `"public_key":"` + encodedKey + `"}` + req := httptest.NewRequest(http.MethodPost, "/", strings.NewReader(body)) + req.Header.Set("Content-Type", "application/json") + rec := httptest.NewRecorder() + router.ServeHTTP(rec, req) + + require.Equal(t, http.StatusCreated, rec.Code) + assert.Contains(t, rec.Body.String(), `"public_key":"`+encodedKey+`"`, + "the pinned key must come back so the CLI can report the anchor it recorded") +} + // TestPluginsInstallCarriesAllowUnsigned pins the API-side half of the // unsigned-install exception: a request body that sets allow_unsigned must // reach the service as InstallOptions.AllowUnsigned, or the flag dies at the diff --git a/pkg/api/v1/plugins_types.go b/pkg/api/v1/plugins_types.go index e515395778..e56ee87be6 100644 --- a/pkg/api/v1/plugins_types.go +++ b/pkg/api/v1/plugins_types.go @@ -35,6 +35,11 @@ type installPluginRequest struct { // verified signature; the exception is recorded in the project's lock // file. AllowUnsigned bool `json:"allow_unsigned,omitempty"` + // PublicKey is the base64-encoded DER SPKI cosign public key the artifact + // must verify against, for artifacts signed with a cosign key pair rather + // than keylessly. Required the first time such an artifact is installed + // project-scoped, and pinned in the lock file from then on. + PublicKey string `json:"public_key,omitempty"` // Group is the group name to add the plugin to after installation Group string `json:"group,omitempty"` } diff --git a/pkg/plugins/client/client.go b/pkg/plugins/client/client.go index 6f6fd7a1ed..e17a018c29 100644 --- a/pkg/plugins/client/client.go +++ b/pkg/plugins/client/client.go @@ -215,6 +215,7 @@ func (c *Client) Install(ctx context.Context, opts plugins.InstallOptions) (*plu Force: opts.Force, Group: opts.Group, AllowUnsigned: opts.AllowUnsigned, + PublicKey: opts.PublicKey, } var resp installResponse diff --git a/pkg/plugins/client/client_test.go b/pkg/plugins/client/client_test.go index aa1db13ff9..6dda1d2f69 100644 --- a/pkg/plugins/client/client_test.go +++ b/pkg/plugins/client/client_test.go @@ -1041,6 +1041,34 @@ func TestInstallCarriesAllowUnsigned(t *testing.T) { assert.True(t, got.AllowUnsigned, "allow_unsigned must reach the server") } +// TestInstallCarriesPublicKey is the same guard for the cosign public key: the +// CLI reads the key file and sends the material, so a field dropped here would +// make every --public-key install fail as though no key had been given. +func TestInstallCarriesPublicKey(t *testing.T) { + t.Parallel() + + const encodedKey = "MFkwEwYHKoZIzj0CAQYIKoZIzj0DAQcDQgAExlVDpbnOEv2fH3gS8n7UCHS9Gs0wKxIPR5" + + "EAcl8F1jSxlxAV/pll0NsSiuAK95Ws4Fpkn+5QkdVKNXy7LHgb2A==" + + var got installRequest + srv := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { + require.NoError(t, json.NewDecoder(r.Body).Decode(&got)) + w.Header().Set("Content-Type", "application/json") + w.WriteHeader(http.StatusCreated) + _ = json.NewEncoder(w).Encode(installResponse{}) + })) + t.Cleanup(srv.Close) + + _, err := newTestClient(t, srv).Install(t.Context(), plugins.InstallOptions{ + Name: "my-plugin", + Scope: plugins.ScopeProject, + ProjectRoot: "/tmp/project", + PublicKey: encodedKey, + }) + require.NoError(t, err) + assert.Equal(t, encodedKey, got.PublicKey, "public_key must reach the server") +} + // TestInstallReturnsTrustState guards the response half of the same DTO // boundary: the CLI is a pure HTTP client, so a provenance block dropped // here would make every signed install print as if it were untracked. diff --git a/pkg/plugins/client/dto.go b/pkg/plugins/client/dto.go index 215e5853f6..fa8e3a0111 100644 --- a/pkg/plugins/client/dto.go +++ b/pkg/plugins/client/dto.go @@ -18,6 +18,10 @@ type installRequest struct { // AllowUnsigned mirrors plugins.InstallOptions.AllowUnsigned; without // it here the CLI flag would silently never reach the server. AllowUnsigned bool `json:"allow_unsigned,omitempty"` + // PublicKey mirrors plugins.InstallOptions.PublicKey: the base64 DER SPKI + // the CLI encoded from the --public-key file, since a path would not + // resolve on a server in another process or on another host. + PublicKey string `json:"public_key,omitempty"` } type validateRequest struct { diff --git a/pkg/plugins/options.go b/pkg/plugins/options.go index f0d64ae166..000501e6c2 100644 --- a/pkg/plugins/options.go +++ b/pkg/plugins/options.go @@ -39,6 +39,15 @@ type InstallOptions struct { // servers to the client that loads it, so this is an explicit // per-install trust decision, never a default. AllowUnsigned bool `json:"allow_unsigned,omitempty"` + // PublicKey is the base64-encoded DER SPKI cosign public key a + // project-scoped install must verify the artifact against, for artifacts + // signed with a cosign key pair rather than keylessly. Required on true + // first use of such an artifact — the signing key is recoverable from + // neither the artifact nor its bundle, so nothing else can supply the + // trust anchor — and pinned into the lock entry, which supplies it on + // every install thereafter. A value that conflicts with what the lock + // already pins is rejected, never ignored. + PublicKey string `json:"public_key,omitempty"` // LayerData is the tar.gz content from an OCI layer. Internal use only — NOT exposed via HTTP API. LayerData []byte `json:"-"` // Reference is the full OCI reference (e.g. ghcr.io/org/plugin:v1). diff --git a/pkg/plugins/pluginsvc/install.go b/pkg/plugins/pluginsvc/install.go index fbfb54d4af..d2905eb6b5 100644 --- a/pkg/plugins/pluginsvc/install.go +++ b/pkg/plugins/pluginsvc/install.go @@ -48,6 +48,16 @@ func (s *service) install( opts.LockSource = opts.Name } + // Checked here, before any resolve or fetch work: this is the only path a + // caller-supplied public key arrives through, and rejecting it now means a + // key that could never be used is reported as bad input rather than as a + // verification failure after the artifact has been pulled. Lock-driven + // callers (sync, upgrade) never set it — they verify against the key the + // lock records. + if err := validateInstallPublicKey(opts, scope); err != nil { + return nil, err + } + // Git references are dispatched first; the prefix is unambiguous and // cannot collide with OCI references. installFromGit holds the per-plugin // lock across extraction, DB, group, lock-file, and rollback unless the diff --git a/pkg/plugins/pluginsvc/verify.go b/pkg/plugins/pluginsvc/verify.go index c372a4c880..e25984556a 100644 --- a/pkg/plugins/pluginsvc/verify.go +++ b/pkg/plugins/pluginsvc/verify.go @@ -50,6 +50,28 @@ func shouldVerifyInstall(opts plugins.InstallOptions, scope plugins.Scope) bool return scope == plugins.ScopeProject && opts.ProjectRoot != "" } +// validateInstallPublicKey rejects a supplied public key before any resolve +// or fetch work begins. Both checks exist so the key is never accepted and +// then quietly unused: an install that does not verify would drop it on the +// floor, and a malformed one would otherwise surface as a verification +// failure deep in the install, long after the input that caused it. +func validateInstallPublicKey(opts plugins.InstallOptions, scope plugins.Scope) error { + if opts.PublicKey == "" { + return nil + } + if !shouldVerifyInstall(opts, scope) { + return httperr.WithCode( + errors.New("public_key (--public-key) applies to project-scoped installs, which are the ones"+ + " whose trust anchor a lock file records; this install would verify nothing"), + http.StatusBadRequest, + ) + } + if _, err := verifier.DecodePublicKey(opts.PublicKey); err != nil { + return httperr.WithCode(fmt.Errorf("public_key: %w", err), http.StatusBadRequest) + } + return nil +} + // provenanceDecision is the outcome of install-time verification: either a // verified identity (with the bundle backing it) or an explicit unsigned // exception. @@ -99,6 +121,12 @@ func signedDecision(result *verifier.Result, pluginName string) (*provenanceDeci // before anything is extracted or recorded. The identity expected by the // lock file (if any) is enforced inside the verifier's Sigstore policy; // trust on first use records whatever identity verification observes. +// +// An entry pinned to a cosign public key, or a first install that supplies +// one, takes the key path instead. Which path runs is decided by the lock +// file and the caller, never by what the artifact turns out to carry: letting +// the artifact select its own verification policy would let a republished +// key-signed artifact walk out of the identity its entry is pinned to. func (s *service) verifyOCIInstall( ctx context.Context, opts plugins.InstallOptions, @@ -108,6 +136,13 @@ func (s *service) verifyOCIInstall( if err != nil { return nil, err } + keyAnchor, err := resolveKeyAnchor(opts, pluginName, expected, expectUnsigned) + if err != nil { + return nil, err + } + if keyAnchor != "" { + return s.verifyOCIInstallWithKey(ctx, keyAnchor, pluginName, ref, digest) + } if opts.AllowSignerChange { // The signer-change guard was explicitly overridden: verify the // chain of trust only and re-record whatever identity is observed. @@ -131,6 +166,167 @@ func (s *service) verifyOCIInstall( return signedDecision(result, pluginName) } +// resolveKeyAnchor decides which cosign public key, if any, this install +// verifies against, returning "" for the ordinary keyless path. +// +// Dispatch is lock-first: a key-pinned entry selects the key path using the +// key the LOCK records, so a supplied key can confirm that pin but never +// replace it. A supplied key is itself the anchor only on true first use, +// where nothing is recorded yet and the key is the only thing that can supply +// one. +// +// Every disagreement between the supplied key and the recorded trust state is +// an error rather than a precedence rule. Silently preferring one of two +// conflicting anchors is how a mistyped --public-key installs as though it had +// been honored — and a caller who names a trust anchor has said they want it +// enforced, so the honest answer to "that is not the anchor here" is to stop. +// +// Unlike skills there is no catalog arm: a plugin install presents no +// catalog-declared provenance expectation, so first use has nothing but the +// supplied key to weigh. +func resolveKeyAnchor( + opts plugins.InstallOptions, + pluginName string, + expected *lockfile.Provenance, + expectUnsigned bool, +) (string, error) { + supplied := opts.PublicKey + locked := "" + if expected != nil { + locked = expected.PublicKey + } + + if opts.AllowSignerChange { + // The override re-verifies from scratch and re-records what it + // observes. For a key there is nothing to observe — a key-pair bundle + // carries no identity — so honoring a key here would mean re-anchoring + // to whatever key the caller named, on the strength of the caller + // having named it. That is the in-place re-anchor v1 deliberately does + // not offer. Without a key the override drops the recorded one and + // takes the keyless path, which is the supported key-to-keyless move. + if supplied != "" { + return "", httperr.WithCode( + fmt.Errorf("plugin %q: a public key cannot be combined with allow_signer_change;"+ + " re-anchoring an entry to a different key is not supported —"+ + " uninstall the plugin and reinstall it with the new key", pluginName), + http.StatusBadRequest, + ) + } + return "", nil + } + + switch { + case locked != "" && supplied != "" && supplied != locked: + return "", keyAnchorConflict(pluginName, + "is pinned to a different cosign public key than the one supplied") + case locked != "": + return locked, nil + case supplied == "": + return "", nil + // A key was supplied and the entry is not key-pinned. Each remaining case + // already records an anchor the key would have to displace. + case expected != nil: + return "", keyAnchorConflict(pluginName, + fmt.Sprintf("is pinned to signer %q, and a cosign key pair carries no certificate identity"+ + " that could satisfy it", expected.SignerIdentity)) + case expectUnsigned: + return "", keyAnchorConflict(pluginName, + "is recorded as an explicit unsigned exception, which a public key cannot upgrade in place") + default: + return supplied, nil + } +} + +// keyAnchorConflict reports a supplied public key that contradicts the trust +// state the lock file already records. The remedy is the same for all of them +// — v1 has no in-place re-anchor path — so it is stated once here. +func keyAnchorConflict(pluginName, problem string) error { + return httperr.WithCode( + fmt.Errorf("plugin %q %s; to install it under a different trust anchor,"+ + " uninstall the plugin and reinstall it", pluginName, problem), + http.StatusForbidden, + ) +} + +// lockedAnchorDescription names the trust anchor an entry records, for error +// messages that report an artifact failing to satisfy it. A key-pinned entry +// has no signer identity, and rendering it as one would print an empty pin +// and read as a bug in the lock file rather than a refusal. +func lockedAnchorDescription(expected *lockfile.Provenance) string { + if expected.PublicKey != "" { + return "a cosign public key" + } + return fmt.Sprintf("signer %q", expected.SignerIdentity) +} + +// verifyOCIInstallWithKey verifies the artifact against a cosign public key +// and records that key as the entry's trust anchor. +// +// Unlike the keyless path there is nothing to observe: a key-pair bundle +// carries no certificate, so the provenance recorded is the key that was +// checked rather than an identity read off the artifact. That makes this a +// weaker claim than keyless provenance — it says the holder of this key signed +// this artifact, and nothing about who that holder is — which is why the key +// has to come from outside the artifact every time. +func (s *service) verifyOCIInstallWithKey( + ctx context.Context, + encodedKey, pluginName, ref, digest string, +) (*provenanceDecision, error) { + // Re-decoded rather than carried down from validateInstallPublicKey: this + // key may instead have come from the lock file, and internal callers reach + // the install path without passing that entry check at all. + pubKeyPEM, err := verifier.DecodePublicKey(encodedKey) + if err != nil { + return nil, httperr.WithCode( + fmt.Errorf("plugin %q: pinned %w", pluginName, err), + http.StatusUnprocessableEntity, + ) + } + result, verifyErr := s.artifactVerifier().VerifyOCIWithKey(ctx, ref, digest, pubKeyPEM) + if verifyErr != nil { + return nil, classifyKeyVerifyError(verifyErr, pluginName) + } + if err := rejectOversizedBlob("sigstore bundle", pluginName, len(result.Bundle)); err != nil { + return nil, err + } + return &provenanceDecision{ + provenance: &lockfile.Provenance{PublicKey: encodedKey}, + bundle: result.Bundle, + }, nil +} + +// classifyKeyVerifyError maps a key-pair verification failure to the 403 the +// install API surfaces. allow_unsigned is deliberately not consulted on any +// arm: an install that named a public key asked for that key to be enforced, +// and the unsigned exception answers a different question. +func classifyKeyVerifyError(verifyErr error, pluginName string) error { + switch { + case errors.Is(verifyErr, verifier.ErrUnsigned): + return httperr.WithCode( + fmt.Errorf("plugin %q must verify against a cosign public key, but the artifact"+ + " carries no signature material at all", pluginName), + http.StatusForbidden, + ) + case errors.Is(verifyErr, verifier.ErrKeylessSigned): + return httperr.WithCode( + fmt.Errorf("plugin %q: %w; install it without a public key so its certificate identity"+ + " is verified and pinned instead", pluginName, verifyErr), + http.StatusForbidden, + ) + default: + // The wrong key and a corrupt signature are indistinguishable here, + // and saying so is more useful than picking one: the bundle records no + // key of its own, so the only fact available is that this key does not + // verify this signature. + return httperr.WithCode( + fmt.Errorf("plugin %q does not verify against the cosign public key it is checked against"+ + " — either the key is not the one that signed it, or the signature is damaged: %w", + pluginName, verifyErr), + http.StatusForbidden, + ) + } +} + // verifyGitInstall verifies the gitsign signature on the resolved commit // before anything is written or recorded. func (s *service) verifyGitInstall( @@ -144,6 +340,17 @@ func (s *service) verifyGitInstall( if err != nil { return nil, err } + // Refused rather than ignored, for the same reason the lock file refuses + // to store a key on a git entry: a commit signature is made with a Fulcio + // certificate, so there is no operation here a public key could take part + // in. + if opts.PublicKey != "" { + return nil, httperr.WithCode( + fmt.Errorf("plugin %q is installed from git, whose commit signature is verified against a"+ + " certificate; a cosign public key cannot verify it", pluginName), + http.StatusBadRequest, + ) + } if opts.AllowSignerChange { expected, expectUnsigned = nil, false } @@ -181,10 +388,20 @@ func verifyLocalInstall(opts plugins.InstallOptions, pluginName string) (*proven if err != nil { return nil, err } + // A local build has no registry signature material at all, so a public key + // would have nothing to check. Saying so beats accepting the key and then + // recording the install as unsigned anyway. + if opts.PublicKey != "" { + return nil, httperr.WithCode( + fmt.Errorf("plugin %q is a local build, which carries no registry signature for a"+ + " cosign public key to verify", pluginName), + http.StatusBadRequest, + ) + } if expected != nil { return nil, httperr.WithCode( - fmt.Errorf("plugin %q is locked to signer %q; a local build cannot satisfy it", - pluginName, expected.SignerIdentity), + fmt.Errorf("plugin %q is locked to %s; a local build cannot satisfy it", + pluginName, lockedAnchorDescription(expected)), http.StatusForbidden, ) } @@ -275,6 +492,7 @@ func provenanceInfoFromLock(p *lockfile.Provenance) *plugins.ProvenanceInfo { RepositoryRef: p.RepositoryRef, RunnerEnvironment: p.RunnerEnvironment, SigstoreURL: p.SigstoreURL, + PublicKey: p.PublicKey, Provisional: p.Provisional, } } @@ -362,18 +580,50 @@ func classifyInstallVerifyError( // deliberately no remedy here: the artifact IS signed, and recording it // as an unsigned exception would file a false trust decision in the lock. case errors.Is(verifyErr, verifier.ErrKeySigned): + return keySignedInstallError(pluginName, verifyErr, expected) + default: return httperr.WithCode( - fmt.Errorf("plugin %q: %w; re-publish it with keyless signing"+ - " (allow_unsigned does not apply — the artifact is signed)", - pluginName, verifyErr), + fmt.Errorf("signature verification failed for %q: %w", pluginName, verifyErr), http.StatusForbidden, ) - default: + } +} + +// keySignedInstallError reports a key-signed artifact that the keyless path +// could not verify. The remedy depends on what the entry already pins, so it +// is chosen from that rather than stated generically. +// +// With no anchor recorded, this is a first install of a key-signed artifact +// and --public-key is exactly the missing input. With a keyless identity +// pinned, --public-key is NOT the remedy: resolveKeyAnchor rejects a key +// against a certificate-pinned entry, because a key pair carries no identity +// that could satisfy it. Naming the flag there would send the caller into a +// conflict error one step later — the most likely way to reach this arm is +// also the one where the obvious advice is wrong. +// +// Either way allow_unsigned is no way out: the artifact IS signed, and +// recording it as an unsigned exception would file a false trust decision in +// the lock. +func keySignedInstallError(pluginName string, verifyErr error, expected *lockfile.Provenance) error { + if expected != nil { return httperr.WithCode( - fmt.Errorf("signature verification failed for %q: %w", pluginName, verifyErr), + fmt.Errorf("plugin %q: %w, but its lock entry is pinned to keyless signer %q;"+ + " a key pair carries no certificate identity that could satisfy that pin, so"+ + " supplying a public key is refused rather than allowed to displace it."+ + " Remove the lock entry and reinstall with `thv ai-plugin install --public-key`"+ + " to anchor it to the key"+ + " (allow_unsigned does not apply — the artifact is signed)", + pluginName, verifyErr, expected.SignerIdentity), http.StatusForbidden, ) } + return httperr.WithCode( + fmt.Errorf("plugin %q: %w; re-run `thv ai-plugin install --public-key` pointing at the cosign"+ + " public key it was signed with, and that key is pinned in the lock file for"+ + " subsequent installs (allow_unsigned does not apply — the artifact is signed)", + pluginName, verifyErr), + http.StatusForbidden, + ) } // classifySignatureError maps verifier sentinels to typed failure reasons diff --git a/pkg/plugins/pluginsvc/verify_test.go b/pkg/plugins/pluginsvc/verify_test.go index cc7fcec8f6..4320a49bb9 100644 --- a/pkg/plugins/pluginsvc/verify_test.go +++ b/pkg/plugins/pluginsvc/verify_test.go @@ -27,6 +27,11 @@ const ( testCertIssuer = "https://token.actions.githubusercontent.com" ) +// testPublicKeyB64 is a real P-256 public key in the base64 DER SPKI form the +// lock file stores, so validation and decoding exercise the real parser rather +// than a placeholder string. +const testPublicKeyB64 = "MFkwEwYHKoZIzj0CAQYIKoZIzj0DAQcDQgAExlVDpbnOEv2fH3gS8n7UCHS9Gs0wKxIPR5EAcl8F1jSxlxAV/pll0NsSiuAK95Ws4Fpkn+5QkdVKNXy7LHgb2A==" + func signedResult() *verifier.Result { return &verifier.Result{ Signed: true, @@ -491,13 +496,43 @@ func TestClassifyInstallVerifyErrorNamesKeySigned(t *testing.T) { err := classifyInstallVerifyError(verifier.ErrKeySigned, "some-plugin", nil, plugins.InstallOptions{}) assert.Contains(t, err.Error(), "cosign key pair") - assert.Contains(t, err.Error(), "re-publish it with keyless signing", - "the message must state the remedy, not merely the refusal") + assert.Contains(t, err.Error(), "--public-key", + "a first install of a key-signed artifact is exactly what --public-key is for") assert.Contains(t, err.Error(), "allow_unsigned does not apply") + assert.NotContains(t, err.Error(), "re-publish it with keyless signing", + "republishing is no longer the remedy — the artifact is verifiable as signed") assert.NotContains(t, err.Error(), "signature verification failed for", "the generic invalid-signature wording is the misdiagnosis this replaces") } +// TestClassifyInstallVerifyErrorKeySignedAgainstKeylessPin covers the arm +// where the obvious advice is wrong: the entry pins a certificate identity, so +// resolveKeyAnchor refuses a supplied key rather than letting it displace the +// pin. Telling this caller to pass --public-key would walk them into that +// conflict one step later, so the message has to name the pin and the fact +// that re-anchoring means removing the entry. +func TestClassifyInstallVerifyErrorKeySignedAgainstKeylessPin(t *testing.T) { + t.Parallel() + + err := classifyInstallVerifyError( + verifier.ErrKeySigned, "some-plugin", + &lockfile.Provenance{SignerIdentity: testSignerIdentity}, plugins.InstallOptions{}) + require.Error(t, err) + assert.Equal(t, http.StatusForbidden, httperr.Code(err)) + assert.Contains(t, err.Error(), testSignerIdentity, + "the pinned identity is the reason the key is refused; naming it explains the refusal") + assert.Contains(t, err.Error(), "Remove the lock entry and reinstall", + "re-anchoring is deliberately not offered in place — say what does work") + assert.Contains(t, err.Error(), "allow_unsigned does not apply") + + // The supplied key really is refused against a certificate pin, so the + // message is not merely cautious wording. + _, anchorErr := resolveKeyAnchor( + plugins.InstallOptions{PublicKey: testPublicKeyB64}, "some-plugin", + &lockfile.Provenance{SignerIdentity: testSignerIdentity}, false) + require.Error(t, anchorErr) +} + // TestClassifySignatureErrorNamesKeySigned keeps the sync/upgrade failure // reason distinct from signature-invalid for the same reason. func TestClassifySignatureErrorNamesKeySigned(t *testing.T) { @@ -835,3 +870,424 @@ func TestInstallVerification_SameCommitGitMigrationRematerializes(t *testing.T) assert.Equal(t, []string{name}, checked.AlreadyCurrent) assert.Empty(t, checked.Drifted) } + +// keyedLockEntry is a lock entry pinned to testPublicKeyB64 — the shape a +// previous key-verified install leaves behind. +func keyedLockEntry(name string) lockfile.Entry { + return lockfile.Entry{ + Name: name, + Source: "example.com/org/" + name, + ResolvedReference: "example.com/org/" + name + ":v1", + Digest: "sha256:" + strings.Repeat("b", 64), + Provenance: &lockfile.Provenance{PublicKey: testPublicKeyB64}, + } +} + +// TestValidateInstallPublicKey covers the entry guard: a key that this install +// could never use is bad input, reported before any resolve or fetch rather +// than as a verification failure afterwards. +func TestValidateInstallPublicKey(t *testing.T) { + t.Parallel() + + tests := []struct { + name string + opts plugins.InstallOptions + scope plugins.Scope + wantMsg string + }{ + { + name: "no key is always fine", + opts: plugins.InstallOptions{}, + scope: plugins.ScopeUser, + }, + { + name: "project scope with a valid key", + opts: plugins.InstallOptions{PublicKey: testPublicKeyB64, ProjectRoot: "/tmp/project"}, + scope: plugins.ScopeProject, + }, + { + // User-scope installs are not lock-managed, so verification never + // runs and the key would be accepted and then dropped. + name: "user scope rejects a key it would never use", + opts: plugins.InstallOptions{PublicKey: testPublicKeyB64}, + scope: plugins.ScopeUser, + wantMsg: "applies to project-scoped installs", + }, + { + name: "project scope without a root rejects a key", + opts: plugins.InstallOptions{PublicKey: testPublicKeyB64}, + scope: plugins.ScopeProject, + wantMsg: "applies to project-scoped installs", + }, + { + name: "malformed base64 rejected", + opts: plugins.InstallOptions{PublicKey: "not!base64", ProjectRoot: "/tmp/project"}, + scope: plugins.ScopeProject, + wantMsg: "not valid base64", + }, + { + // Well-encoded is not well-formed. This value decodes cleanly and + // is not a key, which is exactly the input that would otherwise + // fail deep inside verification with the lock file as the suspect. + name: "valid base64 that is not a public key rejected", + opts: plugins.InstallOptions{PublicKey: "aGVsbG8gd29ybGQ=", ProjectRoot: "/tmp/project"}, + scope: plugins.ScopeProject, + wantMsg: "not a DER SPKI public key", + }, + { + name: "oversized key rejected before decoding", + opts: plugins.InstallOptions{ + PublicKey: strings.Repeat("A", lockfile.MaxEncodedPublicKeyLength+1), + ProjectRoot: "/tmp/project", + }, + scope: plugins.ScopeProject, + wantMsg: "exceeding the", + }, + } + for _, tc := range tests { + t.Run(tc.name, func(t *testing.T) { + t.Parallel() + err := validateInstallPublicKey(tc.opts, tc.scope) + if tc.wantMsg == "" { + require.NoError(t, err) + return + } + require.Error(t, err) + assert.Equal(t, http.StatusBadRequest, httperr.Code(err), + "a key this install cannot use is bad input, not a policy refusal") + assert.Contains(t, err.Error(), tc.wantMsg) + }) + } +} + +// TestInstallRejectsMalformedPublicKeyBeforeFetching pins where the guard sits. +// The verifier mock carries no expectations, so any verification call fails the +// test, and a 400 (rather than a clone or pull failure) shows the key was +// judged as input before the artifact was touched at all. +// +//nolint:paralleltest // serial: real sqlite + on-disk client materialization per test +func TestInstallRejectsMalformedPublicKeyBeforeFetching(t *testing.T) { + repoDir := createPluginTestRepo(t, "") + mv := verifiermocks.NewMockVerifier(gomock.NewController(t)) + + svc, projectRoot := newGitLockTestService(t, repoDir, WithVerifier(mv)) + err := gitInstall(t, svc, projectRoot, func(o *plugins.InstallOptions) { o.PublicKey = "not!base64" }) + + require.Error(t, err) + assert.Equal(t, http.StatusBadRequest, httperr.Code(err)) + assert.Contains(t, err.Error(), "public_key") + _, ok := loadPluginLockEntry(t, projectRoot) + assert.False(t, ok, "a rejected install must not write a lock entry") +} + +// TestVerifyOCIInstall_KeyPathDispatch covers the two ways the key path is +// reached and the fact that reaching it excludes the keyless one. Dispatch is +// lock-first by design: were the artifact allowed to select the policy, a +// republished key-signed artifact could walk an entry out of the certificate +// identity it is pinned to. +func TestVerifyOCIInstall_KeyPathDispatch(t *testing.T) { + t.Parallel() + + keyPEM, err := verifier.DecodePublicKey(testPublicKeyB64) + require.NoError(t, err) + + tests := []struct { + name string + locked bool + opts plugins.InstallOptions + }{ + { + name: "first use verifies against the supplied key", + opts: plugins.InstallOptions{PublicKey: testPublicKeyB64}, + }, + { + name: "pinned entry verifies against the locked key with no flag", + locked: true, + }, + { + name: "supplied key that agrees with the pin is accepted", + locked: true, + opts: plugins.InstallOptions{PublicKey: testPublicKeyB64}, + }, + } + for _, tc := range tests { + t.Run(tc.name, func(t *testing.T) { + t.Parallel() + projectRoot := makeProjectRoot(t) + if tc.locked { + require.NoError(t, lockfile.UpsertPluginEntry( + mustOpenRoot(t, projectRoot), keyedLockEntry("keyed-plugin"))) + } + mv := verifiermocks.NewMockVerifier(gomock.NewController(t)) + mv.EXPECT().VerifyOCIWithKey( + gomock.Any(), gomock.Any(), gomock.Any(), gomock.Eq(keyPEM)). + Return(&verifier.Result{Signed: true, Bundle: []byte(`{"bundle":true}`)}, nil) + + opts := tc.opts + opts.ProjectRoot = projectRoot + svc := newTestService(WithVerifier(mv)) + decision, err := svc.verifyOCIInstall( + t.Context(), opts, "keyed-plugin", "example.com/org/keyed-plugin:v1", + "sha256:"+strings.Repeat("b", 64)) + + require.NoError(t, err) + // The key is recorded, and nothing else is: a key-pair bundle + // carries no certificate, so there is no identity to observe and + // inventing one would file provenance the artifact never asserted. + assert.Equal(t, &lockfile.Provenance{PublicKey: testPublicKeyB64}, decision.provenance) + assert.Equal(t, []byte(`{"bundle":true}`), decision.bundle, + "the bundle must be captured for offline re-verification") + assert.False(t, decision.unsigned) + }) + } +} + +// TestVerifyOCIInstall_SuppliedKeyConflictsAreRefused runs the conflicts +// through the real dispatch rather than through resolveKeyAnchor alone. The +// verifier mock has no expectations, so a conflict that was silently ignored +// instead of refused — the failure mode a mistyped --public-key produces — +// would show up as an unexpected keyless verification call. +func TestVerifyOCIInstall_SuppliedKeyConflictsAreRefused(t *testing.T) { + t.Parallel() + + identityEntry := keyedLockEntry("conflicted-plugin") + identityEntry.Provenance = &lockfile.Provenance{ + SignerIdentity: testSignerIdentity, + CertIssuer: testCertIssuer, + } + unsignedEntry := keyedLockEntry("conflicted-plugin") + unsignedEntry.Provenance = nil + unsignedEntry.Unsigned = true + + tests := []struct { + name string + entry lockfile.Entry + wantMsg string + }{ + { + name: "keyless-pinned entry refuses a supplied key", + entry: identityEntry, + wantMsg: "carries no certificate identity", + }, + { + name: "unsigned exception cannot be upgraded by a key", + entry: unsignedEntry, + wantMsg: "unsigned exception", + }, + } + for _, tc := range tests { + t.Run(tc.name, func(t *testing.T) { + t.Parallel() + projectRoot := makeProjectRoot(t) + require.NoError(t, lockfile.UpsertPluginEntry(mustOpenRoot(t, projectRoot), tc.entry)) + svc := newTestService(WithVerifier(verifiermocks.NewMockVerifier(gomock.NewController(t)))) + + _, err := svc.verifyOCIInstall( + t.Context(), + plugins.InstallOptions{ProjectRoot: projectRoot, PublicKey: testPublicKeyB64}, + "conflicted-plugin", "example.com/org/conflicted-plugin:v1", + "sha256:"+strings.Repeat("b", 64)) + + require.Error(t, err) + assert.Equal(t, http.StatusForbidden, httperr.Code(err)) + assert.Contains(t, err.Error(), tc.wantMsg) + }) + } +} + +// TestResolveKeyAnchor pins the conflict rules. Every disagreement between a +// supplied key and the recorded trust state is an error rather than a +// precedence rule, because silently preferring either one is how a mistyped +// --public-key installs as though it had been honored. +func TestResolveKeyAnchor(t *testing.T) { + t.Parallel() + + const otherKeyB64 = "MFkwEwYHKoZIzj0CAQYIKoZIzj0DAQcDQgAEZ7Bd5Kk7GAOI1PoQFvY6Sw+9zL3fVX" + + "Bqz0mAo0hVW1nQz4Vv9pQmT2yqXqL7NqRk5FvPQZ8DdcW0xTn3Yg6ZBw==" + identityPin := &lockfile.Provenance{SignerIdentity: testSignerIdentity, CertIssuer: testCertIssuer} + keyPin := &lockfile.Provenance{PublicKey: testPublicKeyB64} + + tests := []struct { + name string + opts plugins.InstallOptions + expected *lockfile.Provenance + expectUnsigned bool + want string + wantCode int + wantMsg string + }{ + {name: "nothing supplied, nothing pinned: keyless"}, + {name: "identity pin, no key: keyless", expected: identityPin}, + {name: "key pin selects the locked key", expected: keyPin, want: testPublicKeyB64}, + { + name: "supplied key confirms the locked key", + opts: plugins.InstallOptions{PublicKey: testPublicKeyB64}, + expected: keyPin, + want: testPublicKeyB64, + }, + { + name: "supplied key that differs from the pin is refused", + opts: plugins.InstallOptions{PublicKey: otherKeyB64}, + expected: keyPin, + wantCode: http.StatusForbidden, + wantMsg: "pinned to a different cosign public key", + }, + { + name: "supplied key against an identity pin is refused", + opts: plugins.InstallOptions{PublicKey: testPublicKeyB64}, + expected: identityPin, + wantCode: http.StatusForbidden, + wantMsg: "carries no certificate identity", + }, + { + name: "supplied key cannot upgrade a recorded unsigned exception", + opts: plugins.InstallOptions{PublicKey: testPublicKeyB64}, + expectUnsigned: true, + wantCode: http.StatusForbidden, + wantMsg: "unsigned exception", + }, + { + name: "first use adopts the supplied key", + opts: plugins.InstallOptions{PublicKey: testPublicKeyB64}, + want: testPublicKeyB64, + }, + { + // The override re-records whatever it observes, and a key-pair + // bundle offers nothing to observe — so honoring a key here would + // re-anchor on the caller's say-so alone. v1 has no such path. + name: "allow_signer_change with a key is refused", + opts: plugins.InstallOptions{PublicKey: testPublicKeyB64, AllowSignerChange: true}, + expected: keyPin, + wantCode: http.StatusBadRequest, + wantMsg: "cannot be combined with allow_signer_change", + }, + { + // key -> keyless is the one supported transition: the candidate is + // chain-verifiable, so the override drops the pinned key and lets + // the keyless path record what it observes. + name: "allow_signer_change without a key drops the pinned key", + opts: plugins.InstallOptions{AllowSignerChange: true}, + expected: keyPin, + want: "", + }, + } + for _, tc := range tests { + t.Run(tc.name, func(t *testing.T) { + t.Parallel() + got, err := resolveKeyAnchor(tc.opts, "some-plugin", tc.expected, tc.expectUnsigned) + if tc.wantCode != 0 { + require.Error(t, err) + assert.Equal(t, tc.wantCode, httperr.Code(err)) + assert.Contains(t, err.Error(), tc.wantMsg) + assert.Empty(t, got, "a refused anchor must not also be returned") + return + } + require.NoError(t, err) + assert.Equal(t, tc.want, got) + }) + } +} + +// TestClassifyKeyVerifyError pins the diagnoses the key path reports. None of +// them mention allow_unsigned: an install that named a public key asked for +// that key to be enforced, and the unsigned exception answers a different +// question entirely. +func TestClassifyKeyVerifyError(t *testing.T) { + t.Parallel() + + tests := []struct { + name string + err error + wantMsg string + }{ + { + name: "unsigned artifact", + err: verifier.ErrUnsigned, + wantMsg: "carries no signature material at all", + }, + { + // The likeliest mistake: a key aimed at an artifact that was + // signed keylessly. The remedy is to drop the key, which a bare + // "verification failed" would never suggest. + name: "keyless artifact names the right remedy", + err: verifier.ErrKeylessSigned, + wantMsg: "install it without a public key", + }, + { + // Wrong key and damaged signature are genuinely indistinguishable: + // the bundle records no key of its own to compare against. + name: "verification failure names both possible causes", + err: verifier.ErrSignatureInvalid, + wantMsg: "either the key is not the one that signed it, or the signature is damaged", + }, + } + for _, tc := range tests { + t.Run(tc.name, func(t *testing.T) { + t.Parallel() + err := classifyKeyVerifyError(tc.err, "keyed-plugin") + require.Error(t, err) + assert.Equal(t, http.StatusForbidden, httperr.Code(err)) + assert.Contains(t, err.Error(), tc.wantMsg) + assert.Contains(t, err.Error(), "plugin") + assert.NotContains(t, err.Error(), "allow_unsigned") + }) + } +} + +// TestVerifyGitInstall_RefusesPublicKey guards the git side of the same rule +// the lock file enforces on key-pinned git entries: a commit signature is made +// with a Fulcio certificate, so a public key has no operation to take part in. +func TestVerifyGitInstall_RefusesPublicKey(t *testing.T) { + t.Parallel() + + svc := newTestService(WithVerifier(verifiermocks.NewMockVerifier(gomock.NewController(t)))) + _, err := svc.verifyGitInstall( + t.Context(), + plugins.InstallOptions{ProjectRoot: makeProjectRoot(t), PublicKey: testPublicKeyB64}, + "git-plugin", []byte("payload"), "signature") + + require.Error(t, err) + assert.Equal(t, http.StatusBadRequest, httperr.Code(err)) + assert.Contains(t, err.Error(), "a cosign public key cannot verify it") +} + +// TestVerifyLocalInstall_RefusesPublicKey covers the local-build side: there is +// no registry signature material for a key to check, so the key is refused +// rather than accepted and then recorded as an unsigned install anyway. +func TestVerifyLocalInstall_RefusesPublicKey(t *testing.T) { + t.Parallel() + + _, err := verifyLocalInstall(plugins.InstallOptions{ + ProjectRoot: makeProjectRoot(t), + PublicKey: testPublicKeyB64, + // Set so the refusal cannot be mistaken for the ordinary + // unsigned-needs-a-flag rejection. + AllowUnsigned: true, + }, "local-plugin") + + require.Error(t, err) + assert.Equal(t, http.StatusBadRequest, httperr.Code(err)) + assert.Contains(t, err.Error(), "carries no registry signature") +} + +// TestVerifyLocalInstall_KeyPinnedEntryRendersAnchor pins the message a +// key-pinned entry produces when a local build tries to replace it. Naming the +// (empty) signer identity there would print `locked to signer ""` and read as a +// corrupt lock file rather than as a refusal. +func TestVerifyLocalInstall_KeyPinnedEntryRendersAnchor(t *testing.T) { + t.Parallel() + + projectRoot := makeProjectRoot(t) + require.NoError(t, lockfile.UpsertPluginEntry( + mustOpenRoot(t, projectRoot), keyedLockEntry("local-plugin"))) + + _, err := verifyLocalInstall(plugins.InstallOptions{ + ProjectRoot: projectRoot, + AllowUnsigned: true, + }, "local-plugin") + + require.Error(t, err) + assert.Equal(t, http.StatusForbidden, httperr.Code(err)) + assert.Contains(t, err.Error(), "locked to a cosign public key") + assert.NotContains(t, err.Error(), `signer ""`) +}