diff --git a/CHANGELOG.md b/CHANGELOG.md index 2ba5a4aeb..4400a6acc 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -2770,6 +2770,14 @@ This release closes that milestone with **765 merged pull requests**. ### Security +- **frontend:** Tool steps no longer turn non-web URLs into links. The + `web_fetch` URL and `web_search` / `image_search` result links in the + chain-of-thought panel skipped the scheme allowlist that markdown links use, + so a prompt-injected tool call could put a `file:` or OS protocol-handler + link (`ms-msdt:`, `vscode:`, …) into the chat. They now pass `isSafeHref` + and show an unsafe URL with the same "Unsafe link omitted" marker as + markdown links. A tool call whose args are missing, or whose `web_fetch` URL + is not a string, no longer crashes the message list. ([#5526]) - **skills:** Close gaps that let files skip SkillScan in the public skill review gate. The review analyzer passed SkillScan only files it had decoded as text, so executable binaries and nested archives were never checked; it @@ -4281,3 +4289,4 @@ with **180 merged pull requests** since the first 2.0 milestone tag. [#5504]: https://github.com/bytedance/deer-flow/pull/5504 [#5505]: https://github.com/bytedance/deer-flow/pull/5505 [#5524]: https://github.com/bytedance/deer-flow/pull/5524 +[#5526]: https://github.com/bytedance/deer-flow/pull/5526 diff --git a/CHANGELOG_zh.md b/CHANGELOG_zh.md index 871a73083..89ad96a20 100644 --- a/CHANGELOG_zh.md +++ b/CHANGELOG_zh.md @@ -2085,6 +2085,12 @@ ### 安全 +- **前端：** 工具步骤不再把非 Web URL 渲染为链接。思维链面板中的 `web_fetch` URL 与 + `web_search` / `image_search` 结果链接此前绕过了 Markdown 链接使用的协议白名单， + 被提示注入的工具调用可在聊天中放入 `file:` 或系统协议处理程序链接（`ms-msdt:`、 + `vscode:` 等）。现在它们会经过 `isSafeHref`，不安全的 URL 与 Markdown 链接一样显示 + “Unsafe link omitted” 标记；缺少 args 的工具调用或非字符串的 `web_fetch` URL 也不再导致 + 消息列表崩溃。([#5526]) - **技能：** 修复公共技能审查门禁中文件可绕过 SkillScan 的缺口。审查分析器此前只把解码为 文本的文件交给 SkillScan，可执行二进制文件和嵌套压缩包从未被检查；豁免了任意层级 `evals/fixtures/` 目录下的所有文件；重复的压缩包成员或仅大小写不同的文件名会在扫描前静默 @@ -3492,3 +3498,4 @@ DeerFlow 2.0 是围绕"超级智能体"框架的彻底重写，核心包含子 [#5504]: https://github.com/bytedance/deer-flow/pull/5504 [#5505]: https://github.com/bytedance/deer-flow/pull/5505 [#5524]: https://github.com/bytedance/deer-flow/pull/5524 +[#5526]: https://github.com/bytedance/deer-flow/pull/5526 diff --git a/frontend/src/AGENTS.md b/frontend/src/AGENTS.md index b09aeac45..f543920e3 100644 --- a/frontend/src/AGENTS.md +++ b/frontend/src/AGENTS.md @@ -144,6 +144,7 @@ Array previews coalesce consecutive generated markers only at the end into one o - **SSE replay gaps** are handled in `core/api/api-client.ts`, which wraps both initial and joined run streams because the upstream SDK ignores unknown event names. An id-less backend `gap` control frame clears stale reconnect metadata, emits an internal `stream_replay_gap` custom event, reloads durable thread values, and resumes after the server-provided retained tail when one exists (or rejoins without a cursor if the buffer is empty), with up to five recovery rejoins after the original stream (six total stream calls on an all-gap exhaustion path). The wrapper remains a lazy async iterable because the SDK consumes it with `for await`. `core/threads/hooks.ts` clears optimistic/transient/subtask state, invalidates durable history caches, and shows the localized recovery warning; never let a gap fall through as a normal stream finish or cancel the still-running backend run. - **Streaming Markdown rendering** is owned by `core/streamdown`: Streamdown's `animated` / `isAnimating` API handles incremental word animation, while the shared `streamdownRenderingPlugins` config registers the named code-highlighting and Mermaid plugins required by Streamdown 2.5. Keep wrappers and derived configs wired to that shared object; do not reintroduce a rehype plugin that wraps every word, because reparsing a growing block remounts old words and replays their animation. - Citation links in message and artifact Markdown must derive their `citation:` label from the full `ReactNode` children tree, since Streamdown may provide element or array children during streaming rather than a plain string. +- Tool-step links in `message-group.tsx` (`web_fetch` args, `web_search` / `image_search` result URLs) are model- or provider-controlled, so they pass the markdown `isSafeHref` allowlist; every surface renders a rejected href through the shared `UnsafeLink` marker. - **Environment validation** uses `@t3-oss/env-nextjs` with Zod schemas (`src/env.js`). Skip with `SKIP_ENV_VALIDATION=1` - **Subtask step history and runtime metadata** (`core/tasks/`) — the subtask card shows a subagent's full step timeline (#3779): its assistant reasoning turns interleaved with the tools it ran. The task tool's model-visible `description` is an optional progress label; `MessageList` uses the required `prompt` (then the localized generic subtask label) when a provider omits it, so a valid task call never renders a blank card title. `Subtask.steps[]` is accumulated live from `task_running` events (appended via `mergeSteps`, not overwritten) and backfilled on expand for historical runs by `fetchSubtaskSteps`, which pages the events endpoint scoped to one task (GET `/runs/{runId}/events?event_types=subagent.step&task_id=…&after_seq=…`) until a short page, so the run-wide limit can't truncate the timeline. `task_started` carries the effective `model_name`; `task_running` carries a cumulative usage snapshot after each completed LLM call. `core/tasks/lifecycle.ts` normalizes these additive events, and `computeNextSubtask` keeps the largest cumulative total so replayed or late SSE frames cannot double-count or roll the folded card backward. Terminal ToolMessage metadata (`subagent_model_name` / `subagent_token_usage`) restores the same values from normal history after reload; no per-card event fetch is needed. `core/tasks/steps.ts` is the pure step model: `messageToStep` (live), `eventsToSteps` (reload), `mergeSteps` (dedup by `message_index`), and `stepsForDisplay` (what the card renders — keeps tool steps + AI steps with text, drops the trailing final-answer AI step when completed since it's shown as `result`). `core/tasks/context.tsx`'s `useUpdateSubtask` applies updates against a `tasksRef` mirroring the latest state (not a closure snapshot), so a late-resolving `fetchSubtaskSteps` backfill merges into current state instead of clobbering SSE steps or sibling subtasks that arrived meanwhile. The owning `run_id` is carried onto history content messages in `buildVisibleHistoryMessages` so the card can resolve the events endpoint. diff --git a/frontend/src/components/workspace/citations/artifact-link.tsx b/frontend/src/components/workspace/citations/artifact-link.tsx index 119093753..1560c3086 100644 --- a/frontend/src/components/workspace/citations/artifact-link.tsx +++ b/frontend/src/components/workspace/citations/artifact-link.tsx @@ -2,7 +2,7 @@ import type { AnchorHTMLAttributes } from "react"; import { cn } from "@/lib/utils"; -import { isSafeHref } from "../messages/markdown-link"; +import { isSafeHref, UnsafeLink } from "../messages/markdown-link"; import { CitationLink, extractReactNodeText } from "./citation-link"; @@ -21,16 +21,9 @@ export function ArtifactLink(props: AnchorHTMLAttributes) { // into the DOM / trigger React DOM warnings. const { className, children } = props; return ( - + {children} - + ); } const childrenText = extractReactNodeText(props.children); diff --git a/frontend/src/components/workspace/messages/markdown-link.tsx b/frontend/src/components/workspace/messages/markdown-link.tsx index 38625ef7b..07fdd64af 100644 --- a/frontend/src/components/workspace/messages/markdown-link.tsx +++ b/frontend/src/components/workspace/messages/markdown-link.tsx @@ -1,4 +1,4 @@ -import type { AnchorHTMLAttributes } from "react"; +import type { AnchorHTMLAttributes, ComponentProps } from "react"; import { resolveMarkdownArtifactURL } from "@/core/artifacts/utils"; import { cn } from "@/lib/utils"; @@ -49,6 +49,30 @@ export function isSafeHref(href: string | undefined): boolean { } } +/** + * Inert stand-in for a link whose href failed `isSafeHref`. It keeps the + * visible label and marks the omission on hover and for assistive tech, so + * every surface that applies the allowlist degrades the same way. Extra props + * pass through for wrappers such as Radix `asChild` triggers. + */ +export function UnsafeLink({ + href, + className, + ...props +}: ComponentProps<"span"> & { href: string }) { + return ( + + ); +} + function isExternalUrl(href: string | undefined): boolean { if (typeof href !== "string") { return false; @@ -78,16 +102,9 @@ export function createMarkdownLinkComponent(threadId?: string) { // and would trigger React DOM warnings. const { className, children } = props; return ( - + {children} - + ); } // Safe-href check passed — citation links now route through CitationLink. diff --git a/frontend/src/components/workspace/messages/message-group.tsx b/frontend/src/components/workspace/messages/message-group.tsx index 0d341a98b..3c40e47d6 100644 --- a/frontend/src/components/workspace/messages/message-group.tsx +++ b/frontend/src/components/workspace/messages/message-group.tsx @@ -48,6 +48,7 @@ import { FlipDisplay } from "../flip-display"; import { Tooltip } from "../tooltip"; import { MarkdownContent } from "./markdown-content"; +import { isSafeHref, UnsafeLink } from "./markdown-link"; import { ToolCallDetails } from "./tool-call-details"; interface MessageGroupProps { @@ -759,11 +760,18 @@ function ToolCall({ > {Array.isArray(result) && ( + {/* Tool args and results are model- or provider-controlled, so + every tool link passes the same scheme allowlist as markdown + links and degrades to the same UnsafeLink marker. */} {result.map((item) => ( - - {item.title} - + {isSafeHref(item.url) ? ( + + {item.title} + + ) : ( + {item.title} + )} ))} @@ -794,32 +802,48 @@ function ToolCall({ {Array.isArray(results) && ( {Array.isArray(results) && - results.map((item) => ( - - -
- {item.title} -
- - - ))} + results.map((item) => { + const thumbnail = ( +
+ {item.title} +
+ ); + return ( + + {isSafeHref(item.source_url) ? ( + + {thumbnail} + + ) : ( + + {thumbnail} + + )} + + ); + })} )} ); } else if (kind === "web_fetch") { - const url = (args as { url: string })?.url; + // Models occasionally emit non-string args mid-stream; an object here + // would reach the JSX below and throw. + const url = typeof args.url === "string" ? args.url : undefined; let title = url; if (typeof result === "string") { const potentialTitle = extractTitleFromMarkdown(result); @@ -834,16 +858,19 @@ function ToolCall({ icon={GlobeIcon} > - {url && ( - - {title} - - )} + {url && + (isSafeHref(url) ? ( + + {title} + + ) : ( + {title} + ))} ); @@ -1089,7 +1116,9 @@ function convertToSteps(messages: Message[]): CoTStep[] { messageId: message.id, type: "toolCall", name: tool_call.name, - args: tool_call.args, + // Persisted or mid-stream tool calls can omit args (or send null); + // every ToolCall branch reads them, so normalize once here. + args: tool_call.args ?? {}, }; const toolCallId = tool_call.id; if (toolCallId) { diff --git a/frontend/tests/unit/components/workspace/citations/artifact-link.test.ts b/frontend/tests/unit/components/workspace/citations/artifact-link.test.ts index 96af92e08..b85fdb50f 100644 --- a/frontend/tests/unit/components/workspace/citations/artifact-link.test.ts +++ b/frontend/tests/unit/components/workspace/citations/artifact-link.test.ts @@ -16,6 +16,7 @@ describe("ArtifactLink rendering", () => { expect(html).toContain(" { diff --git a/frontend/tests/unit/components/workspace/messages/markdown-link.test.ts b/frontend/tests/unit/components/workspace/messages/markdown-link.test.ts index b90941a9e..c0eb5e7cc 100644 --- a/frontend/tests/unit/components/workspace/messages/markdown-link.test.ts +++ b/frontend/tests/unit/components/workspace/messages/markdown-link.test.ts @@ -50,6 +50,7 @@ describe("MarkdownLink rendering", () => { expect(html).toContain(" { diff --git a/frontend/tests/unit/components/workspace/messages/message-group.test.ts b/frontend/tests/unit/components/workspace/messages/message-group.test.ts index a3c5b65b1..3e74337b7 100644 --- a/frontend/tests/unit/components/workspace/messages/message-group.test.ts +++ b/frontend/tests/unit/components/workspace/messages/message-group.test.ts @@ -516,6 +516,167 @@ describe("MessageGroup", () => { }); }); +// Tool args come from the model and results from search providers, so a +// prompt-injected URL must not become a navigable anchor. React only rewrites +// javascript: hrefs; local and OS-handler schemes would otherwise pass through. +// A blocked URL keeps the markdown path's "Unsafe link omitted" marker. +describe("MessageGroup tool links", () => { + const unsafeUrls = [ + "javascript:alert(1)", + "file:///etc/passwd", + "ms-msdt:/id PCWDiagnostic", + "vscode://file/etc/passwd", + ]; + + it.each(unsafeUrls)("marks a web_fetch URL of %s as omitted", (url) => { + const html = renderToolCall("web_fetch", { url }); + + expect(html).toContain(`>${url}`); + expect(html).toContain(`title="Unsafe link scheme in ${url}"`); + expect(unsafeMarkerCount(html)).toBe(1); + expect(html).not.toContain(" { + const html = renderToolCall( + "web_search", + { query: "DeerFlow" }, + JSON.stringify([ + { title: "Safe source", url: "https://safe.example" }, + { title: "Injected source", url }, + ]), + ); + + expect(html).toContain('href="https://safe.example"'); + expect(html).toContain(">Injected source"); + expect(unsafeMarkerCount(html)).toBe(1); + expect(anchorCount(html)).toBe(1); + }); + + it.each(unsafeUrls)( + "marks an image_search source at %s as omitted", + (url) => { + const html = renderToolCall( + "image_search", + { query: "DeerFlow" }, + JSON.stringify({ + results: [ + { + title: "Injected image", + source_url: url, + thumbnail_url: "https://images.example/thumb.png", + image_url: "https://images.example/full.png", + }, + ], + }), + ); + + expect(html).toContain('src="https://images.example/thumb.png"'); + expect(unsafeMarkerCount(html)).toBe(1); + expect(html).not.toContain(" { + const fetchHtml = renderToolCall("web_fetch", { + url: "https://example.com/page", + }); + const imageHtml = renderToolCall( + "image_search", + { query: "DeerFlow" }, + JSON.stringify({ + results: [ + { + title: "Image", + source_url: "https://example.com/source", + thumbnail_url: "https://images.example/thumb.png", + image_url: "https://images.example/full.png", + }, + ], + }), + ); + + expect(fetchHtml).toContain('href="https://example.com/page"'); + expect(imageHtml).toContain('href="https://example.com/source"'); + expect(unsafeMarkerCount(fetchHtml + imageHtml)).toBe(0); + }); + + // Models occasionally emit non-string args, and the step renders mid-stream; + // an object reaching the JSX would throw and take down the message list. + it("renders a web_fetch step whose url arg is not a string", () => { + const html = renderToolCall("web_fetch", { + url: { href: "https://example.com/page" }, + }); + + expect(html).toContain("View web page"); + expect(html).not.toContain(" { + for (const args of [undefined, null]) { + const render = () => + renderGroup([ + { + id: "ai-1", + type: "ai", + content: "", + tool_calls: [{ id: "call-1", name, args }], + } as unknown as Message, + ]); + + expect(render).not.toThrow(); + } + }); +}); + +function renderToolCall( + name: string, + args: Record, + content?: string, +) { + const messages: Message[] = [ + { + id: "ai-1", + type: "ai", + content: "", + tool_calls: [{ id: "call-1", name, args }], + } as Message, + ]; + if (content !== undefined) { + messages.push({ + id: "tool-1", + type: "tool", + name, + tool_call_id: "call-1", + content, + } as Message); + } + return renderGroup(messages); +} + +function unsafeMarkerCount(html: string) { + return html.split('aria-label="Unsafe link omitted"').length - 1; +} + +function anchorCount(html: string) { + return html.match(/ html.indexOf(needle));