TightknitAI · StephenTangCook · Sep 1, 2026 · Sep 1, 2026
diff --git a/SECURITY-REVIEW.md b/SECURITY-REVIEW.md
@@ -143,6 +143,44 @@ Chromium.
 
 ---
 
+## Follow-up findings
+
+Reported after the review above, against the same code. Kept here so the
+URL-sanitization story reads in one place.
+
+### F-009 (High) — Video-block URL fields bypassed the F-001 sanitizer and reached an unsanitized `<iframe src>`
+
+- **Reported**: 2026-08-27, external report. **Fixed**: 2026-09-01.
+- **OWASP**: A03 Injection (DOM-XSS).
+- **Surfaces**: the same entry points as F-001 — JSON drawer, `decodeBlocksFromString` / the `?blocks=` URL state, `initialBlocks`, `onLoadMessage`, any consumer backend.
+- **Root cause**: all three F-001 layers were scoped to the URL fields that existed when they were written, and the video block's four are not among them.
+  1. The payload sanitizer keyed on exact-match sets `HREF_KEYS = {'url'}` / `IMAGE_KEYS = {'image_url'}`, so `video_url`, `title_url`, `thumbnail_url`, and `provider_icon_url` were returned verbatim — by `sanitizeBlock` at the preview boundary **and** by `toSlackBlocks`, the documented consumer hardening boundary.
+  2. `slack-blocks-to-jsx@1.1.2` renders `<iframe title={alt_text} src={video_url}>` with no scheme filter, the same gap F-001 documented for its anchors and images.
+  3. The post-render DOM scrub walked `a[href]` and `img[src]` only. `iframe[src]` was never inspected.
+- **Repro**: paste into the JSON drawer — the preview rendered `<iframe src="data:text/html,…">` with no `data-bk-blocked-*` marker, while the same block's `title_url` anchor *was* scrubbed to `href="#"` (the control proving layer 3 worked for anchors and not for frames).
+  ```jsonc
+  [
+    {
+      "type": "video",
+      "alt_text": "poc",
+      "title": { "type": "plain_text", "text": "PoC" },
+      "thumbnail_url": "https://example.com/t.png",
+      "video_url": "data:text/html,<script>top.__pwned=1</script>",
+      "block_id": "poc"
+    }
+  ]
+  ```
+- **Impact**: a `javascript:` `video_url` executes in the embedding app's origin on React 18 (an explicitly supported peer). React 19 blocks that one at `setAttribute` time, but `data:text/html` renders and executes on **every** React version — in an opaque origin, so phishing and UI-redress inside trusted chrome rather than token theft. Reproduced in jsdom: the frame navigation fires before any of our code runs.
+- **Fix**:
+  - `src/lib/url-safety.ts`: new `isSafeEmbedSrc` / `sanitizeEmbedSrc`. Tighter than both existing predicates — http(s) only. `data:` of any media type is rejected (a frame loads with no click, so `data:text/html` executes), as are scheme-less relative URLs (a relative frame source frames the embedding app itself) and the link-only schemes `mailto`/`tel`/`sms`/`xmpp`/`irc`.
+  - `src/lib/sanitize-blocks.ts`: the exact-match key sets are gone. Keys are classified by **name shape** — anything whose last `_`-delimited segment is `url`/`uri`/`link`/`href`/`src` is sanitized, image-ish names (`image`, `thumb`, `icon`, `avatar`, `logo`, `photo`, `picture`) get the image allowlist, `video_url` gets the embed allowlist, everything else gets the link allowlist. Over-matching is free: a URL-shaped key holding a non-URL string has no recognized scheme, so it is treated as relative and passed through. This is the structural half of the fix — a hand-maintained key set had by then missed a field twice.
+  - `src/components/preview/slack-block-preview.tsx`: a third scrub arm over `iframe[src], embed[src], object[data], source[src], video[src], audio[src], track[src]`, applying the embed allowlist and marking blocked elements `data-bk-blocked-src`.
+  - `src/components/editors/video-editor.tsx`: all four URL fields flag an unsafe value with `aria-invalid` and an inline note, matching the structured rich-text editor.
+- **Tests**: [test/preview-url-scrub.test.tsx](test/preview-url-scrub.test.tsx) is the one that would have caught this. It renders hostile payloads and then walks **every** element of the result, asserting that no URL-bearing attribute on any tag that navigates or auto-loads carries anything but an http(s) URL — rather than asserting on the fields we happen to know about today. A renderer upgrade that emits a new URL-bearing tag, or a Slack block that adds a new URL field, fails there. Field-level coverage in [test/sanitize-blocks.test.ts](test/sanitize-blocks.test.ts), [test/url-safety.test.ts](test/url-safety.test.ts), [test/public-api.test.ts](test/public-api.test.ts), and [test/video-editor.test.tsx](test/video-editor.test.tsx). All 14 new assertions fail against the pre-fix tree.
+- **Residual risk**: `slack-blocks-to-jsx` still has no scheme allowlist of its own; ours is applied on both sides of it. Pushing one upstream would close this class at the source for every consumer of that package.
+
+---
+
 ## Items checked and clean
 
 These were inspected, deemed safe as-shipped, and noted here so future reviewers can see what was covered:
