From 97e79a8194bacb188cdfe0eede73142fd1704429 Mon Sep 17 00:00:00 2001 From: Andrey Antukh Date: Tue, 28 Jul 2026 12:31:56 +0000 Subject: [PATCH] :bug: Validate content-type on management upload endpoints Add media type validation to upload-tempfile and upload-org-logo management endpoints. Both stored user-supplied mtype without checking against an allowlist. Only image types and PDF are permitted. Non-public bucket assets now also carry Content-Disposition: attachment to prevent inline rendering. AI-assisted-by: mimo-v2.5-pro --- backend/src/app/http/assets.clj | 23 ++++++++++++------- backend/src/app/rpc/management/exporter.clj | 6 +++-- backend/src/app/rpc/management/nitrate.clj | 2 ++ .../backend_tests/rpc_management_test.clj | 16 +++++++++++++ common/src/app/common/media.cljc | 3 +++ 5 files changed, 40 insertions(+), 10 deletions(-) diff --git a/backend/src/app/http/assets.clj b/backend/src/app/http/assets.clj index 1458b06d27..3c8225abb6 100644 --- a/backend/src/app/http/assets.clj +++ b/backend/src/app/http/assets.clj @@ -48,12 +48,16 @@ [{:keys [::sto/storage ::signature-max-age ::cache-max-age] :as cfg} obj] (let [sig-max-age (or signature-max-age default-signature-max-age) cch-max-age (or cache-max-age default-cache-max-age) - {:keys [host port] :as url} (sto/get-object-url storage obj {:max-age sig-max-age})] + {:keys [host port] :as url} (sto/get-object-url storage obj {:max-age sig-max-age}) + bucket (-> obj meta :bucket) + headers (cond-> {"location" (str url) + "x-host" (cond-> host port (str ":" port)) + "x-mtype" (-> obj meta :content-type) + "cache-control" (str "max-age=" (inst-ms cch-max-age))} + (not (contains? public-buckets bucket)) + (assoc "content-disposition" "attachment"))] {::yres/status 307 - ::yres/headers {"location" (str url) - "x-host" (cond-> host port (str ":" port)) - "x-mtype" (-> obj meta :content-type) - "cache-control" (str "max-age=" (inst-ms cch-max-age))}})) + ::yres/headers headers})) (defn- serve-object-from-fs [{:keys [::path ::cache-max-age]} obj] @@ -61,9 +65,12 @@ purl (u/join (u/uri path) (sto/object->relative-path obj)) mdata (meta obj) - headers {"x-accel-redirect" (:path purl) - "content-type" (:content-type mdata) - "cache-control" (str "max-age=" (inst-ms cch-max-age))}] + bucket (:bucket mdata) + headers (cond-> {"x-accel-redirect" (:path purl) + "content-type" (:content-type mdata) + "cache-control" (str "max-age=" (inst-ms cch-max-age))} + (not (contains? public-buckets bucket)) + (assoc "content-disposition" "attachment"))] {::yres/status 204 ::yres/headers headers})) diff --git a/backend/src/app/rpc/management/exporter.clj b/backend/src/app/rpc/management/exporter.clj index 20e791e7d0..142095419d 100644 --- a/backend/src/app/rpc/management/exporter.clj +++ b/backend/src/app/rpc/management/exporter.clj @@ -6,11 +6,12 @@ (ns app.rpc.management.exporter (:require + [app.common.media :as cm] [app.common.schema :as sm] [app.common.time :as ct] [app.common.uri :as u] [app.config :as cf] - [app.media :refer [schema:upload]] + [app.media :as media] [app.rpc :as-alias rpc] [app.rpc.doc :as doc] [app.storage :as sto] @@ -21,7 +22,7 @@ (def ^:private schema:upload-tempfile-params [:map {:title "upload-templfile-params"} - [:content schema:upload]]) + [:content media/schema:upload]]) (def ^:private schema:upload-tempfile-result @@ -32,6 +33,7 @@ ::sm/params schema:upload-tempfile-params ::sm/result schema:upload-tempfile-result} [cfg {:keys [::rpc/profile-id content]}] + (media/validate-media-type! content cm/tempfile-types) (let [storage (sto/resolve cfg) hash (sto/calculate-hash (:path content)) data (-> (sto/content (:path content)) diff --git a/backend/src/app/rpc/management/nitrate.clj b/backend/src/app/rpc/management/nitrate.clj index e36b04fbb0..d2f01b0108 100644 --- a/backend/src/app/rpc/management/nitrate.clj +++ b/backend/src/app/rpc/management/nitrate.clj @@ -12,6 +12,7 @@ [app.auth.oidc :as oidc] [app.common.data :as d] [app.common.exceptions :as ex] + [app.common.media :as cm] [app.common.schema :as sm] [app.common.time :as ct] [app.common.types.organization :as cto] @@ -135,6 +136,7 @@ ::sm/result schema:upload-organization-logo-result ::nitrate/sso false} [{:keys [::sto/storage]} {:keys [content organization-id previous-id]}] + (media/validate-media-type! content cm/image-types) (when previous-id (sto/touch-object! storage previous-id)) (let [hash (sto/calculate-hash (:path content)) diff --git a/backend/test/backend_tests/rpc_management_test.clj b/backend/test/backend_tests/rpc_management_test.clj index 601e8b3d35..2dd25694a1 100644 --- a/backend/test/backend_tests/rpc_management_test.clj +++ b/backend/test/backend_tests/rpc_management_test.clj @@ -57,6 +57,22 @@ (t/is (not= (get-in out1 [:result :id]) (get-in out2 [:result :id]))))) +(t/deftest upload-tempfile-rejects-html-content-type + ;; N2-13: upload-tempfile must reject non-allowed content types + (let [profile (th/create-profile* 1 {:is-active true}) + path (fs/create-tempfile :dir "/tmp/penpot" :prefix "test-upload-tempfile-") + _ (io/write* path "") + params {::th/type :upload-tempfile + ::rpc/profile-id (:id profile) + :content {:filename "evil.html" + :path path + :mtype "text/html" + :size 27}} + out (th/management-command! params)] + (t/is (some? (:error out))) + (t/is (= :validation (th/ex-type (:error out)))) + (t/is (= :media-type-not-allowed (th/ex-code (:error out)))))) + (t/deftest duplicate-file (let [storage (-> (:app.storage/storage th/*system*) (configure-storage-backend)) diff --git a/common/src/app/common/media.cljc b/common/src/app/common/media.cljc index 3507ba5f59..3d67bc75b6 100644 --- a/common/src/app/common/media.cljc +++ b/common/src/app/common/media.cljc @@ -22,6 +22,9 @@ "image/gif" "image/svg+xml"}) +(def tempfile-types + (conj image-types "application/pdf")) + (defn format->extension [format] (case format