From 6c3e2daac8917a4abd0d055b29a14dcbb2a4fc4f Mon Sep 17 00:00:00 2001 From: Ashish Huddar Date: Tue, 5 May 2026 17:39:53 +0530 Subject: [PATCH] fix(canvas): address PR review + CodeQL findings MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit - 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 
 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) 
---
 .gitignore                                    |   1 +
 .../core/src/__tests__/canvas-log.test.ts     |   2 +
 packages/core/src/canvas-log.ts               |  33 ++++-
 .../DiffCanvas.tsx => CanvasDiff.tsx}         |   2 +-
 .../web/src/components/CanvasMarkdown.tsx     | 132 ++++++++++++++++++
 .../components/{canvas => }/CanvasRail.tsx    |  18 +--
 .../StatsCanvas.tsx => CanvasStats.tsx}       |   2 +-
 .../TableCanvas.tsx => CanvasTable.tsx}       |   2 +-
 packages/web/src/components/SessionDetail.tsx |   2 +-
 .../__tests__/CanvasRail.test.tsx             |   0
 .../canvas/renderers/MarkdownCanvas.tsx       |  13 --
 11 files changed, 177 insertions(+), 30 deletions(-)
 rename packages/web/src/components/{canvas/renderers/DiffCanvas.tsx => CanvasDiff.tsx} (98%)
 create mode 100644 packages/web/src/components/CanvasMarkdown.tsx
 rename packages/web/src/components/{canvas => }/CanvasRail.tsx (87%)
 rename packages/web/src/components/{canvas/renderers/StatsCanvas.tsx => CanvasStats.tsx} (96%)
 rename packages/web/src/components/{canvas/renderers/TableCanvas.tsx => CanvasTable.tsx} (97%)
 rename packages/web/src/components/{canvas => }/__tests__/CanvasRail.test.tsx (100%)
 delete mode 100644 packages/web/src/components/canvas/renderers/MarkdownCanvas.tsx

diff --git a/.gitignore b/.gitignore
index 6b6797b57..369cd8e00 100644
--- a/.gitignore
+++ b/.gitignore
@@ -68,3 +68,4 @@ agent-orchestrator.yaml
 # OS-specific files
 .DS_Store
 Thumbs.db
+.gstack/
diff --git a/packages/core/src/__tests__/canvas-log.test.ts b/packages/core/src/__tests__/canvas-log.test.ts
index 8e6048575..96797de58 100644
--- a/packages/core/src/__tests__/canvas-log.test.ts
+++ b/packages/core/src/__tests__/canvas-log.test.ts
@@ -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: "" },
     ]);
   });
 
diff --git a/packages/core/src/canvas-log.ts b/packages/core/src/canvas-log.ts
index df1db000c..d671552b0 100644
--- a/packages/core/src/canvas-log.ts
+++ b/packages/core/src/canvas-log.ts
@@ -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/ b/" 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("-")
diff --git a/packages/web/src/components/canvas/renderers/DiffCanvas.tsx b/packages/web/src/components/CanvasDiff.tsx
similarity index 98%
rename from packages/web/src/components/canvas/renderers/DiffCanvas.tsx
rename to packages/web/src/components/CanvasDiff.tsx
index 2efe39ead..2083b34ca 100644
--- a/packages/web/src/components/canvas/renderers/DiffCanvas.tsx
+++ b/packages/web/src/components/CanvasDiff.tsx
@@ -5,7 +5,7 @@ import type { CanvasArtifact, CanvasDiffFile } from "@aoagents/ao-core";
 
 type Props = { canvas: Extract };
 
-export function DiffCanvas({ canvas }: Props) {
+export function CanvasDiff({ canvas }: Props) {
   return (
     

       {canvas.payload.files.map((file) => (
diff --git a/packages/web/src/components/CanvasMarkdown.tsx b/packages/web/src/components/CanvasMarkdown.tsx
new file mode 100644
index 000000000..73a461762
--- /dev/null
+++ b/packages/web/src/components/CanvasMarkdown.tsx
@@ -0,0 +1,132 @@
+"use client";
+
+import { Fragment, type ReactNode } from "react";
+import type { CanvasArtifact } from "@aoagents/ao-core";
+
+type Props = { canvas: Extract };
+
+export function CanvasMarkdown({ canvas }: Props) {
+  return (
+    

+      {renderMarkdown(canvas.payload.markdown)}
+    

+  );
+}
+
+// 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(
+        
+          {buf.join("\n")}
+        
,
+      );
+      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(
+        

+          {renderInline(text)}
+        
,
+      );
+      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(
+        
    
+          {items.map((it, idx) => (
+            
  • {renderInline(it)}
    • 
+          ))}
+        
,
+      );
+      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(
+      

+        {renderInline(buf.join(" "))}
+      
,
+    );
+  }
+
+  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({text.slice(last, idx)});
+    const tok = match[0];
+    if (tok.startsWith("**")) {
+      out.push({tok.slice(2, -2)});
+    } else if (tok.startsWith("`")) {
+      out.push(
+        
+          {tok.slice(1, -1)}
+        ,
+      );
+    } else {
+      out.push({tok.slice(1, -1)});
+    }
+    last = idx + tok.length;
+  }
+  if (last < text.length) out.push({text.slice(last)});
+  return out;
+}
diff --git a/packages/web/src/components/canvas/CanvasRail.tsx b/packages/web/src/components/CanvasRail.tsx
similarity index 87%
rename from packages/web/src/components/canvas/CanvasRail.tsx
rename to packages/web/src/components/CanvasRail.tsx
index dda197129..0c21a8b94 100644
--- a/packages/web/src/components/canvas/CanvasRail.tsx
+++ b/packages/web/src/components/CanvasRail.tsx
@@ -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 (
-      

+