From bf84966eec0f60f0e9fdc736814065364653b928 Mon Sep 17 00:00:00 2001 From: Aaron Goldsmith Date: Tue, 7 Jul 2026 21:53:49 -0700 Subject: [PATCH 1/3] feat(desktop): show full URL tooltip on masked link hover MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Masked links — [text](url) where the visible text hides the destination — now show a tooltip with the full URL on hover, mirroring Slack's anti-phishing affordance. Bare pasted URLs and GFM autolinks are left unchanged: their label already is the destination, modulo cosmetic differences (omitted scheme, trailing slash, host case). isMaskedLink parses both sides with the URL API and compares component-wise so cosmetic differences stay tooltip-free while spoof shapes always trigger: scheme downgrades ([https://x](http://x)), case-only path/query changes, protocol-relative hrefs, and userinfo labels (safe.com@evil.com) are all treated as masked. The tooltip content is pointer-events-none so it can never intercept clicks meant for the link or a spoiler wrapping it, and never truncates so the full destination stays auditable. Co-Authored-By: Claude Fable 5 Signed-off-by: Aaron Goldsmith --- desktop/src/shared/lib/maskedLink.test.mjs | 102 ++++++++++++++++++ desktop/src/shared/lib/maskedLink.ts | 62 +++++++++++ desktop/src/shared/ui/markdown.tsx | 9 +- .../shared/ui/markdown/MaskedLinkTooltip.tsx | 41 +++++++ 4 files changed, 213 insertions(+), 1 deletion(-) create mode 100644 desktop/src/shared/lib/maskedLink.test.mjs create mode 100644 desktop/src/shared/lib/maskedLink.ts create mode 100644 desktop/src/shared/ui/markdown/MaskedLinkTooltip.tsx diff --git a/desktop/src/shared/lib/maskedLink.test.mjs b/desktop/src/shared/lib/maskedLink.test.mjs new file mode 100644 index 000000000..035ccf697 --- /dev/null +++ b/desktop/src/shared/lib/maskedLink.test.mjs @@ -0,0 +1,102 @@ +import assert from "node:assert/strict"; +import test from "node:test"; + +import { isMaskedLink } from "./maskedLink.ts"; + +test("masked when label text differs from destination", () => { + assert.equal(isMaskedLink("click here", "https://example.com"), true); + assert.equal( + isMaskedLink("the docs", "https://docs.example.com/guide"), + true, + ); +}); + +test("masked when label is a different URL (spoof)", () => { + assert.equal(isMaskedLink("https://good.com", "https://evil.com"), true); + assert.equal( + isMaskedLink("example.com/safe", "https://example.com/pwn"), + true, + ); +}); + +test("not masked when label matches href exactly", () => { + assert.equal( + isMaskedLink("https://example.com/a/b", "https://example.com/a/b"), + false, + ); +}); + +test("not masked across cosmetic differences", () => { + // scheme omitted in label + assert.equal(isMaskedLink("example.com", "https://example.com"), false); + // trailing slash + assert.equal(isMaskedLink("example.com", "https://example.com/"), false); + // GFM autolink of www URLs prefixes http:// + assert.equal( + isMaskedLink("www.example.com", "http://www.example.com"), + false, + ); + // case-insensitive + assert.equal(isMaskedLink("Example.COM", "https://example.com"), false); + // surrounding whitespace in label + assert.equal(isMaskedLink(" example.com ", "https://example.com"), false); +}); + +test("not masked when there is nothing useful to compare", () => { + assert.equal(isMaskedLink("", "https://example.com"), false); + assert.equal(isMaskedLink(" ", "https://example.com"), false); + assert.equal(isMaskedLink("label", ""), false); +}); + +test("non-http schemes are never masked", () => { + assert.equal(isMaskedLink("email me", "mailto:a@b.com"), false); + assert.equal( + isMaskedLink("open thread", "buzz://message?channel=x&id=y"), + false, + ); +}); + +test("masked on scheme downgrade even when host matches", () => { + assert.equal( + isMaskedLink("https://accounts.example.com", "http://accounts.example.com"), + true, + ); +}); + +test("not masked when label repeats href scheme exactly", () => { + assert.equal(isMaskedLink("http://example.com", "http://example.com"), false); +}); + +test("masked when path or query differ only by case", () => { + assert.equal( + isMaskedLink( + "https://example.com/Reset?token=ABC123", + "https://example.com/reset?token=abc123", + ), + true, + ); +}); + +test("host case differences alone are not masked", () => { + assert.equal( + isMaskedLink("Example.COM/path", "https://example.com/path"), + false, + ); +}); + +test("protocol-relative hrefs are covered", () => { + assert.equal(isMaskedLink("company wiki", "//evil.example/login"), true); + assert.equal( + isMaskedLink("//evil.example/login", "//evil.example/login"), + false, + ); +}); + +test("labels with userinfo are always masked", () => { + // Visually reads as safe.com but actually resolves to evil.com + assert.equal(isMaskedLink("safe.com@evil.com", "https://evil.com"), true); + assert.equal( + isMaskedLink("https://safe.com@evil.com", "https://evil.com"), + true, + ); +}); diff --git a/desktop/src/shared/lib/maskedLink.ts b/desktop/src/shared/lib/maskedLink.ts new file mode 100644 index 000000000..8744ccd1b --- /dev/null +++ b/desktop/src/shared/lib/maskedLink.ts @@ -0,0 +1,62 @@ +/** + * A link is "masked" when its visible text is not the destination URL — + * `[click here](https://example.com)` — so the reader can't tell where it + * goes. Masked links get a hover tooltip showing the full destination + * (anti-phishing, mirrors Slack). Bare pasted URLs and GFM autolinks are + * not masked: their label already is the URL, modulo cosmetic differences + * (omitted scheme, trailing slash, host case). + * + * Deliberately strict where it matters: scheme downgrades (label says + * https, href is http), path/query case changes, and protocol-relative + * hrefs all count as masked. + */ +export function isMaskedLink(label: string, href: string): boolean { + const trimmedLabel = label.trim(); + if (!trimmedLabel) return false; + + const hrefUrl = parseExternal(href); + if (!hrefUrl) return false; + + const labelUrl = parseExternal(trimmedLabel, { assumeScheme: true }); + if (!labelUrl) return true; // label isn't URL-shaped at all + + // A label with userinfo (`safe.com@evil.com`) visually leads with the + // wrong host — always treat it as masked. + if (labelUrl.username || labelUrl.password) return true; + + // If the label spells out a scheme, it must match the destination's — + // otherwise [https://x](http://x) silently downgrades. + const labelHasScheme = /^https?:\/\//i.test(trimmedLabel); + if (labelHasScheme && labelUrl.protocol !== hrefUrl.protocol) return true; + + // Host comparison is case-insensitive (the URL parser lowercases it); + // path/query/hash stay case-sensitive — many backends distinguish them. + return ( + labelUrl.host !== hrefUrl.host || + normalizePath(labelUrl.pathname) !== normalizePath(hrefUrl.pathname) || + labelUrl.search !== hrefUrl.search || + labelUrl.hash !== hrefUrl.hash + ); +} + +function parseExternal( + value: string, + { assumeScheme = false }: { assumeScheme?: boolean } = {}, +): URL | null { + let candidate = value; + if (/^\/\//.test(candidate)) { + candidate = `https:${candidate}`; + } else if (assumeScheme && !/^https?:\/\//i.test(candidate)) { + candidate = `https://${candidate}`; + } + if (!/^https?:\/\//i.test(candidate)) return null; + try { + return new URL(candidate); + } catch { + return null; + } +} + +function normalizePath(pathname: string): string { + return pathname.length > 1 ? pathname.replace(/\/$/, "") : pathname; +} diff --git a/desktop/src/shared/ui/markdown.tsx b/desktop/src/shared/ui/markdown.tsx index e4384efa9..8493b7239 100644 --- a/desktop/src/shared/ui/markdown.tsx +++ b/desktop/src/shared/ui/markdown.tsx @@ -73,6 +73,7 @@ import { FileCard } from "./markdown/FileCard"; import { InlineEmojiPopover } from "./markdown/InlineEmojiPopover"; import { MarkdownInput } from "./markdown/MarkdownInput"; import { MarkdownTable } from "./markdown/MarkdownTable"; +import { MaskedLinkTooltip } from "./markdown/MaskedLinkTooltip"; import { MessageLinkPill } from "./markdown/MessageLinkPill"; import { resolveFileCard } from "./markdownFileCard"; import type { @@ -1658,7 +1659,7 @@ function createMarkdownComponents( : null; const isLinearLink = supportedLinkPreview?.kind === "linear-issue"; - return ( + const anchor = ( ); + + return ( + + {anchor} + + ); }, blockquote: ({ children }) => (
diff --git a/desktop/src/shared/ui/markdown/MaskedLinkTooltip.tsx b/desktop/src/shared/ui/markdown/MaskedLinkTooltip.tsx new file mode 100644 index 000000000..e4d4a91ea --- /dev/null +++ b/desktop/src/shared/ui/markdown/MaskedLinkTooltip.tsx @@ -0,0 +1,41 @@ +import type * as React from "react"; + +import { isMaskedLink } from "@/shared/lib/maskedLink"; +import { Tooltip, TooltipContent, TooltipTrigger } from "@/shared/ui/tooltip"; + +/** + * Masked link (`[text](url)` where the text hides the destination): reveal + * the full URL on hover so readers can see where a "click here" actually + * goes before clicking. Bare/autolinked URLs render children unchanged — + * their label already is the destination. + */ +export function MaskedLinkTooltip({ + href, + label, + disabled = false, + children, +}: { + href: string | undefined; + label: string; + disabled?: boolean; + children: React.ReactElement; +}) { + if (disabled || !href || !isMaskedLink(label, href)) { + return children; + } + return ( + + {children} + {/* pointer-events-none: the tooltip is purely informational and must + never intercept clicks meant for the link (or a spoiler wrapping + it) when it opens mid-interaction. + overflow-wrap:anywhere (not break-all): most URLs fit on one line + at max-w-md; only genuinely long ones wrap, breaking at natural + boundaries rather than mid-word. Never truncate — the full + destination must stay auditable for the anti-phishing use case. */} + + {href} + + + ); +} From e8319f0c1ca48c189f1ada80bed7957d8435b658 Mon Sep 17 00:00:00 2001 From: Aaron Goldsmith Date: Tue, 7 Jul 2026 21:54:05 -0700 Subject: [PATCH 2/3] fix(desktop): stabilize flaky spoiler-link reveal e2e test The spoiler-link reveal test failed intermittently on cold-start runs (~1 in 3), pre-existing on main and surfaced while validating the masked-link tooltip. click({force: true}) computes its target point from the link's bounding box while the freshly sent row is still settling layout, so the click can land a few px above the link on the author name. Wait for the link's position to be stable across two animation frames before clicking. Co-Authored-By: Claude Fable 5 Signed-off-by: Aaron Goldsmith --- desktop/tests/e2e/spoiler.spec.ts | 24 +++++++++++++++++++++++- 1 file changed, 23 insertions(+), 1 deletion(-) diff --git a/desktop/tests/e2e/spoiler.spec.ts b/desktop/tests/e2e/spoiler.spec.ts index b7b672698..c022c486d 100644 --- a/desktop/tests/e2e/spoiler.spec.ts +++ b/desktop/tests/e2e/spoiler.spec.ts @@ -175,10 +175,32 @@ test("hidden spoiler links reveal without opening on the first click", async ({ const spoiler = lastMessage.locator(".buzz-spoiler").first(); await expect(spoiler).toHaveAttribute("data-revealed", "false"); + // The freshly sent row can still be settling layout; a forced click + // computed from a pre-shift bounding box lands on the wrong element + // (force skips Playwright's stability check). Wait until the link's + // position is identical across two animation frames before clicking. + const secretLink = spoiler.getByRole("link", { name: "secret" }); + await secretLink.evaluate( + (el) => + new Promise((resolve) => { + let last = -1; + const tick = () => + requestAnimationFrame(() => { + const { y } = el.getBoundingClientRect(); + if (y === last) resolve(); + else { + last = y; + tick(); + } + }); + tick(); + }), + ); + const popupPromise = page .waitForEvent("popup", { timeout: 500 }) .catch(() => null); - await spoiler.getByRole("link", { name: "secret" }).click({ force: true }); + await secretLink.click({ force: true }); const popup = await popupPromise; await popup?.close(); From 7ae40d1ad55eb1d5f78263c4f3766471db41f833 Mon Sep 17 00:00:00 2001 From: Aaron Goldsmith Date: Tue, 7 Jul 2026 23:16:37 -0700 Subject: [PATCH 3/3] fix(desktop): suppress masked-link tooltip inside hidden spoilers MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit A masked link inside an unrevealed spoiler still installed a Radix tooltip trigger on the hidden anchor. The spoiler only masks its content visually and intercepts pointerdown/click to reveal — it does not suppress hover or focus — and the tooltip content is portaled outside the spoiler, so hovering or tab-focusing the hidden link leaked its URL before the spoiler was opened. SpoilerInline now provides a SpoilerHiddenContext (true while unrevealed, and always true for inert preview spoilers); MaskedLinkTooltip consumes it and renders the plain anchor with no tooltip trigger while hidden. It re-enables reactively when the spoiler is revealed. Covers both the hover and focus vectors. Reported by Codex review on #1625. Co-Authored-By: Claude Fable 5 Signed-off-by: Aaron Goldsmith --- .../shared/ui/markdown/MaskedLinkTooltip.tsx | 8 ++- .../src/shared/ui/markdown/SpoilerInline.tsx | 24 ++++++-- desktop/tests/e2e/spoiler.spec.ts | 59 +++++++++++++++++++ 3 files changed, 85 insertions(+), 6 deletions(-) diff --git a/desktop/src/shared/ui/markdown/MaskedLinkTooltip.tsx b/desktop/src/shared/ui/markdown/MaskedLinkTooltip.tsx index e4d4a91ea..43fe8327f 100644 --- a/desktop/src/shared/ui/markdown/MaskedLinkTooltip.tsx +++ b/desktop/src/shared/ui/markdown/MaskedLinkTooltip.tsx @@ -1,7 +1,8 @@ -import type * as React from "react"; +import * as React from "react"; import { isMaskedLink } from "@/shared/lib/maskedLink"; import { Tooltip, TooltipContent, TooltipTrigger } from "@/shared/ui/tooltip"; +import { SpoilerHiddenContext } from "./SpoilerInline"; /** * Masked link (`[text](url)` where the text hides the destination): reveal @@ -20,7 +21,10 @@ export function MaskedLinkTooltip({ disabled?: boolean; children: React.ReactElement; }) { - if (disabled || !href || !isMaskedLink(label, href)) { + // A masked link inside an unrevealed spoiler must not expose its URL via a + // hover/focus tooltip — that would leak the spoiler's content early. + const hiddenInSpoiler = React.useContext(SpoilerHiddenContext); + if (disabled || hiddenInSpoiler || !href || !isMaskedLink(label, href)) { return children; } return ( diff --git a/desktop/src/shared/ui/markdown/SpoilerInline.tsx b/desktop/src/shared/ui/markdown/SpoilerInline.tsx index a137d8b65..3b1519742 100644 --- a/desktop/src/shared/ui/markdown/SpoilerInline.tsx +++ b/desktop/src/shared/ui/markdown/SpoilerInline.tsx @@ -3,6 +3,14 @@ import * as React from "react"; import { hasBlockMedia } from "../markdownUtils"; import { SpoilerParticles } from "../SpoilerParticles"; +/** + * True for descendants of a spoiler that is currently hidden. Consumers (e.g. + * `MaskedLinkTooltip`) use it to suppress hover/focus affordances that would + * otherwise leak masked content before the spoiler is revealed. Default + * `false` — content outside any spoiler is never hidden. + */ +export const SpoilerHiddenContext = React.createContext(false); + export function SpoilerInline({ block = false, children, @@ -80,7 +88,9 @@ export function SpoilerInline({ >
- {children} + + {children} +
); @@ -94,7 +104,9 @@ export function SpoilerInline({ > - {children} + + {children} + ); @@ -110,7 +122,9 @@ export function SpoilerInline({ >
- {children} + + {children} +
); @@ -125,7 +139,9 @@ export function SpoilerInline({ > - {children} + + {children} + ); diff --git a/desktop/tests/e2e/spoiler.spec.ts b/desktop/tests/e2e/spoiler.spec.ts index c022c486d..738b19a74 100644 --- a/desktop/tests/e2e/spoiler.spec.ts +++ b/desktop/tests/e2e/spoiler.spec.ts @@ -248,6 +248,65 @@ test("hidden spoilers stay masked on hover and focus until reveal", async ({ await expect(particles).toHaveCSS("opacity", "0"); }); +test("masked link inside a hidden spoiler does not leak its URL until revealed", async ({ + page, +}) => { + const SECRET_URL = "https://private.example/leak-path"; + await installSpoilerBridge(page); + await page.goto("/"); + await page.getByTestId("channel-general").click(); + await expect(page.getByTestId("chat-title")).toHaveText("general"); + + const input = page.getByTestId("message-input"); + await input.click(); + await page.keyboard.type(`||[secret](${SECRET_URL})||`); + await page.getByTestId("send-message").click(); + + const lastMessage = page.getByTestId("message-row").last(); + const spoiler = lastMessage.locator(".buzz-spoiler").first(); + await expect(spoiler).toHaveAttribute("data-revealed", "false"); + + const secretLink = spoiler.getByRole("link", { name: "secret" }); + + // Neither hovering nor focusing the still-hidden link may open the URL + // tooltip — that would leak the destination before the spoiler is revealed. + await secretLink.hover({ force: true }); + await page.waitForTimeout(500); // exceed the tooltip open delay + await expect(page.getByRole("tooltip")).toHaveCount(0); + + await page.mouse.move(0, 0); + await secretLink.focus(); + await page.waitForTimeout(500); + await expect(page.getByRole("tooltip")).toHaveCount(0); + await expect(page.getByText(SECRET_URL)).toHaveCount(0); + await secretLink.blur(); + + // Reveal the spoiler (first click reveals; it must not open the link). + await page.mouse.move(0, 0); + await secretLink.evaluate( + (el) => + new Promise((resolve) => { + let last = -1; + const tick = () => + requestAnimationFrame(() => { + const { y } = el.getBoundingClientRect(); + if (y === last) resolve(); + else { + last = y; + tick(); + } + }); + tick(); + }), + ); + await secretLink.click({ force: true }); + await expect(spoiler).toHaveAttribute("data-revealed", "true"); + + // Now revealed, hovering the link reveals the URL tooltip as normal. + await secretLink.hover(); + await expect(page.getByRole("tooltip")).toContainText(SECRET_URL); +}); + test("non-interactive inbox preview spoilers let row clicks pass through", async ({ page, }) => {