From 97ca7f88cfb5ac55bbbd1823c0301463e0e2a924 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?=E9=BB=84=E4=BA=91=E9=BE=99?= <76432572+nankingjing@users.noreply.github.com> Date: Tue, 21 Jul 2026 11:35:44 +0800 Subject: [PATCH] fix(frontend): harden artifact and markdown rendering (#4117) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit * 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 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 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 (never an ), 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 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 --- .../src/app/workspace/agents/new/page.tsx | 5 +- .../artifacts/artifact-file-detail.tsx | 14 ++- .../workspace/citations/artifact-link.tsx | 23 +++++ .../workspace/messages/markdown-link.tsx | 74 ++++++++++++++- frontend/src/core/settings/local.ts | 51 ++++++++++- frontend/tests/e2e/artifact-preview.spec.ts | 45 +++++++++ .../workspace/citations/artifact-link.test.ts | 41 +++++++++ .../workspace/messages/markdown-link.test.ts | 91 +++++++++++++++++++ .../tests/unit/core/settings/local.test.ts | 27 +++++- 9 files changed, 360 insertions(+), 11 deletions(-) create mode 100644 frontend/tests/unit/components/workspace/citations/artifact-link.test.ts create mode 100644 frontend/tests/unit/components/workspace/messages/markdown-link.test.ts diff --git a/frontend/src/app/workspace/agents/new/page.tsx b/frontend/src/app/workspace/agents/new/page.tsx index 67ec49315..e006cf88a 100644 --- a/frontend/src/app/workspace/agents/new/page.tsx +++ b/frontend/src/app/workspace/agents/new/page.tsx @@ -45,6 +45,7 @@ import { type HumanInputResponse, } from "@/core/messages/human-input"; import { isHiddenFromUIMessage } from "@/core/messages/utils"; +import { safeLocalStorage } from "@/core/settings/local"; import { useThreadStream } from "@/core/threads/hooks"; import { uuid } from "@/core/utils/uuid"; import { isIMEComposing } from "@/lib/ime"; @@ -130,11 +131,11 @@ export default function NewAgentPage() { if (typeof window === "undefined" || step !== "chat") { return; } - if (window.localStorage.getItem(SAVE_HINT_STORAGE_KEY) === "1") { + if (safeLocalStorage.getItem(SAVE_HINT_STORAGE_KEY) === "1") { return; } setShowSaveHint(true); - window.localStorage.setItem(SAVE_HINT_STORAGE_KEY, "1"); + safeLocalStorage.setItem(SAVE_HINT_STORAGE_KEY, "1"); }, [step]); const handleConfirmName = useCallback(async () => { diff --git a/frontend/src/components/workspace/artifacts/artifact-file-detail.tsx b/frontend/src/components/workspace/artifacts/artifact-file-detail.tsx index 546a9d8a5..d77d75031 100644 --- a/frontend/src/components/workspace/artifacts/artifact-file-detail.tsx +++ b/frontend/src/components/workspace/artifacts/artifact-file-detail.tsx @@ -129,7 +129,13 @@ export function ArtifactFileDetail({ }, [filepath]); const { isCodeFile, language } = useMemo(() => { if (isWriteFile) { - let language = checkCodeFile(filepath).language; + const codeResult = checkCodeFile(filepath); + // Non-code browser-previewable files (PDF, images, audio, video) + // should render in the sandboxed iframe, not the code editor. + if (!codeResult.isCodeFile && canBrowserPreviewFile(filepath)) { + return codeResult; + } + let language = codeResult.language; language ??= "text"; return { isCodeFile: true, language }; } @@ -360,6 +366,7 @@ export function ArtifactFileDetail({ {!isCodeFile && canPreviewInBrowser && (