fix: resolve all npm audit vulnerabilities#822
Conversation
- Bump @oclif/plugin-plugins to ^5.4.64 to pull in npm@11, which ships fixed bundled versions of brace-expansion (2.x), picomatch, and ip-address - Override @octokit/rest to ^20.0.2 to fix the @octokit/plugin-paginate-rest, @octokit/request, and @octokit/request-error ReDoS chain - Override @tootallnate/once to 3.0.1 to fix the make-fetch-happen / http-proxy-agent vulnerability chain - Override diff to ^8.0.4 scoped to @yeoman/conflicter to fix the jsdiff DoS vulnerability - Override brace-expansion to 1.1.14 scoped to minimatch@3.1.5 to fix the brace-expansion v1.x DoS without affecting safe v2.x instances - Override tmp to 0.2.5 scoped to external-editor to fix the tmp symlink vulnerability (tmpNameSync API is compatible across versions) Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
|
/review |
Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
There was a problem hiding this comment.
🤖 PR Reviewer
This is a package-lock.json dependency update diff. It upgrades numerous Adobe AIO CLI plugins and their transitive dependencies (inquirer, oclif/core versions, etc.), bumps minimum Node.js requirements from >=18 to >=20 for several packages, removes some deprecated/replaced sub-dependencies (sprintf-js, node-fetch, ansi-escapes in some packages), and consolidates duplicate nested module entries. This is a routine dependency maintenance update with no application code changes.
✅ LGTM! This PR looks good to merge.
⚠️ Diff exceeded 100 000 chars and was truncated — some files may not have been reviewed.
💡 How to re-trigger
Comment /review or /pr-reviewer on this PR
Codecov Report✅ All modified and coverable lines are covered by tests. 📢 Thoughts on this report? Let us know! |
… conflict The nested override was hoisting brace-expansion@1.1.14 to the top level, preventing minimatch@10.x (used by @oclif/core and npm@11) from resolving brace-expansion@^5.0.5. Without the override, npm auto-resolves brace-expansion@1.1.14 for minimatch@3.x subtrees and brace-expansion@5.0.6 for minimatch@10.x — all safe, zero vulnerabilities. Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
There was a problem hiding this comment.
🤖 PR Reviewer
This is a package-lock.json dependency update diff that bumps multiple Adobe CLI plugins and their transitive dependencies to newer versions. The changes include Node.js engine requirement bumps from >=18 to >=20 across several packages, @oclif/core upgrades from v1/v2 to v4, and various @inquirer/* package updates. This is a routine dependency maintenance update with no code logic changes to review.
✅ LGTM! This PR looks good to merge.
⚠️ Diff exceeded 100 000 chars and was truncated — some files may not have been reviewed.
💡 How to re-trigger
Comment /review or /pr-reviewer on this PR
Multiple bundled plugins each initialize roarr, which adds an error listener on process.stderr per initialization. With 17 plugins the default limit of 10 is exceeded. Raise the limit on stderr/stdout to 30 at the entry point before any plugins are loaded. Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
There was a problem hiding this comment.
🤖 PR Reviewer
This diff is a large dependency upgrade across multiple Adobe CLI plugins, bumping them to newer versions and consolidating on @oclif/core ^4.x. The only non-lockfile code change is in bin/run, which raises EventEmitter listener limits for process.stderr and process.stdout. The approach is functional but the magic number 30 is undocumented and stdout listeners are unlikely to need raising for the same roarr reason cited.
✅ LGTM! This PR looks good to merge.
⚠️ Diff exceeded 100 000 chars and was truncated — some files may not have been reviewed.
💡 How to re-trigger
Comment /review or /pr-reviewer on this PR
Co-authored-by: github-actions[bot] <41898282+github-actions[bot]@users.noreply.github.com>
There was a problem hiding this comment.
🤖 PR Reviewer
The bin/run file contains a critical bug: the stderr comment and setMaxListeners call are duplicated verbatim, and a separate stdout setMaxListeners call was added without explanation. The duplicate comment block should be removed. The rest of the diff is routine dependency version bumps with no concerns.
🔄 1 re-raised suggestion(s) from previous review
⚠️ Diff exceeded 100 000 chars and was truncated — some files may not have been reviewed.
💡 How to re-trigger
Comment /review or /pr-reviewer on this PR
There was a problem hiding this comment.
🤖 PR Reviewer
The previous suggestion about documenting both stderr and stdout has been addressed: the comment now correctly explains that roarr attaches listeners to both process.stderr and process.stdout, and the duplicate block has been removed. The package-lock.json changes are routine dependency updates with no issues. The bin/run change is clean and well-commented.
✅ LGTM! This PR looks good to merge.
⚠️ Diff exceeded 100 000 chars and was truncated — some files may not have been reviewed.
💡 How to re-trigger
Comment /review or /pr-reviewer on this PR
Summary
@oclif/plugin-pluginsto^5.4.64to bring innpm@11, which ships fixed bundled versions ofbrace-expansion(2.x),picomatch, andip-addressoverridesinpackage.jsonto fix all remaining vulnerability chains without using--forceor downgrading any dependencyOverrides added
@octokit/rest: "^20.0.2"@octokit/plugin-paginate-restReDoS,@octokit/requestReDoS,@octokit/request-errorReDoS@tootallnate/once: "3.0.1"http-proxy-agent→make-fetch-happenchain@yeoman/conflicter: { diff: "^8.0.4" }jsdiffDoS inparsePatch/applyPatch(scoped to avoid breaking other diff consumers)minimatch@3.1.5: { brace-expansion: "1.1.14" }brace-expansionv1 DoS (scoped to avoid downgrading safe v2.x instances)external-editor: { tmp: "0.2.5" }tmpsymlink vulnerability (tmpNameSyncAPI is compatible)Result
npm auditreports 0 vulnerabilities (down from 47).Test plan
npm audit→ 0 vulnerabilitiesnpm testpassesaioCLI smoke test (help, version)🤖 Generated with Claude Code