✨ Add protection for version inconsistency on opening or editing file

backend/src/app/rpc/commands/files.clj

@@ -11,7 +11,7 @@
    [app.common.exceptions :as ex]
    [app.common.features :as cfeat]
    [app.common.files.helpers :as cfh]
-   [app.common.files.migrations :as pmg]
+   [app.common.files.migrations :as fmg]
    [app.common.schema :as sm]
    [app.common.schema.desc-js-like :as-alias smdj]
    [app.common.spec :as us]
@@ -68,6 +68,22 @@
       changes  (assoc :changes (blob/decode changes))
       data     (assoc :data (blob/decode data)))))
 
+(defn check-version!
+  [{:keys [data] :as file}]
+  (dm/assert!
+   "expect data to be decoded"
+   (map? data))
+
+  (let [version (:version data 0)]
+    (when (> version fmg/version)
+      (ex/raise :type :restriction
+                :code :file-version-not-supported
+                :hint "file version is greated that the maximum"
+                :file-version version
+                :max-version fmg/version))
+
+    file))
+
 ;; --- FILE PERMISSIONS
 
 (def ^:private sql:file-permissions
@@ -283,7 +299,7 @@
 
            file   (-> (db/get conn :file params)
                       (decode-row)
-                      (pmg/migrate-file))]
+                      (fmg/migrate-file))]
 
        ;; NOTE: when file is migrated, we break the rule of no perform
        ;; mutations on get operations and update the file with all
@@ -292,7 +308,7 @@
        ;; NOTE: the following code will not work on read-only mode, it
        ;; is a known issue; we keep is not implemented until we really
        ;; need this
