fix(web/embed): tighten media-embed surface — audit findings

Audit fixes from the post-be52cc1 9-dim review:

- CSS selector for the post-detail UrlAutoEmbed now targets the
  .proto-row sibling directly. Previous selector targeted descendants
  inside the sibling, so the wrapper class never matched and the
  player rendered unstyled.
- Drop the allowYoutube alias on RenderMarkdownOptions. The alias
  silently widened semantics from "YouTube only" to all three
  platforms, which is a regression for any caller that opts in by
  the old name. Only allowMediaEmbeds remains; both call sites
  (MarkdownEditor preview, post-detail body render) migrated.
- Drop allow-same-origin from the Spotify and Apple iframe
  sandboxes. Both load cookie-bearing hosts; without
  allow-same-origin the iframe is forced into a unique opaque
  origin so it can't read the reader's session cookies. The player
  ↔ parent communication uses postMessage which is cross-origin
  safe. YouTube keeps allow-same-origin because youtube-nocookie.com
  is the canonical privacy host and removing it breaks the embed
  player controls.
- Centralize per-platform iframe attrs into lib/embed-attrs.ts so
  the markdown sanitizer, the three pre-pass builders, and
  UrlAutoEmbed all consume one source of truth. Future tightening
  lands in one edit.
- End-anchor the Spotify path regex; reject malformed Apple ?i=
  values instead of silently downgrading to a show embed.
- Strict CommonMark fence-close detection across all three
  embed-pre-pass walkers. A line like `~~~more` inside a fence body
  no longer terminates it; closers must carry no info string.

Tests: existing suite green; the spotify/apple/markdown-media tests
reflect the new sandbox shape. Two findings deferred:
- UrlAutoEmbed component-level test (needs project-wide React test
  infrastructure not yet present)
- tokens.css migration (codebase uses theme.css for :root by
  convention; project-wide migration is out of scope)

Author: xiaolai

web/src/components/prototype/MarkdownEditor.tsx

