✨ Restrict optional RPC ids to user-provided UUIDs (#11777)

* ✨ Restrict optional RPC ids to user-provided UUIDs Add ::sm/user-provided-uuid, backed by a version and variant aware regex that only accepts v4, v7 and v8 instances. Use it for the optional :id of the creation RPC commands so reserved versions such as v3 are rejected at validation time. Reads such as get-team keep the lax ::sm/uuid. Cover the predicate and the schema on both JVM and JS runtimes. AI-assisted-by: muse-spark-1.3-contributor * ✨ Cover id version restriction at the RPC boundary Add backend regression tests proving the seven creation commands reject reserved-version ids (v3) with :params-validation and accept v4 ids (plus v7/v8 on create-team) through the real decode and validate path. Also drop two duplicated assertions and document the version and variant of every fixture UUID in user-provided-test. AI-assisted-by: muse-spark-1.3-contributor
2026-09-23 20:36:15 +00:00 · 2026-09-22 09:45:50 +02:00 · 2026-09-22 09:45:50 +02:00 · e05747b546
commit e05747b546
parent d8fddf9942
13 changed files with 332 additions and 7 deletions
--- a/backend/src/app/rpc/commands/files_create.clj
+++ b/backend/src/app/rpc/commands/files_create.clj
@ -75,7 +75,7 @@
  [:map {:title "create-file"}
   [:name [:string {:max 250}]]
   [:project-id ::sm/uuid]
-   [:id {:optional true} ::sm/uuid]
+   [:id {:optional true} ::sm/user-provided-uuid]
   [:is-shared {:optional true} ::sm/boolean]
   [:features {:optional true} ::cfeat/features]])

--- a/backend/src/app/rpc/commands/media.clj
+++ b/backend/src/app/rpc/commands/media.clj
@ -48,7 +48,7 @@

 (def ^:private schema:upload-file-media-object
  [:map {:title "upload-file-media-object"}
-   [:id {:optional true} ::sm/uuid]
+   [:id {:optional true} ::sm/user-provided-uuid]
   [:file-id ::sm/uuid]
   [:is-local ::sm/boolean]
   [:name [:string {:max 250}]]
@ -225,7 +225,7 @@
   [:file-id ::sm/uuid]
   [:is-local ::sm/boolean]
   [:url ::sm/uri]
-   [:id {:optional true} ::sm/uuid]
+   [:id {:optional true} ::sm/user-provided-uuid]
   [:name {:optional true} [:string {:max 250}]]])

 (sv/defmethod ::create-file-media-object-from-url
@ -489,7 +489,7 @@
   [:is-local   ::sm/boolean]
   [:name       [:string {:max 250}]]
   [:mtype      :string]
-   [:id         {:optional true} ::sm/uuid]])
+   [:id         {:optional true} ::sm/user-provided-uuid]])

 (sv/defmethod ::assemble-file-media-object
  {::doc/added "2.17"
--- a/backend/src/app/rpc/commands/projects.clj
+++ b/backend/src/app/rpc/commands/projects.clj
@ -199,7 +199,7 @@
  [:map {:title "create-project"}
   [:team-id ::sm/uuid]
   [:name [:string {:max 250 :min 1}]]
-   [:id {:optional true} ::sm/uuid]])
+   [:id {:optional true} ::sm/user-provided-uuid]])

 (sv/defmethod ::create-project
  {::doc/added "1.18"
--- a/backend/src/app/rpc/commands/teams.clj
+++ b/backend/src/app/rpc/commands/teams.clj
@ -523,7 +523,7 @@
  [:map {:title "create-team"}
   [:name types.team/schema:team-name]
   [:features {:optional true} ::cfeat/features]
-   [:id {:optional true} ::sm/uuid]
+   [:id {:optional true} ::sm/user-provided-uuid]
   [:organization-id {:optional true} ::sm/uuid]
   [:is-default {:optional true} :boolean]])

--- a/backend/src/app/rpc/commands/teams_invitations.clj
+++ b/backend/src/app/rpc/commands/teams_invitations.clj
@ -538,7 +538,7 @@
  [:map {:title "create-team-with-invitations"}
   [:name [:string {:max 250}]]
   [:features {:optional true} ::cfeat/features]
-   [:id {:optional true} ::sm/uuid]
+   [:id {:optional true} ::sm/user-provided-uuid]
   [:emails [::sm/set ::sm/email]]
   [:role types.team/schema:role]])