-       (if (pmg/migrated? file)
+       (if (fmg/migrated? file)
          (let [file     (update file :features cfeat/migrate-legacy-features)
                features (set/union (deref cfeat/*new*) (:features file))]
            (db/update! conn :file
@@ -328,7 +344,8 @@
                                                  :file-id id)
 
                             file (-> (get-file conn id project-id)
-                                     (assoc :permissions perms))
+                                     (assoc :permissions perms)
+                                     (check-version!))
 
                             _    (-> (cfeat/get-team-enabled-features cf/flags team)
                                      (cfeat/check-client-features! (:features params))
@@ -824,7 +841,7 @@
   into the file local libraries"
   [conn {:keys [id] :as library}]
   (let [ldata (binding [pmap/*load-fn* (partial load-pointer conn id)]
-                (-> library decode-row (process-pointers deref) pmg/migrate-file :data))
+                (-> library decode-row (process-pointers deref) fmg/migrate-file :data))
         rows  (db/exec! conn [sql:get-referenced-files id])]
     (doseq [file-id (map :id rows)]
       (binding [pmap/*load-fn* (partial load-pointer conn file-id)
@@ -834,7 +851,7 @@
                                      ::db/remove-deleted? false)
                        (decode-row)
                        (load-all-pointers!)
-                       (pmg/migrate-file))
+                       (fmg/migrate-file))
               data (ctf/absorb-assets (:data file) ldata)]
           (db/update! conn :file
                       {:revn (inc (:revn file))

backend/src/app/rpc/commands/files_update.clj

@@ -10,7 +10,7 @@
    [app.common.exceptions :as ex]
    [app.common.features :as cfeat]
    [app.common.files.changes :as cpc]
-   [app.common.files.migrations :as pmg]
+   [app.common.files.migrations :as fmg]
    [app.common.files.validate :as val]
    [app.common.logging :as l]
    [app.common.schema :as sm]
@@ -39,34 +39,32 @@
 
 ;; --- SCHEMA
 
-(def ^:private schema:changes
+(def ^:private
-  [:vector ::cpc/change])
+  schema:update-file
+  (sm/define
+    [:map {:title "update-file"}
+     [:id ::sm/uuid]
+     [:session-id ::sm/uuid]
+     [:revn {:min 0} :int]
+     [:features {:optional true} ::cfeat/features]
+     [:changes {:optional true} [:vector ::cpc/change]]
+     [:changes-with-metadata {:optional true}
+      [:vector [:map
+                [:changes [:vector ::cpc/change]]
+                [:hint-origin {:optional true} :keyword]
+                [:hint-events {:optional true} [:vector :string]]]]]
+     [:skip-validate {:optional true} :boolean]]))
 
-(def ^:private schema:change-with-metadata
+(def ^:private
-  [:map {:title "ChangeWithMetadata"}
+  schema:update-file-result
-   [:changes schema:changes]
+  (sm/define
-   [:hint-origin {:optional true} :keyword]
+    [:vector {:title "update-file-result"}
-   [:hint-events {:optional true} [:vector :string]]])
+     [:map
-
+      [:changes [:vector ::cpc/change]]
-(def ^:private schema:update-file
+      [:file-id ::sm/uuid]
-  [:map {:title "update-file"}
+      [:id ::sm/uuid]
-   [:id ::sm/uuid]
+      [:revn {:min 0} :int]
-   [:session-id ::sm/uuid]
+      [:session-id ::sm/uuid]]]))
-   [:revn {:min 0} :int]
-   [:features {:optional true} ::cfeat/features]
-   [:changes {:optional true} schema:changes]
-   [:changes-with-metadata {:optional true}
-    [:vector schema:change-with-metadata]]
-   [:skip-validate {:optional true} :boolean]])
-
-(def ^:private schema:update-file-result
-  [:vector {:title "update-file-result"}
-   [:map
-    [:changes schema:changes]
-    [:file-id ::sm/uuid]
-    [:id ::sm/uuid]
-    [:revn {:min 0} :int]
-    [:session-id ::sm/uuid]]])
 
 ;; --- HELPERS
 
@@ -297,7 +295,7 @@
                                   (-> data
                                       (blob/decode)
                                       (assoc :id (:id file))
-                                      (pmg/migrate-data)
+                                      (fmg/migrate-data)
                                       (d/without-nils))))
 
         ;; WARNING: this ruins performance; maybe we need to find
@@ -310,10 +308,10 @@
                                           (-> (db/get conn :file {:id id})
                                               (files/decode-row)
                                               (files/process-pointers deref) ; ensure all pointers resolved
-                                              (pmg/migrate-file))))))
+                                              (fmg/migrate-file))))))
                     (d/index-by :id)))]
 
-    (-> file
+    (-> (files/check-version! file)
         (update :revn inc)
         (update :data cpc/process-changes changes)
 

common/src/app/common/files/migrations.cljc

@@ -10,7 +10,7 @@
    [app.common.data.macros :as dm]
    [app.common.features :as cfeat]
    [app.common.files.changes :as cpc]
-   [app.common.files.defaults :refer [version]]
+   [app.common.files.defaults :as cfd]
    [app.common.files.helpers :as cfh]
    [app.common.geom.matrix :as gmt]
    [app.common.geom.rect :as grc]
@@ -27,6 +27,8 @@
 
 #?(:cljs (l/set-level! :info))
 
+(def version cfd/version)
+
 (defmulti migrate :version)
 
 (defn migrate-data

frontend/src/app/main/errors.cljs

@@ -199,6 +199,15 @@
   (ts/schedule
    #(st/emit! (rt/assign-exception error))))
 
+
+(defn- redirect-to-dashboard
+  []
+  (let [team-id    (:current-team-id @st/state)
+        project-id (:current-project-id @st/state)]
+    (if (and project-id team-id)
+      (st/emit! (rt/nav :dashboard-files {:team-id team-id :project-id project-id}))
+      (set! (.-href glob/location) ""))))
+
 (defmethod ptk/handle-error :restriction
   [{:keys [code] :as error}]
   (cond
@@ -213,31 +222,20 @@
       (st/emit! (modal/show {:type :alert :message message :on-accept on-accept})))
 
     (= :file-feature-mismatch code)
-    (let [message    (tr "errors.file-feature-mismatch" (:feature error))
+    (let [message (tr "errors.file-feature-mismatch" (:feature error))]
-          team-id    (:current-team-id @st/state)
+      (st/emit! (modal/show {:type :alert :message message :on-accept redirect-to-dashboard})))
-          project-id (:current-project-id @st/state)
-          on-accept  #(if (and project-id team-id)
-                        (st/emit! (rt/nav :dashboard-files {:team-id team-id :project-id project-id}))
-                        (set! (.-href glob/location) ""))]
-      (st/emit! (modal/show {:type :alert :message message :on-accept on-accept})))
 
     (= :feature-mismatch code)
-    (let [message    (tr "errors.feature-mismatch" (:feature error))
+    (let [message (tr "errors.feature-mismatch" (:feature error))]
-          team-id    (:current-team-id @st/state)
+      (st/emit! (modal/show {:type :alert :message message :on-accept redirect-to-dashboard})))
-          project-id (:current-project-id @st/state)
-          on-accept  #(if (and project-id team-id)
-                        (st/emit! (rt/nav :dashboard-files {:team-id team-id :project-id project-id}))
-                        (set! (.-href glob/location) ""))]
-      (st/emit! (modal/show {:type :alert :message message :on-accept on-accept})))
 
     (= :feature-not-supported code)
-    (let [message    (tr "errors.feature-not-supported" (:feature error))
+    (let [message (tr "errors.feature-not-supported" (:feature error))]
-          team-id    (:current-team-id @st/state)
+      (st/emit! (modal/show {:type :alert :message message :on-accept redirect-to-dashboard})))
-          project-id (:current-project-id @st/state)
+
-          on-accept  #(if (and project-id team-id)
+    (= :file-version-not-supported code)
-                        (st/emit! (rt/nav :dashboard-files {:team-id team-id :project-id project-id}))
+    (let [message (tr "errors.version-not-supported")]
-                        (set! (.-href glob/location) ""))]
+      (st/emit! (modal/show {:type :alert :message message :on-accept redirect-to-dashboard})))
-      (st/emit! (modal/show {:type :alert :message message :on-accept on-accept})))
 
     (= :max-quote-reached code)
     (let [message (tr "errors.max-quote-reached" (:target error))]
@@ -250,7 +248,7 @@
       (st/emit! (modal/show {:type :alert :message message})))
 
     :else
-    (ptk/handle-error {:type :server-error :data error})))
+    (print-cause! "Restriction Error" error)))
 
 ;; This happens when the backed server fails to process the
 ;; request. This can be caused by an internal assertion or any other

frontend/translations/en.po

@@ -883,6 +883,11 @@ msgstr ""
 "Looks like you are opening a file that has the feature '%s' enabled but "
 "the current penpot version does not supports it or has it disabled."
 
+#: src/app/main/errors.cljs
+msgid "errors.version-not-supported"
+msgstr ""
+"File has an incompatible version number"
+
 #: src/app/main/errors.cljs
 msgid "errors.file-feature-mismatch"
 msgstr ""