Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Update packages to fix vulnerabilities #3712

Closed

Conversation

CalebUsadi
Copy link
Contributor

📑 Summary

Update parse-url and d3 packages to fix security vulnerabilities.

Resolves #3705

@github-actions github-actions bot added the Discussion Discussion which may lead to separate issues or PRs label Oct 23, 2022
@CalebUsadi CalebUsadi force-pushed the fix/3705_package_updates branch 2 times, most recently from 447215f to 309a137 Compare October 25, 2022 01:39
@huineng
Copy link

huineng commented Oct 25, 2022

Thanks, this is an important fix, if this can be merged, that would be great

@sidharthv96
Copy link
Member

@CalebUsadi can you resolve the conflicts?

Copy link
Member

@sidharthv96 sidharthv96 left a comment

Choose a reason for hiding this comment

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

LGTM, as long as the tests pass.

Copy link
Member

@aloisklink aloisklink left a comment

Choose a reason for hiding this comment

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

This won't fix the npm audit security warnings for people that npm install mermaid, e.g. #3705 will NOT be fixed.

To fix that, we'd need to replace dagre-d3 with a version that has updated dependencies (maybe https://www.npmjs.com/package/dagre-d3-es ?). This is because downstream users won't see the pnpm.overrides bit in our package.json, or even if they do, npm/yarn will ignore it.

However, this PR should still fix bundle vulnerabilities.

@sidharthv96
Copy link
Member

sidharthv96 commented Nov 1, 2022

Seems like dagre-d3-esm doesn't expose the functions we need directly. Although it seems to be there in their code.

[0] 'intersect' is not exported by 'node_modules/.pnpm/dagre-d3-es@7.0.1/node_modules/dagre-d3-es/src/index.js'

Raised tbo47/dagre-es#5

sidharthv96 and others added 3 commits November 1, 2022 19:37
* develop: (62 commits)
  chore(docs): Update live editor links
  update user story link
  chore(deps): update all non-major dependencies
  chore(deps): update all non-major dependencies
  docs(git): Regenerate
  docs(git): Add a quoted branch name example
  feat(gantt): Add option 'tickInterval' for custom tick interval
  chore: Update bug report template
  fix(git): Support quoted branch names
  Convert attr to classed
  Convert attr to style
  Ensure example code and rendered output are synced
  Change fill attribute to style.
  chore(deps): update all non-major dependencies
  chore(deps): pin dependencies
  fix: Add default arg to options
  Color fix for default nodes in mindmap, line uses inv color
  add basic render (cypress) test for classDiagram-v2 too
  add basic render (cypress test for notes
  add renedering (cypress) tests
  ...
@aloisklink
Copy link
Member

Seems like dagre-d3-esm doesn't expose the functions we need directly.

I've just tested it, and it does export all the functions we need, although the import paths are a bit different. See #3809 for a working version. We have to import each function individually, which is actually better, since then ESM/tree-shaking can occur to reduce our bundle sizes.

E.g.:

// before
import dagreD3 from 'dagre-d3';
dagreD3.intersect.polygon(node, points, point);

// after
import { intersectPolygon } from 'dagre-d3-es/src/dagre-js/intersect/intersect-polygon';
intersectPolygon(node, points, point);

@tbo47
Copy link

tbo47 commented Nov 18, 2022

Seems like dagre-d3-esm doesn't expose the functions we need directly.

I've just tested it, and it does export all the functions we need, although the import paths are a bit different. See #3809 for a working version. We have to import each function individually, which is actually better, since then ESM/tree-shaking can occur to reduce our bundle sizes.

E.g.:

// before
import dagreD3 from 'dagre-d3';
dagreD3.intersect.polygon(node, points, point);

// after
import { intersectPolygon } from 'dagre-d3-es/src/dagre-js/intersect/intersect-polygon';
intersectPolygon(node, points, point);

I think it will solve your problem https://github.com/tbo47/dagre-es/pull/6/files

Do you mind testing by manually copy and paste the modifications in my PR into your local node_modules/dagre-d3-es/src/?

If it solves your problem I will merge and release a new version.

This should work:

import dagreD3 from 'dagre-d3-es';
dagreD3.intersect.polygon(node, points, point);

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Discussion Discussion which may lead to separate issues or PRs
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Multiple npm security warnings
6 participants