Skip to content

Comments

build(deps): add security resolutions for vulnerable npm packages#1740

Open
motsc wants to merge 8 commits intomainfrom
fix/npm-security-resolutions
Open

build(deps): add security resolutions for vulnerable npm packages#1740
motsc wants to merge 8 commits intomainfrom
fix/npm-security-resolutions

Conversation

@motsc
Copy link
Contributor

@motsc motsc commented Feb 13, 2026

Summary

Addresses npm security vulnerabilities in transitive dependencies. Prefer direct dependency upgrades over broad resolutions where possible.

Changes

Direct upgrade:

  • @slack/webhook: ^6.1.0^7.0.0 — v7 natively uses axios v1, eliminating the axios@0.21.4 SSRF/redirect vulnerabilities. Only breaking change in v7 is dropping Node <18 (we're on Node 22).

Resolutions for transitive deps with no direct upgrade path:

  • fast-xml-parser: ^4.4.0 — fixes prototype pollution (High)
  • systeminformation: ^5.24.0 — fixes command injection (High)

Removed/Not Done

  • axios resolution removed — covered by the @slack/webhook upgrade instead
  • tar resolution removed — was a v6→v7 major jump on build-only tools (cacache, node-gyp); not present in the production image
  • glob resolution removed — was breaking test coverage tooling (test-exclude@6 depends on glob@^7)

Related

Follow-up to #1731 which addressed base image vulnerabilities (Node, Go, ClickHouse).

@vercel
Copy link

vercel bot commented Feb 13, 2026

The latest updates on your projects. Learn more about Vercel for GitHub.

Project Deployment Actions Updated (UTC)
hyperdx-v2-oss-app Ready Ready Preview, Comment Feb 19, 2026 7:14pm

Request Review

@changeset-bot
Copy link

changeset-bot bot commented Feb 13, 2026

⚠️ No Changeset found

Latest commit: 40ec609

Merging this PR will not cause a version bump for any packages. If these changes should not result in a new version, you're good to go. If these changes should result in a version bump, you need to add a changeset.

This PR includes no changesets

When changesets are added to this PR, you'll see the packages that this PR includes changesets for and the associated semver types

Click here to learn what changesets are, and how to add one.

Click here if you're a maintainer who wants to add a changeset to this PR

@claude
Copy link

claude bot commented Feb 13, 2026

PR Review: build(deps): add security resolutions for vulnerable npm packages

Overall this is a well-structured security hardening PR. A couple of items worth flagging:

  • ⚠️ VEX justification field is semantically incorrect → All 20 statements use inline_mitigations_already_exist, but the impact statements describe an architectural constraint ("localhost-only deployment"), not inline code mitigations. The correct VEX justification code would be vulnerable_code_not_in_execute_path or vulnerable_code_cannot_be_controlled_by_adversary. Using the wrong code could cause VEX-aware scanners to misclassify these exceptions.

  • ⚠️ Verify CVE IDs in .vex/openssl-mongodb.vex.json → Several CVE IDs appear suspicious: CVE-2025-9230, CVE-2025-15467, and the sequential trio CVE-2025-69419/69420/69421. Worth confirming these exist in the NVD database before suppressing them—suppressing fabricated or incorrect CVE IDs could mask real vulnerabilities from future scans.

  • ℹ️ concurrently added as a production dependency → It's added to packages/api/package.json purely to expose the binary for the shell entrypoint scripts. This works fine with Yarn workspaces hoisting, but it's a slightly unusual pattern—an alternative would be a root-level devDependency. Low priority, but worth noting for future maintainers.

✅ The @slack/webhook v7 upgrade (eliminating axios@0.21.x), removal of npm/yarn from production images, privilege drop via su-exec, and supply chain attestation flags (--sbom/--provenance) are all solid improvements.

@github-actions
Copy link
Contributor

github-actions bot commented Feb 13, 2026

E2E Test Results

All tests passed • 67 passed • 4 skipped • 858s

Status Count
✅ Passed 67
❌ Failed 0
⚠️ Flaky 1
⏭️ Skipped 4

Tests ran across 4 shards in parallel.

View full report →

Add yarn resolutions to address high and medium severity vulnerabilities
in transitive dependencies:

- tar: ^7.4.4 (fixes 3 high severity CVEs)
- axios: ^1.7.0 (fixes 3 high severity CVEs - SSRF, redirect issues)
- fast-xml-parser: ^4.4.0 (fixes 1 high - prototype pollution)
- systeminformation: ^5.24.0 (fixes 1 high - command injection)
- glob: ^10.4.6 (fixes 1 high - ReDoS)

These resolutions force secure versions of packages that are pulled in
as transitive dependencies, reducing Docker Scout vulnerability count
by ~6 vulnerabilities.

Co-Authored-By: Claude Sonnet 4.5 <noreply@anthropic.com>
Copy link
Contributor

@MikeShi42 MikeShi42 left a comment

Choose a reason for hiding this comment

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

Just noting for further investigation, axios is a major jump from 0 -> 1, tar from 6 to 7.

Looks like Axios is primarily due to an outdated @slack/webhook dependency. We should instead upgrade to @slack/webhook v7, which does not have any listed breaking changes

Strip npm/yarn from production images (~15-20 CVEs removed):
- Add concurrently to @hyperdx/api dependencies so entry scripts can
  use ./node_modules/.bin/concurrently instead of a global install
- Remove npm, npx, corepack, yarn, yarnpkg binaries and modules from
  both prod and all-in-one-base stages (unused at runtime)

Update curl and busybox in all-in-one image (~7 CVEs removed):
- Upgrade curl and busybox via apk --upgrade in the MongoDB RUN block

Add VEX exceptions for OpenSSL 1.1.1k (~55 CVEs suppressed):
- New .vex/openssl-mongodb.vex.json marks 20 OpenSSL CVEs as
  not_affected (MongoDB is localhost-only, no external TLS cert
  processing); reference via: docker scout cves --vex-location /etc/vex/
- Copy VEX files into image at /etc/vex/

Add supply chain attestations to release builds (+15 policy points):
- Replace --squash with --sbom=true --provenance=true on all 6
  docker buildx build release targets in Makefile
- --squash is incompatible with attestation flags; multi-stage build
  already minimizes layers

Drop app process privileges via su-exec in all-in-one:
- Install su-exec in all-in-one-base stage
- Run HyperDX app via su-exec otel in entry.local.base.sh; system
  services (ClickHouse, MongoDB, OTel) continue as root
- chown /app to otel user so the app process has full file access
# Start HyperDX app
concurrently \
# Start HyperDX app (run as otel user via su-exec)
su-exec otel ./node_modules/.bin/concurrently \
Copy link
Member

Choose a reason for hiding this comment

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

Could you elaborate on what this is for? It seems a bit odd to run the main application node process as the otel user (which is intended for the collector).

Comment on lines 304 to 322
@@ -319,7 +319,7 @@ release-local-nightly:
.PHONY: release-all-in-one-nightly
release-all-in-one-nightly:
@echo "Building and pushing nightly tag ${ALL_IN_ONE_IMAGE_NAME_DOCKERHUB}:${IMAGE_NIGHTLY_TAG}..."; \
docker buildx build --squash . -f ./docker/hyperdx/Dockerfile \
docker buildx build --sbom=true --provenance=true . -f ./docker/hyperdx/Dockerfile \
Copy link
Member

@wrn14897 wrn14897 Feb 19, 2026

Choose a reason for hiding this comment

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

does this break the build? I don't think we should remove --squash right?

Copy link
Member

@wrn14897 wrn14897 left a comment

Choose a reason for hiding this comment

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

Some changes are breaking the build. Please review the comments.

"@opentelemetry/host-metrics": "^0.35.5",
"@opentelemetry/sdk-metrics": "^1.30.1",
"@slack/webhook": "^6.1.0",
"@slack/webhook": "^7.0.0",
Copy link
Member

@wrn14897 wrn14897 Feb 19, 2026

Choose a reason for hiding this comment

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

Tested slack webhook and it worked fine

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.

3 participants