mirror of
https://github.com/agentclientprotocol/claude-agent-acp.git
synced 2026-10-27 04:30:59 +00:00
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:
@@ -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",
|
||||
|
||||
39
src/tools.ts
39
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
|
||||
|
||||
Reference in New Issue
Block a user