From b33213787e2eb4f52fade3a257ce82f76b3b3221 Mon Sep 17 00:00:00 2001 From: Andrey Antukh Date: Wed, 26 Aug 2026 08:33:20 +0200 Subject: [PATCH] :bug: Add accumulated storage byte quota for media uploads (#11038) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit * :bug: Add accumulated storage byte quota for media uploads Add media-storage-bytes-per-team quote to prevent persistent DoS via repeated uploads. The quota sums storage_object sizes from both file_media_object (media + thumbnails) and team_font_variant (otf/ttf/woff1/woff2). Default limit is 20 GiB per team, configurable via PENPOT_QUOTES_MEDIA_STORAGE_BYTES_PER_TEAM. The check is invoked in upload-file-media-object before processing, looking up the team-id via file -> project -> team_id join. AI-assisted-by: mimo-v2.5-pro * :bug: Fix deduplicated storage overcounting in media-storage-bytes-per-team quote The SQL query sql:get-media-storage-bytes-per-team used UNION ALL across six SELECT branches that each produce a so_id reference. When deduplication causes multiple file_media_object or team_font_variant rows to point at the same storage_object, UNION ALL counts that objects size once per reference — inflating "used bytes" and causing false :max-quote-reached rejections. Change all five UNION ALL to UNION so that duplicate so_id values are collapsed before the JOIN storage_object / SUM(so.size). Add a test (media-storage-bytes-quote-deduped) that creates one storage_object referenced by two file_media_object rows and asserts the computed usage reflects the deduplicated physical size, not 2x. AI-assisted-by: mimo-v2.5-pro --- backend/src/app/config.clj | 1 + backend/src/app/rpc/commands/media.clj | 12 ++ backend/src/app/rpc/quotes.clj | 70 +++++++++ .../test/backend_tests/rpc_quotes_test.clj | 134 ++++++++++++++++++ 4 files changed, 217 insertions(+) diff --git a/backend/src/app/config.clj b/backend/src/app/config.clj index f02136b1ca..3979f399d7 100644 --- a/backend/src/app/config.clj +++ b/backend/src/app/config.clj @@ -194,6 +194,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] + [:quotes-media-storage-bytes-per-team {:optional true} ::sm/int] [:auth-token-cookie-name {:optional true} :string] [:auth-token-cookie-max-age {:optional true} ::ct/duration] diff --git a/backend/src/app/rpc/commands/media.clj b/backend/src/app/rpc/commands/media.clj index 3dff04fa10..a5e1b9672a 100644 --- a/backend/src/app/rpc/commands/media.clj +++ b/backend/src/app/rpc/commands/media.clj @@ -40,6 +40,12 @@ (declare create-file-media-object) +(def ^:private sql:get-team-id-for-file + "SELECT p.team_id + FROM file AS f + JOIN project AS p ON (p.id = f.project_id) + WHERE f.id = ?") + (def ^:private schema:upload-file-media-object [:map {:title "upload-file-media-object"} [:id {:optional true} ::sm/uuid] @@ -58,6 +64,12 @@ (media.v/validate-media-type! content) (media.v/validate-media-size! content) + (let [team-id (:team-id (db/exec-one! pool [sql:get-team-id-for-file file-id]))] + (quotes/check! cfg {::quotes/id ::quotes/media-storage-bytes-per-team + ::quotes/profile-id profile-id + ::quotes/team-id team-id + ::quotes/incr (:size content)})) + (db/run! cfg (fn [{:keys [::db/conn] :as cfg}] ;; We get the minimal file for proper checking if ;; file is not already deleted diff --git a/backend/src/app/rpc/quotes.clj b/backend/src/app/rpc/quotes.clj index 0a7004cc54..e391f0b0ac 100644 --- a/backend/src/app/rpc/quotes.clj +++ b/backend/src/app/rpc/quotes.clj @@ -546,6 +546,76 @@ (assoc ::count-sql [sql:get-upload-sessions-per-profile profile-id]) (generic-check!))) +;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;; +;; QUOTE: MEDIA-STORAGE-BYTES-PER-TEAM +;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;; + +(def ^:private schema:media-storage-bytes-per-team + [:map + [::profile-id ::sm/uuid] + [::team-id ::sm/uuid]]) + +(def ^:private valid-media-storage-bytes-per-team-quote? + (sm/lazy-validator schema:media-storage-bytes-per-team)) + +(def ^:private sql:get-media-storage-bytes-per-team + "SELECT COALESCE(SUM(so.size), 0) AS total + FROM ( + SELECT fmo.media_id AS so_id + FROM file_media_object AS fmo + JOIN file AS f ON (f.id = fmo.file_id) + JOIN project AS p ON (p.id = f.project_id) + WHERE p.team_id = ? + AND fmo.deleted_at IS NULL + AND f.deleted_at IS NULL + UNION + SELECT fmo.thumbnail_id AS so_id + FROM file_media_object AS fmo + JOIN file AS f ON (f.id = fmo.file_id) + JOIN project AS p ON (p.id = f.project_id) + WHERE p.team_id = ? + AND fmo.thumbnail_id IS NOT NULL + AND fmo.deleted_at IS NULL + AND f.deleted_at IS NULL + UNION + SELECT v.otf_file_id AS so_id + FROM team_font_variant AS v + WHERE v.team_id = ? + AND v.otf_file_id IS NOT NULL + AND v.deleted_at IS NULL + UNION + SELECT v.ttf_file_id AS so_id + FROM team_font_variant AS v + WHERE v.team_id = ? + AND v.ttf_file_id IS NOT NULL + AND v.deleted_at IS NULL + UNION + SELECT v.woff1_file_id AS so_id + FROM team_font_variant AS v + WHERE v.team_id = ? + AND v.woff1_file_id IS NOT NULL + AND v.deleted_at IS NULL + UNION + SELECT v.woff2_file_id AS so_id + FROM team_font_variant AS v + WHERE v.team_id = ? + AND v.woff2_file_id IS NOT NULL + AND v.deleted_at IS NULL + ) AS refs + JOIN storage_object AS so ON (so.id = refs.so_id) + WHERE so.deleted_at IS NULL") + +(defmethod check-quote ::media-storage-bytes-per-team + [{:keys [::profile-id ::team-id ::target] :as quote}] + (assert (valid-media-storage-bytes-per-team-quote? quote) "invalid quote parameters") + (-> quote + (assoc ::default (cf/get :quotes-media-storage-bytes-per-team + (* 20 1024 1024 1024))) + (assoc ::quote-sql [sql:get-quotes-2 target team-id profile-id profile-id]) + (assoc ::count-sql [sql:get-media-storage-bytes-per-team + team-id team-id team-id team-id team-id team-id]) + (generic-check!))) + ;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;; ;; QUOTE: DEFAULT ;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;; diff --git a/backend/test/backend_tests/rpc_quotes_test.clj b/backend/test/backend_tests/rpc_quotes_test.clj index 94db804e17..b279ff96d3 100644 --- a/backend/test/backend_tests/rpc_quotes_test.clj +++ b/backend/test/backend_tests/rpc_quotes_test.clj @@ -338,3 +338,137 @@ (check-ok! 4) (check-ko! 5)))) + +(t/deftest media-storage-bytes-per-team-quote + (with-mocks [mock {:target 'app.config/get + :return (th/config-get-mock + {:quotes-media-storage-bytes-per-team 1000})}] + + (let [profile-1 (th/create-profile* 1) + profile-2 (th/create-profile* 2) + team-id (:default-team-id profile-1) + data {::quotes/id ::quotes/media-storage-bytes-per-team + ::quotes/profile-id (:id profile-1) + ::quotes/team-id team-id + ::quotes/incr 500} + + check-ok! (fn [msg] + (quotes/check! th/*system* data) + (t/is (true? true) msg)) + check-ko! (fn [msg] + (try + (quotes/check! th/*system* data) + (t/is false (str msg " — expected exception but none thrown")) + (catch Exception e + (let [ed (ex-data e)] + (t/is (= :restriction (:type ed))) + (t/is (= :max-quote-reached (:code ed))) + (t/is (= "media-storage-bytes-per-team" (:target ed)))))))] + + ;; Under default limit (1000) with incr=500 and no existing storage — ok + (check-ok! "first check under limit") + + ;; Insert a quote row for another profile on the same team — does not help + (th/db-insert! :usage-quote + {:profile-id (:id profile-2) + :target "media-storage-bytes-per-team" + :quote 100}) + + ;; Insert a team+profile quote that is still too low + (th/db-insert! :usage-quote + {:team-id team-id + :profile-id (:id profile-2) + :target "media-storage-bytes-per-team" + :quote 200}) + + ;; Insert a team-level quote (no profile) that is still too low + (th/db-insert! :usage-quote + {:team-id team-id + :target "media-storage-bytes-per-team" + :quote 400}) + + ;; total=0, incr=500, best quote=400 → 0+500 > 400 → blocked + (check-ko! "blocked by team-level quote") + + ;; Insert a team+profile quote that allows it + (th/db-insert! :usage-quote + {:team-id team-id + :profile-id (:id profile-1) + :target "media-storage-bytes-per-team" + :quote 1000}) + + ;; total=0, incr=500, best quote=1000 → 0+500 <= 1000 → ok + (check-ok! "allowed by team+profile quote")))) + +(t/deftest media-storage-bytes-quote-deduped + (with-mocks [mock {:target 'app.config/get + :return (th/config-get-mock + {:quotes-media-storage-bytes-per-team 1100})}] + + (let [prof (th/create-profile* 1) + team-id (:default-team-id prof) + proj (th/create-project* 1 {:profile-id (:id prof) + :team-id team-id}) + file1 (th/create-file* 1 {:profile-id (:id prof) + :project-id (:id proj) + :is-shared false}) + file2 (th/create-file* 2 {:profile-id (:id prof) + :project-id (:id proj) + :is-shared false}) + + ;; One physical storage object of 500 bytes + so-id (uuid/random) + _ (th/db-insert! :storage-object {:id so-id + :size 500 + :backend "test"}) + + ;; Two file_media_object rows pointing at the SAME storage object + ;; (simulates the deduplication path: same content uploaded twice) + _ (th/create-file-media-object* + {:file-id (:id file1) :media-id so-id + :name "icon" :mtype "image/svg+xml"}) + _ (th/create-file-media-object* + {:file-id (:id file2) :media-id so-id + :name "icon" :mtype "image/svg+xml"}) + + data {::quotes/id ::quotes/media-storage-bytes-per-team + ::quotes/profile-id (:id prof) + ::quotes/team-id team-id + ::quotes/incr 200}] + + ;; Physical size is 500. With UNION (correct), total=500, 500+200=700 ≤ 1100 → ok. + ;; With UNION ALL (buggy), total=1000, 1000+200=1200 > 1100 → rejected. + (quotes/check! th/*system* data) + (t/is (true? true) "deduped storage counted once, under quota")))) + +(t/deftest media-upload-enforces-storage-quote + (with-mocks [mock {:target 'app.config/get + :return (th/config-get-mock + {:quotes-media-storage-bytes-per-team 100})}] + + (let [prof (th/create-profile* 1) + proj (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 (:id proj) + :is-shared false}) + mfile {:filename "sample.jpg" + :path (th/tempfile "backend_tests/test_files/sample.jpg") + :mtype "image/jpeg" + :size 312043} + + params {::th/type :upload-file-media-object + ::rpc/profile-id (:id prof) + :file-id (:id file) + :is-local true + :name "testfile" + :content mfile} + + out (th/command! params)] + + ;; 312043 bytes > 100 byte limit → should be rejected + (t/is (not (th/success? out))) + (let [error (:error out)] + (t/is (= :restriction (th/ex-type error))) + (t/is (= :max-quote-reached (th/ex-code error))) + (t/is (= "media-storage-bytes-per-team" (:target (ex-data error))))))))