Patch vulnerable ip-address dependency in gh-aw-node - #51719
Conversation
Co-authored-by: pelikhan <4175913+pelikhan@users.noreply.github.com>
There was a problem hiding this comment.
Pull request overview
Updates the gh-aw-node image to remediate GHSA-mwp4-54f8-5fhr.
Changes:
- Installs
ip-address@^10.3.1in the dependency overlay. - Copies the patched package into npm’s bundled dependencies.
Show a summary per file
| File | Description |
|---|---|
actions/setup/js/Dockerfile.safe-outputs-mcp |
Adds the patched ip-address overlay. |
Review details
Tip
Add a code-review agent skill for context-aware, tailored reviews. Learn more in the docs.
- Files reviewed: 1/1 changed files
- Comments generated: 0
- Review effort level: Balanced
|
✅ Test Quality Sentinel completed test quality analysis. No test files were added or modified in this PR. Test Quality Sentinel skipped.
|
|
🧠 Matt Pocock Skills Reviewer has completed the skills-based review. ✅
|
|
✅ Ponytail Reviewer completed successfully! Reviewed PR #51719 for over-engineering. The diff is a minimal, consistent addition (4/-3 lines) that follows the exact same pattern already used for brace-expansion, tar, and undici overlay patches. Lean already. Ship.
|
|
✅ Design Decision Gate 🏗️ completed the design decision gate check. No ADR enforcement needed: PR does not have the 'implementation' label and has ≤100 new lines of code in business logic directories.
|
|
✅ PR Code Quality Reviewer completed the code quality review. Warning Threat Detection Engine Failure — The analysis engine could not complete. This is a tooling failure, not a security finding. What happenedThe threat detection engine failed to produce results. Review the workflow run logs for details. Warning Firewall blocked 1 domainThe following domain was blocked by the firewall during workflow execution:
To allow these domains, add them to the network:
allowed:
- defaults
- "api.individual.githubcopilot.com"See Network Configuration for more information.
|
There was a problem hiding this comment.
The patch correctly adds ip-address@^10.3.1 to address GHSA-mwp4-54f8-5fhr, following the same well-established pattern already used for brace-expansion, tar, and undici. No issues found.
🧵 Reviewed using Impeccable skills by Impeccable Skills Reviewer · sonnet46 · 11.8 AIC · ⌖ 7.07 AIC · ⊞ 5.4K
There was a problem hiding this comment.
Skills-Based Review 🧠
Applied /diagnosing-bugs — patch is clean and follows the established overlay pattern.
📋 Key Themes & Highlights
Key Themes
- Consistent remediation pattern: change follows the exact same overlay-install-then-copy idiom already used for
brace-expansion,tar, andundici. - Comment updated: CVE advisory reference (GHSA-mwp4-54f8-5fhr) added alongside the fix — good provenance tracking.
- Potential gap:
ip-address@10.xhas transitive runtime deps (jsbn,sprintf-js). Thecp -rfoverlays only the top-level package directory. If npm's bundled node_modules does not already have those subdeps at the right version, the patchedip-addressmay fail to load. See inline comment.
Positive Highlights
- CVE advisory reference included in the comment block
--no-saveflag preserves offline-install discipline- Alphabetical ordering maintained in both the comment and the
cpcommands
🧠 Reviewed using Matt Pocock's skills by Matt Pocock Skills Reviewer · sonnet46 · 17 AIC · ⌖ 7.49 AIC · ⊞ 7.1K
Comment /matt to run again
| && npm_modules="$(npm root -g)/npm/node_modules" \ | ||
| && cp -rf "$tmpdir/node_modules/brace-expansion/." "$npm_modules/brace-expansion/" \ | ||
| && cp -rf "$tmpdir/node_modules/ip-address/." "$npm_modules/ip-address/" \ | ||
| && cp -rf "$tmpdir/node_modules/tar/." "$npm_modules/tar/" \ |
There was a problem hiding this comment.
[/diagnosing-bugs] ip-address@10.x depends on jsbn and sprintf-js at runtime. The cp -rf overlay copies only the top-level ip-address package; if npm's bundled node_modules lacks those transitive deps (or has incompatible versions), the patched module will throw at load time.
💡 Suggested verification
After the cp, verify the transitive deps are resolvable inside the npm prefix:
&& node -e "require('ip-address')" \If that throws, you may need to overlay jsbn and sprintf-js with the same install-and-copy pattern.
@copilot please address this.
|
@copilot Quick triage nudge for this PR. Please refresh the branch if GitHub can update it cleanly, address the maintainer-facing feedback below, and run the Open review context (newest first):
Branch refresh was requested.
|
Co-authored-by: gh-aw-bot <259018956+gh-aw-bot@users.noreply.github.com>
Addressed the open review thread. |
The published
gh-aw-nodeimage includes a vulnerable npm-bundledip-addressdependency. Update the image’s existing dependency overlay to install a patched version (10.3.1+).ip-address@^10.3.1to the temporary npm overlay installation.node_modules.