From 06c3d7bdbd8cc9415c8cabac060a50e0951c758b Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?Jos=C3=A9=20Gurruchaga?= <jsgrrchg@gmail.com>
Date: Mon, 20 Jul 2026 07:21:52 -0400
Subject: [PATCH] fix: parse Agent/Task trailers without regex (#879)
MIME-Version: 1.0
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 8bit

## Summary

Closes:
https://github.com/agentclientprotocol/claude-agent-acp/issues/878
Fixes the potential ReDoS warning reported by CodeQL in
`stripAgentTrailer()`.

- Replaces regex-based trailer stripping with bounded linear parsing
- Preserves valid `<usage>…</usage>` and `agentId: … (…)` trailer
handling
- Adds coverage for malformed trailers, which are left unchanged

## Testing

- `npm test -- --run src/tests/tools.test.ts`

---------

Co-authored-by: Ben Brandt <benjamin.j.brandt@gmail.com>
Co-authored-by: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
---
 src/tests/tools.test.ts | 56 +++++++++++++++++++++++++++++++++++++++++
 src/tools.ts            | 39 +++++++++++++++++++++++++---
 2 files changed, 92 insertions(+), 3 deletions(-)

diff --git a/src/tests/tools.test.ts b/src/tests/tools.test.ts
index 76a12411..d2adff7a 100644
--- a/src/tests/tools.test.ts
+++ b/src/tests/tools.test.ts
@@ -2275,6 +2275,62 @@ describe("Agent/Task tool_result rendering from tool_use_result", () => {
     ]);
   });
 
+  it("leaves malformed trailers alone", () => {
+    // Incomplete trailers are ordinary report text, not metadata to strip.
+    const malformedResult: ToolResultBlockParam = {
+      type: "tool_result",
+      tool_use_id: "toolu_agent",
+      content: [
+        { type: "text", text: "Report.\n<usage>missing closing tag" },
+        { type: "text", text: "Report.\nagentId: abc-123 (missing closing paren" },
+      ],
+    };
+
+    const update = toolUpdateFromToolResult(malformedResult, agentToolUse, false);
+
+    expect(update.content).toEqual([
+      { type: "content", content: { type: "text", text: "Report.\n<usage>missing closing tag" } },
+      {
+        type: "content",
+        content: { type: "text", text: "Report.\nagentId: abc-123 (missing closing paren" },
+      },
+    ]);
+  });
+
+  it("strips only the trailer when the report itself mentions <usage>", () => {
+    const result: ToolResultBlockParam = {
+      type: "tool_result",
+      tool_use_id: "toolu_agent",
+      content: "Grep for <usage> found 3 hits.\n<usage>subagent_tokens: 5</usage>",
+    };
+    const update = toolUpdateFromToolResult(result, agentToolUse, false);
+
+    expect(update.content).toEqual([
+      { type: "content", content: { type: "text", text: "Grep for <usage> found 3 hits." } },
+    ]);
+  });
+
+  it("handles adversarial trailer-shaped input in linear time", () => {
+    // Regression: the old regex-based strip backtracked quadratically on
+    // these shapes (CodeQL js/polynomial-redos) — at this size it would
+    // blow the test timeout rather than merely run slow.
+    const result: ToolResultBlockParam = {
+      type: "tool_result",
+      tool_use_id: "toolu_agent",
+      content: [
+        { type: "text", text: "agentId: - (".repeat(20000) },
+        { type: "text", text: "<usage>".repeat(30000) },
+      ],
+    };
+    const update = toolUpdateFromToolResult(result, agentToolUse, false);
+
+    // Neither is a real trailer, so both come through unchanged.
+    expect(update.content).toEqual([
+      { type: "content", content: { type: "text", text: "agentId: - (".repeat(20000) } },
+      { type: "content", content: { type: "text", text: "<usage>".repeat(30000) } },
+    ]);
+  });
+
   it("falls back (trailer-stripped) when tool_use_result is the async_launched variant", () => {
     const update = toolUpdateFromToolResult(rawResult, agentToolUse, false, {
       status: "async_launched",
diff --git a/src/tools.ts b/src/tools.ts
index 14cb7450..199b3473 100644
--- a/src/tools.ts
+++ b/src/tools.ts
@@ -515,9 +515,42 @@ function structuredResult<T extends object>(toolUseResult: unknown): T | undefin
  * matching rather than mangle the report.
  */
 function stripAgentTrailer(text: string): string {
-  return text
-    .replace(/\n?<usage>[\s\S]*?<\/usage>\s*$/, "")
-    .replace(/\n?agentId: [\w-]+ \([^)]*\)\s*$/, "");
+  return stripAgentIdLine(stripUsageBlock(text));
+}
+
+const USAGE_OPEN = "<usage>";
+const USAGE_CLOSE = "</usage>";
+
+/** Remove a trailing `<usage>…</usage>` block, plus trailing whitespace and
+ *  one preceding newline. Matches from the *last* `<usage>` so a report that
+ *  merely mentions the marker earlier isn't truncated at the mention. */
+function stripUsageBlock(text: string): string {
+  const body = text.trimEnd();
+  if (!body.endsWith(USAGE_CLOSE)) {
+    return text;
+  }
+  const open = body.lastIndexOf(USAGE_OPEN, body.length - USAGE_CLOSE.length - USAGE_OPEN.length);
+  if (open === -1) {
+    return text;
+  }
+  return body.slice(0, open > 0 && body[open - 1] === "\n" ? open - 1 : open);
+}
+
+/** The continuation line, anchored to a whole line so the regex has a single
+ *  start position and no ambiguous repetition (`[\w-]+` can't consume the
+ *  following space, `[^)]*` can't consume the closing paren) — it runs in
+ *  linear time on any input. */
+const AGENT_ID_LINE = /^agentId: [\w-]+ \([^)]*\)$/;
+
+/** Remove a final `agentId: <id> (…)` line, plus trailing whitespace and the
+ *  newline that preceded the line. */
+function stripAgentIdLine(text: string): string {
+  const body = text.trimEnd();
+  const lineStart = body.lastIndexOf("\n") + 1;
+  if (!AGENT_ID_LINE.test(body.slice(lineStart))) {
+    return text;
+  }
+  return body.slice(0, Math.max(lineStart - 1, 0));
 }
 
 /** Apply {@link stripAgentTrailer} across a raw tool_result `content` (plain