Skip to content

Commit e2f2055

Browse files
committed
Drop --key from plugin push
Key signing has no consume-time path: install verification is keyless-only, and a key-signed artifact fails as ErrSignatureInvalid rather than ErrUnsigned, so --allow-unsigned cannot override it. The flag could only produce plugins nobody can install. Rather than document that trap, remove it. The flag and the API's key field are gone, and pluginsvc rejects a key set by an in-process caller (PushOptions aliases skills.PushOptions, so the field still exists). Plugin publishing is keyless or explicitly unsigned. The flag returns in the change that makes install, sync, and the lock carry a public key, tracked in #6442. Part of #6300. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Signed-off-by: Samuele Verzi <samu@stacklok.com>
1 parent 3990a39 commit e2f2055

15 files changed

Lines changed: 89 additions & 100 deletions

cmd/thv/app/ai_plugin_push.go

Lines changed: 5 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -11,7 +11,6 @@ import (
1111
)
1212

1313
var (
14-
aiPluginPushKey string
1514
aiPluginPushIdentityToken string
1615
aiPluginPushNoSign bool
1716
)
@@ -26,17 +25,13 @@ var aiPluginPushCmd = &cobra.Command{
2625

2726
func init() {
2827
aiPluginCmd.AddCommand(aiPluginPushCmd)
29-
aiPluginPushCmd.Flags().StringVar(&aiPluginPushKey, "key", "",
30-
"Path to a cosign private key to sign the pushed artifact. "+
31-
"Encrypted keys are decrypted with COSIGN_PASSWORD read from the 'thv serve' process, "+
32-
"which performs the signing. NOTE: ToolHive cannot yet verify key-signed artifacts at "+
33-
"install time (only keyless/Fulcio signatures), so a project-scoped install of one is "+
34-
"refused and --allow-unsigned does not override it; prefer keyless signing")
28+
// No --key flag: plugin signing is keyless-only until install-time key
29+
// verification exists (#6442). Pushing a key-signed plugin would produce
30+
// an artifact no project-scoped install can accept.
3531
aiPluginPushCmd.Flags().StringVar(&aiPluginPushIdentityToken, "identity-token", "",
3632
"OIDC identity token (or a path to a file containing one) for keyless signing. "+
37-
"Mutually exclusive with --key. If omitted, one is acquired automatically: from the "+
38-
"ambient CI OIDC token when running with id-token: write permission, otherwise via an "+
39-
"interactive browser sign-in")
33+
"If omitted, one is acquired automatically: from the ambient CI OIDC token when "+
34+
"running with id-token: write permission, otherwise via an interactive browser sign-in")
4035
aiPluginPushCmd.Flags().BoolVar(&aiPluginPushNoSign, "no-sign", false,
4136
"Push without signing (consumers will need an explicit unsigned exception to install project-scoped)")
4237
}
@@ -49,7 +44,6 @@ func aiPluginPushCmdFunc(cmd *cobra.Command, args []string) error {
4944
// Sigstore keyless signing, not of the artifact kind being pushed.
5045
token, err := identitytoken.Acquire(ctx, identitytoken.Options{
5146
FlagValue: aiPluginPushIdentityToken,
52-
Key: aiPluginPushKey,
5347
NoSign: aiPluginPushNoSign,
5448
Confirm: confirmBrowserSignIn,
5549
})
@@ -60,7 +54,6 @@ func aiPluginPushCmdFunc(cmd *cobra.Command, args []string) error {
6054
c := newAIPluginClient(ctx)
6155
err = c.Push(ctx, plugins.PushOptions{
6256
Reference: args[0],
63-
Key: aiPluginPushKey,
6457
IdentityToken: token,
6558
NoSign: aiPluginPushNoSign,
6659
})

cmd/thv/app/ai_plugin_trust_test.go

Lines changed: 8 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -14,18 +14,22 @@ import (
1414
)
1515

1616
// TestAIPluginPushSigningFlags pins the signed-by-default publish surface:
17-
// the three flags must exist, and neither signing method nor the opt-out may
18-
// be preset — a defaulted --no-sign would publish unsigned artifacts silently.
17+
// the keyless flags must exist, the opt-out must not be preset (a defaulted
18+
// --no-sign would publish unsigned artifacts silently), and --key must NOT be
19+
// offered — ToolHive cannot verify key-signed artifacts at install time, so
20+
// the flag would only produce uninstallable plugins (#6442). Re-add it in the
21+
// change that makes key verification work.
1922
func TestAIPluginPushSigningFlags(t *testing.T) {
2023
t.Parallel()
2124

22-
for _, name := range []string{"key", "identity-token", "no-sign"} {
25+
for _, name := range []string{"identity-token", "no-sign"} {
2326
flag := aiPluginPushCmd.Flags().Lookup(name)
2427
require.NotNil(t, flag, "thv ai-plugin push must expose --%s", name)
2528
}
29+
assert.Nil(t, aiPluginPushCmd.Flags().Lookup("key"),
30+
"plugin signing is keyless-only; --key must not be advertised until install can verify it")
2631
assert.Equal(t, "false", aiPluginPushCmd.Flags().Lookup("no-sign").DefValue,
2732
"pushing unsigned must always be an explicit choice")
28-
assert.Empty(t, aiPluginPushCmd.Flags().Lookup("key").DefValue)
2933
assert.Empty(t, aiPluginPushCmd.Flags().Lookup("identity-token").DefValue)
3034
}
3135

docs/arch/14-plugins-system.md

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -181,9 +181,9 @@ Project-scoped plugin installs are verified against Sigstore signatures, and the
181181

182182
On first install of a signed plugin the observed signer identity is recorded (trust on first use) as the entry's `provenance:` block and **displayed to the user** — by `thv ai-plugin install` when it completes, and by `thv ai-plugin info` on every later read. Each subsequent install, sync, and upgrade enforces that identity *inside* the Sigstore verification policy: OCI artifacts through their attached signature bundles, git commits through gitsign signature-and-chain verification (recorded `provisional: true` — the transparency-log proof of signing time is not yet validated, so the replay window is unbounded until that lands, and the marker is rendered wherever the identity is). `thv ai-plugin sync` additionally re-verifies each entry's stored signature bundle offline — embedded trust root, no network — before counting it current, so a tampered-with stored bundle is caught in CI rather than at the next install. `thv ai-plugin upgrade` refuses to move to an artifact signed by a different identity, or to an unsigned one, without an explicit `--allow-signer-change`.
183183

184-
Publishing is signed by default. `thv ai-plugin push` requires `--key` (a cosign private key), an OIDC identity token for keyless signing (`--identity-token`, or acquired automatically from an ambient CI token or an interactive browser sign-in), or an explicit `--no-sign`. Either signing path attaches the signature manifest next to the pushed artifact, where install-time verification finds it. A failed signing fails the push: an artifact published as if it were signed — which consumers then have to install with `--allow-unsigned` — is worse than a push that visibly did not complete.
184+
Publishing is signed by default, and **keyless only**. `thv ai-plugin push` requires either an OIDC identity token (`--identity-token`, or acquired automatically from an ambient CI token or an interactive browser sign-in) or an explicit `--no-sign`. Signing attaches the signature manifest next to the pushed artifact, where install-time verification finds it. A failed signing fails the push: an artifact published as if it were signed — which consumers then have to install with `--allow-unsigned` — is worse than a push that visibly did not complete.
185185

186-
Keyless signing is the only path ToolHive can currently verify end to end. Install-time verification is keyless-only (`verifier.VerifyOCI` checks a Fulcio certificate chain), and neither the lock schema nor the install API carries a public key, so a key-signed artifact fails verification as `ErrSignatureInvalid` — not `ErrUnsigned`, which means `--allow-unsigned` does not override it and a project-scoped install of one is refused outright. `--key` therefore publishes artifacts that are verifiable by cosign but not installable by ToolHive. This applies identically to `thv skill push --key`; closing it means threading a public key through the lock entry, the install options, and offline sync re-verification for both surfaces, tracked in [#6442](https://github.com/stacklok/toolhive/issues/6442).
186+
There is deliberately **no `--key` flag** and no `key` field on the push API, and `pluginsvc.Push` rejects a key set by an in-process caller with a 400. Install-time verification is keyless-only (`verifier.VerifyOCI` checks a Fulcio certificate chain), and neither the lock schema nor the install API carries a public key, so a key-signed artifact fails verification as `ErrSignatureInvalid` — not `ErrUnsigned`, which means `--allow-unsigned` cannot override it and a project-scoped install is refused outright. Offering key signing would therefore only let publishers produce plugins nobody can install. `thv skill push --key` still accepts one and has exactly this problem; the flag returns here once install, sync, and the lock can carry a public key, tracked in [#6442](https://github.com/stacklok/toolhive/issues/6442).
187187

188188
What is still trusted on faith, deliberately and visibly:
189189

docs/cli/thv_ai-plugin_push.md

Lines changed: 1 addition & 2 deletions
Some generated files are not rendered by default. Learn more about customizing how changed files appear on GitHub.

docs/server/docs.go

Lines changed: 1 addition & 5 deletions
Some generated files are not rendered by default. Learn more about customizing how changed files appear on GitHub.

docs/server/swagger.json

Lines changed: 1 addition & 5 deletions
Some generated files are not rendered by default. Learn more about customizing how changed files appear on GitHub.

docs/server/swagger.yaml

Lines changed: 3 additions & 6 deletions
Some generated files are not rendered by default. Learn more about customizing how changed files appear on GitHub.

pkg/api/v1/plugins.go

Lines changed: 0 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -327,7 +327,6 @@ func (s *PluginsRoutes) pushPlugin(w http.ResponseWriter, r *http.Request) error
327327

328328
if err := s.pluginService.Push(r.Context(), plugins.PushOptions{
329329
Reference: req.Reference,
330-
Key: req.Key,
331330
IdentityToken: req.IdentityToken,
332331
NoSign: req.NoSign,
333332
}); err != nil {

pkg/api/v1/plugins_test.go

Lines changed: 7 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -487,14 +487,18 @@ func TestPluginsRouter(t *testing.T) {
487487
expectedStatus: http.StatusNoContent,
488488
},
489489
{
490-
name: "push plugin forwards key and no_sign",
490+
// The request DTO has no key field, so a key sent by an older
491+
// client is dropped rather than reaching the service: plugin
492+
// signing is keyless-only (#6442). It must never arrive as a
493+
// populated PushOptions.Key.
494+
name: "push plugin does not forward a key",
491495
method: "POST",
492496
path: "/push",
493-
body: `{"reference":"ghcr.io/test/plugin:v1","key":"/tmp/cosign.key"}`,
497+
body: `{"reference":"ghcr.io/test/plugin:v1","key":"/tmp/cosign.key","no_sign":true}`,
494498
setupMock: func(svc *plugmocks.MockPluginService, _ string) {
495499
svc.EXPECT().Push(gomock.Any(), plugins.PushOptions{
496500
Reference: "ghcr.io/test/plugin:v1",
497-
Key: "/tmp/cosign.key",
501+
NoSign: true,
498502
}).Return(nil)
499503
},
500504
expectedStatus: http.StatusNoContent,

pkg/api/v1/plugins_types.go

Lines changed: 3 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -77,11 +77,10 @@ type buildPluginRequest struct {
7777
type pushPluginRequest struct {
7878
// OCI reference to push
7979
Reference string `json:"reference"`
80-
// Key is the path to a cosign private key used to sign the pushed
81-
// artifact
82-
Key string `json:"key,omitempty"`
8380
// IdentityToken is a short-lived OIDC identity token used for keyless
84-
// signing, mutually exclusive with Key
81+
// signing. Plugin signing is keyless-only: there is deliberately no key
82+
// field, because ToolHive cannot verify key-signed artifacts at install
83+
// time and would publish an uninstallable plugin (#6442)
8584
IdentityToken string `json:"identity_token,omitempty"`
8685
// NoSign pushes without signing
8786
NoSign bool `json:"no_sign,omitempty"`

0 commit comments

Comments
 (0)