--- a/backend/test/backend_tests/rpc_file_test.clj
+++ b/backend/test/backend_tests/rpc_file_test.clj
@ -166,6 +166,33 @@
        (t/is (th/ex-info? err))
        (t/is (th/ex-of-type? err :not-found))))))

+(t/deftest create-file-id-version
+  (let [prof    (th/create-profile* 1 {:is-active true})
+        proj-id (:default-project-id prof)
+        v3-id   "6fa459ea-ee8a-3ca4-894e-db77e160355e"
+        v4-id   "550e8400-e29b-41d4-a716-446655440000"]
+
+    ;; reserved version (v3) must be rejected at the RPC boundary
+    (let [data {::th/type :create-file
+                ::rpc/profile-id (:id prof)
+                :project-id proj-id
+                :id v3-id
+                :name "file with v3 id"}
+          out  (th/command! data)]
+      (t/is (not (th/success? out)))
+      (t/is (th/ex-of-type? (:error out) :validation))
+      (t/is (th/ex-of-code? (:error out) :params-validation)))
+
+    ;; v4 id is accepted
+    (let [data {::th/type :create-file
+                ::rpc/profile-id (:id prof)
+                :project-id proj-id
+                :id v4-id
+                :name "file with v4 id"}
+          out  (th/command! data)]
+      (t/is (th/success? out))
+      (t/is (= v4-id (str (:id (:result out))))))))
+
 (t/deftest file-gc-with-fragments
  (let [profile (th/create-profile* 1)
        file    (th/create-file* 1 {:profile-id (:id profile)
--- a/backend/test/backend_tests/rpc_media_test.clj
+++ b/backend/test/backend_tests/rpc_media_test.clj
@ -137,6 +137,66 @@
      (t/is (uuid? (:thumbnail-id result))))))


+(t/deftest upload-file-media-object-id-version
+  (let [prof   (th/create-profile* 1)
+        _      (th/create-project* 1 {:profile-id (:id prof)
+                                      :team-id (:default-team-id prof)})
+        file   (th/create-file* 1 {:profile-id (:id prof)
+                                   :project-id (:default-project-id prof)
+                                   :is-shared false})
+        mfile  {:filename "sample.jpg"
+                :path (th/tempfile "backend_tests/test_files/sample.jpg")
+                :mtype "image/jpeg"
+                :size 312043}
+        v3-id  "6fa459ea-ee8a-3ca4-894e-db77e160355e"
+        v4-id  "550e8400-e29b-41d4-a716-446655440000"]
+
+    ;; reserved version (v3) must be rejected at the RPC boundary
+    (let [params {::th/type :upload-file-media-object
+                  ::rpc/profile-id (:id prof)
+                  :file-id (:id file)
+                  :is-local true
+                  :name "testfile"
+                  :content mfile
+                  :id v3-id}
+          out    (th/command! params)]
+      (t/is (not (th/success? out)))
+      (t/is (th/ex-of-type? (:error out) :validation))
+      (t/is (th/ex-of-code? (:error out) :params-validation)))
+
+    ;; v4 id is accepted
+    (let [params {::th/type :upload-file-media-object
+                  ::rpc/profile-id (:id prof)
+                  :file-id (:id file)
+                  :is-local true
+                  :name "testfile"
+                  :content mfile
+                  :id v4-id}
+          out    (th/command! params)]
+      (t/is (th/success? out))
+      (t/is (= v4-id (str (:id (:result out))))))))
+
+(t/deftest create-file-media-object-from-url-id-version
+  (let [prof   (th/create-profile* 1)
+        _      (th/create-project* 1 {:profile-id (:id prof)
+                                      :team-id (:default-team-id prof)})
+        file   (th/create-file* 1 {:profile-id (:id prof)
+                                   :project-id (:default-project-id prof)
+                                   :is-shared false})
+        v3-id  "6fa459ea-ee8a-3ca4-894e-db77e160355e"]
+
+    ;; reserved version (v3) must be rejected before any download happens
+    (let [params {::th/type :create-file-media-object-from-url
+                  ::rpc/profile-id (:id prof)
+                  :file-id (:id file)
+                  :is-local true
+                  :url "https://example.com/sample.jpg"
+                  :id v3-id}
+          out    (th/command! params)]
+      (t/is (not (th/success? out)))
+      (t/is (th/ex-of-type? (:error out) :validation))
+      (t/is (th/ex-of-code? (:error out) :params-validation)))))
+
 (t/deftest media-object-from-url-command
  (let [prof   (th/create-profile* 1)
        proj   (th/create-project* 1 {:profile-id (:id prof)
@ -1078,3 +1138,48 @@
      (t/is (th/ex-info? error))
      (t/is (= :not-found (:type error-data)))
      (t/is (= :object-not-found (:code error-data))))))
+
+(t/deftest assemble-file-media-object-id-version
+  (let [prof   (th/create-profile* 1)
+        _      (th/create-project* 1 {:profile-id (:id prof)
+                                      :team-id (:default-team-id prof)})
+        file   (th/create-file* 1 {:profile-id (:id prof)
+                                   :project-id (:default-project-id prof)
+                                   :is-shared false})
+        v3-id  "6fa459ea-ee8a-3ca4-894e-db77e160355e"
+        v4-id  "550e8400-e29b-41d4-a716-446655440000"
+        mtype  "image/jpeg"]
+
+    ;; reserved version (v3) must be rejected without touching any session
+    (let [out (th/command! {::th/type :assemble-file-media-object
+                            ::rpc/profile-id (:id prof)
+                            :session-id (uuid/next)
+                            :file-id (:id file)
+                            :is-local true
+                            :name "assembled-image"
+                            :mtype mtype
+                            :id v3-id})]
+      (t/is (not (th/success? out)))
+      (t/is (th/ex-of-type? (:error out) :validation))
+      (t/is (th/ex-of-code? (:error out) :params-validation)))
+
+    ;; v4 id is accepted through the full chunked flow
+    (let [source-path (th/tempfile "backend_tests/test_files/sample.jpg")
+          chunks      (split-file-into-chunks source-path 312043)
+          session-id  (create-session! prof 1)
+          mfile       (make-chunk-mfile (first chunks) mtype)
+          _           (th/command! {::th/type :upload-chunk
+                                    ::rpc/profile-id (:id prof)
+                                    :session-id session-id
+                                    :index 0
+                                    :content mfile})
+          out         (th/command! {::th/type :assemble-file-media-object
+                                    ::rpc/profile-id (:id prof)
+                                    :session-id session-id
+                                    :file-id (:id file)
+                                    :is-local true
+                                    :name "assembled-image"
+                                    :mtype mtype
+                                    :id v4-id})]
+      (t/is (th/success? out))
+      (t/is (= v4-id (str (:id (:result out))))))))
--- a/backend/test/backend_tests/rpc_project_test.clj
+++ b/backend/test/backend_tests/rpc_project_test.clj
@ -123,6 +123,33 @@
    (t/is (th/ex-info? error))
    (t/is (th/ex-of-type? error :not-found))))

+(t/deftest create-project-id-version
+  (let [profile (th/create-profile* 1)
+        team    (th/create-team* 1 {:profile-id (:id profile)})
+        v3-id   "6fa459ea-ee8a-3ca4-894e-db77e160355e"
+        v4-id   "550e8400-e29b-41d4-a716-446655440000"]
+
+    ;; reserved version (v3) must be rejected at the RPC boundary
+    (let [data {::th/type :create-project
+                ::rpc/profile-id (:id profile)
+                :team-id (:id team)
+                :id v3-id
+                :name "project with v3 id"}
+          out  (th/command! data)]
+      (t/is (not (th/success? out)))
+      (t/is (th/ex-of-type? (:error out) :validation))
+      (t/is (th/ex-of-code? (:error out) :params-validation)))
+
+    ;; v4 id is accepted
+    (let [data {::th/type :create-project
+                ::rpc/profile-id (:id profile)
+                :team-id (:id team)
+                :id v4-id
+                :name "project with v4 id"}
+          out  (th/command! data)]
+      (t/is (th/success? out))
+      (t/is (= v4-id (str (:id (:result out))))))))
+
 (t/deftest permissions-checks-rename-project
  (let [profile1 (th/create-profile* 1)
        profile2 (th/create-profile* 2)
--- a/backend/test/backend_tests/rpc_team_test.clj
+++ b/backend/test/backend_tests/rpc_team_test.clj
@ -1116,6 +1116,62 @@
          out  (th/command! data)]
      (t/is (th/success? out)))))

+(t/deftest create-team-id-version
+  (let [profile (th/create-profile* 1 {:is-active true})
+        v3-id   "6fa459ea-ee8a-3ca4-894e-db77e160355e"
+        v4-id   "550e8400-e29b-41d4-a716-446655440000"
+        v7-id   "0191062e-3f50-7a5e-9f5a-1a2b3c4d5e6f"]
+
+    ;; reserved version (v3) must be rejected at the RPC boundary
+    (let [data {::th/type :create-team
+                ::rpc/profile-id (:id profile)
+                :name "team with v3 id"
+                :id v3-id}
+          out  (th/command! data)]
+      (t/is (not (th/success? out)))
+      (t/is (th/ex-of-type? (:error out) :validation))
+      (t/is (th/ex-of-code? (:error out) :params-validation)))
+
+    ;; v4, v7 and v8 ids are accepted
+    (doseq [id [v4-id v7-id (str (uuid/next))]]
+      (let [data {::th/type :create-team
+                  ::rpc/profile-id (:id profile)
+                  :name (str "team with id " id)
+                  :id id}
+            out  (th/command! data)]
+        (t/is (th/success? out))
+        (t/is (= id (str (:id (:result out)))))))))
+
+(t/deftest create-team-with-invitations-id-version
+  (with-mocks [mock {:target 'app.email/send! :return nil}]
+    (let [profile (th/create-profile* 1 {:is-active true})
+          v3-id   "6fa459ea-ee8a-3ca4-894e-db77e160355e"
+          v4-id   "550e8400-e29b-41d4-a716-446655440000"]
+
+      ;; reserved version (v3) must be rejected before any invitation is sent
+      (let [data {::th/type :create-team-with-invitations
+                  ::rpc/profile-id (:id profile)
+                  :name "team with v3 id"
+                  :id v3-id
+                  :emails #{"invitee@example.com"}
+                  :role :editor}
+            out  (th/command! data)]
+        (t/is (not (th/success? out)))
+        (t/is (th/ex-of-type? (:error out) :validation))
+        (t/is (th/ex-of-code? (:error out) :params-validation))
+        (t/is (= 0 (:call-count @mock))))
+
+      ;; v4 id is accepted
+      (let [data {::th/type :create-team-with-invitations
+                  ::rpc/profile-id (:id profile)
+                  :name "team with v4 id"
+                  :id v4-id
+                  :emails #{"invitee@example.com"}
+                  :role :editor}
+            out  (th/command! data)]
+        (t/is (th/success? out))
+        (t/is (= v4-id (str (:id (:result out)))))))))
+
 (t/deftest create-team-invitations-email-cooldown
  (with-mocks [mock {:target 'app.email/send! :return nil}]
    (let [profile1 (th/create-profile* 1 {:is-active true})
--- a/common/src/app/common/schema.cljc
+++ b/common/src/app/common/schema.cljc
@ -448,6 +448,21 @@
   ::oapi/type "string"
   ::oapi/format "uuid"}})

+(register!
+ {:type ::user-provided-uuid
+  :pred uuid/user-provided?
+  :type-properties
+  {:title "user-provided-uuid"
+   :description "UUID provided by the user (v4, v7 or v8)"
+   :error/message "should be a user provided uuid (v4, v7 or v8)"
+   :gen/gen (sg/uuid)
+   :decode/string parse-uuid
+   :decode/json parse-uuid
+   :encode/string encode-uuid
+   :encode/json encode-uuid
+   ::oapi/type "string"
+   ::oapi/format "uuid"}})
+
 ;; Strict email regex aligned with app.common.spec/email-re.
 ;; Local part: valid RFC chars, no leading/trailing dot, no consecutive dots.
 ;; Domain: labels can't start/end with hyphen, no empty labels.
--- a/common/src/app/common/uuid.cljc
+++ b/common/src/app/common/uuid.cljc
@ -70,6 +70,20 @@
  [v]
  (= zero v))

+(def ^:private user-provided-regex
+  #"(?i)^[0-9a-f]{8}-[0-9a-f]{4}-[478][0-9a-f]{3}-[89ab][0-9a-f]{3}-[0-9a-f]{12}$")
+
+(defn user-provided?
+  "Returns true when `v` is a UUID instance penpot can accept when
+  it is provided by the user (v4 random, v7 time-ordered or v8 penpot
+  time-ordered). Anything else, including UUID strings, returns false;
+  strings are meant to be decoded into UUID instances first, mirroring
+  how ::sm/uuid behaves."
+  [v]
+  (boolean
+   (and (uuid? v)
+        (re-matches user-provided-regex (str v)))))
+
 (defn get-word-high
  [id]
  #?(:clj (.getMostSignificantBits ^UUID id)
--- a/common/test/common_tests/schema_test.cljc
+++ b/common/test/common_tests/schema_test.cljc
@ -9,6 +9,7 @@
   [app.common.data :as d]
   [app.common.schema :as sm]
   [app.common.schema.generators :as sg]
+   [app.common.uuid :as uuid]
   [clojure.test :as t]))

 (t/deftest test-set-of-email
@ -245,3 +246,40 @@
    (t/is (false? (sm/validate ::sm/email "user@")))
    (t/is (false? (sm/validate ::sm/email "userdomain.com")))
    (t/is (false? (sm/validate ::sm/email "user@@domain.com")))))
+
+(t/deftest test-user-provided-uuid
+  (let [v4 (uuid/uuid "550e8400-e29b-41d4-a716-446655440000")
+        v7 (uuid/uuid "0191062e-3f50-7a5e-9f5a-1a2b3c4d5e6f")
+        v8 (uuid/uuid "0227df82-63d7-8016-8005-48d9c0f33011")
+        v1 (uuid/uuid "6ba7b810-9dad-11d1-80b4-00c04fd430c8")
+        v3 (uuid/uuid "6fa459ea-ee8a-3ca4-894e-db77e160355e")
+        v5 (uuid/uuid "886313e1-3b8a-5372-9b90-0c9aee199e5d")]
+
+    (t/testing "accepts v4, v7 and v8 instances"
+      (t/is (true? (sm/validate ::sm/user-provided-uuid v4)))
+      (t/is (true? (sm/validate ::sm/user-provided-uuid v7)))
+      (t/is (true? (sm/validate ::sm/user-provided-uuid v8)))
+      (t/is (true? (sm/validate ::sm/user-provided-uuid (uuid/random))))
+      (t/is (true? (sm/validate ::sm/user-provided-uuid (uuid/next)))))
+
+    (t/testing "rejects reserved and other versions"
+      (t/is (false? (sm/validate ::sm/user-provided-uuid v1)))
+      (t/is (false? (sm/validate ::sm/user-provided-uuid v3)))
+      (t/is (false? (sm/validate ::sm/user-provided-uuid v5)))
+      (t/is (false? (sm/validate ::sm/user-provided-uuid uuid/zero)))
+      (t/is (false? (sm/validate ::sm/user-provided-uuid nil)))
+      (t/is (false? (sm/validate ::sm/user-provided-uuid "not-an-uuid"))))
+
+    (t/testing "decodes strings like the RPC layer does"
+      (let [decode (sm/decoder ::sm/user-provided-uuid sm/json-transformer)]
+        (t/is (= v4 (decode (str v4))))
+        (t/is (= v3 (decode (str v3))))
+        (t/is (true? (sm/validate ::sm/user-provided-uuid (decode (str v4)))))
+        (t/is (false? (sm/validate ::sm/user-provided-uuid (decode (str v3)))))))
+
+    (t/testing "rejects raw strings without decoding, like ::sm/uuid does"
+      (t/is (false? (sm/validate ::sm/user-provided-uuid (str v4))))
+      (t/is (false? (sm/validate ::sm/user-provided-uuid (str v3)))))
+
+    (t/testing "generate"
+      (t/is (true? (sm/validate ::sm/user-provided-uuid (sg/generate ::sm/user-provided-uuid)))))))
--- a/common/test/common_tests/uuid_test.cljc
+++ b/common/test/common_tests/uuid_test.cljc
@ -95,3 +95,46 @@
           (t/is (= (nth expected 1) (aget parts 1)))
           (t/is (= (nth expected 2) (aget parts 2)))
           (t/is (= (nth expected 3) (aget parts 3))))))))
