Merge remote-tracking branch 'origin/main-2.18.x' into staging

This commit is contained in:
Andrey Antukh 2026-11-03 15:06:32 +01:00
commit 610813281b
13 changed files with 152 additions and 51 deletions

View File

@ -59,8 +59,8 @@
:hint "seems like the url points to resource with unknown size"))
(-> {:size size :mtype mtype}
(validation/validate-media-type!)
(validation/validate-media-size!))))]
(validation/check-media-type)
(validation/check-media-size))))]
(let [{:keys [body] :as response}
(try

View File

@ -32,8 +32,26 @@
(def check-input
(sm/check-fn schema:input))
(defn validate-media-type!
([upload] (validate-media-type! upload cm/image-types))
(def allowed-upload-prefixes
;; Upload temp files are created server-side by the multipart parser
;; (yetti `default-temp-dir`, `/tmp/undertow/`) plus the tmp dir used
;; by tests and storage tmp files (`/tmp/penpot/`). Anything else
;; (e.g. `/etc/passwd` smuggled via Transit `~#path`) is rejected.
["/tmp/undertow/" "/tmp/penpot/"])
(defn check-upload-path
[upload]
(let [real (try (str (fs/real (:path upload)))
(catch Exception _ nil))]
(when (or (nil? real)
(not (some #(str/starts-with? real %) allowed-upload-prefixes)))
(ex/raise :type :validation
:code :params-validation
:hint "invalid upload path"))
upload))
(defn check-media-type
([upload] (check-media-type upload cm/image-types))
([upload allowed]
(when-not (contains? allowed (:mtype upload))
(ex/raise :type :validation
@ -42,7 +60,7 @@
upload))
(defn validate-media-size!
(defn check-media-size
[upload]
(let [max-size (cf/get :media-max-file-size)]
(when (> (:size upload) max-size)
@ -53,10 +71,10 @@
max-size)))
upload))
(defn validate-font-size!
(defn check-font-size
"Validates that the font file `upload` does not exceed the configured
`:font-max-file-size` limit. Accepts the same map shape as
`validate-media-size!` — requires a `:size` key in bytes."
`check-media-size` — requires a `:size` key in bytes."
[upload]
(let [max-size (cf/get :font-max-file-size)]
(when (> (:size upload) max-size)

View File

@ -210,6 +210,8 @@
(assoc params :file file))
params)
_ (media.v/check-upload-path (:file params))
version (or version
(case (bfc/parse-file-format (-> params :file :path))
:binfile-v1 1

View File

@ -290,8 +290,9 @@
::rpc/multipart-only-params [:media]}
[cfg {:keys [::rpc/profile-id file-id object-id media tag]}]
(media.v/validate-media-type! media)
(media.v/validate-media-size! media)
(media.v/check-media-type media)
(media.v/check-media-size media)
(media.v/check-upload-path media)
(db/run! cfg files/check-edition-permissions! profile-id file-id)
(when-let [file (files/get-minimal-file cfg file-id {::db/check-deleted false})]
@ -392,8 +393,9 @@
::rpc/multipart-only-params [:media]}
[cfg {:keys [::rpc/profile-id file-id] :as params}]
(media.v/validate-media-type! (:media params))
(media.v/validate-media-size! (:media params))
(media.v/check-media-type (:media params))
(media.v/check-media-size (:media params))
(media.v/check-upload-path (:media params))
(db/run! cfg files/check-edition-permissions! profile-id file-id)

View File

@ -123,8 +123,8 @@
(fn [acc mtype session-id]
(let [assembled (assemble-chunks cfg profile-id session-id)]
(-> {:mtype mtype :size (:size assembled)}
(media.v/validate-media-type! cm/font-types)
(media.v/validate-font-size!))
(media.v/check-media-type cm/font-types)
(media.v/check-font-size))
(assoc acc mtype (:path assembled))))
{}
uploads)]

View File

@ -65,8 +65,9 @@
[:process-image/global]]}
[{:keys [::db/pool] :as cfg} {:keys [::rpc/profile-id file-id content] :as params}]
(files/check-edition-permissions! pool profile-id file-id)
(media.v/validate-media-type! content)
(media.v/validate-media-size! content)
(media.v/check-media-type content)
(media.v/check-media-size content)
(media.v/check-upload-path content)
(let [team-id (:team-id (db/exec-one! pool [sql:get-team-id-for-file file-id]))]
;; TEMPORARILY DISABLED (2026-10-19): media-storage-bytes-per-team
@ -376,6 +377,7 @@
::sm/result schema:upload-chunk-result}
[{:keys [::db/pool] :as cfg}
{:keys [::rpc/profile-id session-id index content] :as _params}]
(media.v/check-upload-path 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
@ -576,8 +578,8 @@
content (-> content
(assoc :filename (str "upload:" name))
(assoc :mtype mtype)
(media.v/validate-media-type!)
(media.v/validate-media-size!))
(media.v/check-media-type)
(media.v/check-media-size))
mobj (create-file-media-object cfg (assoc params
:from-chunks? true
:content content))]

View File

@ -313,8 +313,9 @@
::sm/result :nil}
[cfg {:keys [::rpc/profile-id file] :as params}]
;; Validate incoming mime type
(media.v/validate-media-type! file #{"image/jpeg" "image/png" "image/webp"})
(media.v/validate-media-size! file)
(media.v/check-media-type file #{"image/jpeg" "image/png" "image/webp"})
(media.v/check-media-size file)
(media.v/check-upload-path file)
(update-profile-photo cfg (assoc params :profile-id profile-id)))
(defn update-profile-photo

View File

@ -1092,8 +1092,9 @@
[cfg {:keys [::rpc/profile-id file] :as params}]
;; Validate incoming mime type
(media.v/validate-media-type! file #{"image/jpeg" "image/png" "image/webp"})
(media.v/validate-media-size! file)
(media.v/check-media-type file #{"image/jpeg" "image/png" "image/webp"})
(media.v/check-media-size file)
(media.v/check-upload-path file)
(update-team-photo cfg (assoc params :profile-id profile-id)))
(defn update-team-photo

View File

@ -34,7 +34,8 @@
::rpc/multipart-only-params [:content]
::sm/result schema:upload-tempfile-result}
[cfg {:keys [::rpc/profile-id content]}]
(media.v/validate-media-type! content cm/tempfile-types)
(media.v/check-media-type content cm/tempfile-types)
(media.v/check-upload-path content)
(let [storage (sto/resolve cfg)
hash (sto/calculate-hash (:path content))
data (-> (sto/content (:path content))

View File

@ -139,7 +139,8 @@
::sm/result schema:upload-organization-logo-result
::nitrate/sso false}
[{:keys [::sto/storage]} {:keys [content organization-id previous-id]}]
(media.v/validate-media-type! content cm/image-types)
(media.v/check-media-type content cm/image-types)
(media.v/check-upload-path content)
(when previous-id
(sto/touch-object! storage previous-id))
(let [hash (sto/calculate-hash (:path content))

View File

@ -146,7 +146,7 @@
(defn- write-temp-svg
"Write `text` to a tempfile and return `[path size]`."
[text]
(let [path (fs/create-tempfile :prefix "penpot-svg-import-test-" :suffix ".svg")
(let [path (fs/create-tempfile :dir "/tmp/penpot" :prefix "penpot-svg-import-test-" :suffix ".svg")
data (utf8bytes text)]
(spit (str path) text :encoding "UTF-8")
[path (alength data)]))

View File

@ -0,0 +1,68 @@
;; This Source Code Form is subject to the terms of the Mozilla Public
;; License, v. 2.0. If a copy of the MPL was not distributed with this
;; file, You can obtain one at http://mozilla.org/MPL/2.0/.
;;
;; Copyright (c) KALEIDOS INC Sucursal en España SL
(ns backend-tests.media-validation-test
(:require
[app.media.validation :as media.v]
[backend-tests.helpers :as th]
[clojure.test :as t]
[datoteka.fs :as fs]))
(t/deftest check-media-type
(t/testing "allowed mtype passes through"
(let [upload {:filename "a.png" :size 1 :path (fs/path "/tmp/penpot/a.png") :mtype "image/png"}]
(t/is (= upload (media.v/check-media-type upload)))))
(t/testing "disallowed mtype is rejected"
(let [upload {:filename "a.txt" :size 1 :path (fs/path "/tmp/penpot/a.txt") :mtype "text/plain"}
error (try (media.v/check-media-type upload) nil
(catch clojure.lang.ExceptionInfo e e))]
(t/is (some? error))
(t/is (th/ex-of-type? error :validation))
(t/is (th/ex-of-code? error :media-type-not-allowed)))))
(t/deftest check-media-size
(t/testing "small file passes through"
(let [upload {:filename "a.png" :size 1 :path (fs/path "/tmp/penpot/a.png") :mtype "image/png"}]
(t/is (= upload (media.v/check-media-size upload)))))
(t/testing "oversized file is rejected"
(let [upload {:filename "a.png" :size (* 1024 1024 31) :path (fs/path "/tmp/penpot/a.png") :mtype "image/png"}
error (try (media.v/check-media-size upload) nil
(catch clojure.lang.ExceptionInfo e e))]
(t/is (some? error))
(t/is (th/ex-of-type? error :restriction))
(t/is (th/ex-of-code? error :media-max-file-size-reached)))))
(t/deftest check-font-size
(t/testing "small font passes through"
(let [upload {:filename "a.ttf" :size 1 :path (fs/path "/tmp/penpot/a.ttf") :mtype "font/ttf"}]
(t/is (= upload (media.v/check-font-size upload)))))
(t/testing "oversized font is rejected"
(let [upload {:filename "a.ttf" :size (* 1024 1024 31) :path (fs/path "/tmp/penpot/a.ttf") :mtype "font/ttf"}
error (try (media.v/check-font-size upload) nil
(catch clojure.lang.ExceptionInfo e e))]
(t/is (some? error))
(t/is (th/ex-of-type? error :restriction))
(t/is (th/ex-of-code? error :font-max-file-size-reached)))))
(t/deftest check-upload-path
(t/testing "tempfile under /tmp/penpot passes through"
(let [path (th/tempfile "backend_tests/test_files/sample.jpg")
upload {:filename "sample.jpg" :size 7923 :path path :mtype "image/jpeg"}]
(t/is (= upload (media.v/check-upload-path upload)))))
(t/testing "system file outside temp is rejected"
(let [upload {:filename "x.png" :size 1 :path (fs/path "/etc/passwd") :mtype "image/png"}
error (try (media.v/check-upload-path upload) nil
(catch clojure.lang.ExceptionInfo e e))]
(t/is (some? error))
(t/is (th/ex-of-type? error :validation))
(t/is (th/ex-of-code? error :params-validation))))
(t/testing "missing file is rejected"
(let [upload {:filename "x.png" :size 1 :path (fs/path "/tmp/penpot/no-such-file-12345.png") :mtype "image/png"}
error (try (media.v/check-upload-path upload) nil
(catch clojure.lang.ExceptionInfo e e))]
(t/is (some? error))
(t/is (th/ex-of-type? error :validation))
(t/is (th/ex-of-code? error :params-validation)))))

View File

@ -915,39 +915,44 @@
(t/is (= 1 (count rows2)))
(t/is (nil? (:deleted-at (first rows2)))))))
(t/deftest create-file-thumbnail-rejects-forged-media
;; A transit body like {"~:media": {"~:path": ["~#path", "/etc/passwd"]}}
;; decodes to this exact map. Without a multipart request it must fail at
;; params validation, before any file is read. NOTE: plain command! on
;; purpose, a multipart header would make it legitimate.
(t/deftest rejects-forged-path-on-create-file-thumbnail
(let [profile (th/create-profile* 1)
file (th/create-file* 1 {:profile-id (:id profile)
:project-id (:default-project-id profile)
:is-shared false})
out (th/command! {::th/type :create-file-thumbnail
::rpc/profile-id (:id profile)
:file-id (:id file)
:revn 0
:media {:filename "evil.jpg"
:size 123
:path (fs/path "/etc/passwd")
:mtype "image/jpeg"}})]
(t/is (th/ex-of-code? (:error out) :params-validation))))
data {::th/type :create-file-thumbnail
::rpc/profile-id (:id profile)
:file-id (:id file)
:revn 1
:media {:filename "x.png"
:size 1
:path (fs/path "/etc/passwd")
:mtype "image/png"}}
out (th/command! data)
error (:error out)]
(t/is (nil? (:result out)))
(t/is (th/ex-info? error))
(t/is (th/ex-of-type? error :validation))
(t/is (th/ex-of-code? error :params-validation))
(t/is (= 0 (count (th/db-query :file-thumbnail {:file-id (:id file)}))))))
(t/deftest create-file-object-thumbnail-rejects-forged-media
;; Same transit-forgery vector through object thumbnails. NOTE: plain
;; command! on purpose, a multipart header would make it legitimate.
(t/deftest rejects-forged-path-on-create-file-object-thumbnail
(let [profile (th/create-profile* 1)
file (th/create-file* 1 {:profile-id (:id profile)
:project-id (:default-project-id profile)
:is-shared false})
out (th/command! {::th/type :create-file-object-thumbnail
::rpc/profile-id (:id profile)
:file-id (:id file)
:object-id "obj"
:media {:filename "evil.jpg"
:size 123
:path (fs/path "/etc/passwd")
:mtype "image/jpeg"}})]
(t/is (th/ex-of-code? (:error out) :params-validation))))
data {::th/type :create-file-object-thumbnail
::rpc/profile-id (:id profile)
:file-id (:id file)
:object-id "test-key-forged"
:media {:filename "x.png"
:size 1
:path (fs/path "/etc/passwd")
:mtype "image/png"}}
out (th/command! data)
error (:error out)]
(t/is (nil? (:result out)))
(t/is (th/ex-info? error))
(t/is (th/ex-of-type? error :validation))
(t/is (th/ex-of-code? error :params-validation))
(t/is (= 0 (count (th/db-query :file-tagged-object-thumbnail {:file-id (:id file)}))))))