fix(canvas): address PR review + CodeQL findings
- Replace ReDoS-vulnerable regex in canvas-log diff parser with lastIndexOf-based parsing. CodeQL js/redos flagged the polynomial backtracking on inputs like "diff --git a/a b/a b/a b/a...". - Flatten components/canvas/ into components/ per CLAUDE.md C-04 (no nested component directories). Renamed for clarity: MarkdownCanvas → CanvasMarkdown, DiffCanvas → CanvasDiff, TableCanvas → CanvasTable, StatsCanvas → CanvasStats. - Add shrink-0 + explicit width to the collapsed rail wrapper so the toggle can't be compressed to zero width by a flex sibling. - Replace the markdown renderer's <pre> stub with a minimal safe parser. Handles ATX headings, **bold**, *italic*, `code`, fenced code blocks, unordered lists, and paragraphs. No HTML pass-through; user content can't inject markup. Tables / links / images stay out of scope (use the table or diff canvas types instead). - Treat bare-empty hunk lines as empty context rather than dropping so non-standard diff generators don't misalign surrounding lines. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
This commit is contained in:
parent
ca0b7ec2ce
commit
6c3e2daac8
|
|
@ -68,3 +68,4 @@ agent-orchestrator.yaml
|
|||
# OS-specific files
|
||||
.DS_Store
|
||||
Thumbs.db
|
||||
.gstack/
|
||||
|
|
|
|||
|
|
@ -163,6 +163,7 @@ describe("parseUnifiedDiff", () => {
|
|||
{ kind: "context", text: "context" },
|
||||
{ kind: "del", text: "old" },
|
||||
{ kind: "add", text: "new" },
|
||||
{ kind: "context", text: "" },
|
||||
]);
|
||||
});
|
||||
|
||||
|
|
@ -184,6 +185,7 @@ describe("parseUnifiedDiff", () => {
|
|||
{ kind: "context", text: "heading" },
|
||||
{ kind: "del", text: "--" },
|
||||
{ kind: "add", text: "++added text" },
|
||||
{ kind: "context", text: "" },
|
||||
]);
|
||||
});
|
||||
|
||||
|
|
|
|||
|
|
@ -238,9 +238,22 @@ export function parseUnifiedDiff(diff: string): { files: CanvasDiffFile[]; trunc
|
|||
truncated = true;
|
||||
break;
|
||||
}
|
||||
const match = /^diff --git a\/(.+) b\/(.+)$/.exec(line);
|
||||
const path = match?.[2] ?? "";
|
||||
const oldPath = match?.[1];
|
||||
// Parse "diff --git a/<oldPath> b/<newPath>" without a regex. The naive
|
||||
// regex `/^diff --git a\/(.+) b\/(.+)$/` is polynomial-backtracking on
|
||||
// pathological inputs like "a/a b/a b/a b/a..." (CodeQL js/redos).
|
||||
// lastIndexOf is O(n) and works for the common case where paths don't
|
||||
// contain " b/"; for paths that do, git quotes them anyway.
|
||||
const prefix = "diff --git a/";
|
||||
let path = "";
|
||||
let oldPath: string | undefined;
|
||||
if (line.startsWith(prefix)) {
|
||||
const rest = line.slice(prefix.length);
|
||||
const sep = rest.lastIndexOf(" b/");
|
||||
if (sep >= 0) {
|
||||
oldPath = rest.slice(0, sep);
|
||||
path = rest.slice(sep + 3);
|
||||
}
|
||||
}
|
||||
current = {
|
||||
path,
|
||||
oldPath: oldPath !== path ? oldPath : undefined,
|
||||
|
|
@ -259,7 +272,19 @@ export function parseUnifiedDiff(diff: string): { files: CanvasDiffFile[]; trunc
|
|||
if (currentHunk) current.hunks.push(currentHunk);
|
||||
currentHunk = { header: line, lines: [] };
|
||||
} else if (currentHunk) {
|
||||
if (line === "") continue;
|
||||
// Bare-empty lines inside a hunk represent a blank context line that some
|
||||
// diff generators emit without the leading space prefix. Treat them as
|
||||
// empty context rather than dropping (which would misalign surrounding
|
||||
// lines in the rendered hunk).
|
||||
if (line === "") {
|
||||
currentHunk.lines.push({ kind: "context", text: "" });
|
||||
totalLines++;
|
||||
if (totalLines >= DIFF_MAX_LINES) {
|
||||
truncated = true;
|
||||
break;
|
||||
}
|
||||
continue;
|
||||
}
|
||||
const kind: "add" | "del" | "context" = line.startsWith("+")
|
||||
? "add"
|
||||
: line.startsWith("-")
|
||||
|
|
|
|||
|
|
@ -5,7 +5,7 @@ import type { CanvasArtifact, CanvasDiffFile } from "@aoagents/ao-core";
|
|||
|
||||
type Props = { canvas: Extract<CanvasArtifact, { type: "diff" }> };
|
||||
|
||||
export function DiffCanvas({ canvas }: Props) {
|
||||
export function CanvasDiff({ canvas }: Props) {
|
||||
return (
|
||||
<div className="flex flex-col gap-3">
|
||||
{canvas.payload.files.map((file) => (
|
||||
|
|
@ -0,0 +1,132 @@
|
|||
"use client";
|
||||
|
||||
import { Fragment, type ReactNode } from "react";
|
||||
import type { CanvasArtifact } from "@aoagents/ao-core";
|
||||
|
||||
type Props = { canvas: Extract<CanvasArtifact, { type: "markdown" }> };
|
||||
|
||||
export function CanvasMarkdown({ canvas }: Props) {
|
||||
return (
|
||||
<div className="text-sm leading-relaxed text-[var(--color-text-primary)]">
|
||||
{renderMarkdown(canvas.payload.markdown)}
|
||||
</div>
|
||||
);
|
||||
}
|
||||
|
||||
// Minimal markdown subset: ATX headings (# .. ######), fenced code blocks
|
||||
// (``` … ```), unordered list groups (- / *), paragraphs, and inline formatting
|
||||
// (**bold**, *italic*, `code`). Plain-text only — no HTML pass-through, so user
|
||||
// content can't inject markup. Heavier markdown features (tables, links, images,
|
||||
// nested lists) are out of scope; agents that need them can ship a `table` or
|
||||
// `diff` canvas type instead.
|
||||
function renderMarkdown(input: string): ReactNode[] {
|
||||
const lines = input.split("\n");
|
||||
const blocks: ReactNode[] = [];
|
||||
let i = 0;
|
||||
let key = 0;
|
||||
|
||||
while (i < lines.length) {
|
||||
const line = lines[i] ?? "";
|
||||
|
||||
if (line.startsWith("```")) {
|
||||
const buf: string[] = [];
|
||||
i++;
|
||||
while (i < lines.length && !(lines[i] ?? "").startsWith("```")) {
|
||||
buf.push(lines[i] ?? "");
|
||||
i++;
|
||||
}
|
||||
if (i < lines.length) i++;
|
||||
blocks.push(
|
||||
<pre
|
||||
key={key++}
|
||||
className="my-2 overflow-x-auto rounded bg-[var(--color-bg-elevated)] p-2 font-mono text-xs"
|
||||
>
|
||||
{buf.join("\n")}
|
||||
</pre>,
|
||||
);
|
||||
continue;
|
||||
}
|
||||
|
||||
const heading = /^(#{1,6})\s+(.*)$/.exec(line);
|
||||
if (heading) {
|
||||
const level = (heading[1] ?? "#").length;
|
||||
const text = heading[2] ?? "";
|
||||
const sizes = ["text-lg", "text-base", "text-sm", "text-sm", "text-sm", "text-sm"];
|
||||
blocks.push(
|
||||
<p key={key++} className={`mb-1 mt-2 font-semibold ${sizes[level - 1]}`}>
|
||||
{renderInline(text)}
|
||||
</p>,
|
||||
);
|
||||
i++;
|
||||
continue;
|
||||
}
|
||||
|
||||
if (/^[-*]\s+/.test(line)) {
|
||||
const items: string[] = [];
|
||||
while (i < lines.length && /^[-*]\s+/.test(lines[i] ?? "")) {
|
||||
items.push((lines[i] ?? "").replace(/^[-*]\s+/, ""));
|
||||
i++;
|
||||
}
|
||||
blocks.push(
|
||||
<ul key={key++} className="my-2 list-disc pl-5">
|
||||
{items.map((it, idx) => (
|
||||
<li key={idx}>{renderInline(it)}</li>
|
||||
))}
|
||||
</ul>,
|
||||
);
|
||||
continue;
|
||||
}
|
||||
|
||||
if (line.trim() === "") {
|
||||
i++;
|
||||
continue;
|
||||
}
|
||||
|
||||
const buf: string[] = [];
|
||||
while (
|
||||
i < lines.length &&
|
||||
(lines[i] ?? "").trim() !== "" &&
|
||||
!(lines[i] ?? "").startsWith("```") &&
|
||||
!/^(#{1,6})\s+/.test(lines[i] ?? "") &&
|
||||
!/^[-*]\s+/.test(lines[i] ?? "")
|
||||
) {
|
||||
buf.push(lines[i] ?? "");
|
||||
i++;
|
||||
}
|
||||
blocks.push(
|
||||
<p key={key++} className="my-2">
|
||||
{renderInline(buf.join(" "))}
|
||||
</p>,
|
||||
);
|
||||
}
|
||||
|
||||
return blocks;
|
||||
}
|
||||
|
||||
function renderInline(text: string): ReactNode[] {
|
||||
// Tokens: **bold**, *italic*, `code`. Plain text fills the gaps. No HTML
|
||||
// escaping needed — React renders strings as text nodes.
|
||||
const out: ReactNode[] = [];
|
||||
const re = /(\*\*[^*]+\*\*|\*[^*]+\*|`[^`]+`)/g;
|
||||
let last = 0;
|
||||
let key = 0;
|
||||
for (const match of text.matchAll(re)) {
|
||||
const idx = match.index ?? 0;
|
||||
if (idx > last) out.push(<Fragment key={key++}>{text.slice(last, idx)}</Fragment>);
|
||||
const tok = match[0];
|
||||
if (tok.startsWith("**")) {
|
||||
out.push(<strong key={key++}>{tok.slice(2, -2)}</strong>);
|
||||
} else if (tok.startsWith("`")) {
|
||||
out.push(
|
||||
<code key={key++} className="rounded bg-[var(--color-bg-elevated)] px-1 font-mono text-xs">
|
||||
{tok.slice(1, -1)}
|
||||
</code>,
|
||||
);
|
||||
} else {
|
||||
out.push(<em key={key++}>{tok.slice(1, -1)}</em>);
|
||||
}
|
||||
last = idx + tok.length;
|
||||
}
|
||||
if (last < text.length) out.push(<Fragment key={key}>{text.slice(last)}</Fragment>);
|
||||
return out;
|
||||
}
|
||||
|
|
@ -3,10 +3,10 @@
|
|||
import { useEffect, useState } from "react";
|
||||
import type { CanvasArtifact } from "@aoagents/ao-core";
|
||||
import { useSessionCanvases } from "@/hooks/useSessionCanvases";
|
||||
import { MarkdownCanvas } from "./renderers/MarkdownCanvas";
|
||||
import { DiffCanvas } from "./renderers/DiffCanvas";
|
||||
import { TableCanvas } from "./renderers/TableCanvas";
|
||||
import { StatsCanvas } from "./renderers/StatsCanvas";
|
||||
import { CanvasMarkdown } from "./CanvasMarkdown";
|
||||
import { CanvasDiff } from "./CanvasDiff";
|
||||
import { CanvasTable } from "./CanvasTable";
|
||||
import { CanvasStats } from "./CanvasStats";
|
||||
|
||||
type Props = { sessionId: string };
|
||||
|
||||
|
|
@ -30,7 +30,7 @@ export function CanvasRail({ sessionId }: Props) {
|
|||
|
||||
if (!isOpen) {
|
||||
return (
|
||||
<div className="flex flex-col border-l border-[var(--color-border-subtle)] bg-[var(--color-bg-surface)]">
|
||||
<div className="flex w-6 shrink-0 flex-col border-l border-[var(--color-border-subtle)] bg-[var(--color-bg-surface)]">
|
||||
<button
|
||||
type="button"
|
||||
onClick={() => setOpen(true)}
|
||||
|
|
@ -96,12 +96,12 @@ function CanvasPanel({ canvas }: { canvas: CanvasArtifact }) {
|
|||
function CanvasBody({ canvas }: { canvas: CanvasArtifact }) {
|
||||
switch (canvas.type) {
|
||||
case "markdown":
|
||||
return <MarkdownCanvas canvas={canvas} />;
|
||||
return <CanvasMarkdown canvas={canvas} />;
|
||||
case "diff":
|
||||
return <DiffCanvas canvas={canvas} />;
|
||||
return <CanvasDiff canvas={canvas} />;
|
||||
case "table":
|
||||
return <TableCanvas canvas={canvas} />;
|
||||
return <CanvasTable canvas={canvas} />;
|
||||
case "stats":
|
||||
return <StatsCanvas canvas={canvas} />;
|
||||
return <CanvasStats canvas={canvas} />;
|
||||
}
|
||||
}
|
||||
|
|
@ -11,7 +11,7 @@ const toneClass: Record<NonNullable<Extract<CanvasArtifact, { type: "stats" }>["
|
|||
neutral: "text-[var(--color-text-primary)]",
|
||||
};
|
||||
|
||||
export function StatsCanvas({ canvas }: Props) {
|
||||
export function CanvasStats({ canvas }: Props) {
|
||||
return (
|
||||
<div className="grid grid-cols-2 gap-2">
|
||||
{canvas.payload.metrics.map((m, i) => (
|
||||
|
|
@ -4,7 +4,7 @@ import type { CanvasArtifact } from "@aoagents/ao-core";
|
|||
|
||||
type Props = { canvas: Extract<CanvasArtifact, { type: "table" }> };
|
||||
|
||||
export function TableCanvas({ canvas }: Props) {
|
||||
export function CanvasTable({ canvas }: Props) {
|
||||
const { columns, rows } = canvas.payload;
|
||||
return (
|
||||
<div className="overflow-auto">
|
||||
|
|
@ -22,7 +22,7 @@ import {
|
|||
} from "./SessionDetailHeader";
|
||||
import { SessionEndedSummary } from "./SessionEndedSummary";
|
||||
import { sessionActivityMeta } from "./session-detail-utils";
|
||||
import { CanvasRail } from "./canvas/CanvasRail";
|
||||
import { CanvasRail } from "./CanvasRail";
|
||||
|
||||
export type { OrchestratorZones } from "./SessionDetailHeader";
|
||||
|
||||
|
|
|
|||
|
|
@ -1,13 +0,0 @@
|
|||
"use client";
|
||||
|
||||
import type { CanvasArtifact } from "@aoagents/ao-core";
|
||||
|
||||
type Props = { canvas: Extract<CanvasArtifact, { type: "markdown" }> };
|
||||
|
||||
export function MarkdownCanvas({ canvas }: Props) {
|
||||
return (
|
||||
<pre className="whitespace-pre-wrap break-words font-sans text-sm leading-relaxed text-[var(--color-text-primary)]">
|
||||
{canvas.payload.markdown}
|
||||
</pre>
|
||||
);
|
||||
}
|
||||
Loading…
Reference in New Issue