chore: sync core lib and CLAUDE.md from agent-core#21
Conversation
There was a problem hiding this comment.
Code Review
This pull request introduces several ReDoS (Regular Expression Denial of Service) fixes across multiple files by bounding previously unbounded quantifiers and character classes. The review feedback points out that some of these arbitrary bounds (such as 50,000 characters for subagent detection and 2,000 characters for line-based action extraction) could fail on exceptionally large inputs. The reviewer suggests safer, more robust alternatives, such as combining simple string checks or using string splitting instead of complex regular expressions.
| // ReDoS fix: bound the unbounded [\s\S]* so a "Task({" with no following | ||
| // subagent_type cannot drive polynomial backtracking; 50k chars covers any | ||
| // realistic Task(...) call body. | ||
| /Task\s{0,100}\(\s{0,100}\{[\s\S]{0,50000}subagent_type/i.test(content); |
There was a problem hiding this comment.
Using a regex with an arbitrary limit like [\s\S]{0,50000} can fail to detect orchestrators in large files if the subagent_type keyword is located more than 50,000 characters after the Task call.
Instead of a single complex regex with arbitrary bounds, we can split this into two simple checks: checking if the content contains subagent_type and testing for the Task call pattern. This is completely safe from ReDoS, has no arbitrary limits, and is much more performant.
| // ReDoS fix: bound the unbounded [\s\S]* so a "Task({" with no following | |
| // subagent_type cannot drive polynomial backtracking; 50k chars covers any | |
| // realistic Task(...) call body. | |
| /Task\s{0,100}\(\s{0,100}\{[\s\S]{0,50000}subagent_type/i.test(content); | |
| (content.toLowerCase().includes('subagent_type') && /Task\s{0,100}\(\s{0,100}\{/i.test(content)); |
| action = line.replace(/[^\n]{0,2000}\bALWAYS\b\s{0,200}/i, '').substring(0, ACTION_COMPARISON_LENGTH); | ||
| } else { | ||
| action = line.replace(/.*\b(?:NEVER|DO NOT)\b\s*/i, '').substring(0, ACTION_COMPARISON_LENGTH); | ||
| action = line.replace(/[^\n]{0,2000}\b(?:NEVER|DO NOT)\b\s{0,200}/i, '').substring(0, ACTION_COMPARISON_LENGTH); |
There was a problem hiding this comment.
Using line.replace with a bounded prefix like [^\n]{0,2000} can fail to match and extract the action correctly if the line is longer than 2,000 characters (which can happen in long markdown paragraphs).
Instead of using a regex with arbitrary bounds to simulate a greedy match up to the last keyword, we can split the string by the keyword regex and take the last element. This is completely safe from ReDoS, has no arbitrary length limits, and is much cleaner.
const parts = line.split(/\bALWAYS\b\s*/i);
action = parts[parts.length - 1].substring(0, ACTION_COMPARISON_LENGTH);
} else {
const parts = line.split(/\b(?:NEVER|DO NOT)\b\s*/i);
action = parts[parts.length - 1].substring(0, ACTION_COMPARISON_LENGTH);|
Superseded by latest sync (post ReDoS + prototype-pollution hardening). |
Automated sync of lib/ and CLAUDE.md from agent-core.