@@ -155,7 +193,7 @@ These were inspected, deemed safe as-shipped, and noted here so future reviewers
 - **Clipboard reads**: none.
 - **Raw `fetch` / XHR**: not performed by the library; all I/O is brokered by consumer callbacks (`loadChannels`, `loadSendAsUserStatus`, `onSend`).
 - **File uploads / `FileReader` / `URL.createObjectURL`**: none.
-- **`<iframe>` elements**: none.
+- **`<iframe>` elements**: none constructed by `src/` itself — but `slack-blocks-to-jsx` renders one for the video block's `video_url`, which is what F-009 missed. The preview's DOM scrub now covers `iframe`/`embed`/`object`/`source`/`video`/`audio`/`track` sources, so "we don't write `<iframe>`" is not the same as "no `<iframe>` renders".
 - **`JSON.parse` of untrusted input**: two sites ([url-state.ts:42](src/lib/url-state.ts:42), [json-drawer.tsx:64](src/components/json-drawer.tsx:64)) — both wrapped in try/catch, top-level array check, and now size-capped. `__proto__` keys in JSON do not pollute `Object.prototype` in modern engines and the sanitizer was verified to be free of `Object.assign`-flavored merges that walk the prototype chain ([test/sanitize-blocks.test.ts](test/sanitize-blocks.test.ts) `prototype pollution shape`).
 - **Random IDs**: `nanoid@5.x` (CSPRNG-backed). Not used for security tokens; appropriate.
 - **Toolbar docs link**: [toolbar.tsx:158-166](src/components/toolbar.tsx:158) is a hardcoded `docs.slack.dev` URL with `rel="noreferrer noopener"`. Safe.
