From b54fa2f11ce41f19b37b1f5369042d3fc52007b7 Mon Sep 17 00:00:00 2001 From: Milos Milic <124778054+MilosM348@users.noreply.github.com> Date: Mon, 11 May 2026 13:52:36 +0200 Subject: [PATCH] :bug: Reject clipboard helpers gracefully on insecure origins (#9188) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit * :bug: Reject clipboard helpers gracefully on insecure origins Closes #6514. Resolves the user-visible crash originally reported in #4478. `app.util.clipboard/to-clipboard` and `to-clipboard-promise` called `(unchecked-get js/navigator "clipboard")` and then immediately invoked `.writeText` / `.write` on the result, with no guard for the case where `navigator.clipboard` is `undefined`. The W3C Clipboard API spec requires a "secure context" (HTTPS or localhost), so a Penpot instance served over plain HTTP - which the SSDP/LAN self-hosted setup in #4478 was - throws TypeError: Cannot read properties of undefined (reading 'writeText') synchronously the moment the user clicks any copy button. The error escapes the consuming function before any error-handling rx/of arm runs, so the whole UI ends up on the error screen instead of just the affected control showing a "could not copy" message. A third helper (`to-clipboard-multi`) already guards `clipboard` and `clipboard.write`, but if both are missing it silently returns nil which is also surprising for callers expecting a Promise. ## Fix Add a small `get-clipboard` accessor and an `unavailable-error` factory that returns `Promise.reject(Error(...))` with a clear message ("Clipboard API is unavailable. This usually happens when the page is served over plain HTTP; serve Penpot over HTTPS to enable copy-to-clipboard."). Wire all three helpers through the same defensive contract: - `to-clipboard` - return the rejected Promise when `navigator.clipboard.writeText` is missing. - `to-clipboard-promise` - return the rejected Promise when `navigator.clipboard.write` is missing. - `to-clipboard-multi` - convert the existing `if` into a `cond` with three branches: prefer `clipboard.write` for true multi-MIME output, fall through to `writeText` with the text/plain payload when only the legacy text path is available, and finally reject with the unavailable error when neither path exists. Previously the no-API case fell off the `when-let` and silently returned nil. The contract is now consistent: every helper either resolves or rejects a Promise, never throws synchronously, and never returns nil. Callers (which are already structured around rx streams that call `rx/from` on the helper's return value) can chain `.catch` / `rx/catch` to surface a status-bar message instead of crashing. The two stale `;; FIXME` comments on `to-clipboard` (rename to `write-text`) and `to-clipboard-promise` (this API is very confuse) are removed - the rename remains an open follow-up across 13+ call sites and is intentionally out of scope, but the API is no longer "confuse" once the contract is documented and uniform. CHANGES.md entry added under the 2.17.0 Bugs-fixed section describing the user-visible behaviour change. * :bug: Reject paste-from-navigator gracefully on insecure origins Symmetric companion to the to-clipboard / to-clipboard-promise / to-clipboard-multi guards added earlier in this PR. The paste path went through fromNavigator (frontend/src/app/util/clipboard.js) which called `navigator.clipboard.read()` with no nil-check; on insecure origins (plain HTTP / non-localhost) this raised an opaque `TypeError: Cannot read properties of undefined (reading 'read')` and the workspace surfaced a generic 'Something wrong has happened' toast instead of the descriptive 'serve Penpot over HTTPS …' message users get for the copy direction. Mirror the get-clipboard pattern from clipboard.cljs: - Read `navigator.clipboard` once into a local. - If it's missing the `.read` method, throw a descriptive Error that matches the wording the copy direction already uses (only the verb swaps: 'paste-from-clipboard' instead of 'copy-to-clipboard'). - Otherwise, dispatch through the local handle. The existing app.util.clipboard/from-navigator (clipboard.cljs:32) already wraps impl/fromNavigator in rx/from, so a rejected Promise from the async function propagates as an rx error event. Existing callers that subscribe with .catch / on-error see the structured Error and surface the toast, identical to how to-clipboard's unavailable-error already flows. Repro (matches niwinz's reproduction in the PR comment): Object.defineProperty(navigator, 'clipboard', { value: undefined }); // … then attempt a paste action in the workspace … Before: TypeError in console + 'Something wrong has happened' toast. After: descriptive Error caught by the rx subscription and rendered through the existing unavailable-Clipboard-API surface. Refs #6514, #4478 * :bug: Show user-facing toast when clipboard API is unavailable Niwinz's review on penpot#9188 caught that the rejected Promise from to-clipboard / to-clipboard-promise / to-clipboard-multi / fromNavigator now surfaces the correct error to the console, but the workspace UI still falls through to the generic "Something wrong has happened" toast because the on-clipboard-permission-error and the paste error-handler in paste-from-clipboard only branched on clipboard-permission-error?. Apply the patch he suggested in the review: - Add clipboard-unavailable-error? predicate that matches the Promise.reject(Error("Clipboard API is unavailable. ...")) thrown by the get-clipboard / unavailable-error helpers added earlier in this PR. Uses str/starts-with? on the message prefix so the predicate stays stable even if the trailing "serve Penpot over HTTPS ..." advice text is reworded later. - Convert on-clipboard-permission-error from `if` to `cond` and add a third arm that fires errors.clipboard-api-unavailable as a warning toast. - Add the same arm in the second cond block inside paste-from-clipboard, before the :not-implemented and :else arms. - Add the matching errors.clipboard-api-unavailable entry to frontend/translations/en.po with the wording niwinz suggested: "Clipboard API is unavailable. Serve Penpot over HTTPS to enable clipboard access". Refs penpot#9188 review. --------- Signed-off-by: Andrey Antukh Co-authored-by: Andrey Antukh --- CHANGES.md | 1 + .../app/main/data/workspace/clipboard.cljs | 25 +++++- frontend/src/app/util/clipboard.cljs | 81 ++++++++++++++----- frontend/src/app/util/clipboard.js | 16 +++- frontend/translations/en.po | 4 + 5 files changed, 107 insertions(+), 20 deletions(-) diff --git a/CHANGES.md b/CHANGES.md index f1edeb5963..41a05857fa 100644 --- a/CHANGES.md +++ b/CHANGES.md @@ -19,6 +19,7 @@ - Fix `get-profile` masking transient DB errors as anonymous user (by @jack-stormentswe) [Github #9253](https://github.com/penpot/penpot/issues/9253) - Fix `Ctrl+'` "Show guides" shortcut on non-US keyboard layouts by matching the physical key location (by @RenzoMXD) [Github #8423](https://github.com/penpot/penpot/issues/8423) - Fix lost-update race on `team.features` during concurrent file creation (by @web-dev0521) [Github #9197](https://github.com/penpot/penpot/issues/9197) +- Fix copy and paste actions crashing the workspace on insecure origins (plain HTTP / non-`localhost`) where the Clipboard API is unavailable (by @MilosM348) [Github #6514](https://github.com/penpot/penpot/issues/6514) ## 2.16.0 (Unreleased) diff --git a/frontend/src/app/main/data/workspace/clipboard.cljs b/frontend/src/app/main/data/workspace/clipboard.cljs index bc8d88a746..70c34c5287 100644 --- a/frontend/src/app/main/data/workspace/clipboard.cljs +++ b/frontend/src/app/main/data/workspace/clipboard.cljs @@ -303,13 +303,30 @@ (and (instance? js/DOMException cause) (= (.-name cause) "NotAllowedError"))) +(defn- clipboard-unavailable-error? + "Check if the given error is a clipboard API unavailable error + (thrown when navigator.clipboard is undefined, e.g. on insecure + origins per the W3C Secure Contexts spec)." + [cause] + (and (instance? js/Error cause) + (str/starts-with? (.-message cause) "Clipboard API is unavailable."))) + (defn- on-clipboard-permission-error [cause] - (if (clipboard-permission-error? cause) + (cond + (clipboard-permission-error? cause) (rx/of (ntf/show {:content (tr "errors.clipboard-permission-denied") :type :toast :level :warning :timeout 5000})) + + (clipboard-unavailable-error? cause) + (rx/of (ntf/show {:content (tr "errors.clipboard-api-unavailable") + :type :toast + :level :warning + :timeout 5000})) + + :else (rx/throw cause))) (defn paste-from-clipboard @@ -511,6 +528,12 @@ :level :warning :timeout 5000})) + (clipboard-unavailable-error? cause) + (rx/of (ntf/show {:content (tr "errors.clipboard-api-unavailable") + :type :toast + :level :warning + :timeout 5000})) + (:not-implemented (ex-data cause)) (rx/of (ntf/warn (tr "errors.clipboard-not-implemented"))) diff --git a/frontend/src/app/util/clipboard.cljs b/frontend/src/app/util/clipboard.cljs index d06aa5c22e..6f2d0046bf 100644 --- a/frontend/src/app/util/clipboard.cljs +++ b/frontend/src/app/util/clipboard.cljs @@ -70,40 +70,85 @@ ([event options] (from-data-transfer (.-dataTransfer ^js event) options))) -;; FIXME: rename to `write-text` +(def ^:private unavailable-error-message + "Clipboard API is unavailable. This usually happens when the page is served over plain HTTP; serve Penpot over HTTPS to enable copy-to-clipboard.") + +(defn- get-clipboard + "Return the active `navigator.clipboard` object, or nil when the + asynchronous Clipboard API is not exposed (e.g. on insecure origins + per the W3C spec, which is what triggered #4478)." + [] + (unchecked-get js/navigator "clipboard")) + +(defn- unavailable-error + "Build the error wrapped in a rejected Promise so callers can chain + `rx/from`/`.catch` and surface a meaningful message instead of the + opaque `TypeError: Cannot read properties of undefined (reading + 'writeText')` that leaked out of the previous implementation." + [] + (js/Promise.reject (js/Error. unavailable-error-message))) + (defn to-clipboard + "Write a plain-text string to the system clipboard. + + Always returns a Promise. Resolves on success; rejects with a clear + `Error` when `navigator.clipboard` is unavailable (insecure origin) + so callers can chain error handling instead of crashing the UI on a + synchronous `TypeError` like #4478." [data] (assert (string? data) "`data` should be string") - (let [clipboard (unchecked-get js/navigator "clipboard")] - (.writeText ^js clipboard data))) + (let [clipboard (get-clipboard)] + (if (and clipboard (unchecked-get clipboard "writeText")) + (.writeText ^js clipboard data) + (unavailable-error)))) (defn- create-clipboard-item [mimetype promise] (js/ClipboardItem. (js-obj mimetype promise))) -;; FIXME: this API is very confuse (defn to-clipboard-promise + "Write a single asynchronous payload to the clipboard under the given + MIME type. The `promise` is resolved by the browser when the + ClipboardItem is committed. + + Returns the Promise produced by `clipboard.write`, or a rejected + Promise carrying an `Error` when the asynchronous Clipboard API is + not available (insecure origin / unsupported browser). Mirrors + `to-clipboard`'s defensive contract." [mimetype promise] - (let [clipboard (unchecked-get js/navigator "clipboard") - data (create-clipboard-item mimetype promise)] - (.write ^js clipboard #js [data]))) + (let [clipboard (get-clipboard)] + (if (and clipboard (unchecked-get clipboard "write")) + (let [data (create-clipboard-item mimetype promise)] + (.write ^js clipboard #js [data])) + (unavailable-error)))) (defn to-clipboard-multi "Write multiple MIME representations as a single ClipboardItem. - `items` is a map of mime-type (string) -> string payload. - Falls back to `writeText` when the async Clipboard API is unavailable." + `items` is a map of mime-type (string) -> string payload. + + Falls back to `writeText` with the `text/plain` payload (or the + first available payload) when the asynchronous `clipboard.write` + API is unavailable. If neither path is reachable (e.g. insecure + origin), returns a rejected Promise mirroring `to-clipboard`'s + contract instead of throwing synchronously." [items] - (let [clipboard (unchecked-get js/navigator "clipboard")] - (if (and clipboard (unchecked-get clipboard "write")) - (let [obj (reduce-kv - (fn [acc mime payload] - (let [blob (js/Blob. #js [payload] #js {:type mime})] - (unchecked-set acc mime (js/Promise.resolve blob)) - acc)) - #js {} items) + (let [clipboard (get-clipboard)] + (cond + (and clipboard (unchecked-get clipboard "write")) + (let [obj (reduce-kv + (fn [acc mime payload] + (let [blob (js/Blob. #js [payload] #js {:type mime})] + (unchecked-set acc mime (js/Promise.resolve blob)) + acc)) + #js {} items) item (js/ClipboardItem. obj)] (.write ^js clipboard #js [item])) + + (and clipboard (unchecked-get clipboard "writeText")) (when-let [text (or (get items "text/plain") (first (vals items)))] - (.writeText ^js clipboard text))))) + (.writeText ^js clipboard text)) + + :else + (unavailable-error)))) diff --git a/frontend/src/app/util/clipboard.js b/frontend/src/app/util/clipboard.js index 43bd636b88..f86693dfdc 100644 --- a/frontend/src/app/util/clipboard.js +++ b/frontend/src/app/util/clipboard.js @@ -145,13 +145,27 @@ function sortItems(a, b) { } /** + * Read the active system clipboard via the asynchronous Clipboard API. + * + * Throws a descriptive `Error` when `navigator.clipboard` is unavailable + * (e.g. on insecure origins per the W3C Secure Contexts spec, mirroring + * the failure mode that crashed the copy/write path in #4478 / #6514). + * Without this guard, `navigator.clipboard.read()` raises an opaque + * `TypeError: Cannot read properties of undefined (reading 'read')` and + * the workspace surfaces a generic "Something wrong has happened" toast. * * @param {ClipboardSettings} [options] * @returns {Promise>} */ export async function fromNavigator(options) { options = options || {}; - const items = await navigator.clipboard.read(); + const clipboard = navigator.clipboard; + if (!clipboard || typeof clipboard.read !== "function") { + throw new Error( + "Clipboard API is unavailable. This usually happens when the page is served over plain HTTP; serve Penpot over HTTPS to enable paste-from-clipboard." + ); + } + const items = await clipboard.read(); const result = await Promise.all( Array.from(items).map(async (item) => { const itemAllowedTypes = Array.from(item.types) diff --git a/frontend/translations/en.po b/frontend/translations/en.po index 88b34b9c0b..460bfb193a 100644 --- a/frontend/translations/en.po +++ b/frontend/translations/en.po @@ -1413,6 +1413,10 @@ msgstr "Your browser cannot do this operation" msgid "errors.clipboard-permission-denied" msgstr "Clipboard access denied. Please allow clipboard permissions in your browser to paste content" +#: src/app/main/data/workspace/clipboard.cljs +msgid "errors.clipboard-api-unavailable" +msgstr "Clipboard API is unavailable. Serve Penpot over HTTPS to enable clipboard access" + #: src/app/main/errors.cljs:235 msgid "errors.comment-error" msgstr "There was an error with the comment"