✨ Cap upload chunk size at 30 MiB by default

Chunks were only bounded by the 350 MiB HTTP body limit while the 30 MiB caps applied to the assembled file. Add :upload-max-chunk-size (default 30 MiB, tunable via env) and reject oversize chunks in upload-chunk with :validation/:chunk-too-large before anything is stored. App clients slice at 25/10 MiB, so no frontend change needed.

AI-assisted-by: muse-spark-1.3-contributor

backend/src/app/config.clj

@@ -91,6 +91,7 @@
 
    :quotes-upload-sessions-per-profile 5
    :quotes-upload-chunks-per-session 20
+   :upload-max-chunk-size (* 1024 1024 30) ; 30MiB
 
    ;; SSRF protection
    :ssrf-allowed-hosts #{}
@@ -202,6 +203,7 @@
     [:quotes-team-access-requests-per-requester {:optional true} ::sm/int]
     [:quotes-upload-sessions-per-profile {:optional true} ::sm/int]
     [:quotes-upload-chunks-per-session {:optional true} ::sm/int]
+    [:upload-max-chunk-size {:optional true} ::sm/int]
     [:quotes-media-storage-bytes-per-team {:optional true} ::sm/int]
 
     [:auth-token-cookie-name {:optional true} :string]

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

@@ -339,6 +339,8 @@
 
 ;; --- Chunked Upload: Upload a single chunk
 
+(declare get-upload-chunk)
+
 (def ^:private schema:upload-chunk
   [:map {:title "upload-chunk"}
    [:session-id ::sm/uuid]
@@ -354,37 +356,54 @@
   {::doc/added "2.17"
    ::sm/params schema:upload-chunk
    ::sm/result schema:upload-chunk-result}
-  [{:keys [::db/pool] :as cfg}
+  [cfg
    {:keys [::rpc/profile-id session-id index content] :as _params}]
-  (let [session (db/get pool :upload-session {:id session-id :profile-id profile-id})]
-    (when (or (neg? index) (>= index (:total-chunks session)))
-      (ex/raise :type :validation
-                :code :invalid-chunk-index
-                :hint "chunk index is out of range for this session"
-                :session-id session-id
-                :total-chunks (:total-chunks session)
-                :index index))
+  (db/tx-run! cfg
+              (fn [{:keys [::db/conn] :as cfg}]
+                (let [session (db/get conn :upload-session {:id session-id :profile-id profile-id} {::db/for-update true})]
+                  (when (or (neg? index) (>= index (:total-chunks session)))
+                    (ex/raise :type :validation
+                              :code :invalid-chunk-index
+                              :hint "chunk index is out of range for this session"
+                              :session-id session-id
+                              :total-chunks (:total-chunks session)
+                              :index index))
 
+                  (when (> (:size content) (cf/get :upload-max-chunk-size))
+                    (ex/raise :type :validation
+                              :code :chunk-too-large
+                              :hint "chunk size exceeds the maximum allowed"
+                              :session-id session-id
+                              :index index
+                              :size (:size content)
+                              :max-size (cf/get :upload-max-chunk-size)))
 
-    (l/trc :hint "upload-chunk"
-           :session-id session-id
-           :chunk (str index "/" (:total-chunks session))
-           :size (:size content)
-           :path (:path content)))
+                  (when (get-upload-chunk conn session-id index)
+                    (ex/raise :type :validation
+                              :code :duplicate-chunk-index
+                              :hint "chunk index already uploaded for this session"
+                              :session-id session-id
+                              :index index))
 
-  (let [storage (sto/resolve cfg)
-        data    (sto/content (:path content))]
-    (sto/put-object! storage
-                     {::sto/content      data
-                      ::sto/deduplicate? false
-                      ::sto/touch        true
-                      :content-type      (:mtype content)
-                      :bucket            sto/tempfile-bucket
-                      :upload-id         (str session-id)
-                      :chunk-index       index}))
+                  (l/trc :hint "upload-chunk"
+                         :session-id session-id
+                         :chunk (str index "/" (:total-chunks session))
+                         :size (:size content)
+                         :path (:path content))
 
-  {:session-id session-id
-   :index      index})
+                  (let [storage (sto/resolve cfg ::db/reuse-conn true)
+                        data    (sto/content (:path content))]
+                    (sto/put-object! storage
+                                     {::sto/content      data
+                                      ::sto/deduplicate? false
+                                      ::sto/touch        true
+                                      :content-type      (:mtype content)
+                                      :bucket            sto/tempfile-bucket
+                                      :upload-id         (str session-id)
+                                      :chunk-index       index})))
+
+                {:session-id session-id
+                 :index      index})))
 
 ;; --- Chunked Upload: shared helpers
 
@@ -399,6 +418,18 @@
   [conn session-id]
   (db/exec! conn [sql:get-upload-chunks (str session-id)]))
 