@@ -197,3 +235,17 @@ test/url-safety.test.ts                               (new)   F-001
 test/sanitize-blocks.test.ts                          (new)   F-001
 test/public-api.test.ts                               (edit)  F-001, F-004
 ```
+
+Follow-up:
+
+```
+src/lib/url-safety.ts                                 (edit)  F-009
+src/lib/sanitize-blocks.ts                            (edit)  F-009
+src/components/preview/slack-block-preview.tsx        (edit)  F-009
+src/components/editors/video-editor.tsx               (edit)  F-009
+test/preview-url-scrub.test.tsx                       (new)   F-009
+test/video-editor.test.tsx                            (new)   F-009
+test/url-safety.test.ts                               (edit)  F-009
+test/sanitize-blocks.test.ts                          (edit)  F-009
+test/public-api.test.ts                               (edit)  F-009
+```
diff --git a/src/components/editors/video-editor.tsx b/src/components/editors/video-editor.tsx
@@ -1,9 +1,24 @@
+import type { ReactNode } from 'react';
 import { Input } from '../../lib/ui/input';
 import { Textarea } from '../../lib/ui/textarea';
+import { isSafeEmbedSrc, isSafeHref, isSafeImageSrc } from '../../lib/url-safety';
 import type { VideoBlock } from '../../types';
 import { EditorField } from './field';
 import type { BlockEditorProps } from './types';
 
+/**
+ * Inline warning shown under a URL field whose value will be stripped by
+ * the sanitizer before it reaches the preview or Slack. Mirrors the
+ * structured rich-text editor's URL field so an unsafe paste explains
+ * itself instead of silently disappearing from the preview.
+ * @param props - warning props
+ * @param props.children - the rule that was broken, in plain language
+ * @returns the rendered warning
+ */
+function UnsafeUrlNote({ children }: { children: ReactNode }) {
+  return <p className="mt-1 text-[11px] text-destructive">{children}</p>;
+}
+
 /**
  * Editor form for video blocks. Edits the title, alt text, thumbnail/video
  * URLs, and optional provider/author/description metadata. Sending a video
@@ -16,6 +31,14 @@ import type { BlockEditorProps } from './types';
  * @returns the rendered video editor form
  */
 export function VideoEditor({ block, onChange }: BlockEditorProps<VideoBlock>) {
+  // The preview renders `video_url` into an `<iframe src>`, so it is held
+  // to http(s) only; the two image fields and the title link get the
+  // image / link allowlists that apply where they land in the DOM.
+  const videoUrlIsUnsafe = (block.video_url ?? '').length > 0 && !isSafeEmbedSrc(block.video_url);
+  const thumbnailIsUnsafe = (block.thumbnail_url ?? '').length > 0 && !isSafeImageSrc(block.thumbnail_url);
+  const titleUrlIsUnsafe = (block.title_url ?? '').length > 0 && !isSafeHref(block.title_url);
+  const providerIconIsUnsafe = (block.provider_icon_url ?? '').length > 0 && !isSafeImageSrc(block.provider_icon_url);
+
   return (
     <div className="flex flex-col gap-4">
       <EditorField label="Title" help="Up to 199 characters. Shown above the embedded player." htmlFor="video-title">
@@ -53,7 +76,13 @@ export function VideoEditor({ block, onChange }: BlockEditorProps<VideoBlock>) {
           value={block.video_url ?? ''}
           placeholder="e.g. https://www.youtube.com/embed/dQw4w9WgXcQ"
           onChange={(e) => onChange({ ...block, video_url: e.target.value })}
+          aria-invalid={videoUrlIsUnsafe || undefined}
         />
+        {videoUrlIsUnsafe && (
+          <UnsafeUrlNote>
+            The embedded player only accepts a full http(s) URL. This URL will be stripped before send and preview.
+          </UnsafeUrlNote>
+        )}
       </EditorField>
 
       <EditorField label="Thumbnail URL" help="Preview image shown before the video loads." htmlFor="video-thumbnail">
@@ -63,7 +92,13 @@ export function VideoEditor({ block, onChange }: BlockEditorProps<VideoBlock>) {
           value={block.thumbnail_url ?? ''}
           placeholder="e.g. https://example.com/thumb.png"
           onChange={(e) => onChange({ ...block, thumbnail_url: e.target.value })}
+          aria-invalid={thumbnailIsUnsafe || undefined}
         />
+        {thumbnailIsUnsafe && (
+          <UnsafeUrlNote>
+            Only http(s) image URLs are allowed. This URL will be stripped before send and preview.
+          </UnsafeUrlNote>
+        )}
       </EditorField>
 
       <EditorField
@@ -77,7 +112,14 @@ export function VideoEditor({ block, onChange }: BlockEditorProps<VideoBlock>) {
           value={block.title_url ?? ''}
           placeholder="e.g. https://www.youtube.com/watch?v=dQw4w9WgXcQ"
           onChange={(e) => onChange({ ...block, title_url: e.target.value || undefined })}
+          aria-invalid={titleUrlIsUnsafe || undefined}
         />
+        {titleUrlIsUnsafe && (
+          <UnsafeUrlNote>
+            Only http(s), mailto, tel, sms, and xmpp links are allowed. This URL will be stripped before send and
+            preview.
+          </UnsafeUrlNote>
+        )}
       </EditorField>
 
       <EditorField label="Description" help="Optional. Up to 199 characters." htmlFor="video-description">
@@ -130,7 +172,13 @@ export function VideoEditor({ block, onChange }: BlockEditorProps<VideoBlock>) {
           value={block.provider_icon_url ?? ''}
           placeholder="e.g. https://example.com/youtube-favicon.png"
           onChange={(e) => onChange({ ...block, provider_icon_url: e.target.value || undefined })}
+          aria-invalid={providerIconIsUnsafe || undefined}
         />
+        {providerIconIsUnsafe && (
+          <UnsafeUrlNote>
+            Only http(s) image URLs are allowed. This URL will be stripped before send and preview.
+          </UnsafeUrlNote>
+        )}
       </EditorField>
     </div>
   );

diff --git a/src/components/preview/slack-block-preview.tsx b/src/components/preview/slack-block-preview.tsx
@@ -4,9 +4,18 @@ import { useEffect, useMemo, useRef } from 'react';
 import type { Block } from 'slack-blocks-to-jsx';
 import { Message } from 'slack-blocks-to-jsx';
 import { sanitizeBlock } from '../../lib/sanitize-blocks';
-import { isSafeHref, isSafeImageSrc } from '../../lib/url-safety';
+import { isSafeEmbedSrc, isSafeHref, isSafeImageSrc } from '../../lib/url-safety';
 import type { PreviewHooks, PreviewTheme, SupportedBlock } from '../../types';
 
+/**
+ * Elements whose source attribute makes the browser load a document or
+ * subresource on its own, with no click. `slack-blocks-to-jsx` renders a
+ * video block's `video_url` into `<iframe src>`; the rest are covered so
+ * a renderer upgrade that starts emitting one of them is scrubbed from
+ * day one rather than after the next report.
+ */
+const EMBED_SELECTOR = 'iframe[src], embed[src], object[data], source[src], video[src], audio[src], track[src]';
+
 /**
  * Renders a Slack block via the `slack-blocks-to-jsx` library's `<Message>`
  * component. We render exactly one block at a time so each row in the
@@ -47,16 +56,19 @@ export function SlackBlockPreview({
   // video blocks without an aria-label, which violates axe's `button-name`
   // rule and is unreachable to screen readers. Post-mount we add a label
   // to any such buttons we find under our wrapper. We also do a final
-  // pass to neutralize any `<a href>` or `<img src>` that carries a
-  // disallowed URI scheme — the block-payload sanitizer catches URLs
-  // that live in structured fields (`url`, `image_url`), but mrkdwn /
-  // rich-text content can encode link URLs inside text strings
-  // (`[label](javascript:...)` or `<javascript:...|label>`) that
-  // `slack-blocks-to-jsx`'s own parser hands straight to `<a href>`
-  // without filtering. React 19 also blocks `javascript:` URLs at
-  // setAttribute time, but we don't rely on that — this loop applies
-  // our allowlist (which is tighter and covers `data:`/`vbscript:`/`file:`
-  // as well) and replaces unsafe values with `#`.
+  // pass to neutralize any `<a href>`, `<img src>`, or frame/subresource
+  // source that carries a disallowed URI scheme — the block-payload
+  // sanitizer catches URLs that live in structured fields (`url`,
+  // `image_url`, `video_url`), but mrkdwn / rich-text content can encode
+  // link URLs inside text strings (`[label](javascript:...)` or
+  // `<javascript:...|label>`) that `slack-blocks-to-jsx`'s own parser
+  // hands straight to `<a href>` without filtering. React 19 also blocks
+  // `javascript:` URLs at setAttribute time, but we don't rely on that —
+  // this loop applies our allowlist (which is tighter and covers
+  // `data:`/`vbscript:`/`file:` as well) and replaces unsafe values with
+  // `#`. Frames get the strictest arm: an `<iframe src>` loads on render
+  // rather than on click, so `data:text/html` there executes script in an
+  // opaque origin on every React version and only http(s) is allowed.
   useEffect(() => {
     const root = rootRef.current;
     if (!root) return;
@@ -82,6 +94,13 @@ export function SlackBlockPreview({
         img.setAttribute('data-bk-blocked-src', '1');
       }
     }
+    for (const el of root.querySelectorAll<HTMLElement>(EMBED_SELECTOR)) {
+      const attr = el.tagName === 'OBJECT' ? 'data' : 'src';
+      if (!isSafeEmbedSrc(el.getAttribute(attr))) {
+        el.removeAttribute(attr);
+        el.setAttribute('data-bk-blocked-src', '1');
+      }
+    }
   });
 
   return (