mirror of
https://github.com/penpot/penpot.git
synced 2026-08-07 05:18:36 +00:00
🐛 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
This commit is contained in:
parent
495e9f059e
commit
f79c18fcb7
@ -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
|
||||
|
||||
75
backend/src/app/rpc/commands/plugins.clj
Normal file
75
backend/src/app/rpc/commands/plugins.clj
Normal file
@ -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))
|
||||
@ -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}
|
||||
|
||||
162
backend/test/backend_tests/rpc_plugins_test.clj
Normal file
162
backend/test/backend_tests/rpc_plugins_test.clj
Normal file
@ -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"))))
|
||||
@ -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
|
||||
|
||||
@ -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]
|
||||
|
||||
Loading…
x
Reference in New Issue
Block a user