+(def ^:private sql:get-upload-chunk
+  "SELECT id
+     FROM storage_object
+    WHERE (metadata->>'~:upload-id') = ?::text
+      AND (metadata->>'~:chunk-index')::integer = ?
+      AND deleted_at IS NULL
+    LIMIT 1")
+
+(defn- get-upload-chunk
+  [conn session-id index]
+  (db/exec-one! conn [sql:get-upload-chunk (str session-id) index]))
+
 (defn- concat-chunks
   "Reads all chunk storage objects in order and writes them to a single
   temporary file on the local filesystem. Returns a path to that file."
@@ -417,18 +448,21 @@
   conforming to `media.v/schema:upload` with `:filename`, `:path` and
   `:size`.
 
-  Raises a :validation/:missing-chunks error when the number of stored
-  chunks does not match `:total-chunks` recorded in the session row.
+  Raises a :validation/:missing-chunks error when the stored chunk
+  indices do not form exactly the `0..total-chunks` range recorded in
+  the session row (wrong count, gaps or duplicates).
   Raises :not-found when the session does not belong to `profile-id`.
   Deletes the session row from `upload_session` on success."
   [{:keys [::db/conn] :as cfg} profile-id session-id]
   (let [session (db/get conn :upload-session {:id session-id :profile-id profile-id})
-        chunks  (get-upload-chunks conn session-id)]
+        chunks  (get-upload-chunks conn session-id)
+        indices (sort (map :chunk-index chunks))]
 
-    (when (not= (count chunks) (:total-chunks session))
+    (when (or (not= (count chunks) (:total-chunks session))
+              (not= indices (range (:total-chunks session))))
       (ex/raise :type :validation
                 :code :missing-chunks
-                :hint "number of stored chunks does not match expected total"
+                :hint "stored chunks do not match expected total"
                 :session-id session-id
                 :expected   (:total-chunks session)
                 :found      (count chunks)))

backend/test/backend_tests/rpc_media_test.clj

@@ -681,6 +681,131 @@
       (t/is (= :validation (-> out :error ex-data :type)))
       (t/is (= :missing-chunks (-> out :error ex-data :code))))))
 