@@ -196,7 +196,7 @@ export function MarkdownEditor({
       return;
     }
     let cancelled = false;
-    renderMarkdown(value, { allowYoutube: kind === "submission" }).then(
+    renderMarkdown(value, { allowMediaEmbeds: kind === "submission" }).then(
       (html) => {
         if (!cancelled) setPreviewHtml(html);
       },

web/src/components/prototype/UrlAutoEmbed.tsx

@@ -11,21 +11,26 @@
  * "open in new tab" / share.
  *
  * Source data is never mutated. The component derives the iframe
- * directly from `submission.url` at render time. Edits to the URL
- * (or future changes to embed src patterns) propagate without a
- * backfill.
+ * directly from `submission.url` at render time.
+ *
+ * Iframe attribute sets are imported from lib/embed-attrs.ts — the
+ * same source the markdown sanitizer uses, so the in-body and
+ * post-detail surfaces can't drift on sandbox/referrer/allow.
  *
  * Privacy posture mirrors the in-body markdown embeds:
  *   - YouTube goes through youtube-nocookie.com.
  *   - Spotify and Apple don't have nocookie equivalents — the embeds
- *     do load each platform's session cookies if the reader is
- *     logged in there. We mitigate with sandbox + strict referrer
- *     policy. The same trade-off exists on every site embedding
- *     these platforms. See lib/markdown.ts for the parallel pattern
- *     applied to in-body URLs.
+ *     do load each platform's pages, but with allow-same-origin
+ *     dropped from the sandbox the iframe can't read those hosts'
+ *     session cookies. See lib/embed-attrs.ts for the trade-off.
  */
 
 import { extractApplePodcastsMatch } from "@/lib/apple-podcasts-embed";
+import {
+  APPLE_PODCASTS_IFRAME_ATTRS,
+  SPOTIFY_IFRAME_ATTRS,
+  YT_IFRAME_ATTRS,
+} from "@/lib/embed-attrs";
 import { extractSpotifyMatch } from "@/lib/spotify-embed";
 import { extractYoutubeId } from "@/lib/youtube-embed";
 
@@ -42,11 +47,11 @@ export function UrlAutoEmbed({ url }: Props) {
       <div className="proto-yt-embed">
         <iframe
           src={`https://www.youtube-nocookie.com/embed/${youtubeId}`}
-          title="YouTube video"
-          loading="lazy"
-          referrerPolicy="strict-origin-when-cross-origin"
-          sandbox="allow-scripts allow-same-origin allow-presentation"
-          allow="accelerometer; clipboard-write; encrypted-media; gyroscope; picture-in-picture; web-share"
+          title={YT_IFRAME_ATTRS.title}
+          loading={YT_IFRAME_ATTRS.loading}
+          referrerPolicy={YT_IFRAME_ATTRS.referrerpolicy}
+          sandbox={YT_IFRAME_ATTRS.sandbox}
+          allow={YT_IFRAME_ATTRS.allow}
           allowFullScreen
         />
       </div>
@@ -59,11 +64,11 @@ export function UrlAutoEmbed({ url }: Props) {
       <div className="proto-spotify-embed">
         <iframe
           src={`https://open.spotify.com/embed/${spotify.kind}/${spotify.id}`}
-          title="Spotify embed"
-          loading="lazy"
-          referrerPolicy="strict-origin-when-cross-origin"
-          sandbox="allow-scripts allow-same-origin allow-popups"
-          allow="autoplay; clipboard-write; encrypted-media; fullscreen; picture-in-picture"
+          title={SPOTIFY_IFRAME_ATTRS.title}
+          loading={SPOTIFY_IFRAME_ATTRS.loading}
+          referrerPolicy={SPOTIFY_IFRAME_ATTRS.referrerpolicy}
+          sandbox={SPOTIFY_IFRAME_ATTRS.sandbox}
+          allow={SPOTIFY_IFRAME_ATTRS.allow}
         />
       </div>
     );
@@ -77,11 +82,11 @@ export function UrlAutoEmbed({ url }: Props) {
       <div className="proto-applepod-embed">
         <iframe
           src={src}
-          title="Apple Podcasts embed"
-          loading="lazy"
-          referrerPolicy="strict-origin-when-cross-origin"
-          sandbox="allow-scripts allow-same-origin allow-popups allow-forms"
-          allow="autoplay; encrypted-media; fullscreen"
+          title={APPLE_PODCASTS_IFRAME_ATTRS.title}
+          loading={APPLE_PODCASTS_IFRAME_ATTRS.loading}
+          referrerPolicy={APPLE_PODCASTS_IFRAME_ATTRS.referrerpolicy}
+          sandbox={APPLE_PODCASTS_IFRAME_ATTRS.sandbox}
+          allow={APPLE_PODCASTS_IFRAME_ATTRS.allow}
         />
       </div>
     );

web/src/lib/apple-podcasts-embed.ts

@@ -19,12 +19,15 @@
  * markdown.ts re-validates the src via APPLE_PODCASTS_EMBED_SRC.
  *
  * Privacy note: Apple does not provide a nocookie-equivalent host.
- * The embed loads embed.podcasts.apple.com which can read Apple's
- * session cookies. Same trade-off as Spotify — mitigated by sandbox +
- * referrerpolicy, but cookies still flow if the reader is logged
- * into Apple in the same browser.
+ * The embed loads embed.podcasts.apple.com, where the reader may
+ * have a session cookie. By dropping `allow-same-origin` from the
+ * sandbox (see lib/embed-attrs.ts) the iframe is forced into a
+ * unique opaque origin and cannot access those cookies. Same
+ * trade-off and mitigation as Spotify.
  */
 
+import { APPLE_PODCASTS_IFRAME_ATTRS } from "@/lib/embed-attrs";
+
 const COUNTRY = /^[a-z]{2}$/;
 const NUMERIC_ID = /^\d+$/;
 
@@ -72,31 +75,36 @@ export function extractApplePodcastsMatch(
   const [, country, slug, showId] = m;
   if (!COUNTRY.test(country) || !NUMERIC_ID.test(showId)) return null;
 
-  // ?i=<episode-id> is the only query param we honor.
-  const episodeRaw = parsed.searchParams.get("i");
-  const episodeId =
-    episodeRaw && NUMERIC_ID.test(episodeRaw) ? episodeRaw : null;
+  // ?i=<episode-id> is the only query param we honor. If it's
+  // present but not numeric, the user signaled episode intent with a
+  // malformed value — reject the whole URL rather than silently
+  // downgrading to a show-level embed.
+  if (parsed.searchParams.has("i")) {
+    const episodeRaw = parsed.searchParams.get("i") ?? "";
+    if (!NUMERIC_ID.test(episodeRaw)) return null;
+    return { country, slug, showId, episodeId: episodeRaw };
+  }
 
-  return { country, slug, showId, episodeId };
+  return { country, slug, showId, episodeId: null };
 }
 
-/** Build the iframe block for a given match. */
+/** Build the iframe block for a given match. Iframe attrs come from
+ *  lib/embed-attrs.ts so the in-body and post-detail surfaces share
+ *  one source of truth. Apple's official embed sandbox is famously
+ *  permissive; we deliberately tighten in the shared module — if a
+ *  show breaks playback, loosen there, not here. */
 function buildEmbed(match: ApplePodcastsMatch): string {
   const base = `https://embed.podcasts.apple.com/${match.country}/podcast/${match.slug}/id${match.showId}`;
   const src = match.episodeId ? `${base}?i=${match.episodeId}` : base;
-  // Apple's official embed sandbox is famously permissive
-  // (allow-storage-access-by-user-activation, allow-top-navigation,
-  // etc.). We deliberately tighten — readers don't need the embed
-  // to navigate the host page or escape its sandbox. If the player
-  // breaks for some content shape, expand on a case-by-case basis.
+  const a = APPLE_PODCASTS_IFRAME_ATTRS;
   return (
     `<div class="proto-applepod-embed">` +
     `<iframe src="${src}"` +
-    ` title="Apple Podcasts embed"` +
-    ` loading="lazy"` +
-    ` referrerpolicy="strict-origin-when-cross-origin"` +
-    ` sandbox="allow-scripts allow-same-origin allow-popups allow-forms"` +
-    ` allow="autoplay; encrypted-media; fullscreen"` +
+    ` title="${a.title}"` +
+    ` loading="${a.loading}"` +
+    ` referrerpolicy="${a.referrerpolicy}"` +
+    ` sandbox="${a.sandbox}"` +
+    ` allow="${a.allow}"` +
     `></iframe>` +
     `</div>`
   );
@@ -117,24 +125,33 @@ export function rewriteApplePodcastsEmbeds(source: string): string {
   const URL_LINE = /^ {0,3}(https?:\/\/\S+)[ \t]*$/;
 
   for (const line of lines) {
-    const fenceMatch = line.match(/^ {0,3}(`{3,}|~{3,})/);
-    if (fenceMatch) {
-      const marker = fenceMatch[1];
-      const ch = marker[0] as "`" | "~";
-      if (!inFence) {
+    // Fence open/close detection — see youtube-embed.ts for the
+    // CommonMark rules. Strict close (no info string) prevents
+    // a line like `~~~more` inside a fence body from closing it.
+    if (inFence) {
+      const closeMatch = line.match(/^ {0,3}(`{3,}|~{3,})[ \t]*$/);
+      if (closeMatch) {
+        const marker = closeMatch[1];
+        const ch = marker[0] as "`" | "~";
+        if (ch === fenceChar && marker.length >= fenceLen) {
+          inFence = false;
+          fenceChar = null;
+          fenceLen = 0;
+          out.push(line);
+          continue;
+        }
+      }
+    } else {
+      const openMatch = line.match(/^ {0,3}(`{3,}|~{3,})/);
+      if (openMatch) {
+        const marker = openMatch[1];
+        const ch = marker[0] as "`" | "~";
         inFence = true;
         fenceChar = ch;
         fenceLen = marker.length;
         out.push(line);
         continue;
       }
-      if (ch === fenceChar && marker.length >= fenceLen) {
-        inFence = false;
-        fenceChar = null;
-        fenceLen = 0;
-        out.push(line);
-        continue;
-      }
     }
 
     if (inFence) {

web/src/lib/embed-attrs.ts

@@ -0,0 +1,61 @@
+/**
+ * Single source of truth for the per-platform iframe attribute sets
+ * used by media embeds (YouTube, Spotify, Apple Podcasts).
+ *
+ * Two consumers must agree on these:
+ *   1. The markdown sanitizer in lib/markdown.ts re-stamps each
+ *      surviving iframe's attrs to the matching set, so a hand-rolled
+ *      iframe can't sneak in a relaxed sandbox.
+ *   2. The UrlAutoEmbed component on the post-detail page emits the
+ *      same iframe directly from submission.url, never going through
+ *      the markdown pipeline.
+ *
+ * Centralizing here lets a tightening (or platform-specific quirk
+ * fix) land in one edit and propagate everywhere. Drift between the
+ * two consumers is the failure mode the centralization is preventing.
+ *
+ * Sandbox posture: minimal permissions. allow-same-origin is
+ * deliberately NOT granted to Spotify or Apple — those embeds run
+ * on cookie-bearing hosts (open.spotify.com, embed.podcasts.apple.com)
+ * where the reader may be logged in. Dropping allow-same-origin
+ * forces the iframe into a unique opaque origin so it can't read
+ * those cookies. The embed players communicate with the parent via
+ * postMessage, which works cross-origin without same-origin. YouTube
+ * gets allow-same-origin because youtube-nocookie.com is the
+ * canonical privacy-aware embed host (no Google-account cookies live
+ * there), so the same-origin combo doesn't grant access to anything
+ * sensitive — and removing it breaks YouTube's embed playback
+ * controls.
+ */
+
+export const YT_IFRAME_ATTRS = {
+  title: "YouTube video",
+  loading: "lazy",
+  referrerpolicy: "strict-origin-when-cross-origin",
+  sandbox: "allow-scripts allow-same-origin allow-presentation",
+  allow:
+    "accelerometer; clipboard-write; encrypted-media; gyroscope; picture-in-picture; web-share",
+  allowfullscreen: "",
+} as const;
+
+export const SPOTIFY_IFRAME_ATTRS = {
+  title: "Spotify embed",
+  loading: "lazy",
+  referrerpolicy: "strict-origin-when-cross-origin",
+  // No allow-same-origin: see file header. allow-popups is kept for
+  // the "Open in Spotify" button to work in a new tab.
+  sandbox: "allow-scripts allow-popups",
+  allow:
+    "autoplay; clipboard-write; encrypted-media; fullscreen; picture-in-picture",
+} as const;
+
+export const APPLE_PODCASTS_IFRAME_ATTRS = {
+  title: "Apple Podcasts embed",
+  loading: "lazy",
+  referrerpolicy: "strict-origin-when-cross-origin",
+  // No allow-same-origin: see file header. allow-popups for "Open in
+  // Apple Podcasts" link; allow-forms for the player's internal
+  // controls (Apple's official sandbox grants both).
+  sandbox: "allow-scripts allow-popups allow-forms",
+  allow: "autoplay; encrypted-media; fullscreen",
+} as const;

web/src/lib/markdown.ts

@@ -12,6 +12,11 @@ import sanitizeHtml from "sanitize-html";
 
 import { highlightCodeToLines } from "@/lib/highlight";
 import { rewriteApplePodcastsEmbeds } from "@/lib/apple-podcasts-embed";
+import {
+  APPLE_PODCASTS_IFRAME_ATTRS,
+  SPOTIFY_IFRAME_ATTRS,
+  YT_IFRAME_ATTRS,
+} from "@/lib/embed-attrs";
 import { rewriteSpotifyEmbeds } from "@/lib/spotify-embed";
 import { rewriteYoutubeEmbeds } from "@/lib/youtube-embed";
 
@@ -32,41 +37,6 @@ const SPOTIFY_EMBED_SRC =
 const APPLE_PODCASTS_EMBED_SRC =
   /^https:\/\/embed\.podcasts\.apple\.com\/[a-z]{2}\/podcast\/[^/?#]+\/id\d+(?:\?i=\d+)?$/;
 
-/** Canonical safe attribute set forced onto every surviving YouTube
- *  iframe. We re-stamp these on every iframe (even pre-pass output)
- *  so a hand-rolled iframe with the right SRC but missing or weakened
- *  sandbox/referrerpolicy/loading values gets normalised to the safe
- *  shape. */
-const YT_IFRAME_ATTRS = {
-  title: "YouTube video",
-  loading: "lazy",
-  referrerpolicy: "strict-origin-when-cross-origin",
-  sandbox: "allow-scripts allow-same-origin allow-presentation",
-  allow:
-    "accelerometer; clipboard-write; encrypted-media; gyroscope; picture-in-picture; web-share",
-  allowfullscreen: "",
-} as const;
-
-/** Canonical safe attribute set for Spotify iframes. Tighter sandbox
- *  than the Spotify default — readers don't need top-navigation. */
-const SPOTIFY_IFRAME_ATTRS = {
-  title: "Spotify embed",
-  loading: "lazy",
-  referrerpolicy: "strict-origin-when-cross-origin",
-  sandbox: "allow-scripts allow-same-origin allow-popups",
-  allow:
-    "autoplay; clipboard-write; encrypted-media; fullscreen; picture-in-picture",
-} as const;
-
-/** Canonical safe attribute set for Apple Podcasts iframes. */
-const APPLE_PODCASTS_IFRAME_ATTRS = {
-  title: "Apple Podcasts embed",
-  loading: "lazy",
-  referrerpolicy: "strict-origin-when-cross-origin",
-  sandbox: "allow-scripts allow-same-origin allow-popups allow-forms",
-  allow: "autoplay; encrypted-media; fullscreen",
-} as const;
-
 export const ALLOWED_TAGS = [
   "p",
   "br",
@@ -506,23 +476,15 @@ export interface RenderMarkdownOptions {
    * Wire only on submission body surfaces — comments and editorial
    * docs should remain text-only so a media URL in a heated reply
    * can't visually dominate the thread.
-   *
-   * Backward-compat alias `allowYoutube` is honored — it now means
-   * "allow all three platforms" since the embed pipeline grew beyond
-   * YouTube. Prefer `allowMediaEmbeds` in new call sites.
    */
   allowMediaEmbeds?: boolean;
-  /** @deprecated Use `allowMediaEmbeds`. Kept so older call sites
-   *  don't drift; behavior is identical. */
-  allowYoutube?: boolean;
 }
 
 export async function renderMarkdown(
   source: string,
   options: RenderMarkdownOptions = {},
 ): Promise<string> {
-  const allowEmbeds =
-    options.allowMediaEmbeds === true || options.allowYoutube === true;
+  const allowEmbeds = options.allowMediaEmbeds === true;
   // Order matters: each rewriter is idempotent on its own output but
   // would re-process foreign output. YouTube's bare-URL match is
   // catch-all-shaped (any https://\S+ URL on its own line), so it