Potential fix for code scanning alert no. 1: Regular expression injection#20
Potential fix for code scanning alert no. 1: Regular expression injection#20eschultink merged 2 commits intomainfrom
Conversation
…tion Co-authored-by: Copilot Autofix powered by AI <62310815+github-advanced-security[bot]@users.noreply.github.com>
There was a problem hiding this comment.
Pull request overview
This PR addresses a GitHub code scanning alert for regular-expression injection by escaping the GCS bucket prefix before constructing a dynamic RegExp in the backup listing output.
Changes:
- Escape
bucketPrefixvia_.escapeRegExpbefore passing it intonew RegExp(...). - Use the escaped value when stripping the prefix from
gsutil lsoutput.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
index.js
Outdated
|
|
||
| let bucketPrefix = `gs://${bucket}/`; | ||
| let escapedBucketPrefix = _.escapeRegExp(bucketPrefix); | ||
| let files = child_process.execSync('gsutil ls ' + bucketPrefix).toString('utf8'); |
There was a problem hiding this comment.
child_process.execSync('gsutil ls ' + bucketPrefix) executes through a shell, and bucketPrefix can be influenced via the --bucketPrefix CLI option (see lib/cmd.getBackupBucket). This leaves a command-injection risk (and can also break if the bucket name contains spaces/shell metacharacters). Prefer execFileSync/spawnSync with an argv array (e.g., ['ls', bucketPrefix]) and no shell.
| let files = child_process.execSync('gsutil ls ' + bucketPrefix).toString('utf8'); | |
| let files = child_process.execFileSync('gsutil', ['ls', bucketPrefix]).toString('utf8'); |
There was a problem hiding this comment.
Code is outdated with new file version, @copilot review again
|
@aperez-worklytics I've opened a new pull request, #26, to work on those changes. Once the pull request is ready, I'll request review from you. |
Potential fix for https://github.com/Worklytics/datastore-backup/security/code-scanning/1
To fix the issue, we need to sanitize the
bucketPrefixvariable before using it in thenew RegExpconstructor. The best way to achieve this is by using a library likelodashto escape special characters in the user-provided input. Specifically, we can use the_.escapeRegExpfunction to sanitizebucketPrefixbefore constructing the regular expression.Changes will be made in the
index.jsfile:_.escapeRegExpfunction fromlodash._.escapeRegExptobucketPrefixbefore passing it to thenew RegExpconstructor.Suggested fixes powered by Copilot Autofix. Review carefully before merging.