+(t/deftest chunked-upload-assemble-rejects-duplicate-indices
+  ;; assemble-chunks must validate the index SET, not just the count: a
+  ;; session declaring 2 chunks but storing [0,0] must fail instead of
+  ;; assembling a corrupt file. Chunks are written at the storage level
+  ;; because upload-chunk itself now rejects the second index.
+  (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})
+        session-id  (create-session! prof 2)
+        storage     (:app.storage/storage th/*system*)
+        source-path (th/tempfile "backend_tests/test_files/sample.jpg")
+        chunks      (split-file-into-chunks source-path 312043)
+        put-chunk!  (fn [idx]
+                      (let [mfile (make-chunk-mfile (first chunks) "image/jpeg")]
+                        (sto/put-object! storage
+                                         {::sto/content      (sto/content (:path mfile))
+                                          ::sto/deduplicate? false
+                                          ::sto/touch        true
+                                          :content-type      "image/jpeg"
+                                          :bucket            sto/tempfile-bucket
+                                          :upload-id         (str session-id)
+                                          :chunk-index       idx})))]
+    (put-chunk! 0)
+    (put-chunk! 0)
+
+    (let [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            "dupe-indices"
+                            :mtype           "image/jpeg"})]
+      (t/is (some? (:error out)))
+      (t/is (= :validation (-> out :error ex-data :type)))
+      (t/is (= :missing-chunks (-> out :error ex-data :code))))))
+
+(t/deftest chunked-upload-duplicate-then-assemble
+  ;; A rejected duplicate must leave the first chunk intact: upload 0,
+  ;; re-upload 0 (rejected), then assemble succeeds with the original size.
+  (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})
+        session-id  (create-session! prof 1)
+        source-path (th/tempfile "backend_tests/test_files/sample.jpg")
+        chunks      (split-file-into-chunks source-path 312043)
+        mtype       "image/jpeg"
+        size        (alength (first chunks))]
+
+    (let [out (th/command! {::th/type        :upload-chunk
+                            ::rpc/profile-id (:id prof)
+                            :session-id      session-id
+                            :index           0
+                            :content         (make-chunk-mfile (first chunks) mtype)})]
+      (t/is (nil? (:error out))))
+
+    (let [out (th/command! {::th/type        :upload-chunk
+                            ::rpc/profile-id (:id prof)
+                            :session-id      session-id
+                            :index           0
+                            :content         (make-chunk-mfile (first chunks) mtype)})]
+      (t/is (some? (:error out)))
+      (t/is (= :duplicate-chunk-index (-> out :error ex-data :code))))
+
+    (let [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            "after-dupe"
+                            :mtype           mtype})]
+      (t/is (nil? (:error out)))
+      (let [storage (:app.storage/storage th/*system*)
+            mobj    (sto/get-object storage (:media-id (:result out)))]
+        (t/is (= size (:size mobj)))))))
+
+(t/deftest chunked-upload-rejected-duplicate-keeps-session-usable
+  ;; Rejecting a duplicate must not poison the session: the remaining
+  ;; distinct indices still accumulate and assemble normally.
+  (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})
+        session-id  (create-session! prof 2)
+        source-path (th/tempfile "backend_tests/test_files/sample.jpg")
+        chunks      (split-file-into-chunks source-path 110000)
+        mtype       "image/jpeg"]
+
+    (t/is (= 3 (count chunks)))
+
+    (let [out (th/command! {::th/type        :upload-chunk
+                            ::rpc/profile-id (:id prof)
+                            :session-id      session-id
+                            :index           0
+                            :content         (make-chunk-mfile (nth chunks 0) mtype)})]
+      (t/is (nil? (:error out))))
+
+    (let [out (th/command! {::th/type        :upload-chunk
+                            ::rpc/profile-id (:id prof)
+                            :session-id      session-id
+                            :index           0
+                            :content         (make-chunk-mfile (nth chunks 0) mtype)})]
+      (t/is (some? (:error out)))
+      (t/is (= :duplicate-chunk-index (-> out :error ex-data :code))))
+
+    (let [out (th/command! {::th/type        :upload-chunk
+                            ::rpc/profile-id (:id prof)
+                            :session-id      session-id
+                            :index           1
+                            :content         (make-chunk-mfile (nth chunks 1) mtype)})]
+      (t/is (nil? (:error out))))
+
+    ;; The live store holds exactly the two distinct indices: the
+    ;; rejected duplicate stored nothing.
+    (let [rows (th/db-exec! ["SELECT (metadata->>'~:chunk-index')::integer AS idx FROM storage_object WHERE (metadata->>'~:upload-id') = ?::text AND deleted_at IS NULL ORDER BY idx"
+                             (str session-id)])]
+      (t/is (= [0 1] (mapv :idx rows))))))
+
 (t/deftest chunked-upload-session-not-found
   (let [prof       (th/create-profile* 1)
         _          (th/create-project* 1 {:profile-id (:id prof)
@@ -767,6 +892,77 @@
       (t/is (= :validation (-> out :error ex-data :type)))
       (t/is (= :invalid-chunk-index (-> out :error ex-data :code))))))
 
+(t/deftest chunked-upload-duplicate-index-rejected
+  ;; Uploading the same chunk index twice into one session must fail:
+  ;; the second call raises :validation / :duplicate-chunk-index and
+  ;; stores nothing, so one session+index keeps at most one object.
+  (let [prof        (th/create-profile* 1)
+        session-id  (create-session! prof 1)
+        source-path (th/tempfile "backend_tests/test_files/sample.jpg")
+        chunks      (split-file-into-chunks source-path 312043)
+        mtype       "image/jpeg"
+        mfile1      (make-chunk-mfile (first chunks) mtype)
+        mfile2      (make-chunk-mfile (first chunks) mtype)]
+
+    ;; First upload succeeds
+    (let [out (th/command! {::th/type        :upload-chunk
+                            ::rpc/profile-id (:id prof)
+                            :session-id      session-id
+                            :index           0
+                            :content         mfile1})]
+      (t/is (nil? (:error out))))
+
+    ;; Second upload of the same index must be rejected
+    (let [out (th/command! {::th/type        :upload-chunk
+                            ::rpc/profile-id (:id prof)
+                            :session-id      session-id
+                            :index           0
+                            :content         mfile2})]
+      (t/is (some? (:error out)))
+      (t/is (= :validation (-> out :error ex-data :type)))
+      (t/is (= :duplicate-chunk-index (-> out :error ex-data :code))))
+
+    ;; Exactly one live object stored for that session/index
+    (let [rows (th/db-exec! ["SELECT id FROM storage_object WHERE (metadata->>'~:upload-id') = ?::text AND (metadata->>'~:chunk-index') = '0' AND deleted_at IS NULL"
+                             (str session-id)])]
+      (t/is (= 1 (count rows))))))
+
+(t/deftest chunked-upload-chunk-too-large
+  ;; Chunks larger than the configured cap must be rejected with
+  ;; :validation / :chunk-too-large before anything is stored, while a
+  ;; chunk exactly at the cap still uploads fine.
+  (with-mocks [mock {:target 'app.config/get
+                     :return (th/config-get-mock
+                              {:upload-max-chunk-size 1024})}]
+    (let [prof        (th/create-profile* 1)
+          session-id  (create-session! prof 1)
+          source-path (th/tempfile "backend_tests/test_files/sample.jpg")
+          chunks      (split-file-into-chunks source-path 312043)
+          mtype       "image/jpeg"]
+
+      ;; 312043 bytes exceeds the mocked 1024-byte cap: rejected
+      (let [out (th/command! {::th/type        :upload-chunk
+                              ::rpc/profile-id (:id prof)
+                              :session-id      session-id
+                              :index           0
+                              :content         (make-chunk-mfile (first chunks) mtype)})]
+        (t/is (some? (:error out)))
+        (t/is (= :validation (-> out :error ex-data :type)))
+        (t/is (= :chunk-too-large (-> out :error ex-data :code))))
+
+      ;; Nothing stored for the rejected chunk
+      (let [rows (th/db-exec! ["SELECT id FROM storage_object WHERE (metadata->>'~:upload-id') = ?::text AND deleted_at IS NULL"
+                               (str session-id)])]
+        (t/is (= 0 (count rows))))
+
+      ;; A chunk exactly at the cap still uploads fine
+      (let [out (th/command! {::th/type        :upload-chunk
+                              ::rpc/profile-id (:id prof)
+                              :session-id      session-id
+                              :index           0
+                              :content         (make-chunk-mfile (byte-array 1024 (byte 1)) mtype)})]
+        (t/is (nil? (:error out)))))))
+
 (t/deftest chunked-upload-sessions-per-profile-quota
   ;; With the session limit set to 2, creating a third session for the
   ;; same profile must fail with :restriction / :max-quote-reached.