Skip to content

chore: sync core lib and CLAUDE.md from agent-core#21

Closed
avifenesh wants to merge 1 commit into
mainfrom
chore/sync-core-learn-20260529-110047
Closed

chore: sync core lib and CLAUDE.md from agent-core#21
avifenesh wants to merge 1 commit into
mainfrom
chore/sync-core-learn-20260529-110047

Conversation

@avifenesh

Copy link
Copy Markdown
Contributor

Automated sync of lib/ and CLAUDE.md from agent-core.

@gemini-code-assist gemini-code-assist Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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.

Comment on lines +135 to +138
// 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);

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

medium

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.

Suggested change
// 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));

Comment on lines +660 to +662
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);

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

medium

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);

@avifenesh

Copy link
Copy Markdown
Contributor Author

Superseded by latest sync (post ReDoS + prototype-pollution hardening).

@avifenesh avifenesh closed this May 29, 2026
@avifenesh avifenesh deleted the chore/sync-core-learn-20260529-110047 branch May 29, 2026 11:37
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant