FazBrowse GitHub Viewer | Trending |
URL:
| Home
Tools: [Download Repo ZIP]   [Original HTTPS Page]

Use brace-expansion >2.0.1 by redsun82 · Pull Request #3065 · github/codeql-action · GitHub

Use brace-expansion >2.0.1 - #3065

Merged
redsun82 merged 4 commits into
mainfrom
redsun82/update-brace-expansion
Sep 12, 2025
Merged

Use brace-expansion >2.0.1#3065
redsun82 merged 4 commits into
mainfrom
redsun82/update-brace-expansion

Conversation

Copy link
Copy Markdown
Contributor

Risk assessment

For internal use only. Please select the risk level of this change:

  • Low risk: Changes are fully under feature flags, or have been fully tested and validated in pre-production environments and are highly observable, or are documentation or test only.

Merge / deployment checklist

  • Confirm this change is backwards compatible with existing workflows.
  • Consider adding a changelog entry for this change.
  • Confirm the readme and docs have been updated if necessary.

redsun82 requested a review from a team as a code owner August 29, 2025 14:53
Copilot AI review requested due to automatic review settings August 29, 2025 14:53

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Choose a reason Spam Abuse Off Topic Outdated Duplicate Resolved Low Quality

Pull Request Overview

This PR updates the brace-expansion dependency to require version >= 2.0.1, which appears to address a security vulnerability by updating the regular expression pattern used in the brace expansion logic.

  • Updates package.json to add explicit brace-expansion dependency with minimum version 2.0.1
  • Updates compiled JavaScript files to reflect the new brace-expansion implementation with improved regex pattern
  • Fixes a potential ReDoS (Regular Expression Denial of Service) vulnerability in the comma-matching pattern

Reviewed Changes

Copilot reviewed 11 out of 13 changed files in this pull request and generated no comments.

File Description
package.json Adds explicit brace-expansion dependency with version constraint >=2.0.1
lib/*.js (multiple files) Generated JavaScript code reflecting the updated brace-expansion package with security fix
Files not reviewed (1)
  • package-lock.json: Language not supported

Comment thread package.json Outdated
"uuid": "^11.1.0",
"zlib": "^1.0.5"
"zlib": "^1.0.5",
"brace-expansion": ">=2.0.1"

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Choose a reason Spam Abuse Off Topic Outdated Duplicate Resolved Low Quality

Minor: I believe the dependencies are in alphabetical order, so it would be good if this addition maintained that order.

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Choose a reason Spam Abuse Off Topic Outdated Duplicate Resolved Low Quality

We don't depend on brace-expansion ourselves, right? Perhaps best to try to update the dependency that pulls in the outdated library version.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Choose a reason Spam Abuse Off Topic Outdated Duplicate Resolved Low Quality

unfortunately, that dependency comes from https://www.npmjs.com/package/@actions/glob, which is stuck at 0.5.0 from a year ago

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Choose a reason Spam Abuse Off Topic Outdated Duplicate Resolved Low Quality

Let's ask the actions folk to update their dependencies.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Choose a reason Spam Abuse Off Topic Outdated Duplicate Resolved Low Quality

looking better, that dependency points to brace-expansion < 2.0.0, which is not problematic. The only problematic ones are 2.0.0 (that we don't have) and 2.0.1. I tried retracing where that comes from, but in the end just resorted to an explicit override of that specific version

Comment thread package-lock.json Outdated
Comment on lines +7214 to +7215
"node_modules/minimatch/node_modules/brace-expansion": {
"version": "1.1.12",

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Choose a reason Spam Abuse Off Topic Outdated Duplicate Resolved Low Quality

Note that we're still pulling in v1.1.12 here — if we want to override this I think we need to modify the overrides property in package.json.

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Choose a reason Spam Abuse Off Topic Outdated Duplicate Resolved Low Quality

Either that or update all the dependencies that rely on old versions of brace-expansion

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Choose a reason Spam Abuse Off Topic Outdated Duplicate Resolved Low Quality

Right, if we can update minimatch, that's better. We've had situations before where we've wanted to bump a transient dependency before the direct dependency had bumped it.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Choose a reason Spam Abuse Off Topic Outdated Duplicate Resolved Low Quality

Aha, now I see why dependabot didn't manage to do this update. I'll get back to it on Monday.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Choose a reason Spam Abuse Off Topic Outdated Duplicate Resolved Low Quality

(notice I didn't tell which Monday 😜 😅 on it now)

redsun82 force-pushed the redsun82/update-brace-expansion branch from 7b474e5 to f11caf4 Compare September 8, 2025 08:53
redsun82 changed the title Use brace-expansion >= 2.0.1 Use brace-expansion >2.0.1 Sep 8, 2025

henrymercer left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Choose a reason Spam Abuse Off Topic Outdated Duplicate Resolved Low Quality

I had a look at the lockfile and we're still pulling in v1.1.11 of brace-expansion — does that need updating too?

Copy link
Copy Markdown
Contributor Author

I had a look at the lockfile and we're still pulling in v1.1.11 of brace-expansion — does that need updating too?

No, I double checked and the versions that need updating are just 2.0.0 (which we don't use) and 2.0.1. Versions <2.0.0 are ok.

henrymercer left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Choose a reason Spam Abuse Off Topic Outdated Duplicate Resolved Low Quality

Thanks!

redsun82 merged commit f92cc3a into main Sep 12, 2025
290 checks passed
redsun82 deleted the redsun82/update-brace-expansion branch September 12, 2025 14:06
github-actions Bot mentioned this pull request Sep 25, 2025
8 tasks
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters. Learn more about bidirectional Unicode characters
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.

5 participants


Back | FazBrowse Home | New Git URL