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/.serena/memories/plugins/core.md b/.serena/memories/plugins/core.md index b042c34aa8..64f4643860 100644 --- a/.serena/memories/plugins/core.md +++ b/.serena/memories/plugins/core.md @@ -26,7 +26,7 @@ ## Lifecycle -- Loading a plugin closes existing non-background plugins and resets the runtime registry. Be careful around `allowBackground` semantics when changing load/close behavior. +- Loading a plugin closes existing non-background plugins and removes them from the runtime registry. Background plugins (`allowBackground`) keep running and must stay registered: the registry routes their UI `postMessage` traffic by sender iframe and is what `ɵunloadPlugin` searches. - If sandbox evaluation fails, the runtime marks the error as plugin-originated, closes the plugin, and rethrows. - `plugin-manager` removes event listeners, timers, intervals, and modal state on close, and marks the plugin destroyed. Listener callbacks check that flag because Penpot events can fire after close. 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 2909e35815..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] @@ -68,11 +71,15 @@ (* 1024 1024 20)) ;; Maximum total decompressed size allowed for all JSON/text zip entries -;; combined within a single import job: 200MiB. Bounds the case where many +;; combined within a single import job: 500MiB. Bounds the case where many ;; entries, each individually under default-max-text-entry-size, still sum -;; to an unreasonable total. +;; to an unreasonable total. Set well above the 200MiB it replaces because +;; legitimate giant exports exist: hundreds of thousands of KB-sized +;; entries (pages, shapes) that never trip the per-entry cap but sum past +;; 400MiB. The per-entry cap still bounds single-entry DEFLATE +;; amplification, so this only moves the cumulative ceiling. (def ^:const default-max-text-total-size - (* 1024 1024 200)) + (* 1024 1024 500)) ;; Maximum number of entries allowed in the import zip: 500,000. (def ^:const default-max-zip-entries @@ -367,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, @@ -384,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] @@ -897,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 ;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;; @@ -940,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 1a4f039f59..c11d65b84b 100644 --- a/backend/src/app/migrations.clj +++ b/backend/src/app/migrations.clj @@ -517,7 +517,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 cf902d4f50..db366020a2 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 c2ec1fa720..8c31676d3d 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 a3931e60aa..40baeb63ee 100644 --- a/backend/test/backend_tests/binfile_test.clj +++ b/backend/test/backend_tests/binfile_test.clj @@ -1985,7 +1985,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"}) @@ -2161,6 +2161,13 @@ (t/is (= :validation (:type out))) (t/is (= :max-file-size-reached (:code out)))))) +(t/deftest default-text-total-budget-fits-giant-legit-files + ;; A real-world giant export held ~437 MiB of JSON across ~178k + ;; KB-sized entries without tripping any per-entry cap. The default + ;; cumulative budget must clear that scale, or such files cannot be + ;; imported out of the box. + (t/is (>= bfc/default-max-text-total-size (* 1024 1024 450)))) + (defn- text-entries-sizes "Returns the decompressed sizes of every `.json` entry in the zip at `zip-path`. Used to pick a cumulative budget that sits between the diff --git a/backend/test/backend_tests/helpers.clj b/backend/test/backend_tests/helpers.clj index 13ac46cf66..d89250eee3 100644 --- a/backend/test/backend_tests/helpers.clj +++ b/backend/test/backend_tests/helpers.clj @@ -309,9 +309,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 11b3190164..f066f53db6 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/logic/libraries.cljc b/common/src/app/common/logic/libraries.cljc index fc3c4d00de..5d003a64b2 100644 --- a/common/src/app/common/logic/libraries.cljc +++ b/common/src/app/common/logic/libraries.cljc @@ -56,6 +56,10 @@ ;; We don't do automatic update of the `layout-grid-cells` property. (remove #(= :layout-grid-cells %)))) +;; Flex-child layout attrs synced between main and copy (interactions are only kept on swap) +(def ^:private flex-child-attrs + (disj ctk/swap-keep-attrs :interactions)) + (defn enabled-shape? [id container] (or (empty? log-shape-ids) @@ -2402,7 +2406,7 @@ (fn [child-copy] (let [child-main (ctf/get-ref-shape main-container main-component child-copy)] (-> child-copy - (propagate-attrs child-main ctk/swap-keep-attrs omit-touched?)))) + (propagate-attrs child-main flex-child-attrs omit-touched?)))) {:ignore-touched true}))] (pcb/concat-changes changes new-changes))) @@ -2432,7 +2436,7 @@ (fn [child-main] (let [child-copy (ctf/get-shape-in-copy copy-container child-main shape-copy)] (-> child-main - (propagate-attrs child-copy ctk/swap-keep-attrs omit-touched?)))) + (propagate-attrs child-copy flex-child-attrs omit-touched?)))) {:ignore-touched true}))] (pcb/concat-changes changes new-changes))) 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/logic/comp_flex_interactions_test.cljc b/common/test/common_tests/logic/comp_flex_interactions_test.cljc new file mode 100644 index 0000000000..5ded09e9a3 --- /dev/null +++ b/common/test/common_tests/logic/comp_flex_interactions_test.cljc @@ -0,0 +1,155 @@ +;; 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 common-tests.logic.comp-flex-interactions-test + (:require + [app.common.files.changes-builder :as pcb] + [app.common.geom.point :as gpt] + [app.common.geom.shapes :as gsh] + [app.common.logic.libraries :as cll] + [app.common.logic.shapes :as cls] + [app.common.math :as mth] + [app.common.test-helpers.components :as thc] + [app.common.test-helpers.compositions :as tho] + [app.common.test-helpers.files :as thf] + [app.common.test-helpers.ids-map :as thi] + [app.common.test-helpers.shapes :as ths] + [app.common.types.container :as ctn] + [clojure.test :as t])) + +(t/use-fixtures :each thi/test-fixture) + +(defn- setup-file + "Flex component and copy whose children navigate to different boards." + [] + (-> (thf/sample-file :file1) + (tho/add-simple-component-with-copy :component1 + :main-root + :main-child + :copy-root + :main-root-params {:layout :flex + :layout-flex-dir :row} + :copy-root-params {:children-labels [:copy-child]}) + (ths/add-sample-shape :main-popup :type :frame) + (ths/add-sample-shape :copy-popup :type :frame) + (ths/add-interaction :main-child :main-popup) + (ths/add-interaction :copy-child :copy-popup))) + +(defn- update-shape + [file label update-fn] + (let [page (thf/current-page file)] + (thf/apply-changes + file + (cls/generate-update-shapes (pcb/empty-changes nil (:id page)) + #{(thi/id label)} + update-fn + (:objects page) + {})))) + +(defn- destinations + [shape] + (mapv :destination (:interactions shape))) + +(t/deftest test-sync-flex-main-keeps-copy-interactions + (let [;; ==== Setup + file (setup-file) + margin {:m1 5 :m2 5 :m3 5 :m4 5} + + ;; ==== Action + updated-file (update-shape file :main-child + #(assoc % :hidden true :layout-item-margin margin)) + changes (cll/generate-sync-file-changes (pcb/empty-changes) + nil + :components + (:id updated-file) + (thi/id :component1) + (:id updated-file) + {(:id updated-file) updated-file} + (:id updated-file)) + file' (thf/apply-changes updated-file changes) + + ;; ==== Get + copy-child' (ths/get-shape file' :copy-child)] + + ;; ==== Check + (t/is (true? (:hidden copy-child'))) + (t/is (= margin (:layout-item-margin copy-child'))) + (t/is (= [(thi/id :copy-popup)] (destinations copy-child'))))) + +(t/deftest test-sync-flex-copy-to-main-keeps-main-interactions + (let [;; ==== Setup + file (setup-file) + + ;; ==== Action + updated-file (update-shape file :copy-child #(assoc % :hidden true)) + page (thf/current-page updated-file) + container (ctn/make-container page :page) + changes (-> (pcb/empty-changes) + (pcb/with-container container) + (cll/generate-sync-shape-inverse (:data updated-file) + {(:id updated-file) updated-file} + container + (thi/id :copy-root))) + file' (thf/apply-changes updated-file changes) + + ;; ==== Get + main-child' (ths/get-shape file' :main-child)] + + ;; ==== Check + (t/is (true? (:hidden main-child'))) + (t/is (= [(thi/id :main-popup)] (destinations main-child'))))) + +(defn- setup-library-and-file + "Library flex component with a nested copy, and a file instance whose + nested copy has an interaction." + [] + (let [library (-> (thf/sample-file :library) + (tho/add-nested-component :component1 + :main1-root + :main1-child + :component2 + :main2-root + :nested-head + :main2-root-params {:layout :flex + :layout-flex-dir :row})) + file (-> (thf/sample-file :file) + (ths/add-sample-shape :copy-popup :type :frame) + (thc/instantiate-component :component2 + :copy2-root + :library library + :children-labels [:copy-nested-head]) + (ths/add-interaction :copy-nested-head :copy-popup))] + [library file])) + +(defn- sync-file-from-library + [file library] + (cll/generate-sync-file-changes (pcb/empty-changes) + nil + nil + (:id file) + nil + (:id library) + {(:id library) library + (:id file) file} + (:id file))) + +(t/deftest test-sync-library-flex-keeps-nested-copy-interactions + (let [;; ==== Setup + [library file] (setup-library-and-file) + copy-x (:x (tho/bottom-shape file :copy2-root)) + + ;; ==== Action + library' (-> library + (update-shape :main1-child #(gsh/move % (gpt/point 1 0))) + (tho/propagate-component-changes :component1)) + file' (thf/apply-changes file (sync-file-from-library file library')) + + ;; ==== Get + copy-nested-head' (ths/get-shape file' :copy-nested-head)] + + ;; ==== Check + (t/is (mth/close? (inc copy-x) (:x (tho/bottom-shape file' :copy2-root)))) + (t/is (= [(thi/id :copy-popup)] (destinations copy-nested-head'))))) 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"))) diff --git a/common/test/common_tests/runner.cljc b/common/test/common_tests/runner.cljc index 081516c73c..eb2e3bb1c7 100644 --- a/common/test/common_tests/runner.cljc +++ b/common/test/common_tests/runner.cljc @@ -56,6 +56,7 @@ [common-tests.logic.chained-propagation-test] [common-tests.logic.comp-creation-test] [common-tests.logic.comp-detach-with-nested-test] + [common-tests.logic.comp-flex-interactions-test] [common-tests.logic.comp-main-edit-breaks-copy-slots-test] [common-tests.logic.comp-remove-swap-slots-test] [common-tests.logic.comp-reset-test] @@ -157,6 +158,7 @@ 'common-tests.logic.chained-propagation-test 'common-tests.logic.comp-creation-test 'common-tests.logic.comp-detach-with-nested-test + 'common-tests.logic.comp-flex-interactions-test 'common-tests.logic.comp-main-edit-breaks-copy-slots-test 'common-tests.logic.comp-remove-swap-slots-test 'common-tests.logic.comp-reset-test diff --git a/docker/devenv/Dockerfile b/docker/devenv/Dockerfile index b198c875a1..619e826549 100644 --- a/docker/devenv/Dockerfile +++ b/docker/devenv/Dockerfile @@ -67,7 +67,7 @@ RUN set -ex; \ FROM base AS setup-node ENV NODE_VERSION=v24.21.0 \ - PNPM_VERSION=12.8.1 \ + PNPM_VERSION=12.9.1 \ PATH=/opt/node/bin:$PATH RUN set -eux; \ @@ -77,13 +77,13 @@ RUN set -eux; \ OPENSSL_ARCH='linux-aarch64'; \ BINARY_URL="https://nodejs.org/dist/${NODE_VERSION}/node-${NODE_VERSION}-linux-arm64.tar.gz"; \ PNPM_ARCH='arm64'; \ - PNPM_SHA256='75c5e34d3164fe3d3549580b1b1b59c9dd16543e558dec722b686193cdb7dfa1'; \ + PNPM_SHA256='468294b77633d29889c6e4a79cefb1277eba30cd62e7d6a754218cb85a1d730d'; \ ;; \ amd64|x86_64) \ OPENSSL_ARCH='linux-x86_64'; \ BINARY_URL="https://nodejs.org/dist/${NODE_VERSION}/node-${NODE_VERSION}-linux-x64.tar.gz"; \ PNPM_ARCH='x64'; \ - PNPM_SHA256='db3906881f63e41b655e3b97d473603dc26326c2470d0b1b8083689e1bcbd1e8'; \ + PNPM_SHA256='b4f58449ac02d9d24023ac89e17e05e8331f8ddc12ca1f489cf231bc2bb2bce1'; \ ;; \ *) \ echo "Unsupported arch: ${ARCH}"; \ @@ -109,18 +109,18 @@ RUN set -eux; \ FROM base AS setup-opencode -ENV OPENCODE_VERSION=2.0.21 +ENV OPENCODE_VERSION=2.0.24 RUN set -eux; \ ARCH="$(dpkg --print-architecture)"; \ case "${ARCH}" in \ aarch64|arm64) \ BINARY_URL="https://registry.npmjs.org/@opencode/cli-linux-arm64/-/cli-linux-arm64-${OPENCODE_VERSION}.tgz"; \ - ESUM='046c0e0e8eec8f6085d46c0eca29ece65fb00e21df0881c81cbcf7392458b1d2'; \ + ESUM='9d0cd2bfc060fd6c2afdf6db69837f8690d46a8d8327bb864421851ce004f453'; \ ;; \ amd64|x86_64) \ BINARY_URL="https://registry.npmjs.org/@opencode/cli-linux-x64/-/cli-linux-x64-${OPENCODE_VERSION}.tgz"; \ - ESUM='57db421ab689d6831178c84736ba1e667c9c27627866849a114fa57afbc81785'; \ + ESUM='21b1ee0683841405d69541fc44481f8e5752e95bea72e8b8ec31dbcdb102e7f8'; \ ;; \ *) \ echo "Unsupported arch: ${ARCH}"; exit 1; \ @@ -140,7 +140,7 @@ RUN set -eux; \ FROM base AS setup-caddy -ENV CADDY_VERSION=2.11.4 +ENV CADDY_VERSION=2.11.7 RUN set -eux; \ ARCH="$(dpkg --print-architecture)"; \ @@ -255,12 +255,12 @@ RUN set -eux; \ FROM base AS setup-utils ENV CLJKONDO_VERSION=2026.08.04 \ - BABASHKA_VERSION=1.13.223 \ - CLJFMT_VERSION=0.16.5 \ + BABASHKA_VERSION=1.13.225 \ + CLJFMT_VERSION=0.16.6 \ PIXI_VERSION=0.81.0 \ - GITHUB_CLI_VERSION=2.101.0 \ - AWS_CLI_VERSION=2.36.49 \ - UV_VERSION=0.12.17 \ + GITHUB_CLI_VERSION=2.102.0 \ + AWS_CLI_VERSION=2.37.9 \ + UV_VERSION=0.12.23 \ UV_TOOL_DIR=/opt/uv/tools \ UV_TOOL_BIN_DIR=/opt/utils/bin \ UV_PYTHON_INSTALL_DIR=/opt/uv/python \ @@ -375,11 +375,11 @@ RUN set -ex; \ case "${ARCH}" in \ aarch64|arm64) \ BINARY_ARCH="aarch64"; \ - BINARY_SHA256="2dec26fdaff2f5e34961ea98f456297104f73d8409fc3599c8f2909c6e27a07e"; \ + BINARY_SHA256="e7d2cca3622af4765871fe9ce7eeed03acd09dca10a99ebc46e92f79d78765c0"; \ ;; \ amd64|x86_64) \ BINARY_ARCH="x86_64"; \ - BINARY_SHA256="6b51b62d232217350e72d8f2493f47d3461239d011d1d5eef960dfcd8cb07069"; \ + BINARY_SHA256="6b3a6a3d7bb3997928f0bdf7b866914224abf242c2e54e1dcebbe84bee64f356"; \ ;; \ *) \ echo "Unsupported arch: ${ARCH}"; \ diff --git a/frontend/playwright/ui/specs/tokens/stroke-per-side.spec.js b/frontend/playwright/ui/specs/tokens/stroke-per-side.spec.js index 61952665bc..50c558478d 100644 --- a/frontend/playwright/ui/specs/tokens/stroke-per-side.spec.js +++ b/frontend/playwright/ui/specs/tokens/stroke-per-side.spec.js @@ -1,3 +1,4 @@ +import { readFile } from "node:fs/promises"; import { test, expect } from "@playwright/test"; import { BaseWebSocketPage } from "../../pages/BaseWebSocketPage"; import { WasmWorkspacePage } from "../../pages/WasmWorkspacePage"; @@ -54,6 +55,76 @@ async function unfoldStrokeWidth(page, workspace) { await unfoldTokenType(workspace.tokensSidebar, "stroke width"); } +const STROKE_TOKEN_FILE = "workspace/get-file-layout-stroke-token-json"; +const PER_SIDE_RECT = "Per side rect"; +const PER_SIDE_TOKEN_BOARD = "Per side token board"; + +// Transit fields appended at the end of a shape, so the earlier "^N" cache +// references stay valid; when a key repeats, the last value wins. +function perSideStrokeFields(name, top, appliedTokens = []) { + const stroke = [ + "^ ", + ...["~:stroke-style", "~:solid", "~:stroke-alignment", "~:inner"], + ...["~:stroke-color", "#000000", "~:stroke-opacity", 1], + ...["~:stroke-width", top, "~:stroke-width-top", top], + ...["~:stroke-width-right", 4, "~:stroke-width-bottom", 8], + ...["~:stroke-width-left", 16], + ]; + return [ + "~:name", + name, + "~:strokes", + [stroke], + "~:applied-tokens", + ["^ ", ...appliedTokens], + ]; +} + +// Serves the stroke token file with a rectangle and a board whose sides +// have different widths (top 2 or 20, right 4, bottom 8, left 16). Only the +// board has a token, `width-big`, on its top side. Both are top-level +// layers, so the layers panel shows them without unfolding anything. +async function mockPerSideStrokeFile(workspace) { + await workspace.mockGetFile(STROKE_TOKEN_FILE); + + const payload = JSON.parse( + await readFile(`playwright/data/${STROKE_TOKEN_FILE}`, "utf-8"), + ); + const [pageData] = Object.values(payload["~:data"]["~:pages-index"]); + const objects = pageData["~:objects"]["~#penpot/objects-map/v2"]; + + const patch = (id, fields) => { + const shape = objects[id]; + const extra = JSON.stringify(fields).slice(1, -1); + objects[id] = `${shape.slice(0, -2)},${extra}]]`; + }; + + patch( + "~u8506e3f3-e05b-807c-8007-6ceac380abc1", + perSideStrokeFields(PER_SIDE_RECT, 2), + ); + patch( + "~u6c65a5dc-fb40-8072-8007-6ce644905054", + perSideStrokeFields(PER_SIDE_TOKEN_BOARD, 20, [ + "~:stroke-width-top", + "width-big", + ]), + ); + + // The last matching route wins over the one set by mockGetFile. + await workspace.page.route(/get\-file\?/, (route) => + route.fulfill({ + status: 200, + contentType: "application/transit+json", + body: JSON.stringify(payload), + }), + ); +} + +function globalWidthInput(page) { + return strokeRow(page).getByRole("textbox", { name: GLOBAL }); +} + test.describe("Tokens: stroke per side", () => { test("per-side toggle is hidden when the feature flag is off", async ({ page, @@ -583,7 +654,58 @@ test.describe("Tokens: stroke per side (inspect)", () => { }); }); +test.describe("Tokens: stroke per side, global width input", () => { + test("shows Mixed for different sides with the new renderer", async ({ + page, + }) => { + const workspace = new WasmWorkspacePage(page); + await workspace.mockConfigFlags([ + "enable-stroke-per-side", + "enable-feature-token-input", + ]); + await workspace.setupEmptyFile(); + await mockPerSideStrokeFile(workspace); + await workspace.goToWorkspace(); + await workspace.waitForFirstRender(); + + await workspace.clickLeafLayer(PER_SIDE_RECT); + await expect(globalWidthInput(page)).toHaveAttribute("placeholder", MIXED); + + await workspace.clickLeafLayer(PER_SIDE_TOKEN_BOARD); + await expect(globalWidthInput(page)).toHaveAttribute("placeholder", MIXED); + }); +}); + test.describe("Tokens: stroke per side (non wasm)", () => { + test("the global width input shows the top side without the new renderer", async ({ + page, + }) => { + const workspace = new WorkspacePage(page); + await workspace.mockConfigFlags([ + "enable-stroke-per-side", + "enable-feature-token-input", + ]); + await workspace.setupEmptyFile(); + await mockPerSideStrokeFile(workspace); + await workspace.goToWorkspace(); + + // Top 2, other sides 4, 8 and 16: the old render draws the top side, + // so the input shows it instead of Mixed. + await workspace.clickLeafLayer(PER_SIDE_RECT); + await expect(globalWidthInput(page)).toHaveValue("2"); + await expect(globalWidthInput(page)).not.toHaveAttribute( + "placeholder", + MIXED, + ); + + // Only the top side has a token: the input shows that token. + await workspace.clickLeafLayer(PER_SIDE_TOKEN_BOARD); + await expect( + strokeRow(page).getByRole("button", { name: "width-big" }), + ).toHaveCount(1); + await expect(globalWidthInput(page)).toHaveCount(0); + }); + test("per-side toggle is disabled without the new renderer", async ({ page, }) => { diff --git a/frontend/src/app/main/data/exports/assets.cljs b/frontend/src/app/main/data/exports/assets.cljs index e1f8f8b508..27c0133cc4 100644 --- a/frontend/src/app/main/data/exports/assets.cljs +++ b/frontend/src/app/main/data/exports/assets.cljs @@ -193,7 +193,7 @@ (update :export assoc :in-progress false :last-update (ct/now) :healthy? healthy?) (= status "ended") - (update :export assoc :in-progress false :last-update (ct/now) :healthy? healthy?)))) + (update :export assoc :in-progress false :progress done :last-update (ct/now) :healthy? healthy?)))) ptk/WatchEvent (watch [_ _ _] diff --git a/frontend/src/app/main/data/workspace/tokens/remapping.cljs b/frontend/src/app/main/data/workspace/tokens/remapping.cljs index e36b9d1b1d..7ff4a8be4b 100644 --- a/frontend/src/app/main/data/workspace/tokens/remapping.cljs +++ b/frontend/src/app/main/data/workspace/tokens/remapping.cljs @@ -115,11 +115,14 @@ shape-changes (reduce-kv (fn [changes container refs] - (let [shape-ids (map :shape-id refs) + (let [;; Each shape keeps only the attributes it already had the + ;; token on, so the rename never applies the token to new + ;; attributes (e.g. a stroke-width token on a single side). + attributes-by-shape (update-vals (group-by :shape-id refs) + #(into #{} (map :attribute) %)) ;; Find the correct token to apply (new or alias) - token (or (some #(when (= (:name (:token %)) new-token-name) %) tokens-with-sets) - (some #(when (= (:name (:token %)) old-token-name) %) tokens-with-sets)) - attributes (set (map :attribute refs))] + token (or (some #(when (= (:name (:token %)) new-token-name) %) tokens-with-sets) + (some #(when (= (:name (:token %)) old-token-name) %) tokens-with-sets))] (if token ;; Create a new independent changes so we can call `with-file-data` after `with-container` ;; otherwise it causes probelms looking up for objects @@ -127,10 +130,11 @@ (-> (pcb/empty-changes) (pcb/with-container container) (pcb/with-file-data file-data) - (pcb/update-shapes shape-ids + (pcb/update-shapes (keys attributes-by-shape) (fn [shape] - (update shape :applied-tokens - #(merge % (cft/attributes-map attributes (:token token))))) + (let [attributes (get attributes-by-shape (:id shape))] + (update shape :applied-tokens + #(merge % (cft/attributes-map attributes (:token token)))))) {:ignore-touched true}))] (pcb/concat-changes changes container-changes)) changes))) diff --git a/frontend/src/app/main/ui/dashboard/import.cljs b/frontend/src/app/main/ui/dashboard/import.cljs index f6962592b4..dd459da077 100644 --- a/frontend/src/app/main/ui/dashboard/import.cljs +++ b/frontend/src/app/main/ui/dashboard/import.cljs @@ -602,7 +602,7 @@ :class (stl/css :context-notification-error)} (tr "dashboard.import.import-error.disclaimer")]) - (when (or (= :import-error status) (and (= :analyze status) errors?)) + (if (or (= :import-error status) (and (= :analyze status) errors?)) [:div {:class (stl/css :import-error-disclaimer)} [:div (tr "dashboard.import.import-error.message1")] [:ul {:class (stl/css :import-error-list)} @@ -624,18 +624,18 @@ :else (tr "dashboard.import.import-error.unknown-error"))])]))] - [:div (tr "dashboard.import.import-error.message2")]]) + [:div (tr "dashboard.import.import-error.message2")]] - (for [entry entries] - [:> import-entry* {:edition edition - :key (dm/str (:uri entry) "/" (:file-id entry)) - :entry entry - :entries entries - :is-progress (= :import-progress status) - :on-edit on-edit - :on-change on-change - :on-delete on-delete - :can-be-deleted (> (count entries) 1)}]) + (for [entry entries] + [:> import-entry* {:edition edition + :key (dm/str (:uri entry) "/" (:file-id entry)) + :entry entry + :entries entries + :is-progress (= :import-progress status) + :on-edit on-edit + :on-change on-change + :on-delete on-delete + :can-be-deleted (> (count entries) 1)}])) (when (some? template) [:> import-entry* {:entry (assoc template :status status) diff --git a/frontend/src/app/main/ui/inspect/styles/panels/stroke.cljs b/frontend/src/app/main/ui/inspect/styles/panels/stroke.cljs index 53ac6d5b8c..039cfd3ce4 100644 --- a/frontend/src/app/main/ui/inspect/styles/panels/stroke.cljs +++ b/frontend/src/app/main/ui/inspect/styles/panels/stroke.cljs @@ -42,20 +42,44 @@ :border-width :stroke-width :border-color :stroke-color}) -(defn- get-applied-tokens-in-shape - [shape-tokens property] - (let [border-prop (get shape-prop->stroke-prop property)] - (if border-prop - (get shape-tokens border-prop) - (get shape-tokens property)))) +(def ^:private per-side-prop->token-attr + {:border-block-start-width :stroke-width-top + :border-inline-end-width :stroke-width-right + :border-block-end-width :stroke-width-bottom + :border-inline-start-width :stroke-width-left}) + +(defn- all-sides-token-name + "Token applied to the four stroke width sides when they all share it." + [applied-tokens] + (let [sides (map #(get applied-tokens %) + [:stroke-width-top :stroke-width-right + :stroke-width-bottom :stroke-width-left])] + (when (and (every? some? sides) (apply = sides)) + (first sides)))) + +(defn stroke-property-token-name + "Name of the token applied to the stroke `property`, read from the shape + `applied-tokens`. Stroke width tokens are stored per side, so a per-side + property reads its side and `:border-width` reads the token shared by + every side. Files not migrated yet keep it in the old `:stroke-width`." + [applied-tokens property] + (cond + (contains? per-side-prop->token-attr property) + (get applied-tokens (get per-side-prop->token-attr property)) + + (= :border-width property) + (or (all-sides-token-name applied-tokens) + (get applied-tokens :stroke-width)) + + :else + (let [border-prop (get shape-prop->stroke-prop property)] + (get applied-tokens (or border-prop property))))) (defn- get-resolved-token "Get the resolved token for a specific property in a shape." [property shape resolved-tokens] - (let [shape-tokens (:applied-tokens shape) - applied-tokens-in-shape (get-applied-tokens-in-shape shape-tokens property) - token (get resolved-tokens applied-tokens-in-shape)] - token)) + (let [token-name (stroke-property-token-name (:applied-tokens shape) property)] + (get resolved-tokens token-name))) ;; Current token implementation on strokes only supports one token per shape and has to be the first stroke ;; This must be improved in the future diff --git a/frontend/src/app/main/ui/workspace/sidebar/options/rows/stroke_row.cljs b/frontend/src/app/main/ui/workspace/sidebar/options/rows/stroke_row.cljs index dd774a3390..278626cab8 100644 --- a/frontend/src/app/main/ui/workspace/sidebar/options/rows/stroke_row.cljs +++ b/frontend/src/app/main/ui/workspace/sidebar/options/rows/stroke_row.cljs @@ -28,6 +28,51 @@ [app.util.i18n :as i18n :refer [tr]] [rumext.v2 :as mf])) +(defn stroke-width-input-value + "Value for the global stroke width input: the width when every side is + equal, `:multiple` otherwise. When per side strokes are disabled (old + render) the input shows the top side, the one that render draws." + [stroke per-side-disabled] + (let [width (:stroke-width stroke) + top (d/nilv (:stroke-width-top stroke) width) + right (d/nilv (:stroke-width-right stroke) width) + bottom (d/nilv (:stroke-width-bottom stroke) width) + left (d/nilv (:stroke-width-left stroke) width)] + (cond + per-side-disabled + top + + (= top right bottom left) + width + + :else + :multiple))) + +(defn stroke-width-input-token + "Token for the global stroke width input: the token when every side has + the same one, `:multiple` when only some sides have a token, nil when no + side has one. When per side strokes are disabled (old render) the input + shows the top side token, like `stroke-width-input-value` does." + [applied-tokens per-side-disabled] + (let [top (get applied-tokens :stroke-width-top) + right (get applied-tokens :stroke-width-right) + bottom (get applied-tokens :stroke-width-bottom) + left (get applied-tokens :stroke-width-left) + per-side [top right bottom left] + per-side-has-token? (some some? per-side)] + (cond + per-side-disabled + top + + (and (every? some? per-side) (apply = per-side)) + top + + per-side-has-token? + :multiple + + :else + nil))) + (mf/defc stroke-row* [{:keys [index stroke @@ -114,35 +159,15 @@ (not per-side-disabled) (true? (get stroke-per-side [ids index]))) - all-sides-equal? - (mf/with-memo [stroke] - (let [width (:stroke-width stroke) - top (d/nilv (:stroke-width-top stroke) width) - right (d/nilv (:stroke-width-right stroke) width) - bottom (d/nilv (:stroke-width-bottom stroke) width) - left (d/nilv (:stroke-width-left stroke) width)] - (= top right bottom left))) + stroke-width-value + (mf/with-memo [stroke per-side-disabled] + (stroke-width-input-value stroke per-side-disabled)) - show-multiple-placeholder? (or per-side? (not all-sides-equal?)) + show-multiple-placeholder? (or per-side? (= :multiple stroke-width-value)) applied-token-width - (mf/with-memo [applied-tokens] - (let [top (get applied-tokens :stroke-width-top) - right (get applied-tokens :stroke-width-right) - bottom (get applied-tokens :stroke-width-bottom) - left (get applied-tokens :stroke-width-left) - - per-side [top right bottom left] - per-side-has-token? (some some? per-side)] - (cond - (and (every? some? per-side) (apply = per-side)) - (first per-side) - - per-side-has-token? - :multiple - - :else - nil))) + (mf/with-memo [applied-tokens per-side-disabled] + (stroke-width-input-token applied-tokens per-side-disabled)) per-side-toggle-label (if per-side-disabled @@ -409,9 +434,7 @@ :placeholder (if show-multiple-placeholder? (tr "settings.multiple") "--") - :value (if all-sides-equal? - (:stroke-width stroke) - :multiple)}] + :value stroke-width-value}] [:> select* {:default-selected (d/name stroke-alignment) :options stroke-alignment-options :variant "icon-only" @@ -445,7 +468,7 @@ :size "s"}] [:> deprecated-input/numeric-input* {:value (if show-multiple-placeholder? nil - stroke-width) + stroke-width-value) :min 0 :placeholder (if show-multiple-placeholder? (tr "settings.multiple") diff --git a/frontend/src/app/main/ui/workspace/sidebar/scroll.cljs b/frontend/src/app/main/ui/workspace/sidebar/scroll.cljs index 7bcaa73bef..235e29dec5 100644 --- a/frontend/src/app/main/ui/workspace/sidebar/scroll.cljs +++ b/frontend/src/app/main/ui/workspace/sidebar/scroll.cljs @@ -45,17 +45,26 @@ (when (needs-restore-retry? prev-height height attempts) (vreset! raf* (tm/raf #(restore-loop! node saved height (inc attempts) raf*)))))) +(defn- restore-position! + "Restores saved, or scrolls to the top when there is none: the node can + stay mounted while the key changes (token sets), so it may still hold + another list's position." + [node saved raf*] + (if (some? saved) + (restore-loop! node saved -1 0 raf*) + (restore-scroll! node 0))) + (defn use-restore-scroll "Restores the scrollTop saved under [panel id] on mount and whenever panel or id change. Retries while the content height keeps changing so deep positions in lazily rendered lists are not clamped to the first - chunk. Missing key or node is a silent no-op." + chunk. With no saved position the node goes to the top. Missing node is + a silent no-op." [store* panel id node-ref] (mf/with-effect [panel id] (let [raf* (volatile! nil)] (when-let [node (mf/ref-val node-ref)] - (when-let [saved (get @store* [panel id])] - (restore-loop! node saved -1 0 raf*))) + (restore-position! node (get @store* [panel id]) raf*)) (fn [] (when-some [raf @raf*] (tm/cancel-af! raf)))))) diff --git a/frontend/src/app/main/ui/workspace/tokens/management/token_tree.scss b/frontend/src/app/main/ui/workspace/tokens/management/token_tree.scss index 87ef7f2e4d..ee686c5f8e 100644 --- a/frontend/src/app/main/ui/workspace/tokens/management/token_tree.scss +++ b/frontend/src/app/main/ui/workspace/tokens/management/token_tree.scss @@ -22,10 +22,6 @@ margin-block-end: var(--sp-s); } } - - & .token-pill { - flex: 0 0 auto; - } } .node-parent { @@ -37,7 +33,9 @@ padding-inline-start: var(--sp-s); border-inline-start: $b-2 solid var(--color-background-quaternary); - &:has(> button) { + // Match pills as "not a group" so the rule does not depend on the + // pill's own markup, which lives in another module. + &:has(> :not(.node-parent)) { display: flex; flex-wrap: wrap; gap: var(--sp-xs); @@ -50,8 +48,4 @@ margin-block-end: var(--sp-s); } } - - & .token-pill { - flex: 0 0 auto; - } } diff --git a/frontend/test/frontend_tests/runner.cljs b/frontend/test/frontend_tests/runner.cljs index 413f40cc08..466358a9db 100644 --- a/frontend/test/frontend_tests/runner.cljs +++ b/frontend/test/frontend_tests/runner.cljs @@ -110,6 +110,7 @@ [frontend-tests.ui.ds-controls-numeric-input-test] [frontend-tests.ui.gradient-handlers-test] [frontend-tests.ui.history-test] + [frontend-tests.ui.inspect-stroke-tokens-test] [frontend-tests.ui.layout-container-multiple-test] [frontend-tests.ui.measures-menu-props-test] [frontend-tests.ui.organization-team-switch-test] @@ -249,6 +250,7 @@ 'frontend-tests.ui.ds-controls-numeric-input-test 'frontend-tests.ui.gradient-handlers-test 'frontend-tests.ui.history-test + 'frontend-tests.ui.inspect-stroke-tokens-test 'frontend-tests.ui.layout-container-multiple-test 'frontend-tests.ui.measures-menu-props-test 'frontend-tests.ui.organization-team-switch-test diff --git a/frontend/test/frontend_tests/tokens/logic/token_remapping_test.cljs b/frontend/test/frontend_tests/tokens/logic/token_remapping_test.cljs index acad25311c..fc47db9132 100644 --- a/frontend/test/frontend_tests/tokens/logic/token_remapping_test.cljs +++ b/frontend/test/frontend_tests/tokens/logic/token_remapping_test.cljs @@ -120,6 +120,39 @@ (let [result (dwtr/validate-token-remapping "color.primary" "brand.primary")] (t/is (true? (:valid? result))))))) +(t/deftest test-remap-keeps-applied-attributes-per-shape + (t/testing "renaming a token only renames the attributes each shape already had" + (let [stroke-token {:id (cthi/new-id! :sw-10) + :name "sw-10" + :value "10" + :type :stroke-width} + file (-> (ctht/sample-file-with-tokens + :lib-fn #(-> % + (ctob/add-set (ctob/make-token-set :id (cthi/new-id! :set-a) + :name "Set A")) + (ctob/add-token (cthi/id :set-a) + (ctob/make-token stroke-token)))) + (ctho/add-rect :top-rect) + (ctho/add-rect :all-rect) + (ctht/apply-token-to-shape :top-rect "sw-10" + [:stroke-width-top] [] nil) + (ctht/apply-token-to-shape :all-rect "sw-10" + cto/stroke-width-keys [] nil) + ;; The production flow renames the token first, + ;; then remaps the shapes that use it. + (update-in [:data :tokens-lib] + ctob/update-token (cthi/id :set-a) (cthi/id :sw-10) + #(assoc % :name "sw-big"))) + changes (dwtr/build-remap-changes (:data file) "sw-10" "sw-big") + file' (cthf/apply-changes file changes) + top-rect' (cths/get-shape file' :top-rect) + all-rect' (cths/get-shape file' :all-rect)] + + (t/is (= {:stroke-width-top "sw-big"} + (:applied-tokens top-rect'))) + (t/is (= (zipmap cto/stroke-width-keys (repeat "sw-big")) + (:applied-tokens all-rect')))))) + (defn- setup-file-with-component-copy-and-token "Create a file containing a component, an instance copy of it, and a single `color.primary` token already applied to both the main and the copy shape. diff --git a/frontend/test/frontend_tests/ui/inspect_stroke_tokens_test.cljs b/frontend/test/frontend_tests/ui/inspect_stroke_tokens_test.cljs new file mode 100644 index 0000000000..9c7638a2dd --- /dev/null +++ b/frontend/test/frontend_tests/ui/inspect_stroke_tokens_test.cljs @@ -0,0 +1,43 @@ +;; 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 frontend-tests.ui.inspect-stroke-tokens-test + (:require + [app.common.types.token :as cto] + [app.main.ui.inspect.styles.panels.stroke :as stroke] + [cljs.test :as t :include-macros true])) + +(def ^:private all-sides-sw-3 + (zipmap cto/per-side-stroke-width-keys (repeat "sw-3"))) + +(t/deftest border-width-shows-the-token-applied-to-every-side + (t/is (= "sw-3" (stroke/stroke-property-token-name all-sides-sw-3 :border-width)))) + +(t/deftest border-width-shows-no-token-when-the-sides-differ + (t/is (nil? (stroke/stroke-property-token-name + (assoc all-sides-sw-3 :stroke-width-top "sw-10") :border-width))) + (t/is (nil? (stroke/stroke-property-token-name + {:stroke-width-top "sw-3"} :border-width)))) + +(t/deftest border-width-reads-the-legacy-key-of-unmigrated-files + (t/is (= "sw-3" (stroke/stroke-property-token-name {:stroke-width "sw-3"} :border-width)))) + +(t/deftest per-side-widths-show-the-token-of-each-side + (let [applied {:stroke-width-top "sw-10" + :stroke-width-right "sw-3" + :stroke-width-bottom "sw-3" + :stroke-width-left "sw-4"}] + (t/is (= "sw-10" (stroke/stroke-property-token-name applied :border-block-start-width))) + (t/is (= "sw-3" (stroke/stroke-property-token-name applied :border-inline-end-width))) + (t/is (= "sw-3" (stroke/stroke-property-token-name applied :border-block-end-width))) + (t/is (= "sw-4" (stroke/stroke-property-token-name applied :border-inline-start-width))))) + +(t/deftest per-side-widths-without-a-token-show-none + (t/is (nil? (stroke/stroke-property-token-name + {:stroke-width-top "sw-10"} :border-inline-end-width)))) + +(t/deftest border-color-keeps-the-stroke-color-token + (t/is (= "red" (stroke/stroke-property-token-name {:stroke-color "red"} :border-color)))) diff --git a/frontend/test/frontend_tests/ui/sidebar_scroll_test.cljs b/frontend/test/frontend_tests/ui/sidebar_scroll_test.cljs index 9cf7bd8bcc..609ed6c35b 100644 --- a/frontend/test/frontend_tests/ui/sidebar_scroll_test.cljs +++ b/frontend/test/frontend_tests/ui/sidebar_scroll_test.cljs @@ -11,6 +11,7 @@ [cljs.test :as t :include-macros true])) (def ^:private restore-loop! @#'sc/restore-loop!) +(def ^:private restore-position! @#'sc/restore-position!) (defn- scroll-event [scroll-top] @@ -64,3 +65,21 @@ ((first @scheduled)) (t/is (= 250 (.-scrollTop node))) (t/is (= 1 (count @scheduled)) "no further frames once settled")))) + +(t/deftest restore-position-without-saved-value-goes-to-top + (let [node #js {:scrollTop 400 :scrollHeight 900} + scheduled (atom []) + raf* (volatile! nil)] + (with-redefs [tm/raf (fn [f] (swap! scheduled conj f) 1)] + (restore-position! node nil raf*) + (t/is (= 0 (.-scrollTop node)) "a node kept mounted drops the old position") + (t/is (empty? @scheduled) "no retry loop for the top")))) + +(t/deftest restore-position-with-saved-value-runs-loop + (let [node #js {:scrollTop 0 :scrollHeight 900} + scheduled (atom []) + raf* (volatile! nil)] + (with-redefs [tm/raf (fn [f] (swap! scheduled conj f) 1)] + (restore-position! node 250 raf*) + (t/is (= 250 (.-scrollTop node))) + (t/is (= 1 (count @scheduled)) "first pass schedules a settle check")))) diff --git a/frontend/test/frontend_tests/ui/stroke_menu_test.cljs b/frontend/test/frontend_tests/ui/stroke_menu_test.cljs index e219333e99..b6fcc7407e 100644 --- a/frontend/test/frontend_tests/ui/stroke_menu_test.cljs +++ b/frontend/test/frontend_tests/ui/stroke_menu_test.cljs @@ -6,8 +6,10 @@ (ns frontend-tests.ui.stroke-menu-test (:require + [app.common.types.token :as cto] [app.config :as cf] [app.main.ui.workspace.sidebar.options.menus.stroke :as stroke] + [app.main.ui.workspace.sidebar.options.rows.stroke-row :as stroke-row] [clojure.test :as t :include-macros true])) (def ^:private per-side-flags (conj cf/flags :stroke-per-side)) @@ -54,3 +56,40 @@ (t/testing "disabled when the feature flag is off" (with-redefs [cf/flags (disj cf/flags :stroke-per-side)] (t/is (false? (stroke/per-side-stroke-enabled? true)))))) + +(def ^:private per-side-stroke + {:stroke-width 2 + :stroke-width-top 2 + :stroke-width-right 4 + :stroke-width-bottom 8 + :stroke-width-left 16}) + +(t/deftest stroke-width-input-value-test + (t/testing "shows the width when every side is equal" + (t/is (= 5 (stroke-row/stroke-width-input-value {:stroke-width 5} false))) + (t/is (= 5 (stroke-row/stroke-width-input-value + (stroke/stroke-width-all-attrs 5) false)))) + + (t/testing "shows multiple when the sides differ" + (t/is (= :multiple (stroke-row/stroke-width-input-value per-side-stroke false)))) + + (t/testing "shows the top side when per side strokes are disabled" + (t/is (= 2 (stroke-row/stroke-width-input-value per-side-stroke true))))) + +(t/deftest stroke-width-input-token-test + (t/testing "shows the token when every side has the same one" + (t/is (= "sw" (stroke-row/stroke-width-input-token + (zipmap cto/per-side-stroke-width-keys (repeat "sw")) false)))) + + (t/testing "shows multiple when only some sides have the token" + (t/is (= :multiple (stroke-row/stroke-width-input-token + {:stroke-width-top "sw"} false)))) + + (t/testing "shows no token when no side has one" + (t/is (nil? (stroke-row/stroke-width-input-token {} false)))) + + (t/testing "shows the top side token when per side strokes are disabled" + (t/is (= "sw" (stroke-row/stroke-width-input-token + {:stroke-width-top "sw" :stroke-width-left "other"} true))) + (t/is (nil? (stroke-row/stroke-width-input-token + {:stroke-width-left "other"} true))))) diff --git a/plugins/CHANGELOG.md b/plugins/CHANGELOG.md index 5c1e07dd74..4b45df68b0 100644 --- a/plugins/CHANGELOG.md +++ b/plugins/CHANGELOG.md @@ -16,6 +16,7 @@ - **plugins-runtime**: Removed the premature deep-hardening of the host plugin context, which froze shared host functions (including `Function.prototype`) before SES override taming, causing `TypeError: Cannot assign to read only property 'toString'` on later host-side function extension. Related to #11001. - **plugins-runtime**: The plugin modal no longer shows two resize grips in Firefox. Firefox now shows only its native grip. Closes #11795. - **plugins-runtime**: Fixed the `fontFamilies` token property mapping so `Shape.applyToken(token, ["fontFamilies"])` resolves to the canonical `:font-family` attribute and applied-token readback exposes the documented `fontFamilies` key instead of the undocumented singular `fontFamily`. Closes #11405. +- **plugins-runtime**: Opening a plugin no longer drops running background plugins (`allowBackground`, such as the MCP plugin) from the runtime registry. They kept running, but their UI messages were no longer delivered to the plugin and they could no longer be unloaded, so MCP tasks timed out after another plugin was opened. ## 1.5.0 (2026-07-08) diff --git a/plugins/libs/plugins-runtime/src/lib/load-plugin.spec.ts b/plugins/libs/plugins-runtime/src/lib/load-plugin.spec.ts index 810ebd54b3..779619279c 100644 --- a/plugins/libs/plugins-runtime/src/lib/load-plugin.spec.ts +++ b/plugins/libs/plugins-runtime/src/lib/load-plugin.spec.ts @@ -3,6 +3,7 @@ import { loadPlugin, ɵloadPlugin, ɵloadPluginByUrl, + ɵunloadPlugin, setContextBuilder, getPlugins, } from './load-plugin'; @@ -200,6 +201,47 @@ describe('plugin-loader', () => { expect(mockPluginApi1.plugin.sendMessage).not.toHaveBeenCalled(); }); + it('should keep background plugins registered when loading another plugin', async () => { + const backgroundIframeWindow = { nodeType: 1 } as unknown as Window; + const backgroundClose = vi.fn(); + const backgroundPluginApi = { + plugin: { + close: backgroundClose, + sendMessage: vi.fn(), + }, + iframeWindow: backgroundIframeWindow, + manifest: { + ...manifest, + pluginId: 'background-plugin', + allowBackground: true, + }, + } as unknown as Awaited>; + + vi.mocked(createPlugin).mockResolvedValue(backgroundPluginApi); + await loadPlugin(manifest); + + vi.mocked(createPlugin).mockResolvedValue(mockPluginApi); + await loadPlugin(manifest); + + expect(backgroundClose).not.toHaveBeenCalled(); + expect(getPlugins()).toContain(backgroundPluginApi); + + const event = new MessageEvent('message', { data: 'from-background' }); + Object.defineProperty(event, 'source', { value: backgroundIframeWindow }); + window.dispatchEvent(event); + + expect(backgroundPluginApi.plugin.sendMessage).toHaveBeenCalledWith( + 'from-background', + ); + + ɵunloadPlugin('background-plugin'); + expect(backgroundClose).toHaveBeenCalledTimes(1); + + // the runtime's close callback deregisters the plugin + vi.mocked(createPlugin).mock.calls[0][2](); + expect(getPlugins()).not.toContain(backgroundPluginApi); + }); + it('should load plugin using ɵloadPlugin', async () => { await ɵloadPlugin(manifest); diff --git a/plugins/libs/plugins-runtime/src/lib/load-plugin.ts b/plugins/libs/plugins-runtime/src/lib/load-plugin.ts index dfd44f4971..e22e62c70c 100644 --- a/plugins/libs/plugins-runtime/src/lib/load-plugin.ts +++ b/plugins/libs/plugins-runtime/src/lib/load-plugin.ts @@ -17,14 +17,20 @@ export function setContextBuilder(builder: ContextBuilder) { export const getPlugins = () => plugins; const closeAllPlugins = () => { + // Background plugins keep running, so they must stay registered: the + // registry routes their UI messages and lets them be unloaded later. + const backgroundPlugins: typeof plugins = []; + plugins.forEach((pluginApi) => { /* eslint-disable @typescript-eslint/no-explicit-any */ - if (!(pluginApi.manifest as any)?.allowBackground) { + if ((pluginApi.manifest as any)?.allowBackground) { + backgroundPlugins.push(pluginApi); + } else { pluginApi.plugin.close(); } }); - plugins = []; + plugins = backgroundPlugins; }; window.addEventListener('message', (event) => { diff --git a/render-wasm/src/render.rs b/render-wasm/src/render.rs index f95f324bfb..65f7665dec 100644 --- a/render-wasm/src/render.rs +++ b/render-wasm/src/render.rs @@ -989,7 +989,12 @@ impl RenderState { } pub fn set_background_color(&mut self, color: skia::Color) { + if self.background_color == color { + return; + } self.background_color = color; + // Tiles bake the background in; old atlas pixels would show on pan/zoom. + self.surfaces.atlas.clear(); } pub fn set_preview_mode(&mut self, enabled: bool) { diff --git a/render-wasm/src/render/surfaces.rs b/render-wasm/src/render/surfaces.rs index be6ccb1fb5..2c60832b45 100644 --- a/render-wasm/src/render/surfaces.rs +++ b/render-wasm/src/render/surfaces.rs @@ -184,6 +184,12 @@ impl DocAtlas { self.size.width <= 0 || self.size.height <= 0 } + /// Drops all atlas content while keeping its allocation and placement. + pub fn clear(&mut self) { + self.surface.canvas().clear(skia::Color::TRANSPARENT); + self.tile_doc_rects.clear(); + } + /// Sets the document-space bounds used to clamp atlas updates. /// Pass `None` to disable clamping. pub fn set_doc_bounds(&mut self, bounds: Option) {