From 4f7bb94bb1f00037dd6d2c0b2aa2c2a7e87c0ddb Mon Sep 17 00:00:00 2001 From: Andrey Antukh Date: Wed, 5 Aug 2026 17:35:59 +0200 Subject: [PATCH 01/13] :bug: Add size limit and rate limiting to send-user-feedback (#10979) (#10990) Prevent email bombing attacks on the send-user-feedback endpoint by limiting the error-report field to 1MiB and adding climit rate limits: by-profile (1 permit, queue 3) and global (4 permits), configured in climit.edn. Make the schema public so it can be exercised by tests, and add schema validation tests covering the new size limit. AI-assisted-by: qwen3.7-plus --- backend/resources/climit.edn | 8 +++- backend/src/app/rpc/commands/feedback.clj | 9 +++-- .../test/backend_tests/rpc_feedback_test.clj | 39 +++++++++++++++++++ 3 files changed, 52 insertions(+), 4 deletions(-) create mode 100644 backend/test/backend_tests/rpc_feedback_test.clj diff --git a/backend/resources/climit.edn b/backend/resources/climit.edn index 7d8234499b..66ac82b174 100644 --- a/backend/resources/climit.edn +++ b/backend/resources/climit.edn @@ -39,4 +39,10 @@ {:permits 3} :create-file-snapshot/by-profile - {:permits 1 :queue 2 :timeout 60000}} + {:permits 1 :queue 2 :timeout 60000} + + :send-user-feedback/global + {:permits 4} + + :send-user-feedback/by-profile + {:permits 1 :queue 3}} diff --git a/backend/src/app/rpc/commands/feedback.clj b/backend/src/app/rpc/commands/feedback.clj index 565f41d30e..b70341fc33 100644 --- a/backend/src/app/rpc/commands/feedback.clj +++ b/backend/src/app/rpc/commands/feedback.clj @@ -14,22 +14,25 @@ [app.db :as db] [app.email :as eml] [app.rpc :as-alias rpc] + [app.rpc.climit :as-alias climit] [app.rpc.commands.profile :as profile] [app.rpc.doc :as-alias doc] [app.util.services :as sv])) (declare ^:private send-user-feedback!) -(def ^:private schema:send-user-feedback +(def schema:send-user-feedback [:map {:title "send-user-feedback"} [:subject [:string {:max 500}]] [:content [:string {:max 2500}]] [:type {:optional true} :string] [:error-href {:optional true} [:string {:max 2500}]] - [:error-report {:optional true} :string]]) + [:error-report {:optional true} [:string {:max 1048576}]]]) (sv/defmethod ::send-user-feedback - {::doc/added "1.18" + {::climit/id [[:send-user-feedback/by-profile ::rpc/profile-id] + [:send-user-feedback/global]] + ::doc/added "1.18" ::sm/params schema:send-user-feedback} [{:keys [::db/pool]} {:keys [::rpc/profile-id] :as params}] (when-not (contains? cf/flags :user-feedback) diff --git a/backend/test/backend_tests/rpc_feedback_test.clj b/backend/test/backend_tests/rpc_feedback_test.clj new file mode 100644 index 0000000000..13231db61b --- /dev/null +++ b/backend/test/backend_tests/rpc_feedback_test.clj @@ -0,0 +1,39 @@ +;; 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.rpc-feedback-test + (:require + [app.common.schema :as sm] + [app.rpc.commands.feedback :as feedback] + [clojure.test :as t])) + +(t/deftest send-user-feedback-schema-validation + (let [schema feedback/schema:send-user-feedback] + + (t/testing "accepts valid feedback with all fields" + (let [params {:subject "Test subject" + :content "Test content" + :type "bug" + :error-href "https://example.com/error" + :error-report "Error details here"}] + (t/is (sm/valid? schema params)))) + + (t/testing "accepts feedback without optional fields" + (let [params {:subject "Test subject" + :content "Test content"}] + (t/is (sm/valid? schema params)))) + + (t/testing "accepts error-report up to 1MiB" + (let [params {:subject "Test subject" + :content "Test content" + :error-report (apply str (repeat 1048576 "x"))}] + (t/is (sm/valid? schema params)))) + + (t/testing "rejects error-report exceeding 1MiB" + (let [params {:subject "Test subject" + :content "Test content" + :error-report (apply str (repeat 1048577 "x"))}] + (t/is (not (sm/valid? schema params))))))) From 9242556da6b87d646d766385c24575cc71a1c6a5 Mon Sep 17 00:00:00 2001 From: Andrey Antukh Date: Wed, 5 Aug 2026 17:37:27 +0200 Subject: [PATCH 02/13] :bug: Close import-binfile schema and remove file-id parameter (#10994) Add :closed true to schema:import-binfile to reject unknown keys. Remove file-id from handler destructuring, config binding, and audit props to prevent specifying a target file on import. AI-assisted-by: mimo-v2.5-pro --- backend/src/app/rpc/commands/binfile.clj | 21 +++------- .../test/backend_tests/rpc_binfile_test.clj | 42 +++++++++++++++++++ 2 files changed, 47 insertions(+), 16 deletions(-) create mode 100644 backend/test/backend_tests/rpc_binfile_test.clj diff --git a/backend/src/app/rpc/commands/binfile.clj b/backend/src/app/rpc/commands/binfile.clj index 74101eadbc..78ba08e5fd 100644 --- a/backend/src/app/rpc/commands/binfile.clj +++ b/backend/src/app/rpc/commands/binfile.clj @@ -118,11 +118,10 @@ (def ^:private schema:import-binfile [:and - [:map {:title "import-binfile"} + [:map {:title "import-binfile" :closed true} [:name [:or [:string {:max 250}] [:map-of ::sm/uuid [:string {:max 250}]]]] [:project-id ::sm/uuid] - [:file-id {:optional true} ::sm/uuid] [:version {:optional true} ::sm/int] [:file {:optional true} media.v/schema:upload] [:upload-id {:optional true} ::sm/uuid]] @@ -131,35 +130,26 @@ (or (some? file) (some? upload-id)))]]) (sv/defmethod ::import-binfile - "Import a penpot file in a binary format. If `file-id` is provided, - an in-place import will be performed instead of creating a new file. - - The in-place imports are only supported for binfile-v3 and when a - .penpot file only contains one penpot file. + "Import a penpot file in a binary format. The file content may be provided either as a multipart `file` upload or as an `upload-id` referencing a completed chunked-upload session, which allows importing files larger than the multipart size limit. " {::doc/added "1.15" - ::doc/changes ["1.20" "Add file-id param for in-place import" - "1.20" "Set default version to 3" - "2.15" "Add upload-id param for chunked upload support"] + ::doc/changes [["1.20" "Set default version to 3"] + ["2.15" "Add upload-id param for chunked upload support"]] ::webhooks/event? true ::sse/stream? true ::sm/params schema:import-binfile} - [{:keys [::db/pool] :as cfg} {:keys [::rpc/profile-id project-id version file-id upload-id] :as params}] + [{:keys [::db/pool] :as cfg} {:keys [::rpc/profile-id project-id version upload-id] :as params}] (projects/check-edition-permissions! pool profile-id project-id) (let [version (or version 3) params (-> params (assoc :profile-id profile-id) (assoc :version version)) - cfg (cond-> cfg - (uuid? file-id) - (assoc ::bfc/file-id file-id)) - params (if (some? upload-id) (let [file (db/tx-run! cfg media-cmd/assemble-chunks upload-id)] @@ -174,6 +164,5 @@ (with-meta (sse/response (partial import-binfile cfg params)) {::audit/props {:file nil - :file-id file-id :generated-by (:generated-by manifest) :referer (:referer manifest)}}))) diff --git a/backend/test/backend_tests/rpc_binfile_test.clj b/backend/test/backend_tests/rpc_binfile_test.clj new file mode 100644 index 0000000000..5ebf83bf18 --- /dev/null +++ b/backend/test/backend_tests/rpc_binfile_test.clj @@ -0,0 +1,42 @@ +;; 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.rpc-binfile-test + (:require + [app.common.schema :as sm] + [app.common.uuid :as uuid] + [app.rpc :as-alias rpc] + [app.rpc.commands.binfile :as binfile] + [backend-tests.helpers :as th] + [clojure.test :as t] + [datoteka.fs :as fs])) + +(t/use-fixtures :once th/state-init) +(t/use-fixtures :each th/database-reset) + +(t/deftest import-binfile-schema-rejects-file-id + ;; N1-06: file-id parameter must be removed from schema for security + ;; The schema should not accept file-id as a valid parameter + (let [schema @#'binfile/schema:import-binfile + validator (sm/lazy-validator schema) + + ;; Valid params without file-id + valid-params {:name "test" + :project-id (uuid/random) + :version 3 + :upload-id (uuid/random)} + + ;; Params with file-id (should be rejected after fix) + params-with-file-id (assoc valid-params :file-id (uuid/random))] + + ;; Valid params without file-id should pass + (t/is (true? (validator valid-params)) + "params without file-id should be valid") + + ;; Params with file-id should fail validation after fix + ;; (Currently this will fail because file-id is still in schema) + (t/is (false? (validator params-with-file-id)) + "params with file-id should be rejected"))) From fb0727389791a92303d6fffaba0cf05b3aa400dc Mon Sep 17 00:00:00 2001 From: Andrey Antukh Date: Wed, 5 Aug 2026 17:40:04 +0200 Subject: [PATCH 03/13] :bug: Validate library belongs to same team in link/unlink/sync handlers (#11016) Add check-library-team-ownership! helper that verifies both the file and library share the same team before creating or modifying library relations. This prevents cross-team library injection where a user with edit permissions on files in different teams could link them across team boundaries. Applied to link-file-to-library, unlink-file-from-library, and update-file-library-sync-status handlers. AI-assisted-by: mimo-v2.5 --- backend/src/app/rpc/commands/files.clj | 22 ++++++++++++++ backend/test/backend_tests/rpc_file_test.clj | 32 ++++++++++++++++++++ 2 files changed, 54 insertions(+) diff --git a/backend/src/app/rpc/commands/files.clj b/backend/src/app/rpc/commands/files.clj index e10c85a7bd..69bead539d 100644 --- a/backend/src/app/rpc/commands/files.clj +++ b/backend/src/app/rpc/commands/files.clj @@ -1069,6 +1069,25 @@ [cfg {:keys [::rpc/profile-id] :as params}] (db/tx-run! cfg delete-file (assoc params :profile-id profile-id))) +;; --- Library relation helpers + +(defn- check-library-team-ownership! + "Verify that file and library belong to the same team. + Prevents cross-team library relation injection." + [conn file-id library-id] + (let [sql "SELECT EXISTS ( + SELECT 1 FROM file AS f + JOIN project AS fp ON (fp.id = f.project_id) + JOIN file AS l ON (l.id = ?) + JOIN project AS lp ON (lp.id = l.project_id) + WHERE f.id = ? AND fp.team_id = lp.team_id + ) AS ok" + row (db/exec-one! conn [sql library-id file-id])] + (when-not (:ok row) + (ex/raise :type :not-found + :code :object-not-found + :hint "file and library must belong to the same team")))) + ;; --- MUTATION COMMAND: link-file-to-library (def sql:link-file-to-library @@ -1104,6 +1123,7 @@ (check-edition-permissions! conn profile-id file-id) (check-edition-permissions! conn profile-id library-id) + (check-library-team-ownership! conn file-id library-id) (let [transitive-deps (bfc/get-libraries cfg [library-id])] (when (contains? transitive-deps file-id) @@ -1135,6 +1155,7 @@ [{:keys [::db/conn] :as cfg} {:keys [::rpc/profile-id file-id library-id] :as params}] (check-edition-permissions! conn profile-id file-id) (check-edition-permissions! conn profile-id library-id) + (check-library-team-ownership! conn file-id library-id) (unlink-file-from-library conn params) nil) @@ -1159,6 +1180,7 @@ [{:keys [::db/conn]} {:keys [::rpc/profile-id file-id library-id] :as params}] (check-edition-permissions! conn profile-id file-id) (check-edition-permissions! conn profile-id library-id) + (check-library-team-ownership! conn file-id library-id) (update-sync conn params)) ;; --- MUTATION COMMAND: ignore-sync diff --git a/backend/test/backend_tests/rpc_file_test.clj b/backend/test/backend_tests/rpc_file_test.clj index 1c07f35971..18ad14e639 100644 --- a/backend/test/backend_tests/rpc_file_test.clj +++ b/backend/test/backend_tests/rpc_file_test.clj @@ -983,6 +983,38 @@ (t/is (some? sync)) (t/is (some? (:synced-at sync))))) +(t/deftest link-file-to-library-rejects-cross-team + ;; N1-08: A file in team2 must not be linked to a library in team1, + ;; even when the user has edit permissions on both (BOLA / CWE-639). + (let [prof1 (th/create-profile* 1) + prof2 (th/create-profile* 2) + team1 (th/create-team* 1 {:profile-id (:id prof1)}) + team2 (th/create-team* 2 {:profile-id (:id prof2)}) + proj1 (th/create-project* 1 {:profile-id (:id prof1) + :team-id (:id team1)}) + proj2 (th/create-project* 2 {:profile-id (:id prof2) + :team-id (:id team2)}) + lib (th/create-file* 1 {:project-id (:id proj1) + :profile-id (:id prof1) + :is-shared true}) + file2 (th/create-file* 2 {:project-id (:id proj2) + :profile-id (:id prof2)})] + + ;; Add prof2 as editor to team1 so they have edit access to the library + (th/db-insert! :team-profile-rel {:team-id (:id team1) + :profile-id (:id prof2) + :is-owner false + :is-admin false + :can-edit true}) + + ;; prof2 tries to link file2 (team2) to lib (team1) — must fail + (let [data {::th/type :link-file-to-library + ::rpc/profile-id (:id prof2) + :file-id (:id file2) + :library-id (:id lib)} + out (th/command! data)] + (t/is (some? (:error out)))))) + (t/deftest update-file-library-sync-status-updates-sync-row (let [profile (th/create-profile* 1) file1 (th/create-file* 1 {:project-id (:default-project-id profile) From 689d3a1be2c3a3bab647c2bea87f74e10598ca5c Mon Sep 17 00:00:00 2001 From: Andrey Antukh Date: Wed, 5 Aug 2026 17:40:58 +0200 Subject: [PATCH 04/13] :bug: Add max-object-size guard to read-obj! in v1 parser (#11018) Prevent unbounded memory allocation when a crafted binfile specifies an excessively large object size. Apply the same 100 MiB limit that read-stream! already enforces. AI-assisted-by: mimo-v2.5 --- backend/src/app/binfile/v1.clj | 4 +++ backend/test/backend_tests/binfile_test.clj | 29 ++++++++++++++++++++- 2 files changed, 32 insertions(+), 1 deletion(-) diff --git a/backend/src/app/binfile/v1.clj b/backend/src/app/binfile/v1.clj index 8dc4120159..5f1834cb74 100644 --- a/backend/src/app/binfile/v1.clj +++ b/backend/src/app/binfile/v1.clj @@ -174,6 +174,10 @@ (assert-mark m :obj) (let [size (read-long! input)] (assert (pos? size) "incorrect header size found on reading header") + (when (> size bfc/max-object-size) + (ex/raise :type :validation + :code :max-file-size-reached + :hint (dm/str "unable to import object with size " size " bytes"))) (let [buff (byte-array size)] (read-bytes! input buff) (fres/decode buff))))) diff --git a/backend/test/backend_tests/binfile_test.clj b/backend/test/backend_tests/binfile_test.clj index 84310241d4..05f1525c5e 100644 --- a/backend/test/backend_tests/binfile_test.clj +++ b/backend/test/backend_tests/binfile_test.clj @@ -8,6 +8,7 @@ "Internal binfile test, no RPC involved" (:require [app.binfile.common :as bfc] + [app.binfile.v1 :as v1] [app.binfile.v3 :as v3] [app.common.features :as cfeat] [app.common.files.validate :as cfv] @@ -25,7 +26,10 @@ [clojure.test :as t] [cuerdas.core :as str] [datoteka.fs :as fs] - [datoteka.io :as io])) + [datoteka.io :as io]) + (:import + java.io.ByteArrayInputStream + java.io.DataInputStream)) (t/use-fixtures :once th/state-init) (t/use-fixtures :each th/database-reset) @@ -202,3 +206,26 @@ (v3/import-files!))] (t/is (= (count result) 1)) (t/is (every? uuid? result))))) + +(t/deftest read-obj-rejects-oversized-buffer + ;; N1-07: read-obj! must reject objects exceeding max-object-size + ;; before attempting to allocate the buffer + (let [size (+ bfc/max-object-size 1) + baos (java.io.ByteArrayOutputStream. 17) + dos (java.io.DataOutputStream. baos)] + (.writeByte dos 5) + (.writeLong dos (long size)) + (.flush dos) + (let [input (java.io.DataInputStream. + (ByteArrayInputStream. (.toByteArray baos)))] + (binding [v1/*position* (atom 0)] + (let [out (try + (v1/read-obj! input) + nil + (catch clojure.lang.ExceptionInfo e + (ex-data e)))] + ;; Without the guard, read-obj! will either OOM or proceed + ;; to read-bytes! on a truncated stream (no :max-file-size-reached). + ;; With the guard, it raises :validation :max-file-size-reached. + (t/is (= :validation (:type out))) + (t/is (= :max-file-size-reached (:code out)))))))) From 0481408531e5447ffc2035b914099c0535f73b59 Mon Sep 17 00:00:00 2001 From: Andrey Antukh Date: Wed, 5 Aug 2026 17:41:59 +0200 Subject: [PATCH 05/13] :bug: Add recursion depth limit to Fressian reader (#11020) Bound read depth at 128 levels to prevent StackOverflowError from crafted deeply-nested payloads. All recursive read handlers go through read-object!, so a single depth check covers all paths. AI-assisted-by: mimo-v2.5-pro --- common/src/app/common/fressian.clj | 13 ++++++++++++- common/test/common_tests/fressian_test.clj | 18 +++++++++++++++++- 2 files changed, 29 insertions(+), 2 deletions(-) diff --git a/common/src/app/common/fressian.clj b/common/src/app/common/fressian.clj index b16d233b42..3de4cc54a7 100644 --- a/common/src/app/common/fressian.clj +++ b/common/src/app/common/fressian.clj @@ -31,6 +31,11 @@ ([^String s, ^String encoding] (.getBytes s encoding))) +;; --- DEPTH TRACKING + +(def ^:dynamic *read-depth* 0) +(def ^:const max-read-depth 128) + ;; --- LOW LEVEL FRESSIAN API (defn write-object! @@ -41,7 +46,13 @@ (defn read-object! [^Reader r] - (.readObject r)) + (when (>= *read-depth* max-read-depth) + (throw (ex-info "maximum Fressian read depth exceeded" + {:type :validation + :code :max-read-depth-reached + :hint "maximum Fressian read depth exceeded"}))) + (binding [*read-depth* (inc *read-depth*)] + (.readObject r))) (defn write-tag! ([^Writer w ^String n] diff --git a/common/test/common_tests/fressian_test.clj b/common/test/common_tests/fressian_test.clj index 9af54464a5..3eda0f34d4 100644 --- a/common/test/common_tests/fressian_test.clj +++ b/common/test/common_tests/fressian_test.clj @@ -21,7 +21,8 @@ (:import java.time.Instant java.time.OffsetDateTime - java.time.ZoneOffset)) + java.time.ZoneOffset + java.util.UUID)) ;; --------------------------------------------------------------------------- ;; Helpers @@ -524,3 +525,18 @@ (t/is (d/ordered-map? rt)) (t/is (= om rt)) (t/is (= (keys om) (keys rt))))) + +(t/deftest decode-rejects-excessive-recursion-depth + ;; N2-01: deeply nested structures must be rejected before stack overflow + (let [depth (+ fres/max-read-depth 50) + data (reduce (fn [acc _i] [acc]) + :leaf + (range depth)) + encoded (fres/encode data)] + (try + (fres/decode encoded) + (t/is false "expected exception for excessive recursion depth") + (catch clojure.lang.ExceptionInfo e + (let [d (ex-data e)] + (t/is (= :validation (:type d))) + (t/is (= :max-read-depth-reached (:code d)))))))) From 3d176d539015bb3ad607e69829254cb4292cf8f3 Mon Sep 17 00:00:00 2001 From: Andrey Antukh Date: Wed, 5 Aug 2026 17:42:49 +0200 Subject: [PATCH 06/13] :bug: Restrict webhook creation/edit/delete to team members only (#11029) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit * :bug: Restrict webhook edit/delete to team members only Remove the creator-id fallback from get-webhooks-permissions. Previously, the webhook creator could always edit/delete their webhook even after being removed from the team. Now can-edit comes from team role only — removed users get :not-found. Webhooks are NOT deleted on member removal; the team owns them and team admins/owners manage them. AI-assisted-by: mimo-v2.5-pro * :bug: Restrict webhook creation to team editors Use team role check (check-edition-permissions!) for create-webhook instead of the custom check that allowed any team member to create webhooks via creator-id self-match override. AI-assisted-by: mimo-v2.5-pro --- backend/src/app/rpc/commands/webhooks.clj | 12 +- .../test/backend_tests/rpc_webhooks_test.clj | 160 +++++++++++++----- 2 files changed, 120 insertions(+), 52 deletions(-) diff --git a/backend/src/app/rpc/commands/webhooks.clj b/backend/src/app/rpc/commands/webhooks.clj index 33341bb34e..85051e8ad7 100644 --- a/backend/src/app/rpc/commands/webhooks.clj +++ b/backend/src/app/rpc/commands/webhooks.clj @@ -23,11 +23,9 @@ [cuerdas.core :as str])) (defn get-webhooks-permissions - [conn profile-id team-id creator-id] + [conn profile-id team-id] (let [permissions (t/get-permissions conn profile-id team-id) - - can-edit (boolean (or (:can-edit permissions) - (= profile-id creator-id)))] + can-edit (boolean (:can-edit permissions))] (assoc permissions :can-edit can-edit))) (def has-webhook-edit-permissions? @@ -120,7 +118,7 @@ {::doc/added "1.17" ::sm/params schema:create-webhook} [{:keys [::db/pool] :as cfg} {:keys [::rpc/profile-id team-id] :as params}] - (check-webhook-edition-permissions! pool profile-id team-id profile-id) + (t/check-edition-permissions! pool profile-id team-id) (validate-quotes! cfg params) (validate-webhook! cfg nil params) (insert-webhook! cfg params)) @@ -137,7 +135,7 @@ ::sm/params schema:update-webhook} [{:keys [::db/pool] :as cfg} {:keys [::rpc/profile-id id] :as params}] (let [whook (-> (db/get pool :webhook {:id id}) (decode-row))] - (check-webhook-edition-permissions! pool profile-id (:team-id whook) (:profile-id whook)) + (check-webhook-edition-permissions! pool profile-id (:team-id whook)) (validate-webhook! cfg whook params) (update-webhook! cfg whook params))) @@ -151,7 +149,7 @@ ::db/transaction true} [{:keys [::db/conn]} {:keys [::rpc/profile-id id]}] (let [whook (-> (db/get conn :webhook {:id id}) decode-row)] - (check-webhook-edition-permissions! conn profile-id (:team-id whook) (:profile-id whook)) + (check-webhook-edition-permissions! conn profile-id (:team-id whook)) (db/delete! conn :webhook {:id id}) nil)) diff --git a/backend/test/backend_tests/rpc_webhooks_test.clj b/backend/test/backend_tests/rpc_webhooks_test.clj index 3b39c8b52d..df4ae3a622 100644 --- a/backend/test/backend_tests/rpc_webhooks_test.clj +++ b/backend/test/backend_tests/rpc_webhooks_test.clj @@ -155,8 +155,7 @@ :return {:status 200}}] (let [owner (th/create-profile* 1 {:is-active true}) viewer (th/create-profile* 2 {:is-active true}) - team (th/create-team* 1 {:profile-id (:id owner)}) - whook (volatile! nil)] + team (th/create-team* 1 {:profile-id (:id owner)})] (th/create-team-role* {:team-id (:id team) :profile-id (:id viewer) :role :viewer}) @@ -164,52 +163,15 @@ (let [roles (th/db-query :team-profile-rel {:team-id (:id team)})] (t/is (= 2 (count roles)))) - (t/testing "viewer creates a webhook" + (t/testing "viewer cannot create a webhook (requires editor role)" (let [viewers-webhook (create-webhook-params (:id viewer) (:id team)) out (th/command! viewers-webhook)] - (t/is (nil? (:error out))) - (t/is (= 1 (:call-count @http-mock))) - - (let [result (:result out)] - (check-webhook-format result) - (t/is (= (:uri viewers-webhook) (:uri result))) - (t/is (= (:team-id viewers-webhook) (:team-id result))) - (t/is (= (::rpc/profile-id viewers-webhook) (:profile-id result))) - (t/is (= (:mtype viewers-webhook) (:mtype result))) - (vreset! whook result)))) - - (th/reset-mock! http-mock) - - (t/testing "viewer updates it's own webhook (success)" - (let [params {::th/type :update-webhook - ::rpc/profile-id (:id viewer) - :id (:id @whook) - :uri (:uri @whook) - :mtype "application/transit+json" - :is-active false} - out (th/command! params) - result (:result out)] - - (t/is (nil? (:error out))) (t/is (= 0 (:call-count @http-mock))) - (check-webhook-format result) - (t/is (= (:is-active params) (:is-active result))) - (t/is (= (:team-id @whook) (:team-id result))) - (t/is (= (:mtype params) (:mtype result))) - (vreset! whook result))) - - (th/reset-mock! http-mock) - - (t/testing "viewer deletes it's own webhook (success)" - (let [params {::th/type :delete-webhook - ::rpc/profile-id (:id viewer) - :id (:id @whook)} - out (th/command! params)] - (t/is (= 0 (:call-count @http-mock))) - (t/is (nil? (:error out))) - (t/is (nil? (:result out))) - (let [rows (th/db-exec! ["select * from webhook"])] - (t/is (= 0 (count rows)))))) + (let [error (:error out) + error-data (ex-data error)] + (t/is (th/ex-info? error)) + (t/is (= (:type error-data) :not-found)) + (t/is (= (:code error-data) :object-not-found))))) (th/reset-mock! http-mock)))) @@ -268,6 +230,26 @@ (t/is (= (:type error-data) :not-found)) (t/is (= (:code error-data) :object-not-found))))))) +(t/deftest webhooks-viewer-cannot-create + (with-mocks [http-mock {:target 'app.http.client/req + :return {:status 200}}] + (let [owner (th/create-profile* 1 {:is-active true}) + viewer (th/create-profile* 2 {:is-active true}) + team (th/create-team* 1 {:profile-id (:id owner)})] + (th/create-team-role* {:team-id (:id team) + :profile-id (:id viewer) + :role :viewer}) + + (t/testing "viewer cannot create a webhook on the team" + (let [params (create-webhook-params (:id viewer) (:id team)) + out (th/command! params)] + (t/is (= 0 (:call-count @http-mock))) + (let [error (:error out) + error-data (ex-data error)] + (t/is (th/ex-info? error)) + (t/is (= (:type error-data) :not-found)) + (t/is (= (:code error-data) :object-not-found)))))))) + (t/deftest webhooks-quotes (with-mocks [http-mock {:target 'app.http.client/req :return {:status 200}}] @@ -304,3 +286,91 @@ (t/is (th/ex-info? error)) (t/is (= (:type error-data) :restriction)) (t/is (= (:code error-data) :webhooks-quote-reached)))))) + +(t/deftest removed-user-cannot-edit-webhook + (with-mocks [http-mock {:target 'app.http.client/req + :return {:status 200}}] + + (let [owner (th/create-profile* 1 {:is-active true}) + editor (th/create-profile* 2 {:is-active true}) + team (th/create-team* 1 {:profile-id (:id owner)})] + + (th/create-team-role* {:team-id (:id team) + :profile-id (:id editor) + :role :editor}) + + (let [params {::th/type :create-webhook + ::rpc/profile-id (:id editor) + :team-id (:id team) + :uri (u/uri "http://example.com") + :mtype "application/json"} + out (th/command! params)] + + (t/is (nil? (:error out))) + (let [whook (:result out)] + + (th/reset-mock! http-mock) + + (t/testing "owner can edit editor's webhook (team owns it)" + (let [params {::th/type :update-webhook + ::rpc/profile-id (:id owner) + :id (:id whook) + :uri (u/uri "http://example.com/updated") + :mtype "application/transit+json" + :is-active true} + out (th/command! params)] + (t/is (nil? (:error out))) + (t/is (= 1 (:call-count @http-mock))))) + + (th/reset-mock! http-mock) + + (t/testing "remove editor from team" + (let [params {::th/type :delete-team-member + ::rpc/profile-id (:id owner) + :team-id (:id team) + :member-id (:id editor)} + out (th/command! params)] + (t/is (nil? (:error out))))) + + (th/reset-mock! http-mock) + + (t/testing "removed editor cannot update webhook" + (let [params {::th/type :update-webhook + ::rpc/profile-id (:id editor) + :id (:id whook) + :uri (u/uri "http://example.com/evil") + :mtype "application/transit+json" + :is-active true} + out (th/command! params)] + (t/is (= 0 (:call-count @http-mock))) + (let [error (:error out) + error-data (ex-data error)] + (t/is (th/ex-info? error)) + (t/is (= (:type error-data) :not-found)) + (t/is (= (:code error-data) :object-not-found))))) + + (th/reset-mock! http-mock) + + (t/testing "removed editor cannot delete webhook" + (let [params {::th/type :delete-webhook + ::rpc/profile-id (:id editor) + :id (:id whook)} + out (th/command! params)] + (t/is (= 0 (:call-count @http-mock))) + (let [error (:error out) + error-data (ex-data error)] + (t/is (th/ex-info? error)) + (t/is (= (:type error-data) :not-found)) + (t/is (= (:code error-data) :object-not-found))))) + + (th/reset-mock! http-mock) + + (t/testing "owner can still delete editor's webhook" + (let [params {::th/type :delete-webhook + ::rpc/profile-id (:id owner) + :id (:id whook)} + out (th/command! params)] + (t/is (nil? (:error out))) + (t/is (nil? (:result out))) + (let [rows (th/db-exec! ["select * from webhook"])] + (t/is (= 0 (count rows))))))))))) From 25066c2f46987c696f2c077e6047e6ec6ebecb21 Mon Sep 17 00:00:00 2001 From: Andrey Antukh Date: Wed, 5 Aug 2026 17:44:01 +0200 Subject: [PATCH 07/13] :bug: Require file read permissions for asset endpoints (#11036) Add authorization check to generic-handler in assets.clj so that /assets/by-file-media-id/:id and its /thumbnail variant verify the requesting profile has read access to the parent file. Return 404 (not 403) when access is denied to avoid confirming existence. Also switch get-file-media-object from db/get to db/get* so that non-existent media objects return nil instead of raising. AI-assisted-by: mimo-v2.5-pro --- backend/src/app/http/assets.clj | 25 ++-- .../test/backend_tests/http_assets_test.clj | 129 ++++++++++++++++++ 2 files changed, 146 insertions(+), 8 deletions(-) diff --git a/backend/src/app/http/assets.clj b/backend/src/app/http/assets.clj index 1458b06d27..04dd7842ca 100644 --- a/backend/src/app/http/assets.clj +++ b/backend/src/app/http/assets.clj @@ -7,6 +7,7 @@ (ns app.http.assets "Assets related handlers." (:require + [app.binfile.common :as bfc] [app.common.data :as d] [app.common.exceptions :as ex] [app.common.time :as ct] @@ -42,7 +43,7 @@ (defn- get-file-media-object [pool id] - (db/get pool :file-media-object {:id id} {::db/remove-deleted false})) + (db/get* pool :file-media-object {:id id} {::db/remove-deleted false})) (defn- serve-object-from-s3 [{:keys [::sto/storage ::signature-max-age ::cache-max-age] :as cfg} obj] @@ -109,13 +110,21 @@ (defn- generic-handler "A generic handler helper/common code for file-media based handlers." [{:keys [::sto/storage] :as cfg} request kf] - (let [pool (::db/pool storage) - id (get-id request) - mobj (get-file-media-object pool id) - sobj (sto/get-object storage (kf mobj))] - (if sobj - (serve-object cfg sobj) - {::yres/status 404}))) + (let [pool (::db/pool storage) + id (get-id request) + mobj (get-file-media-object pool id)] + (if (nil? mobj) + {::yres/status 404} + (let [file-id (:file-id mobj) + profile-id (or (::session/profile-id request) + (::actoken/profile-id request)) + perms (bfc/get-file-permissions pool profile-id file-id)] + (if-not (:can-read perms) + {::yres/status 404} + (let [sobj (sto/get-object storage (kf mobj))] + (if sobj + (serve-object cfg sobj) + {::yres/status 404}))))))) (defn file-objects-handler "Handler that serves storage objects by file media id." diff --git a/backend/test/backend_tests/http_assets_test.clj b/backend/test/backend_tests/http_assets_test.clj index 796306efe2..94510d73d6 100644 --- a/backend/test/backend_tests/http_assets_test.clj +++ b/backend/test/backend_tests/http_assets_test.clj @@ -459,6 +459,135 @@ ;; Tests: objects-handler — expired objects ;; ---------------------------------------------------------------- +;; ---------------------------------------------------------------- +;; Tests: file-objects-handler — authz required (T2-N1-01) +;; ---------------------------------------------------------------- + +(t/deftest file-objects-handler-unauthenticated-returns-404 + ;; Unauthenticated requests to file-media assets must return 404 + (let [storage (-> (:app.storage/storage th/*system*) + (configure-storage-backend)) + cfg (make-handler-cfg storage) + profile (th/create-profile* 1) + team (th/create-team* 1 {:profile-id (:id profile)}) + project (th/create-project* 1 {:profile-id (:id profile) + :team-id (:id team)}) + file (th/create-file* 1 {:profile-id (:id profile) + :project-id (:id project)}) + media-storage (create-storage-object! storage "file-media-object" "image data") + media-obj (th/create-file-media-object* {:file-id (:id file) + :media-id (:id media-storage)}) + request {:path-params {:id (str (:id media-obj))}} + response (assets/file-objects-handler cfg request)] + (t/is (= 404 (::yres/status response))))) + +(t/deftest file-objects-handler-no-file-perms-returns-404 + ;; Authenticated user without file read permissions must get 404 + (let [storage (-> (:app.storage/storage th/*system*) + (configure-storage-backend)) + cfg (make-handler-cfg storage) + owner (th/create-profile* 1) + team (th/create-team* 1 {:profile-id (:id owner)}) + project (th/create-project* 1 {:profile-id (:id owner) + :team-id (:id team)}) + file (th/create-file* 1 {:profile-id (:id owner) + :project-id (:id project)}) + media-storage (create-storage-object! storage "file-media-object" "image data") + media-obj (th/create-file-media-object* {:file-id (:id file) + :media-id (:id media-storage)}) + stranger (th/create-profile* 2) + request {:path-params {:id (str (:id media-obj))} + ::session/profile-id (:id stranger)} + response (assets/file-objects-handler cfg request)] + (t/is (= 404 (::yres/status response))))) + +(t/deftest file-objects-handler-with-file-perms-succeeds + ;; Authenticated user with file read permissions must get the object + (let [storage (-> (:app.storage/storage th/*system*) + (configure-storage-backend)) + cfg (make-handler-cfg storage) + owner (th/create-profile* 1) + team (th/create-team* 1 {:profile-id (:id owner)}) + project (th/create-project* 1 {:profile-id (:id owner) + :team-id (:id team)}) + file (th/create-file* 1 {:profile-id (:id owner) + :project-id (:id project)}) + media-storage (create-storage-object! storage "file-media-object" "image data") + media-obj (th/create-file-media-object* {:file-id (:id file) + :media-id (:id media-storage)}) + request {:path-params {:id (str (:id media-obj))} + ::session/profile-id (:id owner)} + response (assets/file-objects-handler cfg request)] + (t/is (= 204 (::yres/status response))))) + +(t/deftest file-thumbnails-handler-unauthenticated-returns-404 + ;; Unauthenticated requests to file-thumbnail assets must return 404 + (let [storage (-> (:app.storage/storage th/*system*) + (configure-storage-backend)) + cfg (make-handler-cfg storage) + profile (th/create-profile* 1) + team (th/create-team* 1 {:profile-id (:id profile)}) + project (th/create-project* 1 {:profile-id (:id profile) + :team-id (:id team)}) + file (th/create-file* 1 {:profile-id (:id profile) + :project-id (:id project)}) + media-storage (create-storage-object! storage "file-media-object" "image data") + media-obj (th/create-file-media-object* {:file-id (:id file) + :media-id (:id media-storage)}) + request {:path-params {:id (str (:id media-obj))}} + response (assets/file-thumbnails-handler cfg request)] + (t/is (= 404 (::yres/status response))))) + +(t/deftest file-thumbnails-handler-with-file-perms-succeeds + ;; Authenticated user with file read permissions must get the thumbnail + (let [storage (-> (:app.storage/storage th/*system*) + (configure-storage-backend)) + cfg (make-handler-cfg storage) + owner (th/create-profile* 1) + team (th/create-team* 1 {:profile-id (:id owner)}) + project (th/create-project* 1 {:profile-id (:id owner) + :team-id (:id team)}) + file (th/create-file* 1 {:profile-id (:id owner) + :project-id (:id project)}) + thumb-storage (create-storage-object! storage "file-object-thumbnail" "thumb data") + media-obj (th/create-file-media-object* {:file-id (:id file) + :media-id (:id thumb-storage)}) + request {:path-params {:id (str (:id media-obj))} + ::session/profile-id (:id owner)} + response (assets/file-thumbnails-handler cfg request)] + ;; Falls back to media-id since no thumbnail-id, but still serves + (t/is (= 204 (::yres/status response))))) + +(t/deftest file-objects-handler-non-existent-media-returns-404 + ;; Request for non-existent file-media-object returns 404 + (let [storage (-> (:app.storage/storage th/*system*) + (configure-storage-backend)) + cfg (make-handler-cfg storage) + profile (th/create-profile* 1) + request {:path-params {:id (str (uuid/next))} + ::session/profile-id (:id profile)} + response (assets/file-objects-handler cfg request)] + (t/is (= 404 (::yres/status response))))) + +(t/deftest file-objects-handler-nil-profile-id-returns-404 + ;; When profile-id is nil (invalid session), must return 404 + (let [storage (-> (:app.storage/storage th/*system*) + (configure-storage-backend)) + cfg (make-handler-cfg storage) + profile (th/create-profile* 1) + team (th/create-team* 1 {:profile-id (:id profile)}) + project (th/create-project* 1 {:profile-id (:id profile) + :team-id (:id team)}) + file (th/create-file* 1 {:profile-id (:id profile) + :project-id (:id project)}) + media-storage (create-storage-object! storage "file-media-object" "image data") + media-obj (th/create-file-media-object* {:file-id (:id file) + :media-id (:id media-storage)}) + request {:path-params {:id (str (:id media-obj))} + ::session/profile-id nil} + response (assets/file-objects-handler cfg request)] + (t/is (= 404 (::yres/status response))))) + (t/deftest objects-handler-expired-object ;; Expired objects should return 404 (get-object filters them out). (let [storage (-> (:app.storage/storage th/*system*) From 5906312dff3bf5812e4a87f74307fc6afd52a736 Mon Sep 17 00:00:00 2001 From: Andrey Antukh Date: Wed, 5 Aug 2026 17:52:14 +0200 Subject: [PATCH 08/13] :bug: Normalize error response on duplicate file ID (#11050) Capture unique constraint violation in insert-file! and return generic :not-found error instead of propagating raw PostgreSQL exception, preventing file existence oracle. AI-assisted-by: mimo-v2.5-pro --- backend/src/app/binfile/common.clj | 14 ++++++++--- backend/test/backend_tests/rpc_file_test.clj | 25 ++++++++++++++++++++ 2 files changed, 36 insertions(+), 3 deletions(-) diff --git a/backend/src/app/binfile/common.clj b/backend/src/app/binfile/common.clj index a5b73564ea..f984a98550 100644 --- a/backend/src/app/binfile/common.clj +++ b/backend/src/app/binfile/common.clj @@ -748,9 +748,17 @@ (fmigr/upsert-migrations! conn file)) (let [file (encode-file cfg file)] - (db/insert! conn :file - (file->params file) - (assoc opts ::db/return-keys false)) + (try + (db/insert! conn :file + (file->params file) + (assoc opts ::db/return-keys false)) + (catch org.postgresql.util.PSQLException cause + (if (db/duplicate-key-error? cause) + (ex/raise :type :not-found + :code :object-not-found + :hint "file already exists" + :cause cause) + (throw cause)))) (->> (file->file-data-params file) (fdata/upsert! cfg)) diff --git a/backend/test/backend_tests/rpc_file_test.clj b/backend/test/backend_tests/rpc_file_test.clj index 18ad14e639..cb0997576d 100644 --- a/backend/test/backend_tests/rpc_file_test.clj +++ b/backend/test/backend_tests/rpc_file_test.clj @@ -141,6 +141,31 @@ (let [result (:result out)] (t/is (= 0 (count result)))))))) +(t/deftest create-file-with-duplicate-id + (let [prof (th/create-profile* 1 {:is-active true}) + proj-id (:default-project-id prof) + file-id (uuid/next)] + + (t/testing "create file with specific id" + (let [data {::th/type :create-file + ::rpc/profile-id (:id prof) + :project-id proj-id + :id file-id + :name "first-file"} + out (th/command! data)] + (t/is (nil? (:error out))))) + + (t/testing "create file with duplicate id returns normalized error" + (let [data {::th/type :create-file + ::rpc/profile-id (:id prof) + :project-id proj-id + :id file-id + :name "duplicate-file"} + out (th/command! data) + err (:error out)] + (t/is (th/ex-info? err)) + (t/is (th/ex-of-type? err :not-found)))))) + (t/deftest file-gc-with-fragments (let [profile (th/create-profile* 1) file (th/create-file* 1 {:profile-id (:id profile) From bf62e59f731698c96bf2a7e40c5e59ba8b1908dd Mon Sep 17 00:00:00 2001 From: Andrey Antukh Date: Wed, 5 Aug 2026 17:53:15 +0200 Subject: [PATCH 09/13] :bug: Add cooldown to prevent duplicate invitation emails (#11063) --- .../app/rpc/commands/teams_invitations.clj | 91 ++++++++++++------- backend/test/backend_tests/rpc_team_test.clj | 40 ++++++++ 2 files changed, 96 insertions(+), 35 deletions(-) diff --git a/backend/src/app/rpc/commands/teams_invitations.clj b/backend/src/app/rpc/commands/teams_invitations.clj index 8b1a8c357c..7cd8933354 100644 --- a/backend/src/app/rpc/commands/teams_invitations.clj +++ b/backend/src/app/rpc/commands/teams_invitations.clj @@ -46,10 +46,29 @@ (def sql:upsert-organization-invitation "insert into team_invitation(id, team_id, org_id, email_to, created_by, role, valid_until) - values (?, null, ?, ?, ?, ?, ?) - on conflict(org_id, email_to) where team_id is null do - update set role = ?, valid_until = ?, updated_at = now() - returning *") + values (?, null, ?, ?, ?, ?, ?) + on conflict(org_id, email_to) where team_id is null do + update set role = ?, valid_until = ?, updated_at = now() + returning *") + +(def ^:private sql:check-recent-invitation + "SELECT 1 FROM team_invitation + WHERE team_id = ? AND email_to = ? + AND updated_at > now() - interval '5 minutes' + LIMIT 1") + +(def ^:private sql:check-recent-org-invitation + "SELECT 1 FROM team_invitation + WHERE org_id = ? AND email_to = ? + AND updated_at > now() - interval '5 minutes' + LIMIT 1") + +(defn- recently-invited? + [{:keys [::db/conn]} team-id org-id email] + (let [query (if org-id + [sql:check-recent-org-invitation org-id email] + [sql:check-recent-invitation team-id email])] + (some? (db/exec-one! conn query)))) (defn- create-invitation-token [cfg {:keys [profile-id valid-until organization-id organization-name team-id member-id member-email role]}] @@ -185,35 +204,36 @@ (teams/check-email-bounce conn email true) (teams/check-email-spam conn email true) - (let [id (uuid/next) - expire (if organization - (ct/in-future "876000h") ;; Organization invitations doesn't expire - (ct/in-future "168h")) ;; 7 days - invitation (db/exec-one! conn (if organization - [sql:upsert-organization-invitation id - (:id organization) - (str/lower email) - (:id profile) - (name role) expire - (name role) expire] - [sql:upsert-team-invitation id - (:id team) - (str/lower email) - (:id profile) - (name role) expire - (name role) expire])) - updated? (not= id (:id invitation)) - profile-id (:id profile) + (let [id (uuid/next) + expire (if organization + (ct/in-future "876000h") ;; Organization invitations doesn't expire + (ct/in-future "168h")) ;; 7 days + recent? (recently-invited? cfg (:id team) (:id organization) email) + invitation (db/exec-one! conn (if organization + [sql:upsert-organization-invitation id + (:id organization) + (str/lower email) + (:id profile) + (name role) expire + (name role) expire] + [sql:upsert-team-invitation id + (:id team) + (str/lower email) + (:id profile) + (name role) expire + (name role) expire])) + updated? (not= id (:id invitation)) + profile-id (:id profile) team-organization-id (get-in team [:organization :id]) - tprops {:profile-id profile-id - :invitation-id (:id invitation) - :valid-until expire - :team-id (:id team) - :organization-id (:id organization) - :organization-name (:name organization) - :member-email (:email-to invitation) - :member-id (:id member) - :role role} + tprops {:profile-id profile-id + :invitation-id (:id invitation) + :valid-until expire + :team-id (:id team) + :organization-id (:id organization) + :organization-name (:name organization) + :member-email (:email-to invitation) + :member-id (:id member) + :role role} audit-props (cond-> {:invitation-id (:id invitation) :valid-until expire @@ -234,8 +254,8 @@ (and team-organization-id member (contains? all-organization-member-ids (:id member)))))) - itoken (create-invitation-token cfg tprops) - ptoken (create-profile-identity-token cfg profile-id)] + itoken (create-invitation-token cfg tprops) + ptoken (create-profile-identity-token cfg profile-id)] (when (contains? cf/flags :log-invitation-tokens) (l/info :hint "invitation token" :token itoken)) @@ -251,7 +271,8 @@ (assoc :props props))] (audit/submit cfg event)) - (when (allow-invitation-emails? member) + (when (and (allow-invitation-emails? member) + (not recent?)) (if organization (when (contains? cf/flags :admin-console) (eml/send! {::eml/conn conn diff --git a/backend/test/backend_tests/rpc_team_test.clj b/backend/test/backend_tests/rpc_team_test.clj index 7c2b5d0552..5c0477ed17 100644 --- a/backend/test/backend_tests/rpc_team_test.clj +++ b/backend/test/backend_tests/rpc_team_test.clj @@ -1015,6 +1015,46 @@ out (th/command! data)] (t/is (th/success? out))))) +(t/deftest create-team-invitations-email-cooldown + (with-mocks [mock {:target 'app.email/send! :return nil}] + (let [profile1 (th/create-profile* 1 {:is-active true}) + team (th/create-team* 1 {:profile-id (:id profile1)}) + + data {::th/type :create-team-invitations + ::rpc/profile-id (:id profile1) + :team-id (:id team) + :role :editor + :emails ["cooldown-test@example.com"]}] + + ;; First invitation sends email + (let [out (th/command! data)] + (t/is (th/success? out)) + (t/is (= 1 (:call-count @mock)))) + + ;; Resending immediately should NOT send email (cooldown active) + (th/reset-mock! mock) + (let [out (th/command! data)] + (t/is (th/success? out)) + (t/is (= 0 (:call-count @mock)))) + + ;; Resending to a different email should send email + (th/reset-mock! mock) + (let [data (assoc data :emails ["different@example.com"]) + out (th/command! data)] + (t/is (th/success? out)) + (t/is (= 1 (:call-count @mock)))) + + ;; After cooldown expires, resending should send email + (th/reset-mock! mock) + (th/db-update! :team-invitation + {:updated-at (ct/in-past "10m")} + {:team-id (:id team) + :email-to "cooldown-test@example.com"}) + (let [data (assoc data :emails ["cooldown-test@example.com"]) + out (th/command! data)] + (t/is (th/success? out)) + (t/is (= 1 (:call-count @mock))))))) + (t/deftest update-team-with-invalid-name (let [profile (th/create-profile* 1 {:is-active true}) team (th/create-team* 1 {:profile-id (:id profile)})] From 0ac711aa68a317c739f41b8cc06c75dcd46bb270 Mon Sep 17 00:00:00 2001 From: Andrey Antukh Date: Wed, 5 Aug 2026 17:54:07 +0200 Subject: [PATCH 10/13] :bug: Normalize string inputs to prevent unfiltered echo (#11061) Add normalize-string helper in app.common.data that trims whitespace and returns empty string for nil input. Apply to profile, team, and project string fields (fullname, lang, theme, name) before storage. AI-assisted-by: qwen3.7-plus --- backend/src/app/rpc/commands/auth.clj | 8 ++++++-- backend/src/app/rpc/commands/profile.clj | 3 +++ backend/src/app/rpc/commands/projects.clj | 4 +++- backend/src/app/rpc/commands/teams.clj | 9 ++++++--- common/src/app/common/data.cljc | 9 +++++++++ common/test/common_tests/data_test.cljc | 18 ++++++++++++++++++ 6 files changed, 45 insertions(+), 6 deletions(-) diff --git a/backend/src/app/rpc/commands/auth.clj b/backend/src/app/rpc/commands/auth.clj index 5444273862..933411a489 100644 --- a/backend/src/app/rpc/commands/auth.clj +++ b/backend/src/app/rpc/commands/auth.clj @@ -258,7 +258,8 @@ (validate-register-attempt! cfg params) (let [email (profile/clean-email email) - profile (profile/get-profile-by-email pool email)] + profile (profile/get-profile-by-email pool email) + fullname (d/normalize-string fullname)] ;; SECURITY: refuse to issue a prepared-register token when an active ;; profile already exists for this email. @@ -359,6 +360,9 @@ is-active (:is-active params false) theme (:theme params nil) email (str/lower email) + fullname (d/normalize-string (:fullname params)) + locale (d/normalize-string locale) + theme (d/normalize-string theme) photo-id (some->> (or (:oidc/picture props) (:google/picture props) @@ -367,7 +371,7 @@ (import-profile-picture cfg)) params {:id id - :fullname (:fullname params) + :fullname fullname :email email :auth-backend backend :lang locale diff --git a/backend/src/app/rpc/commands/profile.clj b/backend/src/app/rpc/commands/profile.clj index ed4d22f445..36d02ba2d9 100644 --- a/backend/src/app/rpc/commands/profile.clj +++ b/backend/src/app/rpc/commands/profile.clj @@ -164,6 +164,9 @@ ;; it or not for explicit locking and avoid concurrent updates of ;; the same row/object. (let [profile (get-profile conn profile-id ::db/for-update true) + fullname (d/normalize-string fullname) + lang (d/normalize-string lang) + theme (d/normalize-string theme) ;; Update the profile map with direct params profile (-> profile (assoc :fullname fullname) diff --git a/backend/src/app/rpc/commands/projects.clj b/backend/src/app/rpc/commands/projects.clj index 12da9bb7c5..cfb03a2f0d 100644 --- a/backend/src/app/rpc/commands/projects.clj +++ b/backend/src/app/rpc/commands/projects.clj @@ -6,6 +6,7 @@ (ns app.rpc.commands.projects (:require + [app.common.data :as d] [app.common.data.macros :as dm] [app.common.exceptions :as ex] [app.common.schema :as sm] @@ -259,7 +260,8 @@ ::db/transaction true} [{:keys [::db/conn]} {:keys [::rpc/profile-id id name] :as params}] (check-edition-permissions! conn profile-id id) - (let [project (db/get-by-id conn :project id ::sql/for-update true)] + (let [project (db/get-by-id conn :project id ::sql/for-update true) + name (d/normalize-string name)] (db/update! conn :project {:name name} {:id id}) diff --git a/backend/src/app/rpc/commands/teams.clj b/backend/src/app/rpc/commands/teams.clj index 9277a803c1..196b35c051 100644 --- a/backend/src/app/rpc/commands/teams.clj +++ b/backend/src/app/rpc/commands/teams.clj @@ -652,6 +652,7 @@ (let [id (or id (uuid/next)) is-default (if (boolean? is-default) is-default false) features (db/create-array conn "text" features) + name (d/normalize-string name) team (db/insert! conn :team {:id id :name name @@ -688,6 +689,7 @@ [conn {:keys [id team-id name is-default created-at modified-at]}] (let [id (or id (uuid/next)) is-default (if (boolean? is-default) is-default false) + name (d/normalize-string name) params {:id id :name name :team-id team-id @@ -718,9 +720,10 @@ ::db/transaction true} [{:keys [::db/conn] :as cfg} {:keys [::rpc/profile-id id name]}] (check-edition-permissions! conn profile-id id) - (db/update! conn :team - {:name name} - {:id id}) + (let [name (d/normalize-string name)] + (db/update! conn :team + {:name name} + {:id id})) nil) diff --git a/common/src/app/common/data.cljc b/common/src/app/common/data.cljc index 7cbfdcc4f5..418c6e5bd5 100644 --- a/common/src/app/common/data.cljc +++ b/common/src/app/common/data.cljc @@ -1173,6 +1173,15 @@ [key coll] (sort-by key natural-compare coll)) +(defn normalize-string + "Normalizes a string by trimming leading/trailing whitespace. + Returns empty string for nil input. Non-string input is returned unchanged." + [s] + (cond + (nil? s) "" + (string? s) (str/trim s) + :else s)) + (defn sanitize-string [s] (if s (-> s diff --git a/common/test/common_tests/data_test.cljc b/common/test/common_tests/data_test.cljc index 39f3370de8..46f12fd8fb 100644 --- a/common/test/common_tests/data_test.cljc +++ b/common/test/common_tests/data_test.cljc @@ -36,6 +36,24 @@ (t/is (= "" (d/get-initials nil))) (t/is (= "" (d/get-initials "!!! ???")))) +(t/deftest normalize-string-test + ;; nil input returns empty string + (t/is (= "" (d/normalize-string nil))) + ;; empty string returns empty string + (t/is (= "" (d/normalize-string ""))) + ;; leading whitespace is trimmed + (t/is (= "hello" (d/normalize-string " hello"))) + ;; trailing whitespace is trimmed + (t/is (= "hello" (d/normalize-string "hello "))) + ;; both leading and trailing whitespace are trimmed + (t/is (= "hello" (d/normalize-string " hello "))) + ;; internal whitespace is preserved + (t/is (= "hello world" (d/normalize-string " hello world "))) + ;; non-string input is returned unchanged + (t/is (= 42 (d/normalize-string 42))) + (t/is (= :keyword (d/normalize-string :keyword))) + (t/is (= true (d/normalize-string true)))) + ;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;; ;; Ordered Data Structures ;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;; From c4dd04353fcf06c3e10a64e3d8e43945508ae98c Mon Sep 17 00:00:00 2001 From: Andrey Antukh Date: Wed, 29 Jul 2026 10:18:57 +0000 Subject: [PATCH 11/13] :bug: Sanitize SVG files on upload to prevent XSS Add sanitize-svg function that removes dangerous elements and attributes: - script tags - foreignObject elements - Event handler attributes (onload, onmouseover, etc.) - javascript: URLs from href/xlink:href attributes Apply sanitization in process-main-image before storing SVG files. AI-assisted-by: mimo-v2.5-pro --- backend/src/app/media/local.clj | 66 +---------- backend/src/app/media/remote.clj | 4 +- backend/src/app/media/svg.clj | 130 ++++++++++++++++++++++ backend/src/app/rpc/commands/media.clj | 18 ++- backend/test/backend_tests/media_test.clj | 82 ++++++++++++++ 5 files changed, 231 insertions(+), 69 deletions(-) create mode 100644 backend/src/app/media/svg.clj diff --git a/backend/src/app/media/local.clj b/backend/src/app/media/local.clj index b53c5a5f6d..f86e46c02e 100644 --- a/backend/src/app/media/local.clj +++ b/backend/src/app/media/local.clj @@ -7,30 +7,22 @@ (ns app.media.local "Local media processing via ImageMagick and FontForge shell commands." (:require - [app.common.data :as d] - [app.common.data.macros :as dm] [app.common.exceptions :as ex] [app.common.logging :as l] [app.common.media :as cm] [app.common.schema :as sm] [app.common.time :as ct] [app.config :as cf] + [app.media.svg :as svg] [app.media.validation :as validation] [app.storage.tmp :as tmp] [app.util.shell :as shell] [buddy.core.bytes :as bb] [buddy.core.codecs :as bc] [clojure.string] - [clojure.xml :as xml] [cuerdas.core :as str] [datoteka.fs :as fs] - [datoteka.io :as io]) - (:import - clojure.lang.XMLHandler - java.io.InputStream - javax.xml.parsers.SAXParserFactory - javax.xml.XMLConstants - org.apache.commons.io.IOUtils)) + [datoteka.io :as io])) (defmulti process (fn [_system params] (:cmd params))) @@ -40,30 +32,6 @@ :code :not-implemented :hint (str/fmt "No impl found for local process cmd: %s" cmd))) -;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;; -;; SVG PARSING -;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;; - -(defn- secure-parser-factory - [^InputStream input ^XMLHandler handler] - (.. (doto (SAXParserFactory/newInstance) - (.setFeature XMLConstants/FEATURE_SECURE_PROCESSING true) - (.setFeature "http://apache.org/xml/features/disallow-doctype-decl" true)) - (newSAXParser) - (parse input handler))) - -(defn- strip-doctype - [data] - (cond-> data - (str/includes? data "]*>" ""))) - -(defn parse-svg - [text] - (let [text (strip-doctype text)] - (dm/with-open [istream (IOUtils/toInputStream ^String text "UTF-8")] - (xml/parse istream secure-parser-factory)))) - ;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;; ;; IMAGE THUMBNAILS ;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;; @@ -167,34 +135,6 @@ "-extent" (str width "x" height) "-quality" (str quality)])))) -(defn get-basic-info-from-svg - [{:keys [tag attrs] :as data}] - (when (not= tag :svg) - (ex/raise :type :validation - :code :unable-to-parse-svg - :hint "uploaded svg has invalid content")) - (reduce (fn [default f] - (if-let [res (f attrs)] - (reduced res) - default)) - {:width 100 :height 100} - [(fn parse-width-and-height - [{:keys [width height]}] - (when (and (string? width) - (string? height)) - (let [width (d/parse-double width) - height (d/parse-double height)] - (when (and width height) - {:width (int width) - :height (int height)})))) - (fn parse-viewbox - [{:keys [viewBox]}] - (let [[x y width height] (->> (str/split viewBox #"\s+" 4) - (map d/parse-double))] - (when (and x y width height) - {:width (int width) - :height (int height)})))])) - (defn- get-dimensions-with-orientation [system ^String path] ;; Image magick doesn't give info about exif rotation so we use the identify command ;; If we are processing an animated gif we use the first frame with -scene 0 @@ -217,7 +157,7 @@ [system {:keys [input] :as params}] (let [{:keys [path mtype] :as input} (validation/check-input input)] (if (= mtype "image/svg+xml") - (let [info (some-> path slurp parse-svg get-basic-info-from-svg)] + (let [info (some-> path slurp svg/parse-svg svg/get-basic-info-from-svg)] (when-not info (ex/raise :type :validation :code :invalid-svg-file diff --git a/backend/src/app/media/remote.clj b/backend/src/app/media/remote.clj index 447d5f2e55..0b5a0a4a42 100644 --- a/backend/src/app/media/remote.clj +++ b/backend/src/app/media/remote.clj @@ -13,7 +13,7 @@ [app.common.uri :as uri] [app.config :as cf] [app.http.client :as http] - [app.media.local :as local] + [app.media.svg :as svg] [app.media.validation :as validation] [app.setup :as-alias setup] [app.storage.tmp :as tmp] @@ -182,7 +182,7 @@ (let [{:keys [path mtype]} (validation/check-input input)] (if (= mtype "image/svg+xml") ;; SVG: parse locally (Sharp doesn't support SVG) - (let [info (some-> path slurp local/parse-svg local/get-basic-info-from-svg)] + (let [info (some-> path slurp svg/parse-svg svg/get-basic-info-from-svg)] (when-not info (ex/raise :type :validation :code :invalid-svg-file diff --git a/backend/src/app/media/svg.clj b/backend/src/app/media/svg.clj new file mode 100644 index 0000000000..1de52d4030 --- /dev/null +++ b/backend/src/app/media/svg.clj @@ -0,0 +1,130 @@ +;; 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 app.media.svg + "SVG parsing, sanitization, and info extraction. + Centralizes all SVG-related security concerns." + (:require + [app.common.data :as d] + [app.common.data.macros :as dm] + [app.common.exceptions :as ex] + [app.common.logging :as l] + [clojure.xml :as xml] + [cuerdas.core :as str]) + (:import + clojure.lang.XMLHandler + java.io.InputStream + javax.xml.parsers.SAXParserFactory + javax.xml.XMLConstants + org.apache.commons.io.IOUtils)) + +;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;; +;; SVG PARSING +;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;; + +(defn- secure-parser-factory + [^InputStream input ^XMLHandler handler] + (.. (doto (SAXParserFactory/newInstance) + (.setFeature XMLConstants/FEATURE_SECURE_PROCESSING true) + (.setFeature "http://apache.org/xml/features/disallow-doctype-decl" true)) + (newSAXParser) + (parse input handler))) + +(defn- strip-doctype + [data] + (cond-> data + (str/includes? data "]*>" ""))) + +(defn parse-svg + [text] + (let [text (strip-doctype text)] + (dm/with-open [istream (IOUtils/toInputStream ^String text "UTF-8")] + (xml/parse istream secure-parser-factory)))) + +;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;; +;; SVG SANITIZATION +;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;; + +(def ^:private dangerous-attrs-pattern #"(?i)^on\w+$") +(def ^:private javascript-href-pattern #"(?i)^javascript:") + +(defn- sanitize-svg-element + "Recursively sanitize an SVG element by removing dangerous tags and attributes." + [{:keys [tag attrs content] :as element}] + (when (and (map? element) tag) + (let [dangerous-tags #{:script :foreignObject :set :animate :animateTransform :animateColor :animateMotion}] + (when-not (contains? dangerous-tags tag) + (let [clean-attrs (->> attrs + (remove (fn [[k v]] + (or (re-matches dangerous-attrs-pattern (name k)) + (and (#{:href :xlink:href} k) + (string? v) + (re-find javascript-href-pattern (str/trim v)))))) + (into {})) + clean-content (when content + (->> content + (filter #(or (string? %) (map? %))) + (map (fn [child] + (if (map? child) + (sanitize-svg-element child) + child))) + (filter some?) + vec))] + (cond-> {:tag tag :attrs clean-attrs} + (seq clean-content) (assoc :content clean-content))))))) + +(defn sanitize-svg + "Sanitize SVG content by removing dangerous elements and attributes. + Removes " + result (svg/sanitize-svg svg)] + (t/is (not (clojure.string/includes? result "" + result (svg/sanitize-svg svg)] + (t/is (not (clojure.string/includes? result "foreignObject"))) + (t/is (not (clojure.string/includes? result "" + result (svg/sanitize-svg svg)] + (t/is (not (clojure.string/includes? result "" + result (svg/sanitize-svg svg)] + (t/is (not (clojure.string/includes? result " Date: Wed, 5 Aug 2026 21:43:45 +0200 Subject: [PATCH 12/13] :bug: Fix scripts/ci issue with backend lintig --- scripts/ci | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/scripts/ci b/scripts/ci index 3d3c7ed9a8..026bdb7d02 100755 --- a/scripts/ci +++ b/scripts/ci @@ -23,7 +23,7 @@ ALL_MODULES=("frontend" "backend" "common" "render-wasm" "exporter" "mcp" "plugi # Module commands declare -A LINT_CMD=( [frontend]="pnpm run lint:clj && pnpm run lint:js && pnpm run lint:scss" - [backend]="pnpm run lint" + [backend]="pnpm run lint:clj" [common]="pnpm run lint:clj" [render-wasm]="./lint" [exporter]="pnpm run lint" From b6656ee8dd1a41d0c00c5936eb9d7d00d33b17af Mon Sep 17 00:00:00 2001 From: Andrey Antukh Date: Wed, 5 Aug 2026 21:54:33 +0200 Subject: [PATCH 13/13] :bug: Enable SSRF check for organization SSO provider (#11064) (#11065) Remove :skip-ssrf-check? true from prepare-organization-sso-provider so SSRF protection is active when validating organization SSO configs. The endpoint is already protected by shared-key authentication (admin-console), but enabling SSRF protection prevents potential misuse of internal network resources if the shared key were ever compromised (defense-in-depth). Add test prepare-organization-sso-provider-does-not-skip-ssrf-check to verify the SSRF check is not skipped. AI-assisted-by: qwen3.7-plus --- backend/src/app/auth/oidc.clj | 5 ++--- backend/test/backend_tests/auth_oidc_test.clj | 13 +++++++++++++ 2 files changed, 15 insertions(+), 3 deletions(-) diff --git a/backend/src/app/auth/oidc.clj b/backend/src/app/auth/oidc.clj index 09d34532b9..ecf8b658a5 100644 --- a/backend/src/app/auth/oidc.clj +++ b/backend/src/app/auth/oidc.clj @@ -776,7 +776,7 @@ (defn prepare-organization-sso-provider "Build an OIDC provider map dynamically from the Nitrate organization SSO config. - Uses OIDC discovery via :issuer when token/auth/user URIs are absent." + Uses OIDC discovery via :issuer when token/auth/user URIs are absent." [cfg {:keys [client-id client-secret issuer]}] (prepare-oidc-provider cfg {:type "oidc" @@ -785,8 +785,7 @@ :base-uri (some-> (non-blank-uri issuer) (str/rtrim "/") (str "/")) - :scopes default-oidc-scopes - :skip-ssrf-check? true})) + :scopes default-oidc-scopes})) (defn build-organization-sso-auth-redirect-uri "Build the OIDC authorization redirect URI for an organization SSO config. diff --git a/backend/test/backend_tests/auth_oidc_test.clj b/backend/test/backend_tests/auth_oidc_test.clj index 22ccb624fe..62f04fd546 100644 --- a/backend/test/backend_tests/auth_oidc_test.clj +++ b/backend/test/backend_tests/auth_oidc_test.clj @@ -518,3 +518,16 @@ loc (redirect-location result)] (t/is (= 302 (::yres/status result))) (t/is (.contains loc "error=unable-to-auth"))))))) + +(t/deftest prepare-organization-sso-provider-does-not-skip-ssrf-check + (t/testing "organization SSO provider must use SSRF protection" + (let [captured-params (atom nil)] + (with-redefs [oidc/prepare-oidc-provider (fn [_cfg params] + (reset! captured-params params) + {:type "oidc" :id "test"})] + (#'oidc/prepare-organization-sso-provider {} + {:client-id "test-client" + :client-secret "test-secret" + :issuer "https://idp.example.com"}) + (t/is (not (true? (:skip-ssrf-check? @captured-params))) + "SSRF protection must be disabled for organization SSO")))))