From 557fd99d332892579014a5b6274dbe7154ffcb5d Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Jos=C3=A9=20Almeida?= <57680069+zeval@users.noreply.github.com> Date: Wed, 26 Aug 2026 11:26:09 +0000 Subject: [PATCH] fix(plugins): require admin for global mutations (#2816) --- .../internal/plugins/action_access_test.go | 85 ++++++++ .../internal/plugins/action_handlers.go | 24 +++ .../plugins/autoupdate_handlers_test.go | 12 +- apps/backend/internal/plugins/config_test.go | 2 +- apps/backend/internal/plugins/handlers.go | 14 +- .../plugins/handlers_management_auth_test.go | 187 ++++++++++++++++++ .../backend/internal/plugins/handlers_test.go | 18 +- .../plugins/manifest/action_access_test.go | 94 +++++++++ .../internal/plugins/manifest/manifest.go | 20 +- .../internal/plugins/manifest/validate.go | 12 ++ .../internal/plugins/marketplace_handlers.go | 9 +- .../plugins/marketplace_handlers_test.go | 114 ++++++++++- apps/web/app/settings/plugins/page.test.tsx | 17 ++ .../sections/settings/settings-tree.tsx | 7 +- .../settings/plugins/marketplace-browser.tsx | 53 +++-- .../plugins/marketplace-entry-row.test.tsx | 20 ++ .../plugins/marketplace-entry-row.tsx | 18 +- .../settings/plugins/plugin-detail.tsx | 86 ++++---- .../settings/plugins/plugin-row.test.tsx | 63 +++++- .../settings/plugins/plugin-row.tsx | 80 +++++--- .../settings/plugins/plugins-settings.tsx | 93 +++++---- .../tests/auth/plugin-settings-member.spec.ts | 76 +++++++ docs/plans/plugins/PLUGIN-API.md | 4 + docs/public/plugins-authoring.md | 12 +- docs/public/plugins-manifest.md | 6 +- .../plugins/system-design/marketplace.md | 15 +- .../specs/plugins/system-design/plugins-01.md | 5 +- .../specs/plugins/system-design/plugins-02.md | 23 ++- 28 files changed, 971 insertions(+), 198 deletions(-) create mode 100644 apps/backend/internal/plugins/action_access_test.go create mode 100644 apps/backend/internal/plugins/handlers_management_auth_test.go create mode 100644 apps/backend/internal/plugins/manifest/action_access_test.go create mode 100644 apps/web/e2e/tests/auth/plugin-settings-member.spec.ts diff --git a/apps/backend/internal/plugins/action_access_test.go b/apps/backend/internal/plugins/action_access_test.go new file mode 100644 index 0000000000..efa356bb9a --- /dev/null +++ b/apps/backend/internal/plugins/action_access_test.go @@ -0,0 +1,85 @@ +package plugins + +import ( + "bytes" + "fmt" + "net/http" + "net/http/httptest" + "runtime" + "strings" + "testing" + + "github.com/kandev/kandev/internal/auth/authn" + "github.com/kandev/kandev/internal/plugins/pkgtar/pkgtartest" + "github.com/kandev/kandev/pkg/pluginsdk" +) + +func adminActionPackage(t *testing.T) *bytes.Buffer { + t.Helper() + manifestYAML := fmt.Sprintf(` +id: kandev-plugin-admin-action +api_version: 1 +version: "1.0.0" +display_name: Admin Action +min_kandev_version: "0.91.1" +actions: + - key: connection.set + scope: workspace + access: admin + max_body_bytes: 128 +runtime: + type: binary + executables: + %s-%s: server/plugin +`, runtime.GOOS, runtime.GOARCH) + var buf bytes.Buffer + if err := pkgtartest.WritePackage(&buf, map[string][]byte{ + "manifest.yaml": []byte(manifestYAML), + "server/plugin": []byte("#!/bin/sh\necho fake\n"), + }); err != nil { + t.Fatalf("WritePackage: %v", err) + } + return &buf +} + +func doActionRequestAsRole(router http.Handler, role authn.Role) *httptest.ResponseRecorder { + req := httptest.NewRequest( + http.MethodPost, + "/api/plugins/kandev-plugin-admin-action/actions/connection.set", + strings.NewReader(`{"workspaceId":"workspace-1","body":{}}`), + ) + req.Header.Set("Content-Type", "application/json") + req = req.WithContext(authn.WithIdentity(req.Context(), authn.Identity{ + UserID: "user-1", + Role: role, + })) + recorder := httptest.NewRecorder() + router.ServeHTTP(recorder, req) + return recorder +} + +func TestActionHandlerRequiresAdminForAdminAccess(t *testing.T) { + invoker := &recordingActionInvoker{ + response: &pluginsdk.PluginActionResponse{Body: []byte(`{"ok":true}`)}, + } + router, service := newActionTestRouter(t, invoker) + if _, err := service.Install(t.Context(), adminActionPackage(t)); err != nil { + t.Fatalf("Install: %v", err) + } + + member := doActionRequestAsRole(router, authn.RoleMember) + if member.Code != http.StatusForbidden { + t.Fatalf("member status = %d, want 403, body=%s", member.Code, member.Body.String()) + } + if invoker.calls != 0 { + t.Fatalf("member action invocations = %d, want 0", invoker.calls) + } + + admin := doActionRequestAsRole(router, authn.RoleAdmin) + if admin.Code != http.StatusOK { + t.Fatalf("admin status = %d, want 200, body=%s", admin.Code, admin.Body.String()) + } + if invoker.calls != 1 { + t.Fatalf("admin action invocations = %d, want 1", invoker.calls) + } +} diff --git a/apps/backend/internal/plugins/action_handlers.go b/apps/backend/internal/plugins/action_handlers.go index 138c6fb371..de5cb002bc 100644 --- a/apps/backend/internal/plugins/action_handlers.go +++ b/apps/backend/internal/plugins/action_handlers.go @@ -48,6 +48,9 @@ func (c *Controller) action(ctx *gin.Context) { ctx.JSON(http.StatusNotFound, gin.H{actionErrorField: "plugin action not found"}) return } + if !authorizeActionAccess(ctx, declared) { + return + } envelope, ok := readActionEnvelope(ctx, declared.MaxBodyBytes) if !ok { @@ -92,6 +95,27 @@ func (c *Controller) action(ctx *gin.Context) { c.writeActionResponse(ctx, resp) } +func authorizeActionAccess(ctx *gin.Context, action manifest.Action) bool { + identity, authenticated := authn.FromGin(ctx) + if !authenticated || identity.UserID == "" { + ctx.JSON(http.StatusUnauthorized, gin.H{actionErrorField: authenticationRequiredMessage}) + return false + } + switch action.EffectiveAccess() { + case manifest.ActionAccessAuthenticated: + return true + case manifest.ActionAccessAdmin: + if identity.IsAdmin() { + return true + } + ctx.JSON(http.StatusForbidden, gin.H{actionErrorField: "admin role required"}) + return false + default: + ctx.JSON(http.StatusServiceUnavailable, gin.H{actionErrorField: "plugin action has invalid access"}) + return false + } +} + func manifestAction(record *store.Record, key string) (manifest.Action, bool) { for _, action := range record.Actions { if action.Key == key { diff --git a/apps/backend/internal/plugins/autoupdate_handlers_test.go b/apps/backend/internal/plugins/autoupdate_handlers_test.go index b681c79622..28da71cfbc 100644 --- a/apps/backend/internal/plugins/autoupdate_handlers_test.go +++ b/apps/backend/internal/plugins/autoupdate_handlers_test.go @@ -9,11 +9,20 @@ import ( "github.com/jmoiron/sqlx" _ "github.com/mattn/go-sqlite3" + "github.com/kandev/kandev/internal/auth/authn" "github.com/kandev/kandev/internal/db" "github.com/kandev/kandev/internal/plugins/store" ) func newTestRouterWithSettings(t *testing.T) (*gin.Engine, *Service) { + t.Helper() + return newTestRouterWithSettingsIdentity(t, authn.Identity{ + UserID: "admin-1", + Role: authn.RoleAdmin, + }) +} + +func newTestRouterWithSettingsIdentity(t *testing.T, identity authn.Identity) (*gin.Engine, *Service) { t.Helper() gin.SetMode(gin.TestMode) svc, _, _ := newTestService(t) @@ -31,8 +40,7 @@ func newTestRouterWithSettings(t *testing.T) (*gin.Engine, *Service) { } svc.SetSettings(ss) - router := gin.New() - RegisterRoutes(router, svc, nil, testLogger(t)) + router := registerPluginRoutesWithIdentity(t, svc, identity) return router, svc } diff --git a/apps/backend/internal/plugins/config_test.go b/apps/backend/internal/plugins/config_test.go index c536618d27..bb4b7b98de 100644 --- a/apps/backend/internal/plugins/config_test.go +++ b/apps/backend/internal/plugins/config_test.go @@ -400,7 +400,7 @@ func TestGetConfigHandlerMissingReturns404(t *testing.T) { } func TestUpdateConfigHandlerInvalidSchemaReturns400(t *testing.T) { - router, svc := newTestRouter(t) + router, svc := newAdminTestRouter(t) installConfigPlugin(t, svc, "kandev-plugin-github") rec := doRequest(router, http.MethodPatch, "/api/plugins/kandev-plugin-github", diff --git a/apps/backend/internal/plugins/handlers.go b/apps/backend/internal/plugins/handlers.go index e81d0db456..4ecff3c581 100644 --- a/apps/backend/internal/plugins/handlers.go +++ b/apps/backend/internal/plugins/handlers.go @@ -77,22 +77,22 @@ func RegisterRoutes(router *gin.Engine, svc *Service, _ Deliverer, log *logger.L api := router.Group("/api/plugins") api.POST("/install", authn.RequireAdmin(), ctrl.install) - api.POST("/sync", ctrl.sync) + api.POST("/sync", authn.RequireAdmin(), ctrl.sync) // Register the static /marketplace and /settings routes before the /:id // wildcard, matching the /install and /sync ordering — some gin/httprouter // tree versions reject a static sibling added after an existing wildcard for // the same method. ctrl.registerMarketplaceRoutes(api) api.GET("/settings", ctrl.getSettings) - api.PUT("/settings", ctrl.updateSettings) + api.PUT("/settings", authn.RequireAdmin(), ctrl.updateSettings) api.GET("", ctrl.list) api.GET("/:id", ctrl.get) api.GET("/:id/config", ctrl.getConfig) - api.PATCH("/:id", ctrl.updateConfig) - api.PUT("/:id/auto-update", ctrl.setAutoUpdate) - api.DELETE("/:id", ctrl.uninstall) - api.POST("/:id/enable", ctrl.enable) - api.POST("/:id/disable", ctrl.disable) + api.PATCH("/:id", authn.RequireAdmin(), ctrl.updateConfig) + api.PUT("/:id/auto-update", authn.RequireAdmin(), ctrl.setAutoUpdate) + api.DELETE("/:id", authn.RequireAdmin(), ctrl.uninstall) + api.POST("/:id/enable", authn.RequireAdmin(), ctrl.enable) + api.POST("/:id/disable", authn.RequireAdmin(), ctrl.disable) api.GET("/:id/bundle", ctrl.bundle) api.GET("/:id/ui/*path", ctrl.ui) diff --git a/apps/backend/internal/plugins/handlers_management_auth_test.go b/apps/backend/internal/plugins/handlers_management_auth_test.go new file mode 100644 index 0000000000..bafc5b5268 --- /dev/null +++ b/apps/backend/internal/plugins/handlers_management_auth_test.go @@ -0,0 +1,187 @@ +package plugins + +import ( + "errors" + "net/http" + "os" + "testing" + + "github.com/gin-gonic/gin" + + "github.com/kandev/kandev/internal/auth/authn" + "github.com/kandev/kandev/internal/plugins/store" +) + +func registerPluginRoutesWithIdentity(t *testing.T, svc *Service, identity authn.Identity) *gin.Engine { + t.Helper() + gin.SetMode(gin.TestMode) + router := gin.New() + router.Use(func(ctx *gin.Context) { + authn.SetOnGin(ctx, identity) + ctx.Next() + }) + RegisterRoutes(router, svc, nil, testLogger(t)) + return router +} + +func registerAdminPluginRoutes(t *testing.T, svc *Service) *gin.Engine { + t.Helper() + return registerPluginRoutesWithIdentity(t, svc, authn.Identity{ + UserID: "admin-1", + Role: authn.RoleAdmin, + }) +} + +func registerMemberPluginRoutes(t *testing.T, svc *Service) *gin.Engine { + t.Helper() + return registerPluginRoutesWithIdentity(t, svc, authn.Identity{ + UserID: "member-1", + Role: authn.RoleMember, + }) +} + +func requireMemberForbidden(t *testing.T, recStatus int, responseBody string) { + t.Helper() + if recStatus != http.StatusForbidden { + t.Errorf("status = %d, want 403, body=%s", recStatus, responseBody) + } +} + +func TestUpdateConfigHandlerRejectsMemberBeforePersistenceOrRestart(t *testing.T) { + svc, _, runtime := newTestService(t) + router := registerMemberPluginRoutes(t, svc) + installTestPlugin(t, svc, "kandev-plugin-slack") + startCallsBefore := runtime.startCallCount("kandev-plugin-slack") + + rec := doRequest(router, http.MethodPatch, "/api/plugins/kandev-plugin-slack", `{"config":{"default_channel":"#member"}}`, nil) + requireMemberForbidden(t, rec.Code, rec.Body.String()) + config, err := svc.GetMaskedConfig("kandev-plugin-slack") + if err != nil { + t.Fatalf("GetMaskedConfig: %v", err) + } + if len(config) != 0 { + t.Errorf("member config persisted: %v", config) + } + if runtime.stopped("kandev-plugin-slack") { + t.Error("member config request stopped the running plugin") + } + if got := runtime.startCallCount("kandev-plugin-slack"); got != startCallsBefore { + t.Errorf("start calls = %d, want %d", got, startCallsBefore) + } +} + +func TestSyncHandlerRejectsMemberBeforeInstallingDroppedPackage(t *testing.T) { + svc, pluginsDir, _, runtime := newTestServiceWithDir(t) + router := registerMemberPluginRoutes(t, svc) + tarPath := dropTarball(t, pluginsDir, "kandev-plugin-drop", "kandev-plugin-drop-1.0.0.tar.gz") + + rec := doRequest(router, http.MethodPost, "/api/plugins/sync", "", nil) + requireMemberForbidden(t, rec.Code, rec.Body.String()) + if _, err := os.Stat(tarPath); err != nil { + t.Errorf("member sync consumed dropped package: %v", err) + } + if _, err := svc.Get("kandev-plugin-drop"); !errors.Is(err, store.ErrNotFound) { + t.Errorf("member sync registered dropped package, error = %v", err) + } + if runtime.Running("kandev-plugin-drop") { + t.Error("member sync started dropped package") + } +} + +func TestUpdateSettingsHandlerRejectsMemberBeforePersistence(t *testing.T) { + router, svc := newTestRouterWithSettingsIdentity(t, authn.Identity{ + UserID: "member-1", + Role: authn.RoleMember, + }) + + rec := doRequest(router, http.MethodPut, "/api/plugins/settings", + `{"auto_update_default":true}`, map[string]string{"Content-Type": "application/json"}) + requireMemberForbidden(t, rec.Code, rec.Body.String()) + got, err := svc.AutoUpdateDefault() + if err != nil { + t.Fatalf("AutoUpdateDefault: %v", err) + } + if got { + t.Error("member changed the global auto-update default") + } +} + +func TestSetAutoUpdateHandlerRejectsMemberBeforePersistence(t *testing.T) { + router, svc := newTestRouterWithSettingsIdentity(t, authn.Identity{ + UserID: "member-1", + Role: authn.RoleMember, + }) + installTestPlugin(t, svc, "kandev-plugin-slack") + + rec := doRequest(router, http.MethodPut, "/api/plugins/kandev-plugin-slack/auto-update", + `{"auto_update":true}`, map[string]string{"Content-Type": "application/json"}) + requireMemberForbidden(t, rec.Code, rec.Body.String()) + got, err := svc.Get("kandev-plugin-slack") + if err != nil { + t.Fatalf("Get: %v", err) + } + if got.AutoUpdate != nil { + t.Errorf("member persisted auto-update override: %v", *got.AutoUpdate) + } +} + +func TestUninstallHandlerRejectsMemberBeforeRemovalOrStop(t *testing.T) { + svc, _, runtime := newTestService(t) + router := registerMemberPluginRoutes(t, svc) + installTestPlugin(t, svc, "kandev-plugin-slack") + + rec := doRequest(router, http.MethodDelete, "/api/plugins/kandev-plugin-slack", "", nil) + requireMemberForbidden(t, rec.Code, rec.Body.String()) + got, err := svc.Get("kandev-plugin-slack") + if err != nil { + t.Fatalf("member uninstalled plugin: %v", err) + } + if got.Status != StatusActive || !runtime.Running(got.ID) { + t.Errorf("plugin changed after member uninstall: status=%q running=%v", got.Status, runtime.Running(got.ID)) + } + if runtime.stopped(got.ID) { + t.Error("member uninstall stopped the plugin") + } +} + +func TestEnableHandlerRejectsMemberBeforeStart(t *testing.T) { + svc, _, runtime := newTestService(t) + router := registerMemberPluginRoutes(t, svc) + installTestPlugin(t, svc, "kandev-plugin-slack") + if err := svc.Disable("kandev-plugin-slack"); err != nil { + t.Fatalf("Disable: %v", err) + } + startCallsBefore := runtime.startCallCount("kandev-plugin-slack") + + rec := doRequest(router, http.MethodPost, "/api/plugins/kandev-plugin-slack/enable", "", nil) + requireMemberForbidden(t, rec.Code, rec.Body.String()) + got, err := svc.Get("kandev-plugin-slack") + if err != nil { + t.Fatalf("Get: %v", err) + } + if got.Status != StatusDisabled || runtime.Running(got.ID) { + t.Errorf("plugin changed after member enable: status=%q running=%v", got.Status, runtime.Running(got.ID)) + } + if calls := runtime.startCallCount(got.ID); calls != startCallsBefore { + t.Errorf("start calls = %d, want %d", calls, startCallsBefore) + } +} + +func TestDisableHandlerRejectsMemberBeforeStop(t *testing.T) { + svc, _, runtime := newTestService(t) + router := registerMemberPluginRoutes(t, svc) + installTestPlugin(t, svc, "kandev-plugin-slack") + + rec := doRequest(router, http.MethodPost, "/api/plugins/kandev-plugin-slack/disable", "", nil) + requireMemberForbidden(t, rec.Code, rec.Body.String()) + got, err := svc.Get("kandev-plugin-slack") + if err != nil { + t.Fatalf("Get: %v", err) + } + if got.Status != StatusActive || !runtime.Running(got.ID) { + t.Errorf("plugin changed after member disable: status=%q running=%v", got.Status, runtime.Running(got.ID)) + } + if runtime.stopped(got.ID) { + t.Error("member disable stopped the plugin") + } +} diff --git a/apps/backend/internal/plugins/handlers_test.go b/apps/backend/internal/plugins/handlers_test.go index a3931208aa..0df2bbd61c 100644 --- a/apps/backend/internal/plugins/handlers_test.go +++ b/apps/backend/internal/plugins/handlers_test.go @@ -72,13 +72,7 @@ func newTestRouterWithIdentity(t *testing.T, identity authn.Identity) (*gin.Engi // fails closed without one), so wire an in-memory one, matching prod // where Provide always attaches the shared vault. svc.SetSecrets(newFakeSecretRevealer()) - router := gin.New() - router.Use(func(ctx *gin.Context) { - authn.SetOnGin(ctx, identity) - ctx.Next() - }) - RegisterRoutes(router, svc, nil, testLogger(t)) - return router, svc + return registerPluginRoutesWithIdentity(t, svc, identity), svc } func newAdminTestRouter(t *testing.T) (*gin.Engine, *Service) { @@ -313,7 +307,7 @@ func TestGetHandlerMissingReturns404(t *testing.T) { } func TestEnableDisableHandlersTransitionStatus(t *testing.T) { - router, svc := newTestRouter(t) + router, svc := newAdminTestRouter(t) installTestPlugin(t, svc, "kandev-plugin-slack") // already active after install rec := doRequest(router, http.MethodPost, "/api/plugins/kandev-plugin-slack/disable", "", nil) @@ -335,8 +329,8 @@ func TestEnableDisableHandlersTransitionStatus(t *testing.T) { } } -func TestUpdateConfigHandlerPersists(t *testing.T) { - router, svc := newTestRouter(t) +func TestUpdateConfigHandlerAdminPersists(t *testing.T) { + router, svc := newAdminTestRouter(t) installTestPlugin(t, svc, "kandev-plugin-slack") rec := doRequest(router, http.MethodPatch, "/api/plugins/kandev-plugin-slack", `{"config":{"default_channel":"#dev"}}`, nil) @@ -346,7 +340,7 @@ func TestUpdateConfigHandlerPersists(t *testing.T) { } func TestUninstallHandlerRemovesPlugin(t *testing.T) { - router, svc := newTestRouter(t) + router, svc := newAdminTestRouter(t) installTestPlugin(t, svc, "kandev-plugin-slack") rec := doRequest(router, http.MethodDelete, "/api/plugins/kandev-plugin-slack", "", nil) @@ -1074,7 +1068,7 @@ func TestWriteWebhookResponse_ValidStatusRelaysHeadersAndBody(t *testing.T) { } func TestSyncHandlerRegistersDirSideload(t *testing.T) { - router, svc := newTestRouter(t) + router, svc := newAdminTestRouter(t) pluginsDir := svc.pluginsDir versionDir := filepath.Join(pluginsDir, "kandev-plugin-side", "1.0.0") if err := os.MkdirAll(versionDir, 0o755); err != nil { diff --git a/apps/backend/internal/plugins/manifest/action_access_test.go b/apps/backend/internal/plugins/manifest/action_access_test.go new file mode 100644 index 0000000000..2d4abfb4d9 --- /dev/null +++ b/apps/backend/internal/plugins/manifest/action_access_test.go @@ -0,0 +1,94 @@ +package manifest + +import ( + "fmt" + "strings" + "testing" +) + +func TestActionAccessDefaultsToAuthenticatedAndAcceptsAdmin(t *testing.T) { + m, err := Parse([]byte(validManifestYAML + ` +min_kandev_version: "0.91.1" +actions: + - key: connection.get + scope: workspace + max_body_bytes: 1024 + - key: connection.set + scope: workspace + access: admin + max_body_bytes: 1024 +`)) + if err != nil { + t.Fatalf("Parse() unexpected error: %v", err) + } + if err := m.Validate(); err != nil { + t.Fatalf("Validate() unexpected error: %v", err) + } + if got := m.Actions[0].EffectiveAccess(); got != ActionAccessAuthenticated { + t.Fatalf("default action access = %q, want %q", got, ActionAccessAuthenticated) + } + if got := m.Actions[1].EffectiveAccess(); got != ActionAccessAdmin { + t.Fatalf("explicit action access = %q, want %q", got, ActionAccessAdmin) + } +} + +func TestValidateAdminActionRequiresSupportingKandevMinimum(t *testing.T) { + tests := []struct { + name string + minimum string + wantReject bool + }{ + {name: "missing", wantReject: true}, + {name: "older release", minimum: "0.91.0", wantReject: true}, + {name: "malformed", minimum: "main", wantReject: true}, + {name: "first supporting release", minimum: "0.91.1"}, + {name: "tagged first supporting release", minimum: "v0.91.1"}, + {name: "later release", minimum: "0.92.0"}, + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + yaml := validManifestYAML + if tt.minimum != "" { + yaml += fmt.Sprintf("\nmin_kandev_version: %q\n", tt.minimum) + } + yaml += ` +actions: + - key: connection.set + scope: workspace + access: admin + max_body_bytes: 1024 +` + m, err := Parse([]byte(yaml)) + if err != nil { + t.Fatalf("Parse() unexpected error: %v", err) + } + validationErr := m.Validate() + if tt.wantReject { + if validationErr == nil || !strings.Contains(validationErr.Error(), "requires min_kandev_version >= 0.91.1") { + t.Fatalf("Validate() error = %v, want admin action minimum-version rejection", validationErr) + } + return + } + if validationErr != nil { + t.Fatalf("Validate() unexpected error: %v", validationErr) + } + }) + } +} + +func TestValidateRejectsUnknownActionAccess(t *testing.T) { + m, err := Parse([]byte(validManifestYAML + ` +actions: + - key: connection.set + scope: workspace + access: owner + max_body_bytes: 1024 +`)) + if err != nil { + t.Fatalf("Parse() unexpected error: %v", err) + } + if err := m.Validate(); err == nil || !strings.Contains(err.Error(), "access") { + t.Fatalf("Validate() error = %v, want invalid action access", err) + } +} diff --git a/apps/backend/internal/plugins/manifest/manifest.go b/apps/backend/internal/plugins/manifest/manifest.go index a4f2309571..2744737df2 100644 --- a/apps/backend/internal/plugins/manifest/manifest.go +++ b/apps/backend/internal/plugins/manifest/manifest.go @@ -199,15 +199,27 @@ func (w Webhook) EffectiveMaxBodyBytes() int64 { type Action struct { Key string `yaml:"key" json:"key"` ResourceScope string `yaml:"scope" json:"scope"` + Access string `yaml:"access,omitempty" json:"access,omitempty"` MaxBodyBytes int `yaml:"max_body_bytes" json:"max_body_bytes"` } const ( - ActionScopeWorkspace = "workspace" - ActionScopeTask = "task" - ActionScopeRepository = "repository" + ActionScopeWorkspace = "workspace" + ActionScopeTask = "task" + ActionScopeRepository = "repository" + ActionAccessAuthenticated = "authenticated" + ActionAccessAdmin = "admin" ) +// EffectiveAccess preserves the original action contract: actions are +// available to any authenticated caller unless a manifest opts into admin. +func (a Action) EffectiveAccess() string { + if a.Access == "" { + return ActionAccessAuthenticated + } + return a.Access +} + // UnmarshalYAML accepts the frozen manifest field name (scope) and the // pre-release resource_scope spelling so stored local plugin records remain // readable through the contract correction. Marshal paths emit scope only. @@ -216,6 +228,7 @@ func (a *Action) UnmarshalYAML(value *yaml.Node) error { Key string `yaml:"key"` Scope string `yaml:"scope"` LegacyResourceScope string `yaml:"resource_scope"` + Access string `yaml:"access"` MaxBodyBytes int `yaml:"max_body_bytes"` } if err := value.Decode(&raw); err != nil { @@ -229,6 +242,7 @@ func (a *Action) UnmarshalYAML(value *yaml.Node) error { if a.ResourceScope == "" { a.ResourceScope = raw.LegacyResourceScope } + a.Access = raw.Access a.MaxBodyBytes = raw.MaxBodyBytes return nil } diff --git a/apps/backend/internal/plugins/manifest/validate.go b/apps/backend/internal/plugins/manifest/validate.go index ccc86af2bc..c35a1a6c31 100644 --- a/apps/backend/internal/plugins/manifest/validate.go +++ b/apps/backend/internal/plugins/manifest/validate.go @@ -544,6 +544,9 @@ func (m *Manifest) validateWebhooks() []error { } func (m *Manifest) validateActions() []error { + const minimumAdminActionKandevVersion = "0.91.1" + minimum, validMinimum := NormalizeReleaseVersion(m.MinKandevVersion) + adminAccessSupported := validMinimum && CompareVersions(minimum, minimumAdminActionKandevVersion) >= 0 seen := make(map[string]bool, len(m.Actions)) var errs []error for _, action := range m.Actions { @@ -559,6 +562,15 @@ func (m *Manifest) validateActions() []error { action.ResourceScope != ActionScopeRepository { errs = append(errs, fmt.Errorf("action %q has invalid scope %q", action.Key, action.ResourceScope)) } + if access := action.EffectiveAccess(); access != ActionAccessAuthenticated && access != ActionAccessAdmin { + errs = append(errs, fmt.Errorf("action %q has invalid access %q", action.Key, action.Access)) + } else if access == ActionAccessAdmin && !adminAccessSupported { + errs = append(errs, fmt.Errorf( + "action %q with admin access requires min_kandev_version >= %s", + action.Key, + minimumAdminActionKandevVersion, + )) + } if action.MaxBodyBytes <= 0 || action.MaxBodyBytes > MaxActionBodyBytes { errs = append(errs, fmt.Errorf("action %q max_body_bytes must be between 1 and %d", action.Key, MaxActionBodyBytes)) } diff --git a/apps/backend/internal/plugins/marketplace_handlers.go b/apps/backend/internal/plugins/marketplace_handlers.go index 83a3a200d2..c642ce5d91 100644 --- a/apps/backend/internal/plugins/marketplace_handlers.go +++ b/apps/backend/internal/plugins/marketplace_handlers.go @@ -7,6 +7,7 @@ import ( "github.com/gin-gonic/gin" "go.uber.org/zap" + "github.com/kandev/kandev/internal/auth/authn" "github.com/kandev/kandev/internal/plugins/marketplace" ) @@ -15,11 +16,11 @@ import ( // the marketplace HTTP surface stays self-contained. func (c *Controller) registerMarketplaceRoutes(api *gin.RouterGroup) { api.GET("/marketplace", c.marketplaceCatalog) - api.POST("/marketplace/refresh", c.marketplaceRefresh) + api.POST("/marketplace/refresh", authn.RequireAdmin(), c.marketplaceRefresh) api.GET("/marketplace/sources", c.listSources) - api.POST("/marketplace/sources", c.addSource) - api.PATCH("/marketplace/sources/:sid", c.updateSource) - api.DELETE("/marketplace/sources/:sid", c.deleteSource) + api.POST("/marketplace/sources", authn.RequireAdmin(), c.addSource) + api.PATCH("/marketplace/sources/:sid", authn.RequireAdmin(), c.updateSource) + api.DELETE("/marketplace/sources/:sid", authn.RequireAdmin(), c.deleteSource) } // addSourceRequest is the POST /api/plugins/marketplace/sources body. diff --git a/apps/backend/internal/plugins/marketplace_handlers_test.go b/apps/backend/internal/plugins/marketplace_handlers_test.go index 7e34fbf3f4..2b9fbcd818 100644 --- a/apps/backend/internal/plugins/marketplace_handlers_test.go +++ b/apps/backend/internal/plugins/marketplace_handlers_test.go @@ -4,6 +4,7 @@ import ( "encoding/json" "net/http" "net/http/httptest" + "sync/atomic" "testing" "github.com/gin-gonic/gin" @@ -85,8 +86,7 @@ func TestMarketplaceSourceCRUD(t *testing.T) { gin.SetMode(gin.TestMode) svc, _, _ := newTestService(t) attachMarketplaceWithSource(t, svc, "https://official.example/index.json") - router := gin.New() - RegisterRoutes(router, svc, nil, testLogger(t)) + router := registerAdminPluginRoutes(t, svc) // List returns the built-in source. list := doRequest(router, http.MethodGet, "/api/plugins/marketplace/sources", "", nil) @@ -124,8 +124,7 @@ func TestMarketplaceDeleteBuiltinReturns409(t *testing.T) { gin.SetMode(gin.TestMode) svc, _, _ := newTestService(t) attachMarketplaceWithSource(t, svc, "https://official.example/index.json") - router := gin.New() - RegisterRoutes(router, svc, nil, testLogger(t)) + router := registerAdminPluginRoutes(t, svc) sources, err := svc.Marketplace().Sources() if err != nil { @@ -136,3 +135,110 @@ func TestMarketplaceDeleteBuiltinReturns409(t *testing.T) { t.Fatalf("delete builtin: want 409, got %d", rec.Code) } } + +func TestMarketplaceSourceMutationsRejectMemberBeforePersistence(t *testing.T) { + svc, _, _ := newTestService(t) + attachMarketplaceWithSource(t, svc, "https://official.example/index.json") + router := registerMemberPluginRoutes(t, svc) + + before, err := svc.Marketplace().Sources() + if err != nil { + t.Fatalf("sources before add: %v", err) + } + add := doRequest(router, http.MethodPost, "/api/plugins/marketplace/sources", + `{"name":"Member","url":"https://member.example/index.json"}`, + map[string]string{"Content-Type": "application/json"}) + requireMemberForbidden(t, add.Code, add.Body.String()) + after, err := svc.Marketplace().Sources() + if err != nil { + t.Fatalf("sources after add: %v", err) + } + if len(after) != len(before) { + t.Errorf("member added marketplace source: before=%d after=%d", len(before), len(after)) + } + + operatorSource, err := svc.Marketplace().AddSource("Operator", "https://operator.example/index.json") + if err != nil { + t.Fatalf("add operator fixture: %v", err) + } + patch := doRequest(router, http.MethodPatch, "/api/plugins/marketplace/sources/"+operatorSource.ID, + `{"name":"Member renamed","enabled":false}`, + map[string]string{"Content-Type": "application/json"}) + requireMemberForbidden(t, patch.Code, patch.Body.String()) + assertMarketplaceSource(t, svc, operatorSource.ID, "Operator", true) + + del := doRequest(router, http.MethodDelete, "/api/plugins/marketplace/sources/"+operatorSource.ID, "", nil) + requireMemberForbidden(t, del.Code, del.Body.String()) + assertMarketplaceSource(t, svc, operatorSource.ID, "Operator", true) +} + +func TestMarketplaceRefreshRejectsMemberBeforeInvalidatingCache(t *testing.T) { + server, fetches := countingFixtureIndexServer(t) + svc, _, _ := newTestService(t) + attachMarketplaceWithSource(t, svc, server.URL) + router := registerMemberPluginRoutes(t, svc) + + first := doRequest(router, http.MethodGet, "/api/plugins/marketplace", "", nil) + if first.Code != http.StatusOK { + t.Fatalf("initial catalog status = %d, want 200", first.Code) + } + refresh := doRequest(router, http.MethodPost, "/api/plugins/marketplace/refresh", "", nil) + requireMemberForbidden(t, refresh.Code, refresh.Body.String()) + second := doRequest(router, http.MethodGet, "/api/plugins/marketplace", "", nil) + if second.Code != http.StatusOK { + t.Fatalf("second catalog status = %d, want 200", second.Code) + } + if got := fetches.Load(); got != 1 { + t.Errorf("catalog fetches = %d, want 1; member refresh invalidated cache", got) + } +} + +func TestMarketplaceRefreshAllowsAdmin(t *testing.T) { + server, fetches := countingFixtureIndexServer(t) + svc, _, _ := newTestService(t) + attachMarketplaceWithSource(t, svc, server.URL) + router := registerAdminPluginRoutes(t, svc) + + if rec := doRequest(router, http.MethodGet, "/api/plugins/marketplace", "", nil); rec.Code != http.StatusOK { + t.Fatalf("initial catalog status = %d, want 200", rec.Code) + } + if rec := doRequest(router, http.MethodPost, "/api/plugins/marketplace/refresh", "", nil); rec.Code != http.StatusOK { + t.Fatalf("refresh status = %d, want 200, body=%s", rec.Code, rec.Body.String()) + } + if rec := doRequest(router, http.MethodGet, "/api/plugins/marketplace", "", nil); rec.Code != http.StatusOK { + t.Fatalf("second catalog status = %d, want 200", rec.Code) + } + if got := fetches.Load(); got != 2 { + t.Fatalf("catalog fetches = %d, want 2 after admin refresh", got) + } +} + +func assertMarketplaceSource(t *testing.T, svc *Service, id, name string, enabled bool) { + t.Helper() + sources, err := svc.Marketplace().Sources() + if err != nil { + t.Fatalf("sources: %v", err) + } + for _, source := range sources { + if source.ID == id { + if source.Name != name || source.Enabled != enabled { + t.Errorf("source = %+v, want name=%q enabled=%v", source, name, enabled) + } + return + } + } + t.Errorf("source %q was deleted", id) +} + +func countingFixtureIndexServer(t *testing.T) (*httptest.Server, *atomic.Int32) { + t.Helper() + var fetches atomic.Int32 + body := `{"schema_version":1,"generated_at":"2026-07-18T00:00:00Z",` + + `"source":{"name":"Kandev Official","url":""},"plugins":[]}` + server := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, _ *http.Request) { + fetches.Add(1) + _, _ = w.Write([]byte(body)) + })) + t.Cleanup(server.Close) + return server, &fetches +} diff --git a/apps/web/app/settings/plugins/page.test.tsx b/apps/web/app/settings/plugins/page.test.tsx index 395d8e148b..029af226e6 100644 --- a/apps/web/app/settings/plugins/page.test.tsx +++ b/apps/web/app/settings/plugins/page.test.tsx @@ -146,6 +146,7 @@ function installedPlugin(overrides: Partial = {}): PluginRecord { function setStoreState(plugins: PluginRecord[]) { storeState = { + auth: { user: null }, plugins: { items: plugins, loading: false, loaded: true, error: null }, setPlugins: vi.fn(), setPluginsLoading: vi.fn(), @@ -317,6 +318,22 @@ describe("PluginsSettingsPage", () => { }); }); +describe("PluginsSettingsPage member view", () => { + it("keeps instance-global plugin controls hidden from members", () => { + setStoreState([activePlugin()]); + storeState.auth = { mode: "enabled", user: { role: "member" } }; + + render(); + + expect(screen.getByText(PLUGIN_DISPLAY_NAME)).toBeTruthy(); + expect(screen.queryByTestId(SYNC_BUTTON_TESTID)).toBeNull(); + expect(screen.queryByTestId(CHECK_UPDATES_BUTTON_TESTID)).toBeNull(); + expect(screen.queryByTestId(INSTALL_TRIGGER_TESTID)).toBeNull(); + expect(screen.queryByTestId("plugins-auto-update-default")).toBeNull(); + expect(screen.queryByRole("button", { name: /disable|uninstall|update/i })).toBeNull(); + }); +}); + describe("PluginsSettingsPage install dialog", () => { it("shows a spinner while a URL install is pending", async () => { let resolveInstall!: (result: InstallResult) => void; diff --git a/apps/web/components/app-sidebar/sections/settings/settings-tree.tsx b/apps/web/components/app-sidebar/sections/settings/settings-tree.tsx index c9aa91ea77..3a418abddb 100644 --- a/apps/web/components/app-sidebar/sections/settings/settings-tree.tsx +++ b/apps/web/components/app-sidebar/sections/settings/settings-tree.tsx @@ -5,6 +5,7 @@ import { useTranslation } from "react-i18next"; import { PluginSlot } from "@/components/plugins/plugin-slot"; import { useAppStore } from "@/components/state-provider"; import { useFeature } from "@/hooks/domains/features/use-feature"; +import { useIsAdmin } from "@/hooks/domains/auth/use-is-admin"; import { usePlugins } from "@/hooks/domains/plugins/use-plugins"; import { useSecrets } from "@/hooks/domains/settings/use-secrets"; import { useSettingsDiscovery } from "@/hooks/domains/settings/use-settings-discovery"; @@ -35,12 +36,6 @@ export { type SettingsMenuSection, } from "./settings-menu-sections"; -/** null user (disabled/synthetic single-user mode) counts as admin for gating. */ -function useIsAdmin(): boolean { - const role = useAppStore((s) => s.auth.user?.role); - return role === undefined || role === "admin"; -} - /** * Item counts for rows whose page owns a list. `undefined` until the backing * data is loaded, so rows never flash a wrong zero. Secrets and plugins load diff --git a/apps/web/components/settings/plugins/marketplace-browser.tsx b/apps/web/components/settings/plugins/marketplace-browser.tsx index 09765caa74..9c7083b154 100644 --- a/apps/web/components/settings/plugins/marketplace-browser.tsx +++ b/apps/web/components/settings/plugins/marketplace-browser.tsx @@ -17,10 +17,11 @@ const ALL_CATEGORIES = "__all__"; type MarketplaceBrowserProps = { /** Installs a plugin from its package URL; resolves when the install settles. The result is unused here — failures already toast. */ onInstallUrl: (url: string) => Promise<{ ok: boolean; error?: string }>; + canManage?: boolean; }; /** The "Browse" tab: search/filter/sort the catalog and install from it. */ -export function MarketplaceBrowser({ onInstallUrl }: MarketplaceBrowserProps) { +export function MarketplaceBrowser({ onInstallUrl, canManage = true }: MarketplaceBrowserProps) { const [text, setText] = useState(""); const [category, setCategory] = useState(ALL_CATEGORIES); const [sort, setSort] = useState("stars"); @@ -62,6 +63,7 @@ export function MarketplaceBrowser({ onInstallUrl }: MarketplaceBrowserProps) { onSort={setSort} onManageSources={() => setSourcesOpen(true)} onRefresh={reload} + canManage={canManage} /> @@ -73,14 +75,17 @@ export function MarketplaceBrowser({ onInstallUrl }: MarketplaceBrowserProps) { filtered={Boolean(text.trim() || category !== ALL_CATEGORIES)} installingId={installingId} onInstall={install} + canManage={canManage} /> - + {canManage && ( + + )} ); } @@ -105,6 +110,7 @@ type ToolbarProps = { onSort: (v: CatalogQuery["sort"]) => void; onManageSources: () => void; onRefresh: () => void; + canManage: boolean; }; function MarketplaceToolbar(props: ToolbarProps) { @@ -142,20 +148,22 @@ function MarketplaceToolbar(props: ToolbarProps) { {t("plugins:sortName")} -
- - -
+ {props.canManage && ( +
+ + +
+ )} ); } @@ -199,6 +207,7 @@ type ListProps = { filtered: boolean; installingId: string | null; onInstall: (entry: MarketplaceEntry) => void; + canManage: boolean; }; function MarketplaceList({ @@ -208,6 +217,7 @@ function MarketplaceList({ filtered, installingId, onInstall, + canManage, }: ListProps) { const { t } = useTranslation(); if (error) { @@ -243,6 +253,7 @@ function MarketplaceList({ entry={entry} busy={installingId === entry.id} onInstall={onInstall} + canManage={canManage} /> ))} diff --git a/apps/web/components/settings/plugins/marketplace-entry-row.test.tsx b/apps/web/components/settings/plugins/marketplace-entry-row.test.tsx index 2572d0a613..80f95cf380 100644 --- a/apps/web/components/settings/plugins/marketplace-entry-row.test.tsx +++ b/apps/web/components/settings/plugins/marketplace-entry-row.test.tsx @@ -53,3 +53,23 @@ describe("MarketplaceEntryRow repo link", () => { expect(screen.getByText("-")).toBeTruthy(); }); }); + +describe("MarketplaceEntryRow member view", () => { + it("keeps catalog metadata visible without install or update actions", () => { + const { rerender } = render( + , + ); + expect(screen.getByText("Acme")).toBeTruthy(); + expect(screen.queryByRole("button", { name: "Install" })).toBeNull(); + + rerender( + , + ); + expect(screen.queryByRole("button", { name: "Update" })).toBeNull(); + }); +}); diff --git a/apps/web/components/settings/plugins/marketplace-entry-row.tsx b/apps/web/components/settings/plugins/marketplace-entry-row.tsx index f089666a9d..f2035efa56 100644 --- a/apps/web/components/settings/plugins/marketplace-entry-row.tsx +++ b/apps/web/components/settings/plugins/marketplace-entry-row.tsx @@ -18,10 +18,16 @@ type MarketplaceEntryRowProps = { entry: MarketplaceEntry; busy: boolean; onInstall: (entry: MarketplaceEntry) => void; + canManage?: boolean; }; /** One catalog card: a neutral tile, metadata, stars, and an install-state action. */ -export function MarketplaceEntryRow({ entry, busy, onInstall }: MarketplaceEntryRowProps) { +export function MarketplaceEntryRow({ + entry, + busy, + onInstall, + canManage = true, +}: MarketplaceEntryRowProps) { return (
- +
@@ -104,7 +115,7 @@ function PluginTile({ entry }: { entry: MarketplaceEntry }) { ); } -function MarketplaceEntryAction({ entry, busy, onInstall }: MarketplaceEntryRowProps) { +function MarketplaceEntryAction({ entry, busy, onInstall, canManage }: MarketplaceEntryRowProps) { const { t } = useTranslation(); if (entry.install_state === "installed") { return ( @@ -118,6 +129,7 @@ function MarketplaceEntryAction({ entry, busy, onInstall }: MarketplaceEntryRowP ); } + if (!canManage) return null; if (entry.install_state === "update_available") { return (
); } diff --git a/apps/web/components/settings/plugins/plugin-row.test.tsx b/apps/web/components/settings/plugins/plugin-row.test.tsx index dd1f479167..941e6ca83b 100644 --- a/apps/web/components/settings/plugins/plugin-row.test.tsx +++ b/apps/web/components/settings/plugins/plugin-row.test.tsx @@ -61,6 +61,7 @@ const UPDATE_BUTTON_TESTID = "plugin-update-acme"; const LATEST_VERSION_TESTID = "plugin-latest-version-acme"; const UPDATE_BADGE_TESTID = "plugin-update-available-acme"; const NOT_IN_MARKETPLACE_TESTID = "plugin-not-in-marketplace-acme"; +const INLINE_UNINSTALL_CONFIRMATION_TESTID = "plugin-uninstall-inline-confirmation"; // baseProps carries the always-required callbacks/flags so each test only // spells out the props it is actually asserting on. @@ -369,9 +370,69 @@ describe("PluginRow auto-update toggle", () => { }); }); +describe("PluginRow member view", () => { + it("keeps plugin details available without rendering administrator actions", () => { + render( + , + ); + + expect(screen.getByTestId("plugin-row-link-acme")).toBeTruthy(); + expect(screen.queryByRole("button", { name: "Disable" })).toBeNull(); + expect(screen.queryByRole("button", { name: "Uninstall" })).toBeNull(); + expect(screen.queryByTestId("plugin-update-acme")).toBeNull(); + expect(screen.queryByTestId(AUTO_UPDATE_TESTID)).toBeNull(); + }); +}); + describe("PluginRow uninstall confirmation", () => { const pluginName = "Acme Tools"; + it("closes an open confirmation when management permission is revoked", () => { + const p = plugin({ display_name: pluginName }); + const onConfirmUninstall = vi.fn(); + const view = render( + , + ); + + fireEvent.click(screen.getByRole("button", { name: /^Uninstall$/i })); + expect(screen.getByTestId(INLINE_UNINSTALL_CONFIRMATION_TESTID)).toBeTruthy(); + + view.rerender( + , + ); + expect(screen.queryByTestId(INLINE_UNINSTALL_CONFIRMATION_TESTID)).toBeNull(); + + view.rerender( + , + ); + expect(screen.queryByTestId(INLINE_UNINSTALL_CONFIRMATION_TESTID)).toBeNull(); + expect(onConfirmUninstall).not.toHaveBeenCalled(); + }); + it("anchors fine-pointer confirmation to the row action and names the target", () => { const p = plugin({ display_name: pluginName }); const onConfirmUninstall = vi.fn(); @@ -406,7 +467,7 @@ describe("PluginRow uninstall confirmation", () => { fireEvent.click(screen.getByRole("button", { name: /^Uninstall$/i })); - const confirmation = screen.getByTestId("plugin-uninstall-inline-confirmation"); + const confirmation = screen.getByTestId(INLINE_UNINSTALL_CONFIRMATION_TESTID); expect(confirmation.textContent).toContain(pluginName); expect(screen.queryByRole("button", { name: /^Uninstall$/i })).toBeNull(); expect(screen.getByTestId("plugin-uninstall-confirm").className).toContain("h-11"); diff --git a/apps/web/components/settings/plugins/plugin-row.tsx b/apps/web/components/settings/plugins/plugin-row.tsx index e52eb2c652..3c5fb4ee2e 100644 --- a/apps/web/components/settings/plugins/plugin-row.tsx +++ b/apps/web/components/settings/plugins/plugin-row.tsx @@ -1,6 +1,6 @@ "use client"; -import { useRef, useState, type RefObject } from "react"; +import { useEffect, useRef, useState, type RefObject } from "react"; import { IconArrowUpCircle, IconChevronRight, @@ -57,6 +57,8 @@ type PluginRowProps = { autoUpdateBusy: boolean; /** True when the plugin declares required settings the operator has not filled in. */ needsSetup?: boolean; + /** Whether the current user may mutate instance-global plugin state. */ + canManage?: boolean; /** Fine pointers use an anchored popover; coarse pointers use inline row actions. */ isFinePointer?: boolean; /** True while this plugin's uninstall request is in flight. */ @@ -87,6 +89,7 @@ export function PluginRow({ autoUpdateDefault, autoUpdateBusy, needsSetup = false, + canManage = true, isFinePointer = true, uninstallBusy = false, onEnable, @@ -108,6 +111,10 @@ export function PluginRow({ confirmUninstall, } = usePluginUninstallConfirmation(plugin, onConfirmUninstall); + useEffect(() => { + if (!canManage) setConfirmingUninstall(false); + }, [canManage, setConfirmingUninstall]); + return (
-
- -
+ {canManage && ( +
+ +
+ )}
); } @@ -150,6 +160,7 @@ type PluginRowContentProps = { update?: PluginRowUpdateState; autoUpdateDefault: boolean; needsSetup: boolean; + canManage: boolean; isFinePointer: boolean; mutationBusy: boolean; canEnable: boolean; @@ -168,6 +179,7 @@ function PluginRowContent({ update, autoUpdateDefault, needsSetup, + canManage, isFinePointer, mutationBusy, canEnable, @@ -201,20 +213,22 @@ function PluginRowContent({
- + {canManage && ( + + )} )} - + {canManage && ( + + )}
); diff --git a/apps/web/components/settings/plugins/plugins-settings.tsx b/apps/web/components/settings/plugins/plugins-settings.tsx index f1de95a30a..50919c1764 100644 --- a/apps/web/components/settings/plugins/plugins-settings.tsx +++ b/apps/web/components/settings/plugins/plugins-settings.tsx @@ -9,6 +9,7 @@ import { Tabs, TabsContent, TabsList, TabsTrigger } from "@kandev/ui/tabs"; import { SettingsPageTemplate } from "@/components/settings/settings-page-template"; import { useResponsiveBreakpoint } from "@/hooks/use-responsive-breakpoint"; import { useAutoUpdateSettings } from "@/hooks/domains/plugins/use-auto-update-settings"; +import { useIsAdmin } from "@/hooks/domains/auth/use-is-admin"; import { usePlugins } from "@/hooks/domains/plugins/use-plugins"; import { usePluginSetupStatus } from "@/hooks/domains/plugins/use-plugin-setup-status"; import { usePluginUpdates } from "@/hooks/domains/plugins/use-plugin-updates"; @@ -27,6 +28,7 @@ import { settingsActionClassName } from "@/components/settings/settings-control" */ export function PluginsSettings() { const { t } = useTranslation(); + const canManage = useIsAdmin(); const { isFinePointer } = useResponsiveBreakpoint(); const list = usePlugins(); const actions = usePluginActions(); @@ -75,24 +77,27 @@ export function PluginsSettings() { actions={actions} autoUpdate={autoUpdate} updates={updates} + canManage={canManage} updateAction={updateAction} isFinePointer={isFinePointer} /> - + - + {canManage && ( + + )} ); } @@ -101,6 +106,7 @@ type InstalledTabProps = { list: ReturnType; actions: ReturnType; autoUpdate: ReturnType; + canManage: boolean; updates: ReturnType; updateAction: ReturnType; isFinePointer: boolean; @@ -112,6 +118,7 @@ function InstalledTab({ actions, autoUpdate, updates, + canManage, updateAction, isFinePointer, }: InstalledTabProps) { @@ -119,39 +126,41 @@ function InstalledTab({ return ( <> - + {canManage && }
{t("plugins:installedPlugins")}
-
- - - -
+ {canManage && ( +
+ + + +
+ )}
- {actions.syncErrors.length > 0 && ( + {canManage && actions.syncErrors.length > 0 && (
@@ -223,6 +233,7 @@ type PluginListProps = { list: ReturnType; actions: ReturnType; autoUpdateDefault: boolean; + canManage: boolean; updates: ReturnType; updateAction: ReturnType; isFinePointer: boolean; @@ -233,6 +244,7 @@ function PluginList({ actions, autoUpdateDefault, updates, + canManage, updateAction, isFinePointer, }: PluginListProps) { @@ -284,6 +296,7 @@ function PluginList({ autoUpdateDefault={autoUpdateDefault} autoUpdateBusy={actions.autoUpdateBusyId === plugin.id} needsSetup={needsSetup.has(plugin.id)} + canManage={canManage} isFinePointer={isFinePointer} uninstallBusy={actions.uninstallBusy} onEnable={actions.handleEnable} diff --git a/apps/web/e2e/tests/auth/plugin-settings-member.spec.ts b/apps/web/e2e/tests/auth/plugin-settings-member.spec.ts new file mode 100644 index 0000000000..ddda815943 --- /dev/null +++ b/apps/web/e2e/tests/auth/plugin-settings-member.spec.ts @@ -0,0 +1,76 @@ +import { expect } from "@playwright/test"; +import path from "node:path"; +import { backendFixture as test } from "../../fixtures/backend"; +import { login, setupAdmin } from "../../helpers/auth"; +import { installFixturePlugin, PLUGIN_ID } from "../../helpers/plugin-fixture"; + +const ADMIN = { email: "plugin-admin@demo.dev", password: "adminpass123", displayName: "Ada" }; +const MEMBER = { email: "plugin-member@demo.dev", password: "memberpass123", displayName: "Sam" }; + +test.describe.serial("member plugin settings", () => { + test.beforeAll(async ({ backend }) => { + await backend.restart({ + KANDEV_FEATURES_AUTH: "true", + KANDEV_DATABASE_PATH: path.join(backend.tmpDir, "kandev-auth-plugin-member.db"), + }); + }); + + test.afterAll(async ({ backend }) => { + await backend.restart(); + }); + + test("shows plugin and marketplace metadata without administrator controls", async ({ + browser, + backend, + }) => { + const adminContext = await browser.newContext({ baseURL: backend.frontendUrl }); + await setupAdmin(adminContext, backend.baseUrl, ADMIN); + await login(adminContext, backend.baseUrl, ADMIN); + + const adminPage = await adminContext.newPage(); + await installFixturePlugin(adminPage); + const createMember = await adminContext.request.post(`${backend.baseUrl}/api/v1/users`, { + data: { + email: MEMBER.email, + password: MEMBER.password, + display_name: MEMBER.displayName, + role: "member", + }, + }); + expect(createMember.status(), await createMember.text()).toBe(201); + + const memberContext = await browser.newContext({ baseURL: backend.frontendUrl }); + await login(memberContext, backend.baseUrl, MEMBER); + const page = await memberContext.newPage(); + await page.goto("/settings/plugins"); + + const settingsPanel = page.locator('[data-testid="settings-scroll-container"]:visible'); + await expect(settingsPanel).toBeVisible(); + + const row = settingsPanel.getByTestId(`plugin-row-${PLUGIN_ID}`); + await expect(row).toBeVisible({ timeout: 15_000 }); + await expect(row.getByTestId(`plugin-row-link-${PLUGIN_ID}`)).toBeVisible(); + await expect(settingsPanel.getByTestId("install-plugin-trigger")).toHaveCount(0); + await expect(settingsPanel.getByTestId("plugins-sync-button")).toHaveCount(0); + await expect(settingsPanel.getByTestId("plugins-check-updates-button")).toHaveCount(0); + await expect(settingsPanel.getByTestId("plugins-auto-update-default")).toHaveCount(0); + await expect(row.getByRole("button", { name: /Enable|Disable|Uninstall|Update/ })).toHaveCount( + 0, + ); + await expect(row.getByTestId(`plugin-auto-update-${PLUGIN_ID}`)).toHaveCount(0); + + await row.getByTestId(`plugin-row-link-${PLUGIN_ID}`).click(); + await expect(settingsPanel.getByTestId("plugin-manifest-card")).toBeVisible(); + await expect(settingsPanel.getByTestId("plugin-settings-card")).toHaveCount(0); + await expect(settingsPanel.getByRole("button", { name: "Uninstall" })).toHaveCount(0); + + await page.goto("/settings/plugins"); + await settingsPanel.getByTestId("plugins-tab-browse").click(); + await expect(settingsPanel.getByTestId("marketplace-search")).toBeVisible(); + await expect(settingsPanel.getByTestId("marketplace-manage-sources")).toHaveCount(0); + await expect(settingsPanel.getByRole("button", { name: "Refresh" })).toHaveCount(0); + + await memberContext.close(); + await adminContext.close(); + }); +}); diff --git a/docs/plans/plugins/PLUGIN-API.md b/docs/plans/plugins/PLUGIN-API.md index a18044f0d1..31f19f27ba 100644 --- a/docs/plans/plugins/PLUGIN-API.md +++ b/docs/plans/plugins/PLUGIN-API.md @@ -428,6 +428,10 @@ actions: max_body_bytes: 16384 ``` +Browser-invoked actions may additionally declare `access: "admin"`. Omitting +`access` preserves the default `authenticated` policy. The host rejects a +non-administrator before it forwards any request body to an admin action. + This action is invoked by the host backend, not the browser callback. Kandev resolves the active plugin that owns the repository's persisted provider ID and supplies a verified workspace context plus this snake-case body: diff --git a/docs/public/plugins-authoring.md b/docs/public/plugins-authoring.md index 53fe0b810c..9b341409af 100644 --- a/docs/public/plugins-authoring.md +++ b/docs/public/plugins-authoring.md @@ -854,12 +854,21 @@ declared `scope`, strips selectors from the untrusted body, and calls the plugin's optional `ActionHandler` with a `VerifiedActionContext`. ```yaml +min_kandev_version: "0.91.1" actions: - key: "pullrequests.link" scope: "task" + access: "authenticated" max_body_bytes: 32768 ``` +`access` defaults to `authenticated`. Set it to `admin` for instance-wide +configuration or credentials that only a Kandev administrator may manage. The +host checks this policy before reading the bounded envelope or invoking the +plugin, so the plugin does not need to infer roles from actor IDs. An `admin` +action requires `min_kandev_version: "0.91.1"` or later; validation rejects a +missing or older minimum because earlier hosts do not enforce action access. + ```ts const result = await host.api.invokeAction( "pullrequests.link", @@ -882,7 +891,8 @@ Kandev verifies the repository worktree and derives its non-empty head branch; a only host-verified `actorID`, `workspaceID`, `taskID`, `sessionID`, `repositoryID`, and derived `headBranch` plus the bounded JSON `Body`. Unknown actions return 404; bad envelopes and -scope-selector combinations return 400; unauthenticated calls return 401. +scope-selector combinations return 400; unauthenticated calls return 401; and +members calling an `admin` action receive 403. The host gives each action 15 seconds and cancels its RPC when the browser abandons the request. Pass the `AbortSignal` supplied to your UI calls, and in diff --git a/docs/public/plugins-manifest.md b/docs/public/plugins-manifest.md index 740eb045dc..6633dca2b2 100644 --- a/docs/public/plugins-manifest.md +++ b/docs/public/plugins-manifest.md @@ -42,7 +42,7 @@ runtime: server/plugin-windows-amd64.exe # any subset; the host's - key # is required at install time -min_kandev_version: "0.78.0" # optional +min_kandev_version: "0.91.1" # required for admin actions capabilities: events: ["task.created", "task.state_changed", "agent.completed"] @@ -64,6 +64,7 @@ webhooks: actions: # authenticated browser-to-plugin calls - key: "connection.save" scope: "workspace" # workspace | task | repository + access: admin # authenticated (default) | admin max_body_bytes: 65536 # 1 through 1048576 repository_providers: ["acme"] # optional provider ids owned while active @@ -152,7 +153,7 @@ ui: # optional native frontend plugin | `repo_url` | no | string | Absolute `http(s)` URL to the plugin's source repository. Rendered as a "Repo" link in Settings > Plugins (both the installed list and the plugin detail). Any other scheme (e.g. `javascript:`) is rejected at registration. Distinct from the marketplace card's `repo_url`, which the registry derives from `plugins.yaml`; declare this in the manifest so sideloaded and directly-installed plugins also carry the link. | | `runtime.type` | conditionally | string | `"binary"` is the only supported value. Setting it (vs. leaving it empty) makes the manifest **runtime-managed**; see "Managed vs. legacy" below. | | `runtime.executables` | required when `runtime.type: binary` | map\ | Key is `-` (e.g. `linux-amd64`, `darwin-arm64`, `windows-amd64`); value is a clean, package-relative path under `server/` (no leading `/`, no `..` segments). At least one entry required; the running host's key must be present at install time. Windows values end in `.exe`. | -| `min_kandev_version` | no | string | Lowest released Kandev version this plugin supports. Enforced at **install time**: installing onto an older release fails with `requires kandev >= , running ` and nothing is registered. Already-installed plugins are never re-checked at load or start. Release values use up to three dotted numeric segments (`0.78.0`) and may have a leading `v`; malformed minimums are rejected. Non-release development and git-describe builds skip the compatibility gate. | +| `min_kandev_version` | conditionally | string | Lowest released Kandev version this plugin supports. Required to be at least `0.91.1` when any action uses `access: admin`. Enforced at **install time**: installing onto an older release fails with `requires kandev >= , running ` and nothing is registered. Already-installed plugins are never re-checked at load or start. Release values use up to three dotted numeric segments (`0.78.0`) and may have a leading `v`; malformed minimums are rejected. Non-release development and git-describe builds skip the compatibility gate. | | `capabilities.events` | no | string[] | Bus subjects (or wildcard patterns) this plugin subscribes to. See "Event subscription vocabulary" below. | | `capabilities.api_read` | no | string[] | Gates the Host data API's read-only accessors. Each entry is a resource name: `tasks`, `sessions`, `messages`, `workspaces`, `workflows`, `agent_profiles`, `executor_profiles`, `repositories`. Calling the matching `Host` accessor (e.g. `Tasks()`) without its resource declared returns gRPC `PermissionDenied`. See "Host data API resource vocabulary" below. | | `capabilities.api_write` | no | string[] | Gates Host writes independently of `api_read`. `tasks` permits `Host.Tasks().Create` and `.Update`; `messages` permits `Host.Messages().Send`. Undeclared writes return gRPC `PermissionDenied`. | @@ -169,6 +170,7 @@ ui: # optional native frontend plugin | `actions[]` | no | object[] | Authenticated, manifest-declared browser actions. They are distinct from public webhooks: the host validates the action key, limits the JSON body, authorizes the declared resource, and passes only a verified context to the plugin. | | `actions[].key` | yes\* | string | Unique non-empty action key, used at `POST /api/plugins/{id}/actions/{key}`. | | `actions[].scope` | yes\* | `workspace` \| `task` \| `repository` | Resource selector the host authorizes. A task-scoped invocation may additionally name one repository attached to that verified task. Use the canonical field name `scope`; `resource_scope` is only a legacy read-compatibility spelling and must not be authored. | +| `actions[].access` | no | `authenticated` \| `admin` | Defaults to `authenticated`. `admin` is enforced before the action envelope is read and requires `min_kandev_version: "0.91.1"` or later so older hosts cannot silently treat it as authenticated. | | `actions[].max_body_bytes` | yes\* | int | Maximum size of the decoded untrusted `body`, from 1 through 1,048,576 bytes. The whole HTTP envelope has a slightly larger hard cap. | | `repository_providers` | no | string[] | Provider IDs this plugin owns while active. An active provider can register native repository discovery/URL inspection and may supply transient Git credentials. Native task creation must use the authenticated plugin task-create path so the plugin reauthorizes the repository descriptor before Host Tasks.Create. Duplicate ownership is rejected. | | `reference_sources[]` | no | object[] | Dynamic composer-reference source. Each entry declares `source`, `provider`, `kind`, `display_name`, and `kind_label`; `order` is optional. Candidate identity is not trusted: Kandev reauthorizes the canonical reference when it is submitted. | diff --git a/docs/specs/plugins/system-design/marketplace.md b/docs/specs/plugins/system-design/marketplace.md index 4144417b18..16dd0ec87d 100644 --- a/docs/specs/plugins/system-design/marketplace.md +++ b/docs/specs/plugins/system-design/marketplace.md @@ -257,13 +257,14 @@ response but stays `enabled`. ## Permissions -- Browsing the catalog and installing from it require the same operator authority as - the existing plugin-management API — marketplace endpoints sit behind the same - auth as `POST /api/plugins/install`. Plugins ship unflagged in the base - product, so there is no feature gate in front of either. -- Managing marketplace sources (add/remove/enable) is an operator action; adding a - source is an explicit act of trust in that source's maintainer, equivalent to - `brew tap`-ing a third-party tap. +- Authenticated members may browse the catalog and inspect installed-plugin and + manifest metadata. Their UI is read-only: it omits install, update, enable, + disable, uninstall, auto-update, source-management, and plugin-configuration + controls. +- Installing or updating a plugin and managing marketplace sources + (add/remove/enable) require administrator authority. Adding a source is an + explicit act of trust in that source's maintainer, equivalent to `brew tap`-ing + a third-party tap. - Appearing in the **official** source requires a maintainer to approve the registry-list PR. Third-party/corporate sources are governed by whoever controls that source repo, not by kandev. diff --git a/docs/specs/plugins/system-design/plugins-01.md b/docs/specs/plugins/system-design/plugins-01.md index ba76b61381..f8230113eb 100644 --- a/docs/specs/plugins/system-design/plugins-01.md +++ b/docs/specs/plugins/system-design/plugins-01.md @@ -135,7 +135,7 @@ runtime: linux-amd64: server/plugin-linux-amd64 darwin-arm64: server/plugin-darwin-arm64 # ... any subset; kandev requires the running host's platform key at install time -min_kandev_version: "0.78.0" # optional +min_kandev_version: "0.91.1" # required for admin actions capabilities: events: ["task.created", "task.state_changed", "agent.completed"] @@ -151,7 +151,8 @@ capabilities: actions: - key: "connection.save" - scope: workspace # workspace | task | repository + scope: workspace + access: admin # defaults to authenticated max_body_bytes: 65536 repository_providers: ["example-repository-provider"] diff --git a/docs/specs/plugins/system-design/plugins-02.md b/docs/specs/plugins/system-design/plugins-02.md index 7a087ef017..6f5c9d95e5 100644 --- a/docs/specs/plugins/system-design/plugins-02.md +++ b/docs/specs/plugins/system-design/plugins-02.md @@ -60,18 +60,17 @@ record, and state. ### Authenticated plugin actions (browser -> kandev -> plugin) -`POST /api/plugins/{id}/actions/{key}` is the only browser action route. Kandev first -applies ordinary HTTP authentication, then rejects inactive plugins or undeclared keys -and authorizes every referenced workspace, task, or repository. It derives a task's -workspace relation server-side, passes verified actor/resource context separately from -bounded untrusted JSON, invokes `Plugin.HandleAction` with a hard timeout and -cancellation, and relays only an allowlisted response-header set. Provider callback -routes under `/webhooks/` stay public and must not serve authenticated browser actions. -Task-scoped actions require a verified task and may optionally select one persisted -repository attached to that task; Kandev rejects unattached repository IDs and passes -the accepted repository separately in `VerifiedActionContext`. -The manifest's canonical action field is `scope`; `resource_scope` remains a read-only -compatibility alias for packages produced during the prerelease contract rollout. +`POST /api/plugins/{id}/actions/{key}` is the only browser action route. Kandev +authenticates the caller, rejects inactive plugins and undeclared keys, and authorizes +each referenced workspace, task, or repository. It derives a task's workspace +server-side. `access` defaults to `authenticated`. `admin` requires +`min_kandev_version: "0.91.1"` or later; validation rejects older/missing +minimums. Non-admins are rejected before envelope parsing or plugin invocation. +Verified context is separate from untrusted JSON; calls are bounded/cancellable and +response headers allowlisted. Public `/webhooks/` callbacks cannot serve browser +actions. A task action may select one attached persisted repository; other IDs are +rejected, and the accepted repository is separate in `VerifiedActionContext`. +Canonical field: `scope`; `resource_scope` is a read-only prerelease alias. ### Dynamic composer reference sources (plugin -> kandev)