黄云龙 97ca7f88cf
fix(frontend): harden artifact and markdown rendering (#4117)
* fix(frontend): harden artifact and markdown rendering

* fix(#4117): restore allow-scripts for scroll-restore, fix citation XSS bypass

willem-bd identified these issues:

1. [REGRESSION] sandbox removed allow-scripts which broke HTML artifact
   scroll-restoration — the injected postMessage script could not run.
   The prior config (allow-scripts allow-forms without allow-same-origin,
   i.e. opaque origin) was already safe. Restored with corrected comment.

2. [XSS bypass] CitationLink rendered <a href={href}> directly before
   isSafeHref ran, so prompt-injected [citation:x](javascript:...)
   bypassed the safety check. Moved citation block after the guard.

Also:
- Added mailto: and tel: to SAFE_HREF_PROTOCOLS
- Kept anchor-only attributes off the <span> fallback (React DOM warning)

Note: the loadMessages re-throw originally in this commit was dropped
during the rebase — upstream #4065 rewrote useThreadHistory as a
TanStack useInfiniteQuery that already surfaces fetch failures.

* test(e2e): update sandbox assertion to match allow-scripts allow-forms

The artifact preview iframe's sandbox was restored to 'allow-scripts allow-forms'
for scroll-restoration. Update the E2E test to expect the corrected value.

* fix(#4117): address review — ArtifactLink XSS guard, urlOfArtifact sandbox e2e

- Apply the isSafeHref guard to ArtifactLink (artifact-link.tsx) so a
  prompt-injected javascript:/data: href in a .md artifact preview cannot
  reach a real anchor in the main document, matching the guard in
  createMarkdownLinkComponent.
- Add an e2e asserting the urlOfArtifact iframe (non-code
  browser-previewable files like PDF) keeps its empty sandbox.

Note: the loadMessages error-surfacing changes originally in this commit
were dropped during the rebase — upstream #4065 rewrote useThreadHistory
as a TanStack useInfiniteQuery whose queryFn already throws on
!response.ok and surfaces the failure via a toast.

* fix(frontend): let write_file non-code previewable artifacts render in sandboxed iframe

When a write_file artifact such as PDF is clicked in chat, the component receives a path that forced isCodeFile=true, hiding the sandboxed iframe behind the code editor. Now non-code browser-previewable files are detected early so the sandboxed iframe renders correctly.

Fixes the E2E test: renders sandboxed iframe for a browser-previewable non-code file.

* fix(#4117): align PDF artifact route pattern with convention (drop /mock/ prefix)

The test used /mock/api/threads/... for the PDF artifact content route,
but the urlOfArtifact helper generates /api/threads/... (without /mock/)
when isMock=false. The other tests (e.g. presented artifacts) already use
the correct /api/threads/... pattern.

* test(frontend): add render-level coverage for unsafe markdown/artifact links

- Render MarkdownLink and ArtifactLink via renderToStaticMarkup and
  assert an unsafe javascript: href produces a disabled <span> (never an
  <a>), including through the citation-labelled branch, and that safe
  https hrefs render hardened anchors (target=_blank, rel=noopener).
- The new ArtifactLink render test caught the unsafe href leaking onto
  the fallback <span> through the {...rest} spread; drop the spread in
  both span fallbacks so anchor-only attributes (href/target/rel) and
  react-markdown's node prop never reach the span.
- Cover mailto:/tel: in the isSafeHref unit cases and fix the stale
  SAFE_HREF_PROTOCOLS docstring that still claimed http/https-only.

* fix(frontend): route the agent save-hint through the safe localStorage facade

Export safeLocalStorage from core/settings/local and use it for the
agent-create save-hint read/write so blocked browser storage (Safari
private mode, strict containers, embedded WebViews) cannot throw from
the effect. core/agents/feature-cache.ts already guards its
localStorage access with try/catch, so settings + agent pages are now
consistently best-effort.

* fix(frontend): allow scheme-less relative markdown links in isSafeHref
2026-07-21 11:35:44 +08:00

92 lines
3.1 KiB
TypeScript

import { describe, expect, it } from "@rstest/core";
import { createElement } from "react";
import { renderToStaticMarkup } from "react-dom/server";
import {
createMarkdownLinkComponent,
isSafeHref,
} from "@/components/workspace/messages/markdown-link";
describe("isSafeHref", () => {
it("allows web URLs and same-origin paths", () => {
expect(isSafeHref("https://example.com/path")).toBe(true);
expect(isSafeHref("http://example.com/path")).toBe(true);
expect(isSafeHref("/workspace/chats/1")).toBe(true);
expect(isSafeHref("#section")).toBe(true);
});
it("allows scheme-less relative references", () => {
expect(isSafeHref("report.md")).toBe(true);
expect(isSafeHref("./report.md")).toBe(true);
expect(isSafeHref("../assets/chart.png")).toBe(true);
});
it("allows non-executing contact schemes", () => {
expect(isSafeHref("mailto:someone@example.com")).toBe(true);
expect(isSafeHref("tel:+15551234567")).toBe(true);
});
it("rejects executable, local, and protocol-relative URLs", () => {
expect(isSafeHref("javascript:alert(1)")).toBe(false);
expect(isSafeHref("data:text/html,<script>alert(1)</script>")).toBe(false);
expect(isSafeHref("file:///etc/passwd")).toBe(false);
expect(isSafeHref("//example.com/path")).toBe(false);
expect(isSafeHref("\\\\evil.com")).toBe(false);
expect(isSafeHref(undefined)).toBe(false);
});
});
// Render-level coverage: these tests exercise the component itself, so
// removing (or inverting) the isSafeHref guard inside MarkdownLink fails
// them even though isSafeHref stays untouched.
describe("MarkdownLink rendering", () => {
const MarkdownLink = createMarkdownLinkComponent();
it("renders an unsafe href as a disabled span, never an anchor", () => {
const html = renderToStaticMarkup(
createElement(MarkdownLink, { href: "javascript:alert(1)" }, "click me"),
);
expect(html).not.toContain("<a");
expect(html).toContain("<span");
expect(html).toContain("click me");
expect(html).not.toContain("href=");
});
it("blocks unsafe hrefs before the citation branch", () => {
const html = renderToStaticMarkup(
createElement(
MarkdownLink,
{ href: "javascript:alert(1)" },
"citation:evil",
),
);
expect(html).not.toContain("<a");
expect(html).not.toContain("href=");
});
it("renders a safe https href as a hardened anchor", () => {
const html = renderToStaticMarkup(
createElement(
MarkdownLink,
{ href: "https://example.com/report" },
"report",
),
);
expect(html).toContain("<a");
expect(html).toContain('href="https://example.com/report"');
expect(html).toContain('target="_blank"');
expect(html).toContain('rel="noopener noreferrer"');
});
it("renders a scheme-less relative href as a navigable anchor", () => {
const tests = ["report.md", "./report.md", "../assets/chart.png"];
for (const href of tests) {
const html = renderToStaticMarkup(
createElement(MarkdownLink, { href }, "doc"),
);
expect(html).toContain("<a");
expect(html).toContain(`href="${href}"`);
}
});
});