From 8d78e484f39519c3068a083e8421ab82b979c3c9 Mon Sep 17 00:00:00 2001 From: "Molecule AI Dev Engineer A (Kimi)" Date: Thu, 18 Jun 2026 23:56:38 +0000 Subject: [PATCH] defense-in-depth: gate molecule-platform-mcp in the plugin INSTALL path MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Issue #3046. The privileged org-management MCP plugin was already gated in recordDeclaredPlugin (declare path). The INSTALL path — install_plugin -> PluginsHandler.Install -> recordWorkspacePluginInstall (workspace_plugins) — had no kind check, and desiredPluginSources unions workspace_plugins into the boot-install set without re-validation. Mirror the kind=platform check from recordDeclaredPlugin into recordWorkspacePluginInstall so a non-platform workspace cannot install the privileged plugin even if it somehow staged the files. Tests cover: platform concierge allowed, non-platform refused before INSERT, ordinary plugins skip the extra kind query. Fixes #3046 🤖 Generated with [Claude Code](https://claude.com/claude-code) --- .../internal/handlers/plugins_tracking.go | 18 ++++++ .../handlers/plugins_tracking_test.go | 60 ++++++++++++++++++- 2 files changed, 77 insertions(+), 1 deletion(-) diff --git a/workspace-server/internal/handlers/plugins_tracking.go b/workspace-server/internal/handlers/plugins_tracking.go index b1f1f595..dc5efea1 100644 --- a/workspace-server/internal/handlers/plugins_tracking.go +++ b/workspace-server/internal/handlers/plugins_tracking.go @@ -67,6 +67,24 @@ func recordWorkspacePluginInstall( if workspaceID == "" || pluginName == "" || sourceRaw == "" { return errors.New("recordWorkspacePluginInstall: missing required field") } + + // Entitlement gate (defense-in-depth) for the PRIVILEGED org-management MCP + // plugin. The install path (workspace_plugins) unions into the boot-install set + // without re-validation, so a non-platform workspace that somehow staged the + // plugin files could get the management MCP installed. Refuse the privileged + // name here, mirroring the gate in recordDeclaredPlugin. Fail-closed on a kind + // read error. + if pluginName == conciergePlatformMCPPlugin { + var kind string + if err := db.DB.QueryRowContext(ctx, + `SELECT COALESCE(kind, 'workspace') FROM workspaces WHERE id = $1`, workspaceID).Scan(&kind); err != nil { + return fmt.Errorf("recordWorkspacePluginInstall: kind precheck for privileged plugin %q on %s: %w", pluginName, workspaceID, err) + } + if kind != models.KindPlatform { + return fmt.Errorf("recordWorkspacePluginInstall: refusing to install privileged plugin %q on non-platform workspace %s (kind=%s)", pluginName, workspaceID, kind) + } + } + canonicalTrack, err := validateTrackedRef(track) if err != nil { return err diff --git a/workspace-server/internal/handlers/plugins_tracking_test.go b/workspace-server/internal/handlers/plugins_tracking_test.go index 8c33023d..6f09bf2b 100644 --- a/workspace-server/internal/handlers/plugins_tracking_test.go +++ b/workspace-server/internal/handlers/plugins_tracking_test.go @@ -1,6 +1,11 @@ package handlers -import "testing" +import ( + "context" + "testing" + + "github.com/DATA-DOG/go-sqlmock" +) // TestValidateTrackedRef: pin the exact set of accepted track values // the install endpoint stores. Drift detector reads this column; any @@ -52,3 +57,56 @@ func TestValidateTrackedRef(t *testing.T) { } } } + +// TestRecordWorkspacePluginInstall_PrivilegedPluginEntitlement mirrors the +// recordDeclaredPlugin gate in the INSTALL path (workspace_plugins). The +// privileged org-management MCP plugin must only be installable on the +// kind='platform' concierge; any other workspace must be refused before the +// row is written. +func TestRecordWorkspacePluginInstall_PrivilegedPluginEntitlement(t *testing.T) { + const kindQuery = `SELECT COALESCE\(kind, 'workspace'\) FROM workspaces WHERE id =` + const installInsert = `INSERT INTO workspace_plugins` + + t.Run("platform concierge MAY install the privileged management MCP", func(t *testing.T) { + mock := setupTestDB(t) + mock.ExpectQuery(kindQuery).WithArgs("ws-concierge"). + WillReturnRows(sqlmock.NewRows([]string{"kind"}).AddRow("platform")) + mock.ExpectExec(installInsert). + WithArgs("ws-concierge", conciergePlatformMCPPlugin, sqlmock.AnyArg(), sqlmock.AnyArg(), sqlmock.AnyArg()). + WillReturnResult(sqlmock.NewResult(0, 1)) + if err := recordWorkspacePluginInstall(context.Background(), "ws-concierge", conciergePlatformMCPPlugin, "gitea://molecule-ai/molecule-ai-plugin-molecule-platform-mcp", "none", "abc123"); err != nil { + t.Fatalf("platform concierge install of the management MCP must succeed: %v", err) + } + if err := mock.ExpectationsWereMet(); err != nil { + t.Errorf("unmet sqlmock expectations: %v", err) + } + }) + + t.Run("non-platform workspace is REFUSED — no INSERT (privilege-escalation guard)", func(t *testing.T) { + mock := setupTestDB(t) + mock.ExpectQuery(kindQuery).WithArgs("ws-user"). + WillReturnRows(sqlmock.NewRows([]string{"kind"}).AddRow("workspace")) + // NO ExpectExec: the gate MUST refuse before any INSERT fires. + err := recordWorkspacePluginInstall(context.Background(), "ws-user", conciergePlatformMCPPlugin, "gitea://molecule-ai/molecule-ai-plugin-molecule-platform-mcp", "none", "abc123") + if err == nil { + t.Fatal("a non-platform workspace MUST NOT be able to install the privileged management MCP plugin") + } + if err := mock.ExpectationsWereMet(); err != nil { + t.Errorf("unmet sqlmock expectations (an INSERT fired — that is the privilege escalation this gate must stop): %v", err) + } + }) + + t.Run("an ordinary plugin skips the kind precheck entirely (no extra query)", func(t *testing.T) { + mock := setupTestDB(t) + // No kind precheck for non-privileged names — straight to the upsert. + mock.ExpectExec(installInsert). + WithArgs("ws-user", "browser-automation", sqlmock.AnyArg(), sqlmock.AnyArg(), sqlmock.AnyArg()). + WillReturnResult(sqlmock.NewResult(0, 1)) + if err := recordWorkspacePluginInstall(context.Background(), "ws-user", "browser-automation", "gitea://molecule-ai/plugin-browser-automation", "none", "abc123"); err != nil { + t.Fatalf("ordinary plugin install must succeed: %v", err) + } + if err := mock.ExpectationsWereMet(); err != nil { + t.Errorf("unmet sqlmock expectations: %v", err) + } + }) +} -- 2.52.0