+
+(t/deftest user-provided-test
+  ;; The version is the first nibble of the 3rd group, the variant is
+  ;; the first nibble of the 4th group (8, 9, a or b means RFC 4122).
+  (let [v4              (uuid/uuid "550e8400-e29b-41d4-a716-446655440000") ; v4, variant a: valid
+        v7              (uuid/uuid "0191062e-3f50-7a5e-9f5a-1a2b3c4d5e6f") ; v7, variant 9: valid
+        v8              (uuid/uuid "0227df82-63d7-8016-8005-48d9c0f33011") ; v8, variant 8: valid
+        v4-upper        (uuid/uuid "550E8400-E29B-41D4-A716-446655440000") ; v4 uppercase, variant A: valid
+        v1              (uuid/uuid "6ba7b810-9dad-11d1-80b4-00c04fd430c8") ; v1 time-based: rejected
+        v3              (uuid/uuid "6fa459ea-ee8a-3ca4-894e-db77e160355e") ; v3 MD5 name-based (reserved): rejected
+        v5              (uuid/uuid "886313e1-3b8a-5372-9b90-0c9aee199e5d") ; v5 SHA-1 name-based: rejected
+        v6              (uuid/uuid "1e29effc-7a68-6d2c-9b6f-7f6a5f9b5b5b") ; v6 time-ordered: rejected
+        v4-bad-variant (uuid/uuid "550e8400-e29b-41d4-0716-446655440000")] ; v4 but variant 0 (NCS reserved): rejected
+
+    (t/testing "accepts v4, v7 and v8 instances"
+      (t/is (true? (uuid/user-provided? v4)))
+      (t/is (true? (uuid/user-provided? v7)))
+      (t/is (true? (uuid/user-provided? v8)))
+      (t/is (true? (uuid/user-provided? v4-upper)))
+      (t/is (true? (uuid/user-provided? (uuid/random)))) ; generates v4
+      (t/is (true? (uuid/user-provided? (uuid/next))))) ; generates v8
+
+    (t/testing "rejects strings, they must be decoded first"
+      (t/is (false? (uuid/user-provided? (str v4))))
+      (t/is (false? (uuid/user-provided? (str v7))))
+      (t/is (false? (uuid/user-provided? (str v8))))
+      (t/is (false? (uuid/user-provided? (str v3)))))
+
+    (t/testing "rejects nil uuid and reserved and other versions"
+      (t/is (false? (uuid/user-provided? uuid/zero))) ; version 0, variant 0
+      (t/is (false? (uuid/user-provided? v1)))
+      (t/is (false? (uuid/user-provided? v3)))
+      (t/is (false? (uuid/user-provided? v5)))
+      (t/is (false? (uuid/user-provided? v6)))
+      (t/is (false? (uuid/user-provided? (uuid/custom 1))))) ; version 0, variant 0
+
+    (t/testing "rejects right version with wrong variant"
+      (t/is (false? (uuid/user-provided? v4-bad-variant))))
+
+    (t/testing "rejects non-uuid values"
+      (t/is (false? (uuid/user-provided? nil)))
+      (t/is (false? (uuid/user-provided? 42)))
+      (t/is (false? (uuid/user-provided? "not-an-uuid"))))))