fix: parse Agent/Task trailers without regex (#879)

## 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>
This commit is contained in:
José Gurruchaga
2026-07-20 07:21:52 -04:00
committed by GitHub
parent 92548f0435
commit 06c3d7bdbd
2 changed files with 92 additions and 3 deletions

View File

@@ -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",

View File

@@ -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