-
Notifications
You must be signed in to change notification settings - Fork 21
Potential fix for code scanning alert no. 71: Incomplete string escaping or encoding #1376
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Conversation
…ing or encoding Co-authored-by: Copilot Autofix powered by AI <62310815+github-advanced-security[bot]@users.noreply.github.com>
| const targetWithPlaceholder = target.replace(sepPattern, WORDSEP_PLACEHOLDER); | ||
| // Properly escape ALL regex metacharacters (including backslash), leaving the placeholder intact | ||
| const escapedTarget = escapeRegexLiteral(targetWithPlaceholder) | ||
| .replace(new RegExp(escapeRegexLiteral(WORDSEP_PLACEHOLDER), 'g'), '[-_\\s]+'); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
[correctness]
The use of new RegExp(escapeRegexLiteral(WORDSEP_PLACEHOLDER), 'g') to replace the placeholder with the pattern [-_\\s]+ is correct, but consider verifying that WORDSEP_PLACEHOLDER is unique enough to avoid accidental matches in the input string. If WORDSEP_PLACEHOLDER could appear in the input, it might lead to incorrect replacements.
|
@copilot fix the lint as it fails doe to errors. |
|
@kkartunov I've opened a new pull request, #1378, to work on those changes. Once the pull request is ready, I'll request review from you. |
Co-authored-by: kkartunov <5585002+kkartunov@users.noreply.github.com>
Fix ESLint semicolon violations in metadataMatching.ts
| .replace(/ /g, '\\ ') | ||
| const sepInsensitive = new RegExp(`\\b${escapedTarget.replace(/\\ /g, '[-_\\s]+')}\\b`) | ||
| // Replace all sequences of space, underscore, or hyphen in the target with a placeholder | ||
| const WORDSEP_PLACEHOLDER = '__WORDSEP__' |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
[💡 style]
Consider adding semicolons at the end of statements to maintain consistency with the rest of the codebase, which uses semicolons. This can help prevent potential issues with automatic semicolon insertion.
| const targetWithPlaceholder = target.replace(sepPattern, WORDSEP_PLACEHOLDER) | ||
| // Properly escape ALL regex metacharacters (including backslash), leaving the placeholder intact | ||
| const escapedTarget = escapeRegexLiteral(targetWithPlaceholder) | ||
| .replace(new RegExp(escapeRegexLiteral(WORDSEP_PLACEHOLDER), 'g'), '[-_\\s]+') |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
[correctness]
The replacement pattern [-_\s]+ is used to match sequences of separators. Ensure that this pattern aligns with the intended behavior and that no additional separators need to be considered. If the pattern should be more flexible, consider documenting the expected input format.
Potential fix for https://github.com/topcoder-platform/platform-ui/security/code-scanning/71
To fix this issue robustly:
escapeRegexLiteral, or do all escaping (for both standard RegExp metacharacters and for your application-specific substitutions) in a single regular expression.[-_\s]+), after escaping actual special characters, and ensure that no intermediate string manipulation reduces the escaping quality.For this file specifically, edit the block constructing the RegExp.
__WORDSEP__)[-_\s]+)No new dependencies are required since the standard escaping function is already robust, but you may consider (if allowed) to use a trusted library like
escape-string-regexp.Suggested fixes powered by Copilot Autofix. Review carefully before merging.