From f79c18fcb7784bf07656fdb96cdd0d95b5966e93 Mon Sep 17 00:00:00 2001 From: Andrey Antukh Date: Thu, 30 Jul 2026 08:35:38 +0000 Subject: [PATCH] :bug: Add dedicated RPC methods for plugin registry operations Add `add-profile-plugin` and `remove-profile-plugin` RPC methods for atomic plugin registry operations, preventing manipulation via the broader `update-profile-props` endpoint. - Close the `:plugins` field in `update-profile-props` schema to eliminate the mass assignment attack vector for plugin data. - Define `valid-permissions` and a closed `schema:permissions` enum to restrict plugin permissions to known values. - Migrate the frontend to use the new granular RPC methods with optimistic updates and rollback on failure. - Add comprehensive backend tests covering valid/invalid permissions, updates, removal, and rejection via old endpoint. AI-assisted-by: qwen3.7-plus --- backend/src/app/rpc.clj | 1 + backend/src/app/rpc/commands/plugins.clj | 75 ++++++++ backend/src/app/rpc/commands/profile.clj | 3 +- .../test/backend_tests/rpc_plugins_test.clj | 162 ++++++++++++++++++ common/src/app/common/types/plugins.cljc | 16 +- frontend/src/app/plugins/register.cljs | 35 ++-- 6 files changed, 274 insertions(+), 18 deletions(-) create mode 100644 backend/src/app/rpc/commands/plugins.clj create mode 100644 backend/test/backend_tests/rpc_plugins_test.clj diff --git a/backend/src/app/rpc.clj b/backend/src/app/rpc.clj index c7e33312ca..ed35f51514 100644 --- a/backend/src/app/rpc.clj +++ b/backend/src/app/rpc.clj @@ -388,6 +388,7 @@ 'app.rpc.commands.management 'app.rpc.commands.media 'app.rpc.commands.nitrate + 'app.rpc.commands.plugins 'app.rpc.commands.profile 'app.rpc.commands.projects 'app.rpc.commands.search diff --git a/backend/src/app/rpc/commands/plugins.clj b/backend/src/app/rpc/commands/plugins.clj new file mode 100644 index 0000000000..989f59ef36 --- /dev/null +++ b/backend/src/app/rpc/commands/plugins.clj @@ -0,0 +1,75 @@ +;; This Source Code Form is subject to the terms of the Mozilla Public +;; License, v. 2.0. If a copy of the MPL was not distributed with this +;; file, You can obtain one at http://mozilla.org/MPL/2.0/. +;; +;; Copyright (c) KALEIDOS INC Sucursal en España SL + +(ns app.rpc.commands.plugins + (:require + [app.common.exceptions :as ex] + [app.common.schema :as sm] + [app.common.types.plugins :as ctp] + [app.db :as db] + [app.rpc :as-alias rpc] + [app.rpc.commands.profile :as profile] + [app.rpc.doc :as-alias doc] + [app.util.services :as sv])) + +(defn- validate-plugin-permissions! + "Validates that all permissions in the plugin are within the valid set." + [plugin] + (let [permissions (:permissions plugin) + invalid (remove ctp/valid-permissions permissions)] + (when (seq invalid) + (ex/raise :type :validation + :code :invalid-plugin-permissions + :hint (str "Invalid permissions: " (pr-str (set invalid))) + :invalid-permissions (set invalid))))) + +(def ^:private + schema:add-profile-plugin + [:map {:title "add-profile-plugin"} + [:plugin ctp/schema:registry-entry]]) + +(sv/defmethod ::add-profile-plugin + {::doc/added "2.18" + ::sm/params schema:add-profile-plugin + ::sm/result ctp/schema:registry-entry + ::db/transaction true} + [{:keys [::db/conn] :as cfg} {:keys [::rpc/profile-id plugin]}] + (validate-plugin-permissions! plugin) + + (let [profile (profile/get-profile conn profile-id ::db/for-update true) + plugins (get-in profile [:props :plugins] {:ids [] :data {}}) + plugin-id (:plugin-id plugin) + plugins (-> plugins + (update :ids #(vec (distinct (conj % plugin-id)))) + (assoc-in [:data plugin-id] plugin))] + (db/update! conn :profile + {:props (db/tjson (assoc (:props profile) :plugins plugins))} + {:id profile-id} + {::db/return-keys false}) + plugin)) + +(def ^:private + schema:remove-profile-plugin + [:map {:title "remove-profile-plugin"} + [:plugin-id ::sm/uuid]]) + +(sv/defmethod ::remove-profile-plugin + {::doc/added "2.18" + ::sm/params schema:remove-profile-plugin + ::sm/result :nil + ::db/transaction true} + [{:keys [::db/conn] :as cfg} {:keys [::rpc/profile-id plugin-id]}] + (let [profile (profile/get-profile conn profile-id ::db/for-update true) + plugins (get-in profile [:props :plugins] {:ids [] :data {}}) + plugin-id-str (str plugin-id) + plugins (-> plugins + (update :ids #(vec (remove (partial = plugin-id-str) %))) + (update :data dissoc plugin-id-str))] + (db/update! conn :profile + {:props (db/tjson (assoc (:props profile) :plugins plugins))} + {:id profile-id} + {::db/return-keys false}) + nil)) diff --git a/backend/src/app/rpc/commands/profile.clj b/backend/src/app/rpc/commands/profile.clj index 36d02ba2d9..103962eb24 100644 --- a/backend/src/app/rpc/commands/profile.clj +++ b/backend/src/app/rpc/commands/profile.clj @@ -11,7 +11,6 @@ [app.common.exceptions :as ex] [app.common.schema :as sm] [app.common.time :as ct] - [app.common.types.plugins :refer [schema:plugin-registry]] [app.common.uuid :as uuid] [app.config :as cf] [app.db :as db] @@ -53,7 +52,7 @@ (def system-managed-props "Props keys managed by the system (not user-writable via RPC)." - #{:subscription}) + #{:subscription :plugins}) (def schema:props [:map {:title "ProfileProps" :closed true} diff --git a/backend/test/backend_tests/rpc_plugins_test.clj b/backend/test/backend_tests/rpc_plugins_test.clj new file mode 100644 index 0000000000..9850d90ba9 --- /dev/null +++ b/backend/test/backend_tests/rpc_plugins_test.clj @@ -0,0 +1,162 @@ +;; This Source Code Form is subject to the terms of the Mozilla Public +;; License, v. 2.0. If a copy of the MPL was not distributed with this +;; file, You can obtain one at http://mozilla.org/MPL/2.0/. +;; +;; Copyright (c) KALEIDOS INC Sucursal en España SL + +(ns backend-tests.rpc-plugins-test + (:require + [app.common.uuid :as uuid] + [app.rpc :as-alias rpc] + [app.rpc.commands.profile :as profile] + [backend-tests.helpers :as th] + [clojure.test :as t])) + +(t/use-fixtures :once th/state-init) +(t/use-fixtures :each th/database-reset) + +(def ^:private plugin-id-1 (str (uuid/next))) +(def ^:private plugin-id-2 (str (uuid/next))) + +(def ^:private valid-plugin + {:plugin-id plugin-id-1 + :name "Test Plugin" + :description "A test plugin" + :host "https://example.com" + :code "(function() { console.log('hello'); })()" + :icon "icon.svg" + :permissions #{"content:read" "content:write"}}) + +(t/deftest add-profile-plugin-accepts-valid-permissions + (let [profile (th/create-profile* 1) + data {::th/type :add-profile-plugin + ::rpc/profile-id (:id profile) + :plugin valid-plugin} + out (th/command! data)] + + (t/is (nil? (:error out))) + (t/is (some? (:result out))) + + (let [saved (th/db-get :profile {:id (:id profile)}) + props (profile/decode-row saved) + plugins (get-in props [:props :plugins])] + (t/is (= [plugin-id-1] (:ids plugins))) + (t/is (= valid-plugin (get-in plugins [:data plugin-id-1])))))) + +(t/deftest add-profile-plugin-rejects-invalid-permissions + (let [profile (th/create-profile* 1) + plugin (assoc valid-plugin :permissions #{"content:read" "admin:delete"}) + data {::th/type :add-profile-plugin + ::rpc/profile-id (:id profile) + :plugin plugin} + out (th/command! data)] + + ;; Schema validation catches invalid permissions before custom validation + (t/is (th/ex-info? (:error out))) + (t/is (th/ex-of-type? (:error out) :validation)) + (t/is (th/ex-of-code? (:error out) :params-validation)) + + (let [saved (th/db-get :profile {:id (:id profile)}) + props (profile/decode-row saved) + plugins (get-in props [:props :plugins])] + (t/is (nil? plugins) "No plugins should be persisted when validation fails")))) + +(t/deftest add-profile-plugin-updates-existing-plugin + (let [profile (th/create-profile* 1) + data1 {::th/type :add-profile-plugin + ::rpc/profile-id (:id profile) + :plugin valid-plugin} + _ (th/command! data1) + + updated-plugin (assoc valid-plugin :name "Updated Plugin") + data2 {::th/type :add-profile-plugin + ::rpc/profile-id (:id profile) + :plugin updated-plugin} + out (th/command! data2)] + + (t/is (nil? (:error out))) + + (let [saved (th/db-get :profile {:id (:id profile)}) + props (profile/decode-row saved) + plugins (get-in props [:props :plugins])] + (t/is (= 1 (count (:ids plugins))) "Should still have only one plugin") + (t/is (= "Updated Plugin" (get-in plugins [:data plugin-id-1 :name])))))) + +(t/deftest remove-profile-plugin-removes-plugin + (let [profile (th/create-profile* 1) + data1 {::th/type :add-profile-plugin + ::rpc/profile-id (:id profile) + :plugin valid-plugin} + _ (th/command! data1) + + data2 {::th/type :remove-profile-plugin + ::rpc/profile-id (:id profile) + :plugin-id (uuid/uuid plugin-id-1)} + out (th/command! data2)] + + (t/is (nil? (:error out))) + + (let [saved (th/db-get :profile {:id (:id profile)}) + props (profile/decode-row saved) + plugins (get-in props [:props :plugins])] + (t/is (= [] (:ids plugins))) + (t/is (empty? (:data plugins)))))) + +(t/deftest remove-profile-plugin-handles-nonexistent-plugin + (let [profile (th/create-profile* 1) + data {::th/type :remove-profile-plugin + ::rpc/profile-id (:id profile) + :plugin-id (uuid/next)} + out (th/command! data)] + + (t/is (nil? (:error out))) + + (let [saved (th/db-get :profile {:id (:id profile)}) + props (profile/decode-row saved) + plugins (get-in props [:props :plugins])] + (t/is (or (nil? plugins) + (and (empty? (:ids plugins)) + (empty? (:data plugins)))) + "Plugins should be nil or empty when no plugins exist")))) + +(t/deftest add-profile-plugin-multiple-plugins + (let [profile (th/create-profile* 1) + plugin1 valid-plugin + plugin2 (assoc valid-plugin + :plugin-id plugin-id-2 + :name "Second Plugin") + + data1 {::th/type :add-profile-plugin + ::rpc/profile-id (:id profile) + :plugin plugin1} + _ (th/command! data1) + + data2 {::th/type :add-profile-plugin + ::rpc/profile-id (:id profile) + :plugin plugin2} + _ (th/command! data2)] + + (let [saved (th/db-get :profile {:id (:id profile)}) + props (profile/decode-row saved) + plugins (get-in props [:props :plugins])] + (t/is (= 2 (count (:ids plugins)))) + (t/is (contains? (set (:ids plugins)) plugin-id-1)) + (t/is (contains? (set (:ids plugins)) plugin-id-2)) + (t/is (= "Test Plugin" (get-in plugins [:data plugin-id-1 :name]))) + (t/is (= "Second Plugin" (get-in plugins [:data plugin-id-2 :name])))))) + +(t/deftest update-profile-props-rejects-plugins + (let [profile (th/create-profile* 1) + data {::th/type :update-profile-props + ::rpc/profile-id (:id profile) + :props {:plugins {:ids ["test"] :data {"test" valid-plugin}}}} + out (th/command! data)] + + (t/is (th/ex-info? (:error out))) + (t/is (th/ex-of-type? (:error out) :validation)) + (t/is (th/ex-of-code? (:error out) :params-validation)) + + (let [saved (th/db-get :profile {:id (:id profile)}) + props (profile/decode-row saved)] + (t/is (nil? (get-in props [:props :plugins])) + ":plugins must not be writable via update-profile-props")))) diff --git a/common/src/app/common/types/plugins.cljc b/common/src/app/common/types/plugins.cljc index 7fe8a4c7d4..f87b4d8452 100644 --- a/common/src/app/common/types/plugins.cljc +++ b/common/src/app/common/types/plugins.cljc @@ -27,6 +27,20 @@ schema:string schema:string]]) +(def valid-permissions + "Set of valid plugin permissions that can be granted to plugins." + #{"content:read" "content:write" + "library:read" "library:write" + "comment:read" "comment:write" + "clipboard:read" "clipboard:write" + "user:read" + "allow:downloads" + "allow:localstorage"}) + +(def schema:permissions + "Schema for plugin permissions - a set of valid permission strings." + [:set {:gen/max 11} (into [:enum] (sort valid-permissions))]) + (def schema:registry-entry [:map [:plugin-id :string] @@ -36,7 +50,7 @@ [:host :string] [:code :string] [:icon {:optional true} :string] - [:permissions [:set :string]]]) + [:permissions schema:permissions]]) (def schema:plugin-registry [:map diff --git a/frontend/src/app/plugins/register.cljs b/frontend/src/app/plugins/register.cljs index 9180463c69..6562238cf1 100644 --- a/frontend/src/app/plugins/register.cljs +++ b/frontend/src/app/plugins/register.cljs @@ -112,13 +112,6 @@ manifest (.error js/console (clj->js (sm/explain ctp/schema:registry-entry manifest)))))) -(defn save-to-store - [] - ;; TODO: need this for the transition to the new schema. We can remove eventually - (let [registry (update @registry :data d/update-vals d/without-nils)] - (->> (rp/cmd! :update-profile-props {:props {:plugins registry}}) - (rx/subs! identity)))) - (defn load-from-store [] (reset! registry (get-in @st/state [:profile :props :plugins] {}))) @@ -127,6 +120,8 @@ [] (load-from-store)) +(declare remove-plugin!) + (defn install-plugin! [plugin] (letfn [(update-ids [ids] @@ -136,17 +131,27 @@ (swap! registry #(-> % (update :ids update-ids) (update :data assoc (:plugin-id plugin) plugin))) - (save-to-store))) + (->> (rp/cmd! :add-profile-plugin {:plugin plugin}) + (rx/subs! identity + (fn [err] + (remove-plugin! plugin) + (.error js/console "Failed to install plugin:" err)))))) (defn remove-plugin! [{:keys [plugin-id]}] - (letfn [(update-ids [ids] - (->> ids - (remove #(= % plugin-id))))] - (swap! registry #(-> % - (update :ids update-ids) - (update :data dissoc plugin-id))) - (save-to-store))) + (let [plugin (get-plugin plugin-id)] + (letfn [(update-ids [ids] + (->> ids + (remove #(= % plugin-id))))] + (swap! registry #(-> % + (update :ids update-ids) + (update :data dissoc plugin-id))) + (->> (rp/cmd! :remove-profile-plugin {:plugin-id plugin-id}) + (rx/subs! identity + (fn [err] + (when plugin + (install-plugin! plugin)) + (.error js/console "Failed to remove plugin:" err))))))) (defn check-permission [plugin-id permission]