diff --git a/backend/src/app/config.clj b/backend/src/app/config.clj index d514c5c9f4..f3a4b7f517 100644 --- a/backend/src/app/config.clj +++ b/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] diff --git a/backend/src/app/rpc/commands/media.clj b/backend/src/app/rpc/commands/media.clj index ffa94d5a6b..51f87c320c 100644 --- a/backend/src/app/rpc/commands/media.clj +++ b/backend/src/app/rpc/commands/media.clj @@ -339,6 +339,9 @@ ;; --- Chunked Upload: Upload a single chunk +(declare ^:private get-upload-chunk) +(declare ^:private check-upload-chunk-slot) + (def ^:private schema:upload-chunk [:map {:title "upload-chunk"} [:session-id ::sm/uuid] @@ -354,9 +357,31 @@ {::doc/added "2.17" ::sm/params schema:upload-chunk ::sm/result schema:upload-chunk-result} - [{:keys [::db/pool] :as 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})] + [cfg {:keys [::rpc/profile-id session-id index content]}] + (let [session (db/tx-run! cfg check-upload-chunk-slot session-id profile-id index content)] + (l/trc :hint "upload-chunk" + :session-id session-id + :chunk (str index "/" (:total-chunks session)) + :size (:size content) + :path (:path content)) + + (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})) + + {:session-id session-id + :index index})) + +(defn- check-upload-chunk-slot + [{:keys [::db/conn]} session-id profile-id index content] + (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 @@ -365,26 +390,23 @@ :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})) - - {:session-id session-id - :index index}) + session)) ;; --- Chunked Upload: shared helpers @@ -399,6 +421,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 +451,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))) diff --git a/backend/test/backend_tests/rpc_media_test.clj b/backend/test/backend_tests/rpc_media_test.clj index d22eabe64b..e22ddb5afd 100644 --- a/backend/test/backend_tests/rpc_media_test.clj +++ b/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.