diff --git a/.serena/memories/backend/auth-permissions-product-domains.md b/.serena/memories/backend/auth-permissions-product-domains.md index 02e2e83462..ddc3aac31a 100644 --- a/.serena/memories/backend/auth-permissions-product-domains.md +++ b/.serena/memories/backend/auth-permissions-product-domains.md @@ -19,6 +19,7 @@ - `app.rpc.permissions` provides predicate/check factories. Failed permission checks intentionally raise `:not-found` / `:object-not-found`, not an authorization-specific error, to avoid leaking object existence. - Team role flags are normalized as owner > admin > editor > viewer. Owner/admin imply edit; any membership row implies read. +- File/project role rows only count with a live team membership: the file/project permission queries require a matching `team_profile_rel` row, and leaving or removing a member deletes their team-scoped `file`/`project`/`team-project-profile-rel` rows (GHSA-v9r9-h77c-55m2). - File/project/comment checks are implemented in the owning command namespaces, often via helpers imported from `files`, `teams`, or `projects`; do not bypass those helpers with direct DB lookups unless preserving their not-found semantics. - Comment permission includes both logged-in state and the file/team comment policy. Shared viewer paths may pass `share-id`; preserve that path when changing comment queries. diff --git a/CHANGES.md b/CHANGES.md index 75a9c866b4..2abeb0612b 100644 --- a/CHANGES.md +++ b/CHANGES.md @@ -47,6 +47,20 @@ - Export multiple fills to SVG [#11466](https://github.com/penpot/penpot/issues/11466) (PR: [#11467](https://github.com/penpot/penpot/pull/11467)) - Add Penpot-specific board size presets (file thumbnail, template cover, plugin icon/cover) [#11561](https://github.com/penpot/penpot/issues/11561) (PR: [#11565](https://github.com/penpot/penpot/pull/11565)) +## 2.18.3 + +### :bug: Bugs fixed + +- Fix binfile import storing unsanitized SVG and missing obfuscated SVG content-types [#12104](https://github.com/penpot/penpot/issues/12104) (PR: [#12105](https://github.com/penpot/penpot/pull/12105)) +- Fix removed team members keeping file access after leaving the team [#12106](https://github.com/penpot/penpot/issues/12106) (PR: [#12097](https://github.com/penpot/penpot/pull/12097)) + +## 2.18.2 + +### :bug: Bugs fixed + +- Fix workspace showing an internal error when syncing components or dragging in the color picker [#11933](https://github.com/penpot/penpot/issues/11933) (PR: [#11941](https://github.com/penpot/penpot/pull/11941)) +- Fix SVG sanitizer keeping namespace-prefixed script elements when uploading SVG files [#12071](https://github.com/penpot/penpot/issues/12071) (PR: [#12073](https://github.com/penpot/penpot/pull/12073)) + ## 2.18.1 ### :bug: Bugs fixed diff --git a/backend/src/app/binfile/common.clj b/backend/src/app/binfile/common.clj index 951b225818..2275a2cc6d 100644 --- a/backend/src/app/binfile/common.clj +++ b/backend/src/app/binfile/common.clj @@ -15,6 +15,7 @@ [app.common.files.migrations :as fmg] [app.common.files.validate :as fval] [app.common.logging :as l] + [app.common.media :as cm] [app.common.schema :as sm] [app.common.time :as ct] [app.common.types.file :as ctf] @@ -27,6 +28,8 @@ [app.features.file-migrations :as fmigr] [app.loggers.audit :as-alias audit] [app.loggers.webhooks :as-alias webhooks] + [app.media.svg :as svg] + [app.storage :as sto] [app.util.blob :as blob] [app.util.pointer-map :as pmap] [app.worker :as-alias wrk] @@ -371,8 +374,13 @@ fpr.can_edit from file_profile_rel as fpr inner join file as f on (f.id = fpr.file_id) + inner join project as p on (p.id = f.project_id) where fpr.file_id = ? and fpr.profile_id = ? + and exists (select 1 + from team_profile_rel as tpr + where tpr.team_id = p.team_id + and tpr.profile_id = fpr.profile_id) union all select tpr.is_owner, tpr.is_admin, @@ -388,8 +396,13 @@ ppr.can_edit from project_profile_rel as ppr inner join file as f on (f.project_id = ppr.project_id) + inner join project as p on (p.id = ppr.project_id) where f.id = ? - and ppr.profile_id = ?") + and ppr.profile_id = ? + and exists (select 1 + from team_profile_rel as tpr + where tpr.team_id = p.team_id + and tpr.profile_id = ppr.profile_id)") (defn- get-file-permissions* [conn profile-id file-id] @@ -901,6 +914,7 @@ load-fn #(get-file cfg % :migrate? false)] (weak/loadable-weak-value-map library-ids load-fn {id file}))) +;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;; ;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;; ;; EXTERNAL LIBRARY RESOLUTION HELPERS ;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;; @@ -944,3 +958,74 @@ [cfg team-id slug] (->> (get-shared-files-for-team cfg team-id) (filter #(= slug (slugify-name (:name %)))))) + +;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;; +;; SVG IMPORT SANITIZATION +;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;; + +(def svg-content-type + "Canonical content type of SVG media objects." + "image/svg+xml") + +(defn normalize-content-type + "Canonicalize a stored content-type: lowercase, trimmed, + parameters after `;` dropped. Returns nil for missing or unusable + values so ancient bundle entries without content-type keep passing + through untouched." + [ctype] + (when (string? ctype) + (let [clean (-> ctype + (str/split #";" 2) + (first) + (str/trim) + (str/lower))] + (when-not (str/empty? clean) + clean)))) + +(defn svg-object? + "True when the storage `object` claims the canonical SVG content + type. Expects an already normalized object (see + `normalize-content-type`): every import path normalizes the + metadata right after reading it, so the comparison stays an exact + match in a single place." + [object] + (= svg-content-type (:content-type object))) + +(def schema:content-type + "Typed content-type for binfile storage objects: a member of + `cm/storage-object-types`. Values are canonicalized with + `normalize-content-type` before validation, so legacy spellings + keep importing while unknown types are rejected." + [::sm/one-of {:format :string} cm/storage-object-types]) + +(defn check-storage-content-type + "Check an imported storage `object` (already normalized, see + `normalize-content-type`) against `cm/storage-object-types`. + Returns nil when the type is allowed; raises `:type :validation` + with `:code :media-type-not-allowed` (same as the upload path) + when it is missing or unknown, so crafted bundles fail closed." + [object] + (when-not (contains? cm/storage-object-types (:content-type object)) + (ex/raise :type :validation + :code :media-type-not-allowed + :hint "storage object declares an unknown content-type" + :content-type (:content-type object)))) + +(defn sanitize-imported-svg + "Sanitize the raw `bytes` of an imported storage `object` when it + holds an SVG document. Expects an already normalized object (see + `normalize-content-type`); returns nil when the object is not an + SVG. + + Otherwise returns a map with the sanitized `:bytes`, their `:size` + and their blake2b `:hash`, ready to persist with `sto/put-object!`. + + Raises a `:validation` exception when the SVG cannot be parsed, the + same error the upload path reports." + [object ^bytes raw] + (when (svg-object? object) + (let [sanitized (svg/sanitize-svg (String. ^bytes raw "UTF-8")) + bytes (.getBytes ^String sanitized "UTF-8")] + {:bytes bytes + :size (alength ^bytes bytes) + :hash (sto/calculate-hash (java.io.ByteArrayInputStream. bytes))}))) diff --git a/backend/src/app/binfile/v1.clj b/backend/src/app/binfile/v1.clj index cb786db487..d7468eaa38 100644 --- a/backend/src/app/binfile/v1.clj +++ b/backend/src/app/binfile/v1.clj @@ -607,16 +607,26 @@ (doseq [expected-storage-id ids] (let [id (read-uuid! input) - mdata (read-obj! input)] + mdata (d/update-when (read-obj! input) :content-type bfc/normalize-content-type)] (when (not= id expected-storage-id) (ex/raise :type :validation :code :inconsistent-penpot-file :hint "the penpot file seems corrupt, found unexpected uuid (storage-object-id)")) + (bfc/check-storage-content-type mdata) + (l/dbg :hint "readed storage object" :id (str id) ::l/sync? true) (let [[size resource] (read-stream! input) + [resource size] (if (bfc/svg-object? mdata) + (let [raw (if (bytes? resource) + resource + (with-open [istream (jio/input-stream resource)] + (io/read istream))) + {:keys [bytes size]} (bfc/sanitize-imported-svg mdata raw)] + [bytes size]) + [resource size]) hash (sto/calculate-hash resource) content (-> (sto/content resource size) (sto/wrap-with-hash hash)) diff --git a/backend/src/app/binfile/v2.clj b/backend/src/app/binfile/v2.clj index 9e9644c47c..b339505ef8 100644 --- a/backend/src/app/binfile/v2.clj +++ b/backend/src/app/binfile/v2.clj @@ -197,9 +197,17 @@ (defn- read-storage-object! [{:keys [::sto/storage ::bfc/timestamp] :as cfg} id] - (let [mdata (read-obj cfg :storage-object id) + (let [mdata (d/update-when (read-obj cfg :storage-object id) + :content-type bfc/normalize-content-type) data (read-blob cfg :storage-object id) - hash (sto/calculate-hash data) + ;; NOTE: v2 bundles are legacy and no longer produced; the + ;; sanitizer call stays as defense in depth and is covered by + ;; the shared helper unit tests, with no dedicated v2 + ;; import test. + clean (when (bfc/svg-object? mdata) + (bfc/sanitize-imported-svg mdata data)) + data (or (:bytes clean) data) + hash (or (:hash clean) (sto/calculate-hash data)) content (-> (sto/content data) (sto/wrap-with-hash hash)) diff --git a/backend/src/app/binfile/v3.clj b/backend/src/app/binfile/v3.clj index cdb3037714..1d411336f7 100644 --- a/backend/src/app/binfile/v3.clj +++ b/backend/src/app/binfile/v3.clj @@ -86,7 +86,7 @@ [:map {:title "StorageObject"} [:id ::sm/uuid] [:size ::sm/int] - [:content-type :string] + [:content-type bfc/schema:content-type] [:bucket [::sm/one-of {:format :string} sto/valid-buckets]] [:hash {:optional true} :string]]) @@ -1003,6 +1003,7 @@ (doseq [{:keys [id entry]} entries] (let [object (-> (read-entry cfg input entry) (decode-storage-object) + (d/update-when :content-type bfc/normalize-content-type) (update :bucket d/nilv sto/default-bucket) (validate-storage-object)) @@ -1038,7 +1039,17 @@ :expected-hash (:hash object) :found-hash (sto/get-hash content)))) - (let [params (-> object + (let [clean (when (bfc/svg-object? object) + (let [limit (::max-binary-entry-size cfg) + raw (with-open [istream (cond-> (zip-entry-stream input (get-zip-entry input path)) + limit (size-limiting-stream limit (atom 0) path))] + (io/read istream))] + (bfc/sanitize-imported-svg object raw))) + content (if-let [{:keys [bytes hash]} clean] + (-> (sto/content bytes) + (sto/wrap-with-hash hash)) + content) + params (-> object (dissoc :id :size) (assoc ::sto/content content) (assoc ::sto/deduplicate? true) diff --git a/backend/src/app/media/svg.clj b/backend/src/app/media/svg.clj index 287322d460..2ef7112815 100644 --- a/backend/src/app/media/svg.clj +++ b/backend/src/app/media/svg.clj @@ -52,30 +52,47 @@ (def ^:private dangerous-attrs-pattern #"(?i)^on\w+$") (def ^:private javascript-href-pattern #"(?i)^javascript:") +(def ^:private dangerous-tags + #{"script" "foreignObject" "set" + "animate" "animateTransform" "animateColor" "animateMotion"}) + +(defn- local-name + "Return the local name of a parsed tag or attribute keyword. + + The XML parser interns qualified names with the prefix included in + the keyword name and no keyword namespace: `` parses as + the keyword `:x:script`. Dangerous elements and attributes behave + the same in the browser regardless of the prefix used, so matching + is done on the local name: the part after the last `:`." + [k] + (let [n (name k)] + (if (str/includes? n ":") + (last (str/split n ":")) + n))) + (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))))))) + (when-not (contains? dangerous-tags (local-name tag)) + (let [clean-attrs (->> attrs + (remove (fn [[k v]] + (or (re-matches dangerous-attrs-pattern (local-name k)) + (and (= "href" (local-name 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. diff --git a/backend/src/app/migrations.clj b/backend/src/app/migrations.clj index ff9b9a4c6f..8f42f8b9e4 100644 --- a/backend/src/app/migrations.clj +++ b/backend/src/app/migrations.clj @@ -514,7 +514,10 @@ :fn (mg/resource "app/migrations/sql/0155-drop-http-session-table.sql")} {:name "0156-add-storage-object-json-dedup-index" - :fn (mg/resource "app/migrations/sql/0156-add-storage-object-json-dedup-index.sql")}]) + :fn (mg/resource "app/migrations/sql/0156-add-storage-object-json-dedup-index.sql")} + + {:name "0153-del-orphan-profile-rels-after-team-leave" + :fn (mg/resource "app/migrations/sql/0153-del-orphan-profile-rels-after-team-leave.sql")}]) (defn apply-migrations! [pool name migrations] diff --git a/backend/src/app/migrations/sql/0153-del-orphan-profile-rels-after-team-leave.sql b/backend/src/app/migrations/sql/0153-del-orphan-profile-rels-after-team-leave.sql new file mode 100644 index 0000000000..41f30d7796 --- /dev/null +++ b/backend/src/app/migrations/sql/0153-del-orphan-profile-rels-after-team-leave.sql @@ -0,0 +1,27 @@ +-- Remove team-scoped role rows whose profile is no longer a member of +-- the team that owns the project/file. Leftover from before +-- leave-team/delete-team-member cleaned them up (GHSA-v9r9-h77c-55m2). +-- Rows with a live team membership are kept. + +DELETE FROM file_profile_rel AS fpr +USING file AS f +INNER JOIN project AS p ON (p.id = f.project_id) +WHERE fpr.file_id = f.id + AND NOT EXISTS (SELECT 1 + FROM team_profile_rel AS tpr + WHERE tpr.team_id = p.team_id + AND tpr.profile_id = fpr.profile_id); + +DELETE FROM project_profile_rel AS ppr +USING project AS p +WHERE ppr.project_id = p.id + AND NOT EXISTS (SELECT 1 + FROM team_profile_rel AS tpr + WHERE tpr.team_id = p.team_id + AND tpr.profile_id = ppr.profile_id); + +DELETE FROM team_project_profile_rel AS tppr +WHERE NOT EXISTS (SELECT 1 + FROM team_profile_rel AS tpr + WHERE tpr.team_id = tppr.team_id + AND tpr.profile_id = tppr.profile_id); diff --git a/backend/src/app/rpc/commands/projects.clj b/backend/src/app/rpc/commands/projects.clj index e69c704270..157b44310a 100644 --- a/backend/src/app/rpc/commands/projects.clj +++ b/backend/src/app/rpc/commands/projects.clj @@ -41,8 +41,13 @@ ppr.is_admin, ppr.can_edit from project_profile_rel as ppr + inner join project as p on (p.id = ppr.project_id) where ppr.project_id = ? - and ppr.profile_id = ?") + and ppr.profile_id = ? + and exists (select 1 + from team_profile_rel as tpr + where tpr.team_id = p.team_id + and tpr.profile_id = ppr.profile_id)") (defn- get-permissions [conn profile-id project-id] @@ -143,7 +148,12 @@ inner join team as t on (t.id = p2.team_id) where p2.id in (select project_id from project_profile_rel as ppr + inner join project as p on (p.id = ppr.project_id) where ppr.profile_id = ? + and exists (select 1 + from team_profile_rel as tpr + where tpr.team_id = p.team_id + and tpr.profile_id = ppr.profile_id) and (ppr.can_edit = true or ppr.is_owner = true or ppr.is_admin = true)) diff --git a/backend/src/app/rpc/commands/search.clj b/backend/src/app/rpc/commands/search.clj index 1186b15554..576d9f513f 100644 --- a/backend/src/app/rpc/commands/search.clj +++ b/backend/src/app/rpc/commands/search.clj @@ -32,6 +32,10 @@ where ppr.profile_id = ? and p.team_id = ? and (p.deleted_at is null) + and exists (select 1 + from team_profile_rel as tpr + where tpr.team_id = p.team_id + and tpr.profile_id = ppr.profile_id) and (ppr.is_admin = true or ppr.is_owner = true or ppr.can_edit = true) diff --git a/backend/src/app/rpc/commands/teams.clj b/backend/src/app/rpc/commands/teams.clj index 405933a463..a878018511 100644 --- a/backend/src/app/rpc/commands/teams.clj +++ b/backend/src/app/rpc/commands/teams.clj @@ -735,6 +735,37 @@ ;; --- Mutation: Leave Team +;; Remove every team-scoped relation of a profile that leaves (or is +;; removed from) a team. File and project roles hang from the team +;; membership: keeping them would let the former member keep reading +;; and editing files through `sql:file-permissions`. Pins are personal +;; state of the same team, so they go too. Other teams are untouched, +;; and rejoining grants fresh roles. Runs inside the caller's +;; transaction. +(def ^:private sql:delete-team-file-rels + "DELETE FROM file_profile_rel AS fpr + USING file AS f + INNER JOIN project AS p ON (p.id = f.project_id) + WHERE fpr.file_id = f.id + AND p.team_id = ? + AND fpr.profile_id = ?") + +(def ^:private sql:delete-team-project-rels + "DELETE FROM project_profile_rel AS ppr + USING project AS p + WHERE ppr.project_id = p.id + AND p.team_id = ? + AND ppr.profile_id = ?") + +(defn- delete-team-rels + [conn profile-id team-id] + (db/exec! conn [sql:delete-team-file-rels team-id profile-id]) + (db/exec! conn [sql:delete-team-project-rels team-id profile-id]) + (db/delete! conn :team-project-profile-rel {:profile-id profile-id + :team-id team-id}) + (db/delete! conn :team-profile-rel {:profile-id profile-id + :team-id team-id})) + (defn leave-team [{:keys [::db/conn ::mbus/msgbus]} {:keys [profile-id id reassign-to]}] (let [perms (get-permissions conn profile-id id) @@ -785,9 +816,7 @@ :code :owner-cant-leave-team :hint "releasing owner before leave")) - (db/delete! conn :team-profile-rel - {:profile-id profile-id - :team-id id}) + (delete-team-rels conn profile-id id) nil)) @@ -969,8 +998,7 @@ (ex/raise :type :validation :code :cant-remove-owner)) - (db/delete! conn :team-profile-rel {:profile-id member-id - :team-id team-id}) + (delete-team-rels conn member-id team-id) ;; A removed member that owns the organization of this team keeps ;; read-only access to it, so instead of kicking them out we degrade diff --git a/backend/src/app/storage/s3.clj b/backend/src/app/storage/s3.clj index cde4afc3c5..5316664620 100644 --- a/backend/src/app/storage/s3.clj +++ b/backend/src/app/storage/s3.clj @@ -370,7 +370,7 @@ ;; to the filesystem and then read with buffered inputstream; if ;; not, read the contento into memory using bytearrays. (if (> ^long size (* 1024 1024 2)) - (let [path (tmp/tempfile :prefix "penpot.storage.s3." :min-age "6h") + (let [path (tmp/temp-path :prefix "penpot.storage.s3." :min-age "6h") rxf (AsyncResponseTransformer/toFile ^Path path)] (->> (.getObject ^S3AsyncClient client ^GetObjectRequest gor diff --git a/backend/src/app/storage/tmp.clj b/backend/src/app/storage/tmp.clj index 7a4ae3b901..3ad6ac8c63 100644 --- a/backend/src/app/storage/tmp.clj +++ b/backend/src/app/storage/tmp.clj @@ -94,6 +94,25 @@ (sp/offer! queue [path (some-> min-age ct/duration)]) path)) +(defn temp-path + "Reserve a unique temp path WITHOUT creating the file. + + Use it when an external writer needs to create the file itself and + fails if it already exists (like the S3 SDK file download, which + opens with CREATE_NEW). The path is still registered for cleanup, + so a crash between reserve and write does not leak." + [& {:keys [suffix prefix dir min-age] + :or {prefix "penpot." + suffix ".tmp" + dir default-tmp-dir}}] + (fs/create-dir dir) + (loop [path (fs/join dir (str prefix (uuid/next) suffix))] + (if (fs/exists? path) + (recur (fs/join dir (str prefix (uuid/next) suffix))) + (do + (sp/offer! queue [path (some-> min-age ct/duration)]) + path)))) + (defn tempfile-from "Create a new tempfile from from consuming the stream" [input & {:as options}] diff --git a/backend/test/backend_tests/binfile_svg_test.clj b/backend/test/backend_tests/binfile_svg_test.clj new file mode 100644 index 0000000000..500c497df1 --- /dev/null +++ b/backend/test/backend_tests/binfile_svg_test.clj @@ -0,0 +1,425 @@ +;; 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.binfile-svg-test + "SVG sanitization on the binfile import path. + + Second case of GHSA-ffhp-m958-qxvr: a crafted `.penpot` bundle can + smuggle an unsanitized SVG storage object past the upload-path + sanitizer, and the raw bytes are served same-origin as + `image/svg+xml`." + (:require + [app.binfile.common :as bfc] + [app.binfile.v1 :as v1] + [app.binfile.v3 :as v3] + [app.common.features :as cfeat] + [app.common.types.shape :as cts] + [app.common.uuid :as uuid] + [app.db :as db] + [app.rpc :as-alias rpc] + [app.storage :as sto] + [app.storage.tmp :as tmp] + [backend-tests.helpers :as th] + [clojure.data.json :as json] + [clojure.java.io :as jio] + [clojure.string :as str] + [clojure.test :as t] + [datoteka.fs :as fs] + [datoteka.io :as io]) + (:import + java.io.ByteArrayInputStream + java.util.zip.ZipEntry + java.util.zip.ZipFile + java.util.zip.ZipOutputStream)) + +(t/use-fixtures :once th/state-init) +(t/use-fixtures :each th/database-reset) + +(def ^:private evil-svg + "") + +(def ^:private evil-xhref-svg + "click") + +(def ^:private clean-svg + "") + +(defn- utf8bytes + [^String s] + (.getBytes s "UTF-8")) + +(defn- bytes-str + [^bytes data] + (String. data "UTF-8")) + +(t/deftest sanitize-imported-svg-removes-script + (let [result (bfc/sanitize-imported-svg {:content-type "image/svg+xml" :bucket "file-media-object"} + (utf8bytes evil-svg))] + (t/is (some? result)) + (t/is (not (str/includes? (bytes-str (:bytes result)) "")))))) + +(t/deftest check-storage-content-type-allows-known-types + (doseq [ctype ["image/svg+xml" "image/jpeg" "font/woff2" "application/octet-stream"]] + (t/is (nil? (bfc/check-storage-content-type {:content-type ctype})) + (str "expected pass for " (pr-str ctype))))) + +(t/deftest check-storage-content-type-rejects-unknown-types + (doseq [ctype ["text/html" "application/x-font-woff" nil "" " "]] + (let [object (if (nil? ctype) {} {:content-type ctype}) + out (try (bfc/check-storage-content-type object) + nil + (catch clojure.lang.ExceptionInfo e + (ex-data e)))] + (t/is (= :validation (:type out)) (str "expected validation for " (pr-str ctype))) + (t/is (= :media-type-not-allowed (:code out)) (str "expected media-type-not-allowed for " (pr-str ctype)))))) + +(t/deftest storage-object-schema-constrains-content-type + (t/testing "known stored types validate, unknown types do not" + (let [base {:id (uuid/random) :size 10 :bucket "file-media-object"}] + (doseq [ctype ["image/svg+xml" "image/jpeg" "image/png" + "font/woff2" "font/ttf" "application/octet-stream" "application/pdf"]] + (t/is (some? (v3/validate-storage-object (assoc base :content-type ctype))) + (str "expected valid for " (pr-str ctype)))) + (doseq [ctype ["" " " "image /svg" "image/apng" "image/avif" + "application/x-font-woff" (apply str (repeat 200 "x"))]] + (t/is (thrown? clojure.lang.ExceptionInfo + (v3/validate-storage-object (assoc base :content-type ctype))) + (str "expected rejection for " (pr-str ctype))))))) + +(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") + data (utf8bytes text)] + (spit (str path) text :encoding "UTF-8") + [path (alength data)])) + +(defn- tamper-svg-entry! + "Copy `src` zip to `dst` zip replacing the `objects/.svg` + payload with `evil-bytes` and fixing its `.json` sidecar (size and + blake2b hash) so the bundle passes the integrity checks. When + `content-type` is given, the sidecar declares it instead of the + original spelling." + [src dst storage-id ^bytes evil-bytes & {:keys [content-type]}] + (let [svg-name (str "objects/" storage-id ".svg") + json-name (str "objects/" storage-id ".json")] + (with-open [zin (ZipFile. (.toFile ^java.nio.file.Path src)) + zout (ZipOutputStream. (io/output-stream dst))] + (doseq [entry (enumeration-seq (.entries zin))] + (let [name (.getName ^ZipEntry entry)] + (cond + (= name svg-name) + (do (.putNextEntry zout (ZipEntry. ^String name)) + (.write zout evil-bytes 0 (alength evil-bytes)) + (.closeEntry zout)) + + (= name json-name) + (let [raw (slurp (.getInputStream zin entry) :encoding "UTF-8") + parsed (json/read-str raw) + updated (cond-> (assoc parsed + "size" (alength evil-bytes) + "hash" (sto/calculate-hash (ByteArrayInputStream. evil-bytes))) + content-type (assoc "contentType" content-type))] + (.putNextEntry zout (ZipEntry. ^String name)) + (.write zout (utf8bytes (json/write-str updated))) + (.closeEntry zout)) + + :else + (do (.putNextEntry zout (ZipEntry. ^String name)) + (with-open [in (.getInputStream zin entry)] + (io/copy in zout)) + (.closeEntry zout)))))))) + +(defn- setup-file-with-svg-media + "Upload `clean-svg`, attach it to a shape fill and return + `[file uploaded]` ready to export." + [profile n] + (let [file (th/create-file* n {:profile-id (:id profile) + :project-id (:default-project-id profile) + :is-shared false}) + [svg-path svg-size] (write-temp-svg clean-svg) + out (th/command! {::th/type :upload-file-media-object + ::rpc/profile-id (:id profile) + :file-id (:id file) + :is-local true + :name "benign.svg" + :content {:filename "benign.svg" + :path svg-path + :mtype "image/svg+xml" + :size svg-size}}) + _ (t/is (nil? (:error out)) (str "upload failed: " (pr-str (:error out)))) + uploaded (:result out) + page-id (first (get-in file [:data :pages])) + shape-id (uuid/random) + upd (th/command! {::th/type :update-file + ::rpc/profile-id (:id profile) + :id (:id file) + :session-id (uuid/random) + :revn 0 + :vern 0 + :features cfeat/supported-features + :changes + [{:type :add-obj + :page-id page-id + :id shape-id + :parent-id uuid/zero + :frame-id uuid/zero + :components-v2 true + :obj (cts/setup-shape + {:id shape-id + :name "image" + :frame-id uuid/zero + :parent-id uuid/zero + :type :rect + :fills [{:fill-opacity 1 + :fill-image {:id (:id uploaded) + :width (:width uploaded) + :height (:height uploaded) + :mtype "image/svg+xml"}}]})}]})] + (t/is (nil? (:error upd)) (str "update-file failed: " (pr-str (:error upd)))) + (t/is (uuid? (:media-id uploaded))) + [file uploaded])) + +(defn- export-tamper-import-v3 + "Export `file` to v3, tamper its SVG payload with `evil-text` + (declaring `content-type` when given) and import the bundle. + Returns `[served-bytes stored-content-type]` of the imported + media." + [profile file media-id evil-text content-type] + (let [bundle (tmp/tempfile :prefix "penpot-export-" :suffix ".zip")] + (v3/export-files! + (-> th/*system* + (assoc ::bfc/ids #{(:id file)}) + (assoc ::bfc/embed-assets false) + (assoc ::bfc/include-libraries false)) + (io/output-stream bundle)) + + (let [evil (tmp/tempfile :prefix "penpot-evil-" :suffix ".zip") + _ (tamper-svg-entry! bundle evil media-id (utf8bytes evil-text) :content-type content-type) + result (-> th/*system* + (assoc ::bfc/project-id (:default-project-id profile)) + (assoc ::bfc/profile-id (:id profile)) + (assoc ::bfc/input evil) + (v3/import-files!))] + (t/is (= 1 (count (:file-ids result)))) + (let [rows (th/db-query :file-media-object {:file-id (first (:file-ids result))}) + storage (:app.storage/storage th/*system*)] + (t/is (= 1 (count rows))) + (let [object (sto/get-object storage (:media-id (first rows)))] + [(bytes-str (sto/get-object-bytes storage object)) + (:content-type (meta object))]))))) + +(t/deftest import-binfile-v3-sanitizes-smuggled-svg + (let [profile (th/create-profile* 1) + [file uploaded] (setup-file-with-svg-media profile 1) + [served _stored] (export-tamper-import-v3 profile file (:media-id uploaded) evil-svg nil)] + (t/is (not (str/includes? served " (sto/content raw) + (sto/wrap-with-hash (sto/calculate-hash (ByteArrayInputStream. raw)))) + sobj (sto/put-object! storage {::sto/content content + :content-type content-type + :bucket "file-media-object"}) + fmo-id (uuid/random) + _ (th/db-insert! :file-media-object + {:id fmo-id + :file-id (:id file) + :is-local true + :name "seed.svg" + :media-id (:id sobj) + :width 64 + :height 64 + :mtype content-type}) + page-id (first (get-in file [:data :pages])) + shape-id (uuid/random) + upd (th/command! {::th/type :update-file + ::rpc/profile-id (:id profile) + :id (:id file) + :session-id (uuid/random) + :revn 0 + :vern 0 + :features cfeat/supported-features + :changes + [{:type :add-obj + :page-id page-id + :id shape-id + :parent-id uuid/zero + :frame-id uuid/zero + :components-v2 true + :obj (cts/setup-shape + {:id shape-id + :name "image" + :frame-id uuid/zero + :parent-id uuid/zero + :type :rect + :fills [{:fill-opacity 1 + :fill-image {:id fmo-id + :width 64 + :height 64 + :mtype "image/svg+xml"}}]})}]})] + (t/is (nil? (:error upd)) (str "update-file failed: " (pr-str (:error upd)))) + [file (:id sobj)])) + +(defn- export-v1 + "Export `file` to v1 and return the bundle path." + [profile file] + (let [bundle (tmp/tempfile :prefix "penpot-export-v1-" :suffix ".bin")] + (v1/export-files! + (-> th/*system* + (assoc ::bfc/ids #{(:id file)}) + (assoc ::bfc/embed-assets false) + (assoc ::bfc/include-libraries false)) + (io/output-stream bundle)) + bundle)) + +(defn- export-import-v1 + "Export `file` to v1, import it back and return the served bytes + of its single media object." + [profile file] + (let [bundle (export-v1 profile file)] + (let [result (-> th/*system* + (assoc ::bfc/project-id (:default-project-id profile)) + (assoc ::bfc/profile-id (:id profile)) + (assoc ::bfc/input (io/input-stream bundle)) + (v1/import-files!))] + (t/is (= 1 (count result))) + (let [rows (th/db-query :file-media-object {:file-id (first result)}) + storage (:app.storage/storage th/*system*)] + (t/is (= 1 (count rows))) + (let [object (sto/get-object storage (:media-id (first rows)))] + [(bytes-str (sto/get-object-bytes storage object)) + (:content-type (meta object))]))))) + +(t/deftest import-binfile-v1-sanitizes-seeded-svg + (let [profile (th/create-profile* 4) + [file _seed] (seed-file-with-svg-media profile 4 evil-svg "IMAGE/SVG+XML") + [served stored] (export-import-v1 profile file)] + (t/is (not (str/includes? served " th/*system* + (assoc ::bfc/project-id (:default-project-id profile)) + (assoc ::bfc/profile-id (:id profile)) + (assoc ::bfc/input (io/input-stream bundle)) + (v1/import-files!)) + nil + (catch clojure.lang.ExceptionInfo e + (ex-data e)))] + (t/is (= :validation (:type out))) + (t/is (= :media-type-not-allowed (:code out))) + (t/is (= [(:id file)] + (mapv :id (th/db-query :file {:project-id (:default-project-id profile)})))))) + +(def ^:private large-clean-svg + (let [pad (apply str (repeat 110000 ""))] + (str "" pad ""))) + +(t/deftest import-binfile-v1-roundtrips-large-svg + (t/testing "objects over the tempfile threshold (tempfile branch) import fine" + (t/is (> (alength (utf8bytes large-clean-svg)) bfc/temp-file-threshold)) + (let [profile (th/create-profile* 5) + [file _seed] (seed-file-with-svg-media profile 5 large-clean-svg "image/svg+xml") + [served _stored] (export-import-v1 profile file)] + (t/is (str/includes? served "circle"))))) diff --git a/backend/test/backend_tests/binfile_test.clj b/backend/test/backend_tests/binfile_test.clj index 0ffb1d2262..df12541d16 100644 --- a/backend/test/backend_tests/binfile_test.clj +++ b/backend/test/backend_tests/binfile_test.clj @@ -1980,7 +1980,7 @@ (let [storage (-> (:app.storage/storage th/*system*) (stt/configure-storage-backend)) - sobject (sto/put-object! storage {::sto/content (sto/content "media-bytes") + sobject (sto/put-object! storage {::sto/content (sto/content "") :content-type "image/svg+xml" :bucket "file-media-object"}) diff --git a/backend/test/backend_tests/helpers.clj b/backend/test/backend_tests/helpers.clj index 1ee446af6a..a32d4facb8 100644 --- a/backend/test/backend_tests/helpers.clj +++ b/backend/test/backend_tests/helpers.clj @@ -306,9 +306,7 @@ ([params] (create-project-role* *system* params)) ([system {:keys [project-id profile-id role] :or {role :owner}}] (dm/with-open [conn (db/open system)] - (#'teams/create-project-role conn {:project-id project-id - :profile-id profile-id - :role role})))) + (#'teams/create-project-role conn profile-id project-id role)))) (defn create-file-role* ([params] (create-file-role* *system* params)) diff --git a/backend/test/backend_tests/media_test.clj b/backend/test/backend_tests/media_test.clj index 2bb80bf30d..60334ebf77 100644 --- a/backend/test/backend_tests/media_test.clj +++ b/backend/test/backend_tests/media_test.clj @@ -137,6 +137,72 @@ (t/is (not (clojure.string/includes? result "onmouseover"))) (t/is (clojure.string/includes? result "alert('xss')" + result (svg/sanitize-svg svg)] + (t/is (not (clojure.string/includes? result "alert('xss')" + result (svg/sanitize-svg svg)] + (t/is (not (clojure.string/includes? result "alert('xss')" + 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 "javascript:"))) + (t/is (not (clojure.string/includes? result "alert"))) + (t/is (clojure.string/includes? result "" + result (svg/sanitize-svg svg)] + (t/is (not (clojure.string/includes? result "onload"))) + (t/is (not (clojure.string/includes? result "onclick"))) + (t/is (not (clojure.string/includes? result "alert"))) + (t/is (clojure.string/includes? result "" + result (svg/sanitize-svg svg)] + (t/is (not (clojure.string/includes? result "javascript:"))) + (t/is (not (clojure.string/includes? result "alert"))) + (t/is (clojure.string/includes? result "hola" + result (svg/sanitize-svg svg)] + (t/is (clojure.string/includes? result "xml:space")) + (t/is (clojure.string/includes? result "hola"))))) + (t/deftest info-invalid-image (t/testing "info on invalid image raises error" (let [path (fs/create-tempfile :prefix "penpot-test-" :suffix ".jpg")] diff --git a/backend/test/backend_tests/rpc_team_test.clj b/backend/test/backend_tests/rpc_team_test.clj index 5f6a5d6543..ed6520b815 100644 --- a/backend/test/backend_tests/rpc_team_test.clj +++ b/backend/test/backend_tests/rpc_team_test.clj @@ -1439,3 +1439,315 @@ (t/is (th/ex-info? (:error out))) (t/is (th/ex-of-type? (:error out) :validation)) (t/is (th/ex-of-code? (:error out) :params-validation)))) + +(t/deftest leaver-loses-file-access + ;; GHSA-v9r9-h77c-55m2: leaving the team must revoke the file and + ;; project roles granted in that team. + (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 [project (th/create-project* 1 {:profile-id (:id editor) + :team-id (:id team)}) + file (th/create-file* 1 {:profile-id (:id editor) + :project-id (:id project)})] + + (t/testing "editor can rename and read summary before leaving" + (let [out (th/command! {::th/type :rename-file + ::rpc/profile-id (:id editor) + :id (:id file) + :name "renamed"})] + (t/is (th/success? out))) + (let [out (th/command! {::th/type :get-file-summary + ::rpc/profile-id (:id editor) + :id (:id file)})] + (t/is (th/success? out)))) + + (t/testing "leave team" + (let [out (th/command! {::th/type :leave-team + ::rpc/profile-id (:id editor) + :id (:id team)})] + (t/is (th/success? out)))) + + (t/testing "renaming after leaving fails with not-found" + (let [out (th/command! {::th/type :rename-file + ::rpc/profile-id (:id editor) + :id (:id file) + :name "renamed-again"}) + error (:error out)] + (t/is (th/ex-info? error)) + (t/is (th/ex-of-type? error :not-found)))) + + (t/testing "file summary after leaving fails with not-found" + (let [out (th/command! {::th/type :get-file-summary + ::rpc/profile-id (:id editor) + :id (:id file)}) + error (:error out)] + (t/is (th/ex-info? error)) + (t/is (th/ex-of-type? error :not-found)))) + + (t/testing "get-file after leaving fails with not-found" + (let [out (th/command! {::th/type :get-file + ::rpc/profile-id (:id editor) + :id (:id file) + :components-v2 true}) + error (:error out)] + (t/is (th/ex-info? error)) + (t/is (th/ex-of-type? error :not-found))))))) + +(t/deftest removed-member-loses-file-access + ;; Same as above through the expulsion path. + (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 [project (th/create-project* 1 {:profile-id (:id editor) + :team-id (:id team)}) + file (th/create-file* 1 {:profile-id (:id editor) + :project-id (:id project)})] + + (t/testing "remove member" + (let [out (th/command! {::th/type :delete-team-member + ::rpc/profile-id (:id owner) + :team-id (:id team) + :member-id (:id editor)})] + (t/is (th/success? out)))) + + (t/testing "renaming after removal fails with not-found" + (let [out (th/command! {::th/type :rename-file + ::rpc/profile-id (:id editor) + :id (:id file) + :name "renamed-again"}) + error (:error out)] + (t/is (th/ex-info? error)) + (t/is (th/ex-of-type? error :not-found)))) + + (t/testing "file summary after removal fails with not-found" + (let [out (th/command! {::th/type :get-file-summary + ::rpc/profile-id (:id editor) + :id (:id file)}) + error (:error out)] + (t/is (th/ex-info? error)) + (t/is (th/ex-of-type? error :not-found))))))) + +(t/deftest orphan-file-role-without-team-membership-grants-nothing + ;; Defense in depth: a file role row without a live team membership + ;; (leftover from before the leave-team cleanup) grants no access. + (let [owner (th/create-profile* 1 {:is-active true}) + stranger (th/create-profile* 2 {:is-active true}) + 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)})] + + ;; Plant an orphan file role for a profile that was never a member. + (th/create-file-role* {:file-id (:id file) + :profile-id (:id stranger) + :role :editor}) + + (t/testing "orphan file role cannot rename" + (let [out (th/command! {::th/type :rename-file + ::rpc/profile-id (:id stranger) + :id (:id file) + :name "renamed"}) + error (:error out)] + (t/is (th/ex-info? error)) + (t/is (th/ex-of-type? error :not-found)))) + + (t/testing "orphan file role cannot read summary" + (let [out (th/command! {::th/type :get-file-summary + ::rpc/profile-id (:id stranger) + :id (:id file)}) + error (:error out)] + (t/is (th/ex-info? error)) + (t/is (th/ex-of-type? error :not-found)))))) + +(t/deftest leaver-loses-project-access + ;; Same revocation at project level (covers sql:project-permissions). + (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 [project (th/create-project* 1 {:profile-id (:id editor) + :team-id (:id team)})] + + (t/testing "leave team" + (let [out (th/command! {::th/type :leave-team + ::rpc/profile-id (:id editor) + :id (:id team)})] + (t/is (th/success? out)))) + + (t/testing "get-project after leaving fails with not-found" + (let [out (th/command! {::th/type :get-project + ::rpc/profile-id (:id editor) + :id (:id project)}) + error (:error out)] + (t/is (th/ex-info? error)) + (t/is (th/ex-of-type? error :not-found)))) + + (t/testing "rename-project after leaving fails with not-found" + (let [out (th/command! {::th/type :rename-project + ::rpc/profile-id (:id editor) + :id (:id project) + :name "renamed"}) + error (:error out)] + (t/is (th/ex-info? error)) + (t/is (th/ex-of-type? error :not-found))))))) + +(t/deftest leaver-vanishes-from-search + ;; The old team's files are no longer searchable by the leaver. + (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 [project (th/create-project* 1 {:profile-id (:id editor) + :team-id (:id team)}) + file (th/create-file* 1 {:profile-id (:id editor) + :project-id (:id project) + :name "searchable-file"})] + + (t/testing "member finds the file before leaving" + (let [out (th/command! {::th/type :search-files + ::rpc/profile-id (:id editor) + :team-id (:id team) + :search-term "searchable"})] + (t/is (th/success? out)) + (t/is (= (:id file) (-> out :result first :id))))) + + (t/testing "leave team" + (let [out (th/command! {::th/type :leave-team + ::rpc/profile-id (:id editor) + :id (:id team)})] + (t/is (th/success? out)))) + + (t/testing "search on the old team fails with not-found" + (let [out (th/command! {::th/type :search-files + ::rpc/profile-id (:id editor) + :team-id (:id team) + :search-term "searchable"}) + error (:error out)] + (t/is (th/ex-info? error)) + (t/is (th/ex-of-type? error :not-found))))))) + +(t/deftest pins-are-cleaned-on-leave + ;; The project pin (team-project-profile-rel) goes with the leaver. + (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 [project (th/create-project* 1 {:profile-id (:id editor) + :team-id (:id team)})] + + (t/testing "pin the project" + (let [out (th/command! {::th/type :update-project-pin + ::rpc/profile-id (:id editor) + :id (:id project) + :team-id (:id team) + :is-pinned true})] + (t/is (th/success? out))) + (t/is (= 1 (count (th/db-query :team-project-profile-rel + {:team-id (:id team) + :profile-id (:id editor)}))))) + + (t/testing "leave team removes the pin row" + (let [out (th/command! {::th/type :leave-team + ::rpc/profile-id (:id editor) + :id (:id team)})] + (t/is (th/success? out))) + (t/is (= 0 (count (th/db-query :team-project-profile-rel + {:team-id (:id team) + :profile-id (:id editor)})))))))) + +(t/deftest leaving-one-team-keeps-other-team-access + ;; Deletes are scoped by team: leaving A keeps B's files usable. + (let [owner (th/create-profile* 1 {:is-active true}) + editor (th/create-profile* 2 {:is-active true}) + team-a (th/create-team* 1 {:profile-id (:id owner)}) + team-b (th/create-team* 2 {:profile-id (:id owner)})] + + (th/create-team-role* {:team-id (:id team-a) + :profile-id (:id editor) + :role :editor}) + (th/create-team-role* {:team-id (:id team-b) + :profile-id (:id editor) + :role :editor}) + + (let [project-a (th/create-project* 1 {:profile-id (:id editor) + :team-id (:id team-a)}) + file-a (th/create-file* 1 {:profile-id (:id editor) + :project-id (:id project-a)}) + project-b (th/create-project* 2 {:profile-id (:id editor) + :team-id (:id team-b)}) + file-b (th/create-file* 2 {:profile-id (:id editor) + :project-id (:id project-b)})] + + (t/testing "leave team A" + (let [out (th/command! {::th/type :leave-team + ::rpc/profile-id (:id editor) + :id (:id team-a)})] + (t/is (th/success? out)))) + + (t/testing "team A file is gone" + (let [out (th/command! {::th/type :rename-file + ::rpc/profile-id (:id editor) + :id (:id file-a) + :name "renamed"}) + error (:error out)] + (t/is (th/ex-info? error)) + (t/is (th/ex-of-type? error :not-found)))) + + (t/testing "team B file still works" + (let [out (th/command! {::th/type :rename-file + ::rpc/profile-id (:id editor) + :id (:id file-b) + :name "renamed"})] + (t/is (th/success? out))))))) + +(t/deftest orphan-project-role-grants-nothing-and-lists-nothing + ;; Mirror of the file orphan test at project level, including the + ;; get-all-projects listing (covers the F1 guard). + (let [owner (th/create-profile* 1 {:is-active true}) + stranger (th/create-profile* 2 {:is-active true}) + team (th/create-team* 1 {:profile-id (:id owner)}) + project (th/create-project* 1 {:profile-id (:id owner) + :team-id (:id team)})] + + ;; Plant an orphan project role for a profile that was never a member. + (th/create-project-role* {:project-id (:id project) + :profile-id (:id stranger) + :role :editor}) + + (t/testing "orphan project role cannot read the project" + (let [out (th/command! {::th/type :get-project + ::rpc/profile-id (:id stranger) + :id (:id project)}) + error (:error out)] + (t/is (th/ex-info? error)) + (t/is (th/ex-of-type? error :not-found)))) + + (t/testing "orphan project is not listed" + (let [out (th/command! {::th/type :get-all-projects + ::rpc/profile-id (:id stranger)})] + (t/is (th/success? out)) + (t/is (nil? (some #(= (:id project) (:id %)) (:result out)))))))) diff --git a/backend/test/backend_tests/storage_tmp_test.clj b/backend/test/backend_tests/storage_tmp_test.clj new file mode 100644 index 0000000000..b9141cf20b --- /dev/null +++ b/backend/test/backend_tests/storage_tmp_test.clj @@ -0,0 +1,40 @@ +;; 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 SUBSIDIARY SL + +(ns backend-tests.storage-tmp-test + (:require + [app.storage.tmp :as tmp] + [clojure.test :as t] + [datoteka.fs :as fs]) + (:import + java.nio.file.Files + java.nio.file.Path)) + +(t/deftest temp-path-does-not-create-the-file + (let [path (tmp/temp-path :prefix "penpot.test." :min-age "6h")] + (t/testing "the reserved path does not exist yet" + (t/is (not (fs/exists? path)))) + (t/testing "an external writer can create it with CREATE_NEW" + (Files/createFile ^Path path (into-array java.nio.file.attribute.FileAttribute [])) + (t/is (fs/exists? path))) + (fs/delete path))) + +(t/deftest temp-path-is-unique + (let [a (tmp/temp-path :prefix "penpot.test.") + b (tmp/temp-path :prefix "penpot.test.")] + (t/is (not= a b)) + (t/is (not (fs/exists? a))) + (t/is (not (fs/exists? b))))) + +(t/deftest tempfile-creates-the-file-so-create-new-fails + (t/testing "documents why s3 downloads cannot use tempfile: the SDK opens with CREATE_NEW" + (let [path (tmp/tempfile :prefix "penpot.test.")] + (try + (t/is (fs/exists? path)) + (t/is (thrown? java.nio.file.FileAlreadyExistsException + (Files/createFile ^Path path (into-array java.nio.file.attribute.FileAttribute [])))) + (finally + (fs/delete path)))))) diff --git a/common/src/app/common/exceptions.cljc b/common/src/app/common/exceptions.cljc index 7a4b6b1a1e..5a8a9acaec 100644 --- a/common/src/app/common/exceptions.cljc +++ b/common/src/app/common/exceptions.cljc @@ -245,7 +245,6 @@ (defn format-throwable [cause & {:as opts}] (with-out-str - (println "====================") (when-let [exdata (ex-data cause)] (when-let [hint (or (get exdata :hint) (ex-message cause))] @@ -275,9 +274,7 @@ (when-let [trace (.-stack cause)] (println "Trace:") (println "--------------------") - (println (.-stack cause))) - - (println "====================")))) + (println (.-stack cause)))))) (defn first-line [s] diff --git a/common/src/app/common/media.cljc b/common/src/app/common/media.cljc index 8e6038ea11..ea0c11b530 100644 --- a/common/src/app/common/media.cljc +++ b/common/src/app/common/media.cljc @@ -25,6 +25,23 @@ (def tempfile-types (conj image-types "application/pdf" "application/zip")) +(def storage-object-types + "Every content-type the system stores in storage objects: uploaded + images and fonts, generated thumbnails, temporary files and the + `application/octet-stream` fallback used by file data and font + variants. Sorted for determinism. Every member must have a + producing code path; anticipated but unproduced types (such as + `image/apng`) stay out until some flow actually stores them. + Import boundaries normalize the stored string (lowercase, no + parameters) before checking membership, so legacy spelling + variants keep importing while unknown types are rejected." + (into (sorted-set) + (concat image-types + font-types + ["application/octet-stream" + "application/pdf" + "application/zip"]))) + (defn format->extension [format] (case format diff --git a/common/test/common_tests/media_test.cljc b/common/test/common_tests/media_test.cljc index 24302a02d1..00bbe932ef 100644 --- a/common/test/common_tests/media_test.cljc +++ b/common/test/common_tests/media_test.cljc @@ -9,6 +9,13 @@ [app.common.media :as media] [clojure.test :as t])) +(t/deftest test-storage-object-types + (t/testing "covers every image and font type the system stores" + (t/is (every? #(contains? media/storage-object-types %) media/image-types)) + (t/is (every? #(contains? media/storage-object-types %) media/font-types)) + (t/is (contains? media/storage-object-types "image/svg+xml")) + (t/is (contains? media/storage-object-types "application/octet-stream")))) + (t/deftest test-parse-font-weight (t/testing "matches weight tokens with proper boundaries" (t/is (= 700 (media/parse-font-weight "Roboto-Bold")))