diff --git a/react-compiler.config.js b/react-compiler.config.js index 4b03d8437f..7988f40891 100644 --- a/react-compiler.config.js +++ b/react-compiler.config.js @@ -79,6 +79,7 @@ export const REACT_COMPILER_ENABLED_DIRS = [ "src/components/shared/SecretsManagement/components/SecretsBackendUnavailable.tsx", "src/components/shared/HighlightText.tsx", "src/components/shared/AnnouncementBanners.tsx", + "src/components/shared/Markdown", "src/components/shared/ReactFlow/FlowCanvas/TaskNode/TaskOverview/IOSection", "src/components/ui/typography.tsx", diff --git a/src/components/shared/Markdown/Markdown.test.tsx b/src/components/shared/Markdown/Markdown.test.tsx new file mode 100644 index 0000000000..d1ec58a7b5 --- /dev/null +++ b/src/components/shared/Markdown/Markdown.test.tsx @@ -0,0 +1,191 @@ +import { cleanup, render, screen } from "@testing-library/react"; +import { afterEach, describe, expect, it } from "vitest"; + +import { Markdown, UntrustedMarkdown } from "./Markdown"; + +function fontClasses(element: HTMLElement) { + return element.className + .split(" ") + .filter((name) => name.startsWith("text-")); +} + +describe("", () => { + afterEach(cleanup); + + it("gives headings a visible hierarchy", () => { + render(); + + const [h1, h2, h3, h4] = ["One", "Two", "Three", "Four"].map((text) => + screen.getByText(text), + ); + + expect(h1.tagName).toBe("H1"); + expect(h4.tagName).toBe("H4"); + expect(fontClasses(h1)).toContain("text-lg"); + expect(fontClasses(h2)).toContain("text-base"); + expect(fontClasses(h3)).toContain("text-sm"); + expect(h1.className).toContain("font-bold"); + expect(h2.className).toContain("font-semibold"); + }); + + it("renders a fenced block as a block, not a run of inline pills", () => { + const { container } = render( + , + ); + + const pre = container.querySelector("pre"); + expect(pre).not.toBeNull(); + expect(pre?.className).toContain("overflow-x-auto"); + expect(pre?.querySelector("code")).not.toBeNull(); + expect(pre?.textContent).toContain("const b = 2;"); + expect(pre?.className).toContain("[&>code]:block"); + }); + + it("renders an unlabelled fence the same way as a labelled one", () => { + const { container } = render(); + + expect(container.querySelector("pre")?.className).toContain( + "[&>code]:block", + ); + expect(container.querySelector("pre code")?.textContent).toContain( + "plain text", + ); + }); + + it("renders GFM tables with delineated rows", () => { + const { container } = render( + , + ); + + expect(container.querySelector("table")).not.toBeNull(); + expect(container.querySelector("thead")?.className).toContain( + "bg-muted/50", + ); + expect(container.querySelector("tr")?.className).toContain("border-b"); + expect(container.querySelector("td")?.className).toContain("px-2"); + }); + + it("drops the bullet from a task list item", () => { + const { container } = render( + , + ); + + expect(container.querySelector("li")?.className).toContain("list-none"); + expect(container.querySelectorAll('input[type="checkbox"]')).toHaveLength( + 2, + ); + }); + + it("does not render raw HTML", () => { + const { container } = render( + window.__mdXss = true;bold"} />, + ); + + expect(container.querySelector("script")).toBeNull(); + expect(screen.queryByText("bold")).not.toBeInTheDocument(); + expect( + (window as unknown as Record).__mdXss, + ).toBeUndefined(); + }); + + it("renders an image and keeps a relative link", () => { + const { container } = render( + , + ); + + expect(container.querySelector("img")).toHaveAttribute("src", "/local.png"); + expect(screen.getByRole("link", { name: "runs" })).toHaveAttribute( + "href", + "/runs", + ); + }); + + it("keeps an internal link in the same tab and sends an external one out", () => { + render( + , + ); + + const internal = screen.getByRole("link", { name: "runs" }); + expect(internal).not.toHaveAttribute("target"); + expect(internal).not.toHaveAttribute("rel"); + expect(internal.querySelector("svg")).toBeNull(); + + const external = screen.getByRole("link", { name: "docs" }); + expect(external).toHaveAttribute("target", "_blank"); + expect(external).toHaveAttribute("rel", "noopener noreferrer"); + expect(external.querySelector("svg")).not.toBeNull(); + }); + + it("renders links through the design system, inline with the text", () => { + const { container } = render( + , + ); + + const link = screen.getByRole("link", { name: "notes" }); + expect(link.className).toContain("inline-flex"); + expect(container.querySelector("p a")).toBe(link); + expect(container.querySelector("a div")).toBeNull(); + }); + + it("lets a caller override a base element", () => { + render( +
{children}
, + }} + />, + ); + + expect(screen.getByTestId("custom")).toHaveTextContent("text"); + }); +}); + +describe("", () => { + afterEach(cleanup); + + it("shows an image's alt text without requesting the image", () => { + const { container } = render( + , + ); + + expect(container.querySelector("img")).toBeNull(); + expect(screen.getByText("Diagram of the outage")).toBeInTheDocument(); + }); + + it("renders an absolute http link as an external anchor", () => { + render(); + + const link = screen.getByRole("link", { name: "docs" }); + expect(link).toHaveAttribute("href", "https://example.com/docs"); + expect(link).toHaveAttribute("rel", "noopener noreferrer"); + }); + + it("renders a rejected link as plain text rather than a dead anchor", () => { + const { container } = render( + , + ); + + expect(screen.queryByRole("link")).not.toBeInTheDocument(); + expect(container.querySelector("a")).toBeNull(); + expect(container.textContent).toBe("script and relative"); + }); + + it("cannot have its image guard overridden by a caller", () => { + const { container } = render( + , + }} + />, + ); + + expect(container.querySelector("img")).toBeNull(); + expect(screen.getByText("Diagram")).toBeInTheDocument(); + }); +}); diff --git a/src/components/shared/Markdown/Markdown.tsx b/src/components/shared/Markdown/Markdown.tsx new file mode 100644 index 0000000000..156b2e945c --- /dev/null +++ b/src/components/shared/Markdown/Markdown.tsx @@ -0,0 +1,185 @@ +import type { ComponentProps, ReactNode } from "react"; +import ReactMarkdown, { + type Components, + defaultUrlTransform, +} from "react-markdown"; +import remarkGfm from "remark-gfm"; + +import { Link } from "@/components/ui/link"; +import { Separator } from "@/components/ui/separator"; +import { Heading, Paragraph, Text } from "@/components/ui/typography"; +import { cn } from "@/lib/utils"; +import { toAbsoluteHttpUrl } from "@/utils/URL"; + +interface ElementProps { + children?: ReactNode; + className?: string; +} + +export const INLINE_CODE_CLASS = + "rounded bg-muted px-1 py-0.5 text-xs font-mono"; + +type HeadingLevel = 1 | 2 | 3 | 4 | 5 | 6; + +const HEADING_STYLES: Record< + HeadingLevel, + Omit, "level" | "children"> +> = { + 1: { size: "lg", weight: "bold", className: "mt-2 mb-1 block" }, + 2: { size: "md", weight: "semibold", className: "mt-2 mb-1 block" }, + 3: { size: "sm", weight: "semibold", className: "mt-2 mb-1 block" }, + 4: { + size: "sm", + weight: "semibold", + tone: "subdued", + className: "mt-1 block", + }, + 5: { + size: "xs", + weight: "semibold", + tone: "subdued", + className: "mt-1 block", + }, + 6: { + size: "xs", + weight: "semibold", + tone: "subdued", + className: "mt-1 block", + }, +}; + +function renderHeading(level: HeadingLevel) { + const MarkdownHeading = ({ children }: ElementProps) => ( + + {children} + + ); + + return MarkdownHeading; +} + +const baseComponents = { + p: ({ children }: ElementProps) => ( + + {children} + + ), + a: ({ href, children }: ElementProps & { href?: string }) => { + if (!href) return <>{children}; + + return ( + + {children} + + ); + }, + h1: renderHeading(1), + h2: renderHeading(2), + h3: renderHeading(3), + h4: renderHeading(4), + h5: renderHeading(5), + h6: renderHeading(6), + ul: ({ children, className }: ElementProps) => ( +
    + {children} +
+ ), + ol: ({ children }: ElementProps) => ( +
    + {children} +
+ ), + li: ({ children, className }: ElementProps) => ( +
  • input]:mr-1.5 [&>input]:align-middle", + className?.includes("task-list-item") && "list-none", + )} + > + {children} +
  • + ), + blockquote: ({ children }: ElementProps) => ( +
    + {children} +
    + ), + code: ({ children }: ElementProps) => ( + {children} + ), + pre: ({ children }: ElementProps) => ( +
    +      {children}
    +    
    + ), + table: ({ children }: ElementProps) => ( +
    + {children}
    +
    + ), + thead: ({ children }: ElementProps) => ( + {children} + ), + tr: ({ children }: ElementProps) => ( + {children} + ), + th: ({ children }: ElementProps) => ( + {children} + ), + td: ({ children }: ElementProps) => ( + {children} + ), + img: ({ alt, src }: { alt?: string; src?: string }) => ( + {alt} + ), + hr: () => , +} satisfies Components; +const AltTextOnlyImage = ({ alt }: { alt?: string }) => + alt ? ( + + {alt} + + ) : null; + +interface MarkdownProps { + body: string; + components?: Components; + urlTransform?: (url: string) => string; +} + +export const Markdown = ({ + body, + components, + urlTransform = defaultUrlTransform, +}: MarkdownProps) => ( + + {body} + +); + +export const UntrustedMarkdown = ({ + body, + components, +}: Omit) => ( + toAbsoluteHttpUrl(url) ?? ""} + /> +); diff --git a/src/components/ui/link.test.tsx b/src/components/ui/link.test.tsx new file mode 100644 index 0000000000..a535b8bdb5 --- /dev/null +++ b/src/components/ui/link.test.tsx @@ -0,0 +1,30 @@ +import { cleanup, render } from "@testing-library/react"; +import { afterEach, describe, expect, it, vi } from "vitest"; + +import { Link } from "./link"; + +afterEach(() => { + cleanup(); + vi.restoreAllMocks(); +}); + +describe("", () => { + it("sits inside running text without breaking the markup", () => { + const consoleError = vi + .spyOn(console, "error") + .mockImplementation(() => {}); + + const { container } = render( +

    + Read the{" "} + + notes + + . +

    , + ); + + expect(consoleError).not.toHaveBeenCalled(); + expect(container.querySelector("a div")).toBeNull(); + }); +}); diff --git a/src/components/ui/link.tsx b/src/components/ui/link.tsx index 69fdce8a70..4f8c0c25a5 100644 --- a/src/components/ui/link.tsx +++ b/src/components/ui/link.tsx @@ -53,7 +53,7 @@ function Link({ {...props} className={cn(linkVariants({ variant, size }), className)} > - + {children} {external && ( diff --git a/src/routes/v2/shared/components/AiChat/components/renderMarkdown.tsx b/src/routes/v2/shared/components/AiChat/components/renderMarkdown.tsx index 9a2e983253..377392924f 100644 --- a/src/routes/v2/shared/components/AiChat/components/renderMarkdown.tsx +++ b/src/routes/v2/shared/components/AiChat/components/renderMarkdown.tsx @@ -5,12 +5,13 @@ import { type ReactNode, useContext, } from "react"; -import Markdown, { defaultUrlTransform } from "react-markdown"; -import remarkGfm from "remark-gfm"; +import { type Components, defaultUrlTransform } from "react-markdown"; +import { + INLINE_CODE_CLASS, + Markdown, +} from "@/components/shared/Markdown/Markdown"; import { Link } from "@/components/ui/link"; -import { Separator } from "@/components/ui/separator"; -import { Heading, Paragraph } from "@/components/ui/typography"; import { getComponentQueryKey } from "@/hooks/useHydrateComponentReference"; import type { ComponentRefData } from "@/routes/v2/shared/components/AiChat/types"; import { CodeBlock } from "@/routes/v2/shared/components/CodeBlock"; @@ -22,8 +23,6 @@ import { EntityChip } from "./EntityChip"; const ENTITY_PROTOCOL = "entity://"; const COMPONENT_PROTOCOL = "component://"; -const INLINE_CODE_CLASS = "rounded bg-muted px-1 py-0.5 text-xs font-mono"; - const ComponentRefsContext = createContext< Record | undefined >(undefined); @@ -135,74 +134,13 @@ function MarkdownCode({ ); } -const markdownComponents = { - h1: ({ children }: { children?: ReactNode }) => ( - - {children} - - ), - h2: ({ children }: { children?: ReactNode }) => ( - - {children} - - ), - h3: ({ children }: { children?: ReactNode }) => ( - - {children} - - ), - h4: ({ children }: { children?: ReactNode }) => ( - - {children} - - ), - p: ({ children }: { children?: ReactNode }) => ( - - {children} - - ), - ul: ({ children }: { children?: ReactNode }) => ( -
      {children}
    - ), - ol: ({ children }: { children?: ReactNode }) => ( -
      {children}
    - ), - li: ({ children }: { children?: ReactNode }) => ( -
  • {children}
  • - ), - blockquote: ({ children }: { children?: ReactNode }) => ( -
    - {children} -
    - ), - table: ({ children }: { children?: ReactNode }) => ( -
    - {children}
    -
    - ), - thead: ({ children }: { children?: ReactNode }) => ( - {children} - ), - tbody: ({ children }: { children?: ReactNode }) => {children}, - tr: ({ children }: { children?: ReactNode }) => ( - {children} - ), - th: ({ children }: { children?: ReactNode }) => ( - {children} - ), - td: ({ children }: { children?: ReactNode }) => ( - {children} - ), - hr: () => , +const chatComponents = { a: MarkdownLink, code: MarkdownCode, + // `MarkdownCode` already renders a fenced block as a `CodeBlock`, which brings + // its own surround. pre: ({ children }: { children?: ReactNode }) => <>{children}, -} as const; +} satisfies Components; export function renderMarkdown( text: string, @@ -210,12 +148,10 @@ export function renderMarkdown( ): ReactNode { const markdown = ( - {text} - + /> ); if (!componentReferences) return markdown; diff --git a/src/utils/URL.test.ts b/src/utils/URL.test.ts index b62bfbf63f..faf1aa67d1 100644 --- a/src/utils/URL.test.ts +++ b/src/utils/URL.test.ts @@ -9,12 +9,60 @@ import { getIdOrTitleFromPath, normalizeUrl, parseHttpUrl, + toAbsoluteHttpUrl, } from "./URL"; vi.mock("@/routes/router", () => ({ RUNS_BASE_PATH: "/runs", })); +// Kept ahead of the download tests, which delete `global.URL` in their teardown. +describe("toAbsoluteHttpUrl", () => { + it("accepts absolute http and https urls", () => { + expect(toAbsoluteHttpUrl("https://example.com/docs")).toBe( + "https://example.com/docs", + ); + expect(toAbsoluteHttpUrl("http://example.com")).toBe("http://example.com/"); + }); + + it("trims surrounding whitespace before parsing", () => { + expect(toAbsoluteHttpUrl(" https://example.com/docs ")).toBe( + "https://example.com/docs", + ); + }); + + it("rejects script-bearing and non-web protocols", () => { + expect(toAbsoluteHttpUrl("javascript:alert(1)")).toBeNull(); + expect(toAbsoluteHttpUrl("JavaScript:alert(1)")).toBeNull(); + expect( + toAbsoluteHttpUrl("data:text/html,"), + ).toBeNull(); + expect(toAbsoluteHttpUrl("vbscript:msgbox(1)")).toBeNull(); + expect(toAbsoluteHttpUrl("file:///etc/passwd")).toBeNull(); + }); + + it("rejects anything that is not already absolute", () => { + expect(toAbsoluteHttpUrl("/runs")).toBeNull(); + expect(toAbsoluteHttpUrl("runs/123")).toBeNull(); + expect(toAbsoluteHttpUrl("//evil.example.com")).toBeNull(); + expect(toAbsoluteHttpUrl("#anchor")).toBeNull(); + }); + + it("rejects empty and whitespace-only input", () => { + expect(toAbsoluteHttpUrl("")).toBeNull(); + expect(toAbsoluteHttpUrl(" ")).toBeNull(); + }); + + it("rejects non-string input, since callers pass unvalidated host data", () => { + expect(toAbsoluteHttpUrl(undefined)).toBeNull(); + expect(toAbsoluteHttpUrl(null)).toBeNull(); + expect(toAbsoluteHttpUrl(42)).toBeNull(); + expect(toAbsoluteHttpUrl({ url: "https://example.com" })).toBeNull(); + expect(toAbsoluteHttpUrl(["https://example.com"])).toBeNull(); + }); +}); + +// normalizeUrl tests describe("normalizeUrl", () => { it("returns empty string for empty input", () => { expect(normalizeUrl("")).toBe(""); diff --git a/src/utils/URL.ts b/src/utils/URL.ts index cb27548a08..8602ab1be4 100644 --- a/src/utils/URL.ts +++ b/src/utils/URL.ts @@ -195,6 +195,17 @@ const parseHttpUrl = (value?: string | null): string | undefined => { } }; +const toAbsoluteHttpUrl = (value: unknown): string | null => { + if (typeof value !== "string" || !value.trim()) return null; + try { + const url = new URL(value.trim()); + if (url.protocol !== "http:" && url.protocol !== "https:") return null; + return url.toString(); + } catch { + return null; + } +}; + const normalizeUrl = (url: string) => { if (url.trim() === "") { return ""; @@ -240,4 +251,5 @@ export { isGithubUrl, normalizeUrl, parseHttpUrl, + toAbsoluteHttpUrl, };