From 5ed28a6744f27709c08863cd63adb71300d02a08 Mon Sep 17 00:00:00 2001 From: chengyongru <61816729+chengyongru@users.noreply.github.com> Date: Wed, 15 Jul 2026 10:45:40 +0800 Subject: [PATCH] fix(webui): validate inferred file paths before preview (#4935) --- nanobot/webui/file_preview.py | 67 ++++++++----- nanobot/webui/ws_http.py | 21 ++-- tests/channels/test_websocket_channel.py | 75 ++++++++++++++ .../FilePreviewAvailabilityContext.tsx | 25 +++++ webui/src/components/MarkdownTextRenderer.tsx | 57 ++++++++++- webui/src/components/thread/ThreadShell.tsx | 97 +++++++++++++++---- webui/src/lib/api.ts | 18 ++++ webui/src/tests/api.test.ts | 27 ++++++ .../src/tests/markdown-text-renderer.test.tsx | 72 +++++++++++++- webui/src/tests/thread-shell.test.tsx | 53 ++++++++++ 10 files changed, 461 insertions(+), 51 deletions(-) create mode 100644 webui/src/components/FilePreviewAvailabilityContext.tsx diff --git a/nanobot/webui/file_preview.py b/nanobot/webui/file_preview.py index 82e6b852..dac0d0e3 100644 --- a/nanobot/webui/file_preview.py +++ b/nanobot/webui/file_preview.py @@ -30,28 +30,7 @@ def file_preview_payload( ) -> dict[str, Any]: """Return a text preview for a file allowed by the session workspace scope.""" - path = _clean_preview_path(raw_path) - if not path: - raise WebUIFilePreviewError(400, "missing path") - if len(path) > 4096: - raise WebUIFilePreviewError(400, "path is too long") - - try: - resolved = resolve_allowed_path( - path, - workspace=scope.project_path, - allowed_root=scope.project_path if scope.restrict_to_workspace else None, - strict=True, - ) - except FileNotFoundError as e: - raise WebUIFilePreviewError(404, "file not found") from e - except WorkspaceBoundaryError as e: - raise WebUIFilePreviewError(403, "file is outside the current workspace") from e - except OSError as e: - raise WebUIFilePreviewError(400, "invalid path") from e - - if not resolved.is_file(): - raise WebUIFilePreviewError(404, "file not found") + resolved = _resolve_preview_path(raw_path, scope=scope) try: with open(resolved, "rb") as f: @@ -81,6 +60,50 @@ def file_preview_payload( } +def file_preview_availability_payload( + raw_path: str | None, + *, + scope: WorkspaceScope, +) -> dict[str, bool]: + """Confirm that a path is a readable text preview candidate without loading it fully.""" + + resolved = _resolve_preview_path(raw_path, scope=scope) + try: + with open(resolved, "rb") as f: + prefix = f.read(4096) + except OSError as e: + raise WebUIFilePreviewError(500, "failed to read file") from e + if b"\0" in prefix: + raise WebUIFilePreviewError(415, "binary files cannot be previewed") + return {"available": True} + + +def _resolve_preview_path(raw_path: str | None, *, scope: WorkspaceScope) -> Path: + path = _clean_preview_path(raw_path) + if not path: + raise WebUIFilePreviewError(400, "missing path") + if len(path) > 4096: + raise WebUIFilePreviewError(400, "path is too long") + + try: + resolved = resolve_allowed_path( + path, + workspace=scope.project_path, + allowed_root=scope.project_path if scope.restrict_to_workspace else None, + strict=True, + ) + except FileNotFoundError as e: + raise WebUIFilePreviewError(404, "file not found") from e + except WorkspaceBoundaryError as e: + raise WebUIFilePreviewError(403, "file is outside the current workspace") from e + except OSError as e: + raise WebUIFilePreviewError(400, "invalid path") from e + + if not resolved.is_file(): + raise WebUIFilePreviewError(404, "file not found") + return resolved + + def _clean_preview_path(raw_path: str | None) -> str: if raw_path is None: return "" diff --git a/nanobot/webui/ws_http.py b/nanobot/webui/ws_http.py index 19c05b86..ed2f0dc8 100644 --- a/nanobot/webui/ws_http.py +++ b/nanobot/webui/ws_http.py @@ -29,7 +29,11 @@ from nanobot.cron.types import CronJob, CronSchedule from nanobot.runtime_context import public_history_messages from nanobot.triggers.local_types import LocalTrigger from nanobot.utils.subagent_channel_display import scrub_subagent_messages_for_channel -from nanobot.webui.file_preview import WebUIFilePreviewError, file_preview_payload +from nanobot.webui.file_preview import ( + WebUIFilePreviewError, + file_preview_availability_payload, + file_preview_payload, +) from nanobot.webui.gateway_tokens import GatewayTokenStore, token_response_payload from nanobot.webui.http_utils import ( case_insensitive_header as _case_insensitive_header, @@ -494,13 +498,18 @@ class GatewayHTTPHandler: return _http_error(400, "invalid session key") if not _is_websocket_channel_session_key(decoded_key): return _http_error(404, "session not found") - path = _query_first(_parse_query(request.path), "path") + query = _parse_query(request.path) + path = _query_first(query, "path") + is_probe = _query_first(query, "probe") == "1" try: - payload = file_preview_payload( - path, - scope=self.workspaces.scope_for_session_key(decoded_key), - ) + scope = self.workspaces.scope_for_session_key(decoded_key) + if is_probe: + payload = file_preview_availability_payload(path, scope=scope) + else: + payload = file_preview_payload(path, scope=scope) except WebUIFilePreviewError as e: + if is_probe and e.status in {400, 403, 404, 415}: + return _http_json_response({"available": False}) return _http_error(e.status, e.message) return _http_json_response(payload) diff --git a/tests/channels/test_websocket_channel.py b/tests/channels/test_websocket_channel.py index 553577ae..42410956 100644 --- a/tests/channels/test_websocket_channel.py +++ b/tests/channels/test_websocket_channel.py @@ -2994,6 +2994,81 @@ def test_handle_file_preview_returns_workspace_file(tmp_path) -> None: assert body["truncated"] is False +def test_handle_file_preview_probe_checks_availability_without_content(tmp_path) -> None: + from urllib.parse import quote + + from websockets.datastructures import Headers + from websockets.http11 import Request + + workspace = tmp_path / "workspace" + source = workspace / "notes" / "ready.md" + source.parent.mkdir(parents=True) + source.write_text("ready\n", encoding="utf-8") + + gateway = _basic_handler(MagicMock(), workspace_path=workspace) + gateway.tokens.api_tokens["tok"] = time.monotonic() + 300.0 + key = "websocket:file-preview" + enc = quote(key, safe="") + path = quote("notes/ready.md", safe="") + req = Request( + f"/api/sessions/{enc}/file-preview?path={path}&probe=1", + Headers([("Authorization", "Bearer tok")]), + ) + + resp = gateway.http._handle_file_preview(req, enc) + + assert resp.status_code == 200 + assert json.loads(resp.body.decode()) == {"available": True} + + +def test_handle_file_preview_probe_reports_missing_file_as_unavailable(tmp_path) -> None: + from urllib.parse import quote + + from websockets.datastructures import Headers + from websockets.http11 import Request + + workspace = tmp_path / "workspace" + workspace.mkdir() + gateway = _basic_handler(MagicMock(), workspace_path=workspace) + gateway.tokens.api_tokens["tok"] = time.monotonic() + 300.0 + key = "websocket:file-preview" + enc = quote(key, safe="") + req = Request( + f"/api/sessions/{enc}/file-preview?path=notes%2Fmissing.md&probe=1", + Headers([("Authorization", "Bearer tok")]), + ) + + resp = gateway.http._handle_file_preview(req, enc) + + assert resp.status_code == 200 + assert json.loads(resp.body.decode()) == {"available": False} + + +def test_handle_file_preview_probe_reports_binary_file_as_unavailable(tmp_path) -> None: + from urllib.parse import quote + + from websockets.datastructures import Headers + from websockets.http11 import Request + + workspace = tmp_path / "workspace" + source = workspace / "image.png" + workspace.mkdir() + source.write_bytes(b"\x89PNG\r\n\0binary") + gateway = _basic_handler(MagicMock(), workspace_path=workspace) + gateway.tokens.api_tokens["tok"] = time.monotonic() + 300.0 + key = "websocket:file-preview" + enc = quote(key, safe="") + req = Request( + f"/api/sessions/{enc}/file-preview?path=image.png&probe=1", + Headers([("Authorization", "Bearer tok")]), + ) + + resp = gateway.http._handle_file_preview(req, enc) + + assert resp.status_code == 200 + assert json.loads(resp.body.decode()) == {"available": False} + + def test_file_preview_normalizes_windows_file_url() -> None: from nanobot.webui.file_preview import _clean_preview_path diff --git a/webui/src/components/FilePreviewAvailabilityContext.tsx b/webui/src/components/FilePreviewAvailabilityContext.tsx new file mode 100644 index 00000000..e098a96d --- /dev/null +++ b/webui/src/components/FilePreviewAvailabilityContext.tsx @@ -0,0 +1,25 @@ +import { createContext, useContext, type ReactNode } from "react"; + +export type FilePreviewAvailabilityResolver = (path: string) => Promise; + +const FilePreviewAvailabilityContext = createContext< + FilePreviewAvailabilityResolver | undefined +>(undefined); + +export function FilePreviewAvailabilityProvider({ + children, + resolve, +}: { + children: ReactNode; + resolve?: FilePreviewAvailabilityResolver; +}) { + return ( + + {children} + + ); +} + +export function useFilePreviewAvailabilityResolver() { + return useContext(FilePreviewAvailabilityContext); +} diff --git a/webui/src/components/MarkdownTextRenderer.tsx b/webui/src/components/MarkdownTextRenderer.tsx index 0a4c9f98..fd3069c0 100644 --- a/webui/src/components/MarkdownTextRenderer.tsx +++ b/webui/src/components/MarkdownTextRenderer.tsx @@ -1,7 +1,9 @@ import { Children, isValidElement, + useEffect, useMemo, + useState, type ReactNode, } from "react"; import type { Components, Options as ReactMarkdownOptions } from "react-markdown"; @@ -14,6 +16,10 @@ import remarkMath from "remark-math"; import { AttachmentTile } from "@/components/AttachmentTile"; import { CodeBlock } from "@/components/CodeBlock"; +import { + useFilePreviewAvailabilityResolver, + type FilePreviewAvailabilityResolver, +} from "@/components/FilePreviewAvailabilityContext"; import { FileReferenceChip, isFilePatternReference, @@ -50,6 +56,50 @@ type InlineLinkPreview = { title: string; }; +type AvailabilityResult = { + available: boolean; + path: string; + resolve: FilePreviewAvailabilityResolver; +}; + +function InferredFileReferenceChip({ + path, + onOpen, +}: { + path: string; + onOpen?: (path: string) => void; +}) { + const resolve = useFilePreviewAvailabilityResolver(); + const [result, setResult] = useState(null); + + useEffect(() => { + if (!resolve || !onOpen) return; + let cancelled = false; + resolve(path) + .then((available) => { + if (!cancelled) setResult({ available, path, resolve }); + }) + .catch(() => { + if (!cancelled) setResult({ available: false, path, resolve }); + }); + return () => { + cancelled = true; + }; + }, [onOpen, path, resolve]); + + const resolvedAvailable = !resolve || ( + result?.resolve === resolve + && result.path === path + && result.available + ); + return ( + + ); +} + const SAFE_INLINE_HTML_TAGS = new Set(["mark", "sub", "sup"]); function extensionOf(value: string): string { @@ -402,7 +452,12 @@ export default function MarkdownTextRenderer({ } const raw = String(kids).replace(/\n$/, ""); if (isLikelyFilePath(raw)) { - return ; + return ( + + ); } /** Plain fenced ``` blocks (no language) & wide one-liners: block monospace, not inline pill. */ const widePlainBlock = raw.includes("\n") || raw.length > 120; diff --git a/webui/src/components/thread/ThreadShell.tsx b/webui/src/components/thread/ThreadShell.tsx index 3dd4fb63..9a52c845 100644 --- a/webui/src/components/thread/ThreadShell.tsx +++ b/webui/src/components/thread/ThreadShell.tsx @@ -2,6 +2,7 @@ import { useCallback, useEffect, useLayoutEffect, useMemo, useRef, useState } fr import type { PointerEvent as ReactPointerEvent } from "react"; import { useTranslation } from "react-i18next"; +import { FilePreviewAvailabilityProvider } from "@/components/FilePreviewAvailabilityContext"; import { FilePreviewPanel } from "@/components/FilePreviewPanel"; import { PromptNavigator } from "@/components/thread/PromptNavigator"; import { SessionInfoPopover } from "@/components/thread/SessionInfoPopover"; @@ -12,6 +13,8 @@ import { ThreadViewport, type ThreadViewportHandle } from "@/components/thread/T import { useNanobotStream, type SendAttachment, type SendOptions } from "@/hooks/useNanobotStream"; import { useSessionHistory } from "@/hooks/useSessions"; import { + ApiError, + fetchFilePreviewAvailability, fetchInstalledCliApps, fetchMcpPresets, fetchSettings, @@ -109,6 +112,12 @@ const FILE_PREVIEW_MAX_WIDTH = 860; const FILE_PREVIEW_MIN_MAIN_WIDTH = 420; const FILE_PREVIEW_CLOSE_ANIMATION_MS = 320; +type FilePreviewAvailabilityCacheEntry = { + available?: boolean; + promise: Promise; + revision: number; +}; + function clampFilePreviewWidth(width: number, maxWidth: number): number { return Math.min(Math.max(width, FILE_PREVIEW_MIN_WIDTH), maxWidth); } @@ -397,6 +406,48 @@ export function ThreadShell({ }, []); const displayMessages = useMemo(() => projectWebuiThreadMessages(messages), [messages]); + const filePreviewAvailabilityCache = useMemo( + () => new Map(), + [historyKey, token], + ); + const filePreviewAvailabilityRevision = displayMessages.length; + const resolveFilePreviewAvailability = useCallback((path: string) => { + if (!historyKey) return Promise.resolve(false); + const cached = filePreviewAvailabilityCache.get(path); + if ( + cached + && (cached.available !== false || cached.revision === filePreviewAvailabilityRevision) + ) { + return cached.promise; + } + const pending = fetchFilePreviewAvailability(token, historyKey, path).catch( + (error: unknown) => { + if (error instanceof ApiError) { + if (error.status === 404 && /API route not found/i.test(error.message)) { + return true; + } + if ([400, 403, 404, 415].includes(error.status)) return false; + } + return false; + }, + ); + const entry: FilePreviewAvailabilityCacheEntry = { + promise: pending, + revision: filePreviewAvailabilityRevision, + }; + filePreviewAvailabilityCache.set(path, entry); + void pending.then((available) => { + if (filePreviewAvailabilityCache.get(path) === entry) { + entry.available = available; + } + }); + return pending; + }, [ + filePreviewAvailabilityCache, + filePreviewAvailabilityRevision, + historyKey, + token, + ]); const showHeroComposer = messages.length === 0 && !loading; const wasShowingHeroComposerRef = useRef(showHeroComposer); @@ -830,27 +881,31 @@ export function ThreadShell({ sessionInfoAction={sessionInfoAction} /> ) : null} - + + + {filePreviewPath && historyKey ? ( { + const query = new URLSearchParams(); + query.set("path", path); + query.set("probe", "1"); + const payload = await request<{ available?: boolean }>( + `${base}/api/sessions/${encodeURIComponent(key)}/file-preview?${query}`, + token, + undefined, + API_READ_TIMEOUT_MS, + ); + return payload.available !== false; +} + export async function fetchSessionAutomations( token: string, key: string, diff --git a/webui/src/tests/api.test.ts b/webui/src/tests/api.test.ts index 34718621..914f92fe 100644 --- a/webui/src/tests/api.test.ts +++ b/webui/src/tests/api.test.ts @@ -5,6 +5,7 @@ import { createModelConfiguration, deleteSession, fetchFilePreview, + fetchFilePreviewAvailability, fetchAutomations, fetchApiService, fetchCliApps, @@ -102,6 +103,32 @@ describe("webui API helpers", () => { ); }); + it("probes file preview availability without requesting contents", async () => { + await expect( + fetchFilePreviewAvailability("tok", "websocket:chat-1", "notes/ready.md"), + ).resolves.toBe(true); + + expect(fetch).toHaveBeenCalledWith( + "/api/sessions/websocket%3Achat-1/file-preview?path=notes%2Fready.md&probe=1", + expect.objectContaining({ + headers: { Authorization: "Bearer tok" }, + credentials: "same-origin", + }), + ); + }); + + it("returns false when a file preview probe is unavailable", async () => { + vi.mocked(fetch).mockResolvedValueOnce({ + ok: true, + status: 200, + json: async () => ({ available: false }), + } as Response); + + await expect( + fetchFilePreviewAvailability("tok", "websocket:chat-1", "notes/missing.md"), + ).resolves.toBe(false); + }); + it("percent-encodes websocket keys when fetching session automations", async () => { await fetchSessionAutomations("tok", "websocket:chat-1"); diff --git a/webui/src/tests/markdown-text-renderer.test.tsx b/webui/src/tests/markdown-text-renderer.test.tsx index 9543a6bd..9f54159e 100644 --- a/webui/src/tests/markdown-text-renderer.test.tsx +++ b/webui/src/tests/markdown-text-renderer.test.tsx @@ -1,6 +1,7 @@ -import { fireEvent, render, screen } from "@testing-library/react"; +import { act, fireEvent, render, screen, waitFor } from "@testing-library/react"; import { describe, expect, it, vi } from "vitest"; +import { FilePreviewAvailabilityProvider } from "@/components/FilePreviewAvailabilityContext"; import MarkdownTextRenderer from "@/components/MarkdownTextRenderer"; describe("MarkdownTextRenderer", () => { @@ -34,6 +35,75 @@ describe("MarkdownTextRenderer", () => { ); }); + it("keeps unavailable inferred inline file paths non-interactive", async () => { + const onOpenFilePreview = vi.fn(); + const resolve = vi.fn().mockResolvedValue(false); + render( + + + {"Future file: `notes/missing.md`"} + + , + ); + + const reference = screen.getByTestId("inline-file-path"); + expect(reference).toHaveTextContent("missing.md"); + await waitFor(() => expect(resolve).toHaveBeenCalledWith("notes/missing.md")); + expect(reference).not.toHaveAttribute("role"); + expect(reference).not.toHaveAttribute("tabindex"); + + fireEvent.click(reference); + + expect(onOpenFilePreview).not.toHaveBeenCalled(); + }); + + it("keeps inferred inline file paths non-interactive when availability lookup fails", async () => { + const onOpenFilePreview = vi.fn(); + let rejectAvailability!: (reason?: unknown) => void; + const resolve = vi.fn(() => new Promise((_resolve, reject) => { + rejectAvailability = reject; + })); + render( + + + {"Unreadable file: `notes/locked.md`"} + + , + ); + + const reference = screen.getByTestId("inline-file-path"); + await waitFor(() => expect(resolve).toHaveBeenCalledWith("notes/locked.md")); + await act(async () => { + rejectAvailability(new Error("probe failed")); + await Promise.resolve(); + }); + + expect(reference).not.toHaveAttribute("role"); + expect(reference).not.toHaveAttribute("tabindex"); + fireEvent.click(reference); + expect(onOpenFilePreview).not.toHaveBeenCalled(); + }); + + it("makes available inferred inline file paths previewable", async () => { + const onOpenFilePreview = vi.fn(); + const resolve = vi.fn().mockResolvedValue(true); + render( + + + {"Existing file: `notes/ready.md`"} + + , + ); + + const reference = screen.getByTestId("inline-file-path"); + await waitFor(() => expect(reference).toHaveAttribute("role", "button")); + expect(reference).toHaveAttribute("tabindex", "0"); + + fireEvent.click(reference); + + expect(onOpenFilePreview).toHaveBeenCalledWith("notes/ready.md"); + }); + it("does not treat non-file hrefs as previews just because the label looks like a file", () => { const onOpenFilePreview = vi.fn(); render( diff --git a/webui/src/tests/thread-shell.test.tsx b/webui/src/tests/thread-shell.test.tsx index 5f6c071e..8d4fb374 100644 --- a/webui/src/tests/thread-shell.test.tsx +++ b/webui/src/tests/thread-shell.test.tsx @@ -231,6 +231,59 @@ describe("ThreadShell", () => { ); }); + it("keeps inferred file paths non-interactive when the availability probe fails", async () => { + const client = makeClient(); + let resolveProbe!: (value: Response) => void; + const probe = new Promise((resolve) => { + resolveProbe = resolve; + }); + const fetchMock = vi.fn((input: RequestInfo | URL) => { + const url = String(input); + if (url.includes("websocket%3Apreview-error/webui-thread")) { + return Promise.resolve(httpJson(transcriptFromSimpleMessages([ + { role: "assistant", content: "Unreadable file: `prompts/dream.md`" }, + ]))); + } + if (url.includes("websocket%3Apreview-error/file-preview?")) return probe; + return Promise.resolve({ + ok: false, + status: 404, + json: async () => ({}), + }); + }); + vi.stubGlobal("fetch", fetchMock); + + render(wrap( + client, + {}} + />, + )); + + const reference = await screen.findByTestId("inline-file-path"); + await waitFor(() => expect(fetchMock).toHaveBeenCalledWith( + expect.stringContaining("file-preview?path=prompts%2Fdream.md&probe=1"), + expect.anything(), + )); + await act(async () => { + resolveProbe({ + ok: false, + status: 500, + text: async () => "failed to read file", + json: async () => ({}), + } as Response); + await probe; + await Promise.resolve(); + }); + + expect(reference).not.toHaveAttribute("role"); + expect(reference).not.toHaveAttribute("tabindex"); + fireEvent.click(reference); + expect(screen.queryByText("failed to read file")).not.toBeInTheDocument(); + }); + it("does not navigate away when clicking the chat title", async () => { const client = makeClient(); const onGoHome = vi.fn();