fix: resolve 30 CodeQL polynomial-redos alerts#211
Conversation
Fixes all open js/polynomial-redos alerts by eliminating overlapping
quantifiers in regexes across 7 lib files:
- Replace `\s*(.+)$` with `[ \t]*(.+)$` to prevent \s+/.+ overlap
- Replace `.*` in mid-pattern with `[^\n]*` or `.{0,N}` bounds
- Replace `[\s\S]*?` with `[\s\S]{0,N}?` length limits
- Replace `\s*\n\s*` with `[ \t]*\n[ \t]*` to prevent \s/\n overlap
- Replace `\d+` with `\d{1,10}` in array index patterns
- Limit `[^\]]+` / `[^)]+` in markdown link extraction
Files: fixer.js, prompt-patterns.js, security-patterns.js,
projectmemory-patterns.js, projectmemory-analyzer.js,
docs-patterns.js, slop-analyzers.js (+ plugin copies via sync-lib)
Summary of ChangesHello @avifenesh, I'm Gemini Code Assist1! I'm currently reviewing this pull request and will post my feedback shortly. In the meantime, here's a summary to help you and other reviewers quickly get up to speed! This pull request significantly enhances the application's security and performance by addressing 30 high-severity CodeQL alerts related to regular expression denial of service (ReDoS). The changes involve meticulously updating various regular expression patterns to be more precise and efficient, thereby preventing potential vulnerabilities arising from unbounded or overlapping quantifiers. This ensures the stability and robustness of the regex-dependent functionalities. Highlights
Changelog
Activity
Using Gemini Code AssistThe full guide for Gemini Code Assist can be found on our documentation page, here are some quick tips. Invoking Gemini You can request assistance from Gemini at any point by creating a comment using either
Customization To customize Gemini Code Assist for GitHub experience, repository maintainers can create a configuration file and/or provide a custom code review style guide (such as PEP-8 for Python) by creating and adding files to a Limitations & Feedback Gemini Code Assist may make mistakes. Please leave feedback on any instances where its feedback is incorrect or counter productive. You can react with 👍 and 👎 on @gemini-code-assist comments. If you're interested in giving your feedback about your experience with Gemini Code Assist for Github and other Google products, sign up here. You can also get AI-powered code generation, chat, as well as code reviews directly in the IDE at no cost with the Gemini Code Assist IDE Extension. Footnotes
|
![github-advanced-security[bot]](https://avatars.githubusercontent.com/in/57789?s=60&v=4){kind=small}
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 48fe4d5785
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
lib/enhance/security-patterns.js
Outdated
| const line = lines[i]; | ||
| // Look for shell commands with interpolation | ||
| if (/(?:exec|spawn|system|shell|`|Bash)\s*[(`].*\$\{/.test(line)) { | ||
| if (/(?:exec|spawn|system|shell|`|Bash)\s*[(`][^\n]{0,200}\$\{/.test(line)) { |
There was a problem hiding this comment.
Because check() already iterates over content.split('\n'), each line cannot contain newlines; adding [^ ]{0,200} here introduces a hard 200-character cutoff that causes false negatives for long command lines (for example, an exec( template where ${userInput} appears after many flags/arguments). This silently weakens the command-injection detector without being necessary for ReDoS mitigation on single-line input.
Useful? React with 👍 / 👎.
lib/enhance/security-patterns.js
Outdated
| const line = lines[i]; | ||
| // Look for user-controlled paths with ../ | ||
| if (/(?:path|file|dir).*\$.*\.\.\/|\.\.\/.*\$/.test(line)) { | ||
| if (/(?:path|file|dir)[^\n]{0,200}\$[^\n]{0,200}\.\.\/|\.\.\/[^\n]{0,200}\$/.test(line)) { |
There was a problem hiding this comment.
This pattern now only matches when $ and ../ are within 200 characters of each other, but the analyzer processes one line at a time, so the [^\n]{0,200} bounds are artificial and create false negatives on longer path-building expressions. In real prompts/scripts with long interpolated paths, risky ../ usage can now be missed entirely.
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
Pull request overview
Resolves CodeQL js/polynomial-redos alerts by rewriting a set of regex-based analyzers/fixers to avoid overlapping quantifiers and to add explicit bounds, then syncing the updated shared lib/ into each plugin’s vendored plugins/*/lib/ copy.
Changes:
- Replaced high-risk regex constructs (e.g.,
.*,[\s\S]*?,\s*\n\s*) with safer bounded/non-overlapping equivalents across pattern matchers and fixers. - Added explicit length limits to several extractors (e.g., markdown link parsing) to prevent worst‑case backtracking.
- Synced root
lib/updates into all pluginplugins/*/lib/copies.
Reviewed changes
Copilot reviewed 77 out of 77 changed files in this pull request and generated 2 comments.
Show a summary per file
| File | Description |
|---|---|
| lib/patterns/slop-analyzers.js | Safer regex in dead-code analyzer. |
| lib/enhance/security-patterns.js | Safer/bounded security detection regexes. |
| lib/enhance/prompt-patterns.js | Safer/bounded prompt analysis regexes. |
| lib/enhance/projectmemory-patterns.js | Bounded cross-line markdown/codeblock regexes. |
| lib/enhance/projectmemory-analyzer.js | Bounded markdown link extraction regexes. |
| lib/enhance/fixer.js | Safer/bounded fixer regexes + bounded index parsing. |
| lib/enhance/docs-patterns.js | Safer list-detection regex. |
| plugins/sync-docs/lib/patterns/slop-analyzers.js | Synced lib change: safer dead-code analyzer regex. |
| plugins/sync-docs/lib/enhance/security-patterns.js | Synced lib change: safer/bounded security regexes. |
| plugins/sync-docs/lib/enhance/prompt-patterns.js | Synced lib change: safer/bounded prompt regexes. |
| plugins/sync-docs/lib/enhance/projectmemory-patterns.js | Synced lib change: bounded cross-line regexes. |
| plugins/sync-docs/lib/enhance/projectmemory-analyzer.js | Synced lib change: bounded link extraction. |
| plugins/sync-docs/lib/enhance/fixer.js | Synced lib change: safer/bounded fixer regexes. |
| plugins/sync-docs/lib/enhance/docs-patterns.js | Synced lib change: safer list-detection regex. |
| plugins/ship/lib/patterns/slop-analyzers.js | Synced lib change: safer dead-code analyzer regex. |
| plugins/ship/lib/enhance/security-patterns.js | Synced lib change: safer/bounded security regexes. |
| plugins/ship/lib/enhance/prompt-patterns.js | Synced lib change: safer/bounded prompt regexes. |
| plugins/ship/lib/enhance/projectmemory-patterns.js | Synced lib change: bounded cross-line regexes. |
| plugins/ship/lib/enhance/projectmemory-analyzer.js | Synced lib change: bounded link extraction. |
| plugins/ship/lib/enhance/fixer.js | Synced lib change: safer/bounded fixer regexes. |
| plugins/ship/lib/enhance/docs-patterns.js | Synced lib change: safer list-detection regex. |
| plugins/repo-map/lib/patterns/slop-analyzers.js | Synced lib change: safer dead-code analyzer regex. |
| plugins/repo-map/lib/enhance/security-patterns.js | Synced lib change: safer/bounded security regexes. |
| plugins/repo-map/lib/enhance/prompt-patterns.js | Synced lib change: safer/bounded prompt regexes. |
| plugins/repo-map/lib/enhance/projectmemory-patterns.js | Synced lib change: bounded cross-line regexes. |
| plugins/repo-map/lib/enhance/projectmemory-analyzer.js | Synced lib change: bounded link extraction. |
| plugins/repo-map/lib/enhance/fixer.js | Synced lib change: safer/bounded fixer regexes. |
| plugins/repo-map/lib/enhance/docs-patterns.js | Synced lib change: safer list-detection regex. |
| plugins/perf/lib/patterns/slop-analyzers.js | Synced lib change: safer dead-code analyzer regex. |
| plugins/perf/lib/enhance/security-patterns.js | Synced lib change: safer/bounded security regexes. |
| plugins/perf/lib/enhance/prompt-patterns.js | Synced lib change: safer/bounded prompt regexes. |
| plugins/perf/lib/enhance/projectmemory-patterns.js | Synced lib change: bounded cross-line regexes. |
| plugins/perf/lib/enhance/projectmemory-analyzer.js | Synced lib change: bounded link extraction. |
| plugins/perf/lib/enhance/fixer.js | Synced lib change: safer/bounded fixer regexes. |
| plugins/perf/lib/enhance/docs-patterns.js | Synced lib change: safer list-detection regex. |
| plugins/next-task/lib/patterns/slop-analyzers.js | Synced lib change: safer dead-code analyzer regex. |
| plugins/next-task/lib/enhance/security-patterns.js | Synced lib change: safer/bounded security regexes. |
| plugins/next-task/lib/enhance/prompt-patterns.js | Synced lib change: safer/bounded prompt regexes. |
| plugins/next-task/lib/enhance/projectmemory-patterns.js | Synced lib change: bounded cross-line regexes. |
| plugins/next-task/lib/enhance/projectmemory-analyzer.js | Synced lib change: bounded link extraction. |
| plugins/next-task/lib/enhance/fixer.js | Synced lib change: safer/bounded fixer regexes. |
| plugins/next-task/lib/enhance/docs-patterns.js | Synced lib change: safer list-detection regex. |
| plugins/learn/lib/patterns/slop-analyzers.js | Synced lib change: safer dead-code analyzer regex. |
| plugins/learn/lib/enhance/security-patterns.js | Synced lib change: safer/bounded security regexes. |
| plugins/learn/lib/enhance/prompt-patterns.js | Synced lib change: safer/bounded prompt regexes. |
| plugins/learn/lib/enhance/projectmemory-patterns.js | Synced lib change: bounded cross-line regexes. |
| plugins/learn/lib/enhance/projectmemory-analyzer.js | Synced lib change: bounded link extraction. |
| plugins/learn/lib/enhance/fixer.js | Synced lib change: safer/bounded fixer regexes. |
| plugins/learn/lib/enhance/docs-patterns.js | Synced lib change: safer list-detection regex. |
| plugins/enhance/lib/patterns/slop-analyzers.js | Synced lib change: safer dead-code analyzer regex. |
| plugins/enhance/lib/enhance/security-patterns.js | Synced lib change: safer/bounded security regexes. |
| plugins/enhance/lib/enhance/prompt-patterns.js | Synced lib change: safer/bounded prompt regexes. |
| plugins/enhance/lib/enhance/projectmemory-patterns.js | Synced lib change: bounded cross-line regexes. |
| plugins/enhance/lib/enhance/projectmemory-analyzer.js | Synced lib change: bounded link extraction. |
| plugins/enhance/lib/enhance/fixer.js | Synced lib change: safer/bounded fixer regexes. |
| plugins/enhance/lib/enhance/docs-patterns.js | Synced lib change: safer list-detection regex. |
| plugins/drift-detect/lib/patterns/slop-analyzers.js | Synced lib change: safer dead-code analyzer regex. |
| plugins/drift-detect/lib/enhance/security-patterns.js | Synced lib change: safer/bounded security regexes. |
| plugins/drift-detect/lib/enhance/prompt-patterns.js | Synced lib change: safer/bounded prompt regexes. |
| plugins/drift-detect/lib/enhance/projectmemory-patterns.js | Synced lib change: bounded cross-line regexes. |
| plugins/drift-detect/lib/enhance/projectmemory-analyzer.js | Synced lib change: bounded link extraction. |
| plugins/drift-detect/lib/enhance/fixer.js | Synced lib change: safer/bounded fixer regexes. |
| plugins/drift-detect/lib/enhance/docs-patterns.js | Synced lib change: safer list-detection regex. |
| plugins/deslop/lib/patterns/slop-analyzers.js | Synced lib change: safer dead-code analyzer regex. |
| plugins/deslop/lib/enhance/security-patterns.js | Synced lib change: safer/bounded security regexes. |
| plugins/deslop/lib/enhance/prompt-patterns.js | Synced lib change: safer/bounded prompt regexes. |
| plugins/deslop/lib/enhance/projectmemory-patterns.js | Synced lib change: bounded cross-line regexes. |
| plugins/deslop/lib/enhance/projectmemory-analyzer.js | Synced lib change: bounded link extraction. |
| plugins/deslop/lib/enhance/fixer.js | Synced lib change: safer/bounded fixer regexes. |
| plugins/deslop/lib/enhance/docs-patterns.js | Synced lib change: safer list-detection regex. |
| plugins/audit-project/lib/patterns/slop-analyzers.js | Synced lib change: safer dead-code analyzer regex. |
| plugins/audit-project/lib/enhance/security-patterns.js | Synced lib change: safer/bounded security regexes. |
| plugins/audit-project/lib/enhance/prompt-patterns.js | Synced lib change: safer/bounded prompt regexes. |
| plugins/audit-project/lib/enhance/projectmemory-patterns.js | Synced lib change: bounded cross-line regexes. |
| plugins/audit-project/lib/enhance/projectmemory-analyzer.js | Synced lib change: bounded link extraction. |
| plugins/audit-project/lib/enhance/fixer.js | Synced lib change: safer/bounded fixer regexes. |
| plugins/audit-project/lib/enhance/docs-patterns.js | Synced lib change: safer list-detection regex. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| const requestsJson = ( | ||
| (/\b(?:respond|output|return)\s+(?:with|in|as)?\s*JSON\b/i.test(content) && | ||
| (/\b(?:respond|output|return)\s+(?:(?:with|in|as)[ \t])?JSON\b/i.test(content) && | ||
| !/--output\s+json/i.test(content) && |
There was a problem hiding this comment.
The updated requestsJson regex requires exactly one space/tab after with|in|as (because of (?:with|in|as)[ \t]), so it will no longer match cases like respond with JSON (multiple spaces) or respond with\t\tJSON. Consider using a 1+ quantifier (e.g. [ \t]+) after the keyword to preserve prior behavior while keeping the ReDoS fix.
lib/enhance/prompt-patterns.js
Outdated
| /\bsee\s+\S+\s+(?:for|as)\s+(?:an?\s+)?example\b/i, | ||
| /\blook\s+at\s+(?:how|the)\b/i, | ||
| /\S+\.(?:js|ts|py)\s+(?:is|as)\s+(?:a\s+)?(?:good\s+)?example/i | ||
| /\S+\.(?:js|ts|py)\s+(?:is|as)\s+(?:a\s+)?(?:good )?example/i |
There was a problem hiding this comment.
This change from (?:good\s+)?example to (?:good )?example only matches a single literal space after good, so it won't match common variants like good\t example or good example. If the intent is just to avoid ReDoS while keeping whitespace flexibility, use a bounded whitespace class (e.g. good[ \t]+) instead of a single space.
| /\S+\.(?:js|ts|py)\s+(?:is|as)\s+(?:a\s+)?(?:good )?example/i | |
| /\S+\.(?:js|ts|py)\s+(?:is|as)\s+(?:a\s+)?(?:good[ \t]+)?example/i |
![gemini-code-assist[bot]](https://avatars.githubusercontent.com/in/956858?s=60&v=4){kind=small}
There was a problem hiding this comment.
Code Review
This pull request effectively resolves 30 high-severity CodeQL alerts for polynomial regular expression denial of service (ReDoS) by systematically addressing vulnerable regex patterns across affected files. However, a systemic Prototype Pollution vulnerability was identified in the applyAtPath function within fixer.js (present in both lib/ and various plugins/ directories). The regex used to parse array access patterns allows sensitive property names like __proto__, which could be exploited if the path string is derived from untrusted input. It is recommended to restrict the allowed property names in these regexes to prevent arbitrary prototype access.
lib/enhance/fixer.js
Outdated
| if (part.includes('[')) { | ||
| // Array access | ||
| const match = part.match(/(\w+)\[(\d+)\]/); | ||
| const match = part.match(/(\w+)\[(\d{1,10})\]/); |
There was a problem hiding this comment.
The regex (\w+) matches sensitive property names such as __proto__, constructor, and prototype. When used in applyAtPath, this can lead to Prototype Pollution if the pathStr is influenced by untrusted input (e.g., keys in a malicious plugin.json being analyzed). Although the PR fixes a ReDoS vulnerability in this line, it should also restrict the property names to prevent arbitrary prototype access.
| const match = part.match(/(\w+)\[(\d{1,10})\]/); | |
| const match = part.match(/^((?!__proto__|constructor|prototype)\w+)\[(\d{1,10})\]$/); |
References
- When a pattern (like a regex) is used in multiple places, maintain consistency across the codebase, even if a localized improvement is possible. Prioritize consistency over isolated perfection.
lib/enhance/fixer.js
Outdated
| const lastPart = parts[parts.length - 1]; | ||
| if (lastPart.includes('[')) { | ||
| const match = lastPart.match(/(\w+)\[(\d+)\]/); | ||
| const match = lastPart.match(/(\w+)\[(\d{1,10})\]/); |
There was a problem hiding this comment.
Similar to the issue on line 182, the regex (\w+) allows matching sensitive property names like __proto__. This should be restricted to prevent Prototype Pollution.
| const match = lastPart.match(/(\w+)\[(\d{1,10})\]/); | |
| const match = lastPart.match(/^((?!__proto__|constructor|prototype)\w+)\[(\d{1,10})\]$/); |
References
- When a pattern (like a regex) is used in multiple places, maintain consistency across the codebase, even if a localized improvement is possible. Prioritize consistency over isolated perfection.
| if (part.includes('[')) { | ||
| // Array access | ||
| const match = part.match(/(\w+)\[(\d+)\]/); | ||
| const match = part.match(/(\w+)\[(\d{1,10})\]/); |
There was a problem hiding this comment.
The regex (\w+) matches sensitive property names such as __proto__, constructor, and prototype. When used in applyAtPath, this can lead to Prototype Pollution if the pathStr is influenced by untrusted input. It is recommended to restrict the allowed property names in this regex.
| const match = part.match(/(\w+)\[(\d{1,10})\]/); | |
| const match = part.match(/^((?!__proto__|constructor|prototype)\w+)\[(\d{1,10})\]$/); |
References
- When a pattern (like a regex) is used in multiple places, maintain consistency across the codebase, even if a localized improvement is possible. Prioritize consistency over isolated perfection.
| const lastPart = parts[parts.length - 1]; | ||
| if (lastPart.includes('[')) { | ||
| const match = lastPart.match(/(\w+)\[(\d+)\]/); | ||
| const match = lastPart.match(/(\w+)\[(\d{1,10})\]/); |
There was a problem hiding this comment.
The regex (\w+) allows matching sensitive property names like __proto__. This should be restricted to prevent Prototype Pollution.
| const match = lastPart.match(/(\w+)\[(\d{1,10})\]/); | |
| const match = lastPart.match(/^((?!__proto__|constructor|prototype)\w+)\[(\d{1,10})\]$/); |
References
- When a pattern (like a regex) is used in multiple places, maintain consistency across the codebase, even if a localized improvement is possible. Prioritize consistency over isolated perfection.
plugins/deslop/lib/enhance/fixer.js
Outdated
| if (part.includes('[')) { | ||
| // Array access | ||
| const match = part.match(/(\w+)\[(\d+)\]/); | ||
| const match = part.match(/(\w+)\[(\d{1,10})\]/); |
There was a problem hiding this comment.
The regex (\w+) matches sensitive property names such as __proto__, constructor, and prototype. When used in applyAtPath, this can lead to Prototype Pollution if the pathStr is influenced by untrusted input. It is recommended to restrict the allowed property names in this regex.
| const match = part.match(/(\w+)\[(\d{1,10})\]/); | |
| const match = part.match(/^((?!__proto__|constructor|prototype)\w+)\[(\d{1,10})\]$/); |
References
- When a pattern (like a regex) is used in multiple places, maintain consistency across the codebase, even if a localized improvement is possible. Prioritize consistency over isolated perfection.
| const lastPart = parts[parts.length - 1]; | ||
| if (lastPart.includes('[')) { | ||
| const match = lastPart.match(/(\w+)\[(\d+)\]/); | ||
| const match = lastPart.match(/(\w+)\[(\d{1,10})\]/); |
There was a problem hiding this comment.
The regex (\w+) allows matching sensitive property names like __proto__. This should be restricted to prevent Prototype Pollution.
| const match = lastPart.match(/(\w+)\[(\d{1,10})\]/); | |
| const match = lastPart.match(/^((?!__proto__|constructor|prototype)\w+)\[(\d{1,10})\]$/); |
References
- When a pattern (like a regex) is used in multiple places, maintain consistency across the codebase, even if a localized improvement is possible. Prioritize consistency over isolated perfection.
plugins/ship/lib/enhance/fixer.js
Outdated
| if (part.includes('[')) { | ||
| // Array access | ||
| const match = part.match(/(\w+)\[(\d+)\]/); | ||
| const match = part.match(/(\w+)\[(\d{1,10})\]/); |
There was a problem hiding this comment.
The regex (\w+) matches sensitive property names such as __proto__, constructor, and prototype. When used in applyAtPath, this can lead to Prototype Pollution if the pathStr is influenced by untrusted input. It is recommended to restrict the allowed property names in this regex.
| const match = part.match(/(\w+)\[(\d{1,10})\]/); | |
| const match = part.match(/^((?!__proto__|constructor|prototype)\w+)\[(\d{1,10})\]$/); |
References
- When a pattern (like a regex) is used in multiple places, maintain consistency across the codebase, even if a localized improvement is possible. Prioritize consistency over isolated perfection.
plugins/ship/lib/enhance/fixer.js
Outdated
| const lastPart = parts[parts.length - 1]; | ||
| if (lastPart.includes('[')) { | ||
| const match = lastPart.match(/(\w+)\[(\d+)\]/); | ||
| const match = lastPart.match(/(\w+)\[(\d{1,10})\]/); |
There was a problem hiding this comment.
The regex (\w+) allows matching sensitive property names like __proto__. This should be restricted to prevent Prototype Pollution.
| const match = lastPart.match(/(\w+)\[(\d{1,10})\]/); | |
| const match = lastPart.match(/^((?!__proto__|constructor|prototype)\w+)\[(\d{1,10})\]$/); |
References
- When a pattern (like a regex) is used in multiple places, maintain consistency across the codebase, even if a localized improvement is possible. Prioritize consistency over isolated perfection.
| if (part.includes('[')) { | ||
| // Array access | ||
| const match = part.match(/(\w+)\[(\d+)\]/); | ||
| const match = part.match(/(\w+)\[(\d{1,10})\]/); |
There was a problem hiding this comment.
The regex (\w+) matches sensitive property names such as __proto__, constructor, and prototype. When used in applyAtPath, this can lead to Prototype Pollution if the pathStr is influenced by untrusted input. It is recommended to restrict the allowed property names in this regex.
| const match = part.match(/(\w+)\[(\d{1,10})\]/); | |
| const match = part.match(/^((?!__proto__|constructor|prototype)\w+)\[(\d{1,10})\]$/); |
References
- When a pattern (like a regex) is used in multiple places, maintain consistency across the codebase, even if a localized improvement is possible. Prioritize consistency over isolated perfection.
| const lastPart = parts[parts.length - 1]; | ||
| if (lastPart.includes('[')) { | ||
| const match = lastPart.match(/(\w+)\[(\d+)\]/); | ||
| const match = lastPart.match(/(\w+)\[(\d{1,10})\]/); |
There was a problem hiding this comment.
The regex (\w+) allows matching sensitive property names like __proto__. This should be restricted to prevent Prototype Pollution.
| const match = lastPart.match(/(\w+)\[(\d{1,10})\]/); | |
| const match = lastPart.match(/^((?!__proto__|constructor|prototype)\w+)\[(\d{1,10})\]$/); |
References
- When a pattern (like a regex) is used in multiple places, maintain consistency across the codebase, even if a localized improvement is possible. Prioritize consistency over isolated perfection.
Reviewer feedback:
- security-patterns.js: Replace [^\n]{0,200} bounds with string-based
checks (indexOf) since lines are already split — no false negatives
- prompt-patterns.js:965: Fix [ \t] → [ \t]+ for multi-space matching
- prompt-patterns.js:1142: Fix (good ) → (good[ \t]+) for tab support
- fixer.js:182,193: Add prototype pollution guard with negative lookahead
for __proto__, constructor, prototype + anchor with ^...$
Deeper ReDoS fixes:
- Convert spawn/invoke orchestrator detection to string includes()
- Convert CoT/redundancy detection to string includes()
- Replace [\s\S]*? section extraction with line-by-line wrapSection()
- Replace markdown link regex with indexOf-based scanning
- Convert directory tree and code block detection to split checks
- Use [ \t]+ instead of \s+ for horizontal-only whitespace
- Use (\S.*) instead of (.+) to prevent [ \t]*/.+ overlap
Split compound regex patterns into simpler non-overlapping checks:
- const/let/var assignment: split keyword check from = { check
- JSON quoted properties: explicit alternation for with/without newline
- prompt-patterns.js: Convert pattern-ref regex array to string includes() checks — eliminates overlapping optional [ \t]+ groups - prompt-patterns.js: LOOKS_LIKE_PYTHON if detection uses [^:\n]*: instead of [^\n]*: — disjoint character classes prevent backtracking - slop-analyzers.js: Same fix for Python if-condition detection
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 77 out of 77 changed files in this pull request and generated 4 comments.
Comments suppressed due to low confidence (1)
lib/enhance/prompt-patterns.js:1002
json_without_schemabehavior changed substantially in this hunk (notably the newhasSchemaheuristics). There are comprehensive tests for other prompt patterns in__tests__/prompt-analyzer-comprehensive.test.js, but none appear to coverjson_without_schema; adding a couple of focused tests (schema present vs absent) would help prevent regressions from these heuristic changes.
json_without_schema: {
id: 'json_without_schema',
category: 'output',
certainty: 'MEDIUM',
autoFix: false,
description: 'Requests JSON output without providing schema/example',
check: (content) => {
if (!content || typeof content !== 'string') return null;
// Check if requests JSON (exclude CLI flags and function descriptions)
// Exclude: "--output json", "analyzer returns JSON", "function returns JSON"
const requestsJson = (
(/\b(?:respond|output|return)[ \t]+(?:(?:with|in|as)[ \t]+)?JSON\b/i.test(content) &&
!/--output\s+json/i.test(content) &&
!/(?:analyzer|function|method)\s+returns?\s+JSON/i.test(content))
) ||
/\bJSON\s+(?:object|response|format)\b/i.test(content);
if (!requestsJson) return null;
// Check if provides schema or example
const hasSchema = /\bproperties\b.{1,200}\btype\b/is.test(content) ||
(content.includes('```json') && content.includes('{')) ||
/<json[_-]?schema>/i.test(content) ||
// JSON in JavaScript/TypeScript code blocks (quoted keys)
(/```(?:javascript|js|typescript|ts)\b/.test(content) && /\{\s*"[a-zA-Z]+"/i.test(content)) ||
// JavaScript object literal assignment (const x = { prop: ... })
(/\b(?:const|let|var)\b/.test(content) && /=\s*\{/.test(content)) ||
// JSON example with quoted property names in prose
/\{"[a-zA-Z_]+"[ \t]*:/i.test(content) || /\{\n[ \t]*"[a-zA-Z_]+"[ \t]*:/i.test(content) ||
// Inline schema description: { prop, prop, prop } or { prop: type, ... }
/\{[ \t]*[a-zA-Z_]+[ \t]*,[ \t]*[a-zA-Z_]+[ \t]*,[ \t]*[a-zA-Z_]+/i.test(content) ||
// Interface-style: { prop: value } patterns with multiple lines
/\{\n[ \t]+[a-zA-Z_]+[ \t]*:[ \t]*[\[\{"']/i.test(content);
if (!hasSchema) {
return {
issue: 'Requests JSON output but no schema or example provided',
fix: 'Add JSON schema or example structure to ensure consistent output format'
};
}
return null;
}
},
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| const hasDirectoryTree = content.includes('```') && /├──|└──|lib\/|src\//.test(content); | ||
|
|
There was a problem hiding this comment.
hasDirectoryTree is now content.includes('```') && /├──|└──|lib\/|src\//.test(content), which no longer checks that the tree markers appear inside a fenced code block. This can incorrectly treat prose that mentions src/ (plus any unrelated code fence) as a directory tree and suppress the intended "missing architecture/structure" finding. Consider a ReDoS-safe approach that still scopes the search to fenced blocks (e.g., scanning for ``` fences and testing the text between them for the tree markers).
| const hasDirectoryTree = content.includes('```') && /├──|└──|lib\/|src\//.test(content); | |
| let hasDirectoryTree = false; | |
| if (content.includes('```')) { | |
| const fence = '```'; | |
| let searchStart = 0; | |
| while (!hasDirectoryTree) { | |
| const start = content.indexOf(fence, searchStart); | |
| if (start === -1) break; | |
| const end = content.indexOf(fence, start + fence.length); | |
| if (end === -1) break; | |
| const blockContent = content.slice(start + fence.length, end); | |
| if (/├──|└──|lib\/|src\//.test(blockContent)) { | |
| hasDirectoryTree = true; | |
| break; | |
| } | |
| searchStart = end + fence.length; | |
| } | |
| } |
| continue; | ||
| } | ||
| } | ||
| pos = openBracket + 1; |
There was a problem hiding this comment.
The new link extraction loop can become O(n^2) on large inputs with many [/] pairs (each iteration does indexOf(']') scanning potentially most of the remaining string, and pos only advances by 1 on the fallback path). To keep the ReDoS fix without introducing a different potential CPU hotspot, consider advancing pos to closeBracket + 1 when a ] is found but it isn't a link, and/or otherwise ensure the loop always makes proportional progress based on the furthest delimiter found.
| pos = openBracket + 1; | |
| // Advance past the closing bracket to avoid repeatedly rescanning the same region | |
| pos = closeBracket + 1; |
| const isOrchestrator = /##\s*Phase\s+\d+/i.test(content) || content.includes('Task({') || | ||
| (lc.includes('spawn') && lc.includes('agent')) || content.includes('subagent_type') || | ||
| content.includes('await Task(') || (lc.includes('invoke') && lc.includes('skill')) || | ||
| (/\bSkill\b/.test(content) && lc.includes('tool')); |
There was a problem hiding this comment.
In isOrchestrator, the Skill tool detection changed from a case-insensitive regex (Skill\s*tool/i) to (/\bSkill\b/.test(content) && lc.includes('tool')), which is now case-sensitive for “skill”. This can miss orchestrator files that use skill tool/SKILL tool, changing behavior beyond the ReDoS fix. Consider making the Skill check case-insensitive (e.g., test against lc or use /\bskill\b/i).
| (/\bSkill\b/.test(content) && lc.includes('tool')); | |
| (/\bskill\b/.test(lc) && lc.includes('tool')); |
| // Check if provides schema or example | ||
| const hasSchema = /\bproperties\b.{1,200}\btype\b/is.test(content) || | ||
| /```json\s*\n\s*\{/i.test(content) || | ||
| (content.includes('```json') && content.includes('{')) || | ||
| /<json[_-]?schema>/i.test(content) || | ||
| // JSON in JavaScript/TypeScript code blocks (quoted keys) | ||
| /```(?:javascript|js|typescript|ts)\s*\n[\s\S]*?\{\s*\n?\s*"[a-zA-Z]+"/i.test(content) || | ||
| (/```(?:javascript|js|typescript|ts)\b/.test(content) && /\{\s*"[a-zA-Z]+"/i.test(content)) || | ||
| // JavaScript object literal assignment (const x = { prop: ... }) | ||
| /(?:const|let|var)\s+\w+\s*=\s*\{\s*\n\s*[a-zA-Z_]+\s*:/i.test(content) || | ||
| (/\b(?:const|let|var)\b/.test(content) && /=\s*\{/.test(content)) || | ||
| // JSON example with quoted property names in prose | ||
| /\{\s*\n?\s*"[a-zA-Z_]+"\s*:\s*["\[\{]/i.test(content) || | ||
| /\{"[a-zA-Z_]+"[ \t]*:/i.test(content) || /\{\n[ \t]*"[a-zA-Z_]+"[ \t]*:/i.test(content) || | ||
| // Inline schema description: { prop, prop, prop } or { prop: type, ... } | ||
| /\{\s*[a-zA-Z_]+\s*,\s*[a-zA-Z_]+\s*,\s*[a-zA-Z_]+/i.test(content) || | ||
| /\{[ \t]*[a-zA-Z_]+[ \t]*,[ \t]*[a-zA-Z_]+[ \t]*,[ \t]*[a-zA-Z_]+/i.test(content) || | ||
| // Interface-style: { prop: value } patterns with multiple lines | ||
| /\{\s*\n\s+[a-zA-Z_]+\s*:\s*[\[\{"']/i.test(content); | ||
| /\{\n[ \t]+[a-zA-Z_]+[ \t]*:[ \t]*[\[\{"']/i.test(content); |
There was a problem hiding this comment.
hasSchema detection was changed from a bounded json-code-block regex to (content.includes('```json') && content.includes('{')), which can produce false positives (e.g., any { elsewhere in the file) and cause json_without_schema to miss cases where JSON is requested but no schema/example is actually provided. Consider using a ReDoS-safe, more specific pattern (e.g., matching { near the json fence or scanning within the json code block) to keep the original intent while avoiding catastrophic backtracking.
Summary
js/polynomial-redosCodeQL alerts (high severity)Changes
\s*(.+)$→[ \t]*(.+)$\s/.overlap at line boundaries.*→[^\n]*or.{0,N}[\s\S]*?→[\s\S]{0,N}?\s*\n\s*→[ \t]*\n[ \t]*\s/\nclass overlap\d+→\d{1,10}[^\]]+→[^\]]{1,500}Files (7 lib sources + 70 plugin copies)
lib/enhance/prompt-patterns.js(14 alerts)lib/enhance/fixer.js(6 alerts)lib/enhance/security-patterns.js(4 alerts)lib/enhance/projectmemory-patterns.js(2 alerts)lib/enhance/projectmemory-analyzer.js(2 alerts)lib/enhance/docs-patterns.js(1 alert)lib/patterns/slop-analyzers.js(1 alert)Test plan
npx awesome-slash-dev validatepasses all validatorsnpx awesome-slash-dev sync-libapplied to all plugin copies