From e24169d7f0b2bd101428f8a7534218e26305e4ab Mon Sep 17 00:00:00 2001 From: Andrey Antukh Date: Wed, 9 Sep 2026 19:06:11 +0000 Subject: [PATCH] :recycle: Address review findings on link-preview branch Keep subpath in the link-preview human redirect, fail fast on absolute join-uri segments, clarify the valueless-param behavior, and detect Applebot, Googlebot, Bing and DuckDuckGo crawlers. AI-assisted-by: muse-spark-1.3-contributor --- backend/resources/app/templates/link-preview.tmpl | 2 +- backend/src/app/config.clj | 2 ++ backend/test/backend_tests/config_test.clj | 5 +++++ docker/devenv/files/nginx.conf | 2 +- docker/images/files/nginx.conf.template | 2 +- docs/technical-guide/developer/subsystems/link-preview.md | 8 +++++--- frontend/src/app/main/router.cljs | 6 ++++-- 7 files changed, 19 insertions(+), 8 deletions(-) diff --git a/backend/resources/app/templates/link-preview.tmpl b/backend/resources/app/templates/link-preview.tmpl index 4b7753c595..997e92267d 100644 --- a/backend/resources/app/templates/link-preview.tmpl +++ b/backend/resources/app/templates/link-preview.tmpl @@ -17,6 +17,6 @@ - + diff --git a/backend/src/app/config.clj b/backend/src/app/config.clj index 3a332bffb5..2916c66fa7 100644 --- a/backend/src/app/config.clj +++ b/backend/src/app/config.clj @@ -383,6 +383,8 @@ a trailing slash; segments must not start with `/` (a leading slash would resolve against the host root and drop the subpath)." [base & segments] + (assert (not (some #(str/starts-with? % "/") segments)) + "URI segments must be relative (no leading slash)") (str (apply u/join (u/ensure-path-slash base) segments))) (defn get-public-uri diff --git a/backend/test/backend_tests/config_test.clj b/backend/test/backend_tests/config_test.clj index ce070ec20b..bae0c3e6af 100644 --- a/backend/test/backend_tests/config_test.clj +++ b/backend/test/backend_tests/config_test.clj @@ -33,3 +33,8 @@ (cf/join-uri "https://nitrate.example.com" "api/teams/123"))) (t/is (= "https://nitrate.example.com/api/teams/123" (cf/join-uri "https://nitrate.example.com/" "api/teams/123"))))) + +(t/deftest join-uri-rejects-leading-slash + (t/testing "a leading slash would drop the subpath, so it fails fast" + (t/is (thrown? AssertionError + (cf/join-uri "https://example.com/penpot" "/assets/by-id/123"))))) diff --git a/docker/devenv/files/nginx.conf b/docker/devenv/files/nginx.conf index ea060f2084..1a75c6f206 100644 --- a/docker/devenv/files/nginx.conf +++ b/docker/devenv/files/nginx.conf @@ -53,7 +53,7 @@ http { # are served with dynamic Open Graph metadata from the backend. map $http_user_agent $penpot_link_preview_agent { default 0; - ~*(slackbot|discordbot|twitterbot|facebookexternalhit|facebookcatalog|whatsapp|telegrambot|linkedinbot|skypeuripreview|pinterestbot|redditbot|embedly|iframely|mastodon|bluesky) 1; + ~*(slackbot|discordbot|twitterbot|facebookexternalhit|facebookcatalog|whatsapp|telegrambot|linkedinbot|skypeuripreview|pinterestbot|redditbot|embedly|iframely|mastodon|bluesky|applebot|googlebot|bingbot|bingpreview|duckduckbot) 1; } proxy_cache_path /tmp/cache/ levels=2:2 keys_zone=penpot:20m; diff --git a/docker/images/files/nginx.conf.template b/docker/images/files/nginx.conf.template index 380a542e4b..33c5702419 100644 --- a/docker/images/files/nginx.conf.template +++ b/docker/images/files/nginx.conf.template @@ -61,7 +61,7 @@ http { # are served with dynamic Open Graph metadata from the backend. map $http_user_agent $penpot_link_preview_agent { default 0; - ~*(slackbot|discordbot|twitterbot|facebookexternalhit|facebookcatalog|whatsapp|telegrambot|linkedinbot|skypeuripreview|pinterestbot|redditbot|embedly|iframely|mastodon|bluesky) 1; + ~*(slackbot|discordbot|twitterbot|facebookexternalhit|facebookcatalog|whatsapp|telegrambot|linkedinbot|skypeuripreview|pinterestbot|redditbot|embedly|iframely|mastodon|bluesky|applebot|googlebot|bingbot|bingpreview|duckduckbot) 1; } proxy_cache_path /tmp/cache/ levels=2:2 keys_zone=penpot:20m; diff --git a/docs/technical-guide/developer/subsystems/link-preview.md b/docs/technical-guide/developer/subsystems/link-preview.md index 2f5e087624..50202b1c48 100644 --- a/docs/technical-guide/developer/subsystems/link-preview.md +++ b/docs/technical-guide/developer/subsystems/link-preview.md @@ -86,7 +86,7 @@ A `map` block classifies the request by `User-Agent`: ```nginx map $http_user_agent $penpot_link_preview_agent { default 0; - ~*(slackbot|discordbot|twitterbot|facebookexternalhit|facebookcatalog|whatsapp|telegrambot|linkedinbot|skypeuripreview|pinterestbot|redditbot|embedly|iframely|mastodon|bluesky) 1; + ~*(slackbot|discordbot|twitterbot|facebookexternalhit|facebookcatalog|whatsapp|telegrambot|linkedinbot|skypeuripreview|pinterestbot|redditbot|embedly|iframely|mastodon|bluesky|applebot|googlebot|bingbot|bingpreview|duckduckbot) 1; } ``` @@ -163,12 +163,14 @@ equivalent `twitter:*` card tags and ``. The body contains a single script: ```html - + ``` so that if a *human* somehow lands on `/link-preview` (e.g. some clients let users click through to the fetched URL), the browser bounces back to the SPA root -keeping the query string and the fragment, and the app loads normally. Crawlers do not execute +keeping the query string and the fragment, and the app loads normally. The +redirect strips only the trailing `link-preview` segment so subpath +deployments keep their prefix. Crawlers do not execute JavaScript, so they just read the meta tags. ### Making file thumbnails publicly accessible diff --git a/frontend/src/app/main/router.cljs b/frontend/src/app/main/router.cljs index 174635135f..cdaa6197cb 100644 --- a/frontend/src/app/main/router.cljs +++ b/frontend/src/app/main/router.cljs @@ -103,8 +103,10 @@ ;; the effect always runs after the update. The sharing-context ids ;; are synced into the pre-fragment query (the fragment never reaches ;; the server, so shared links need them there); every other param is - ;; left untouched. The backend applies its own file > project > team - ;; priority, so no filtering is needed here. + ;; left untouched, except valueless ones (`?flag`), which the query + ;; codec cannot round-trip and are dropped. The backend applies its + ;; own file > project > team priority, so no filtering is needed + ;; here. (let [params (:query-params (:route state)) uri (u/uri (.-href globals/location)) search (reduce (fn [m k]