chore: resolve open dependabot security alerts#1544
Conversation
MattIPv4
left a comment
There was a problem hiding this comment.
Same thought as in the other PR, do we need to keep the overrides, or can they be removed now that the lockfiles have been bumped?
There was a problem hiding this comment.
Code Review
This pull request updates several dependencies across the monorepo, including axios, fast-xml-builder, fast-uri, and json-logic-engine, and introduces dependency overrides to address security vulnerabilities. Feedback identifies that the lodash version 4.18.0 specified in the overrides does not exist and should be corrected to 4.17.21. Furthermore, the overrides in sub-packages are redundant as they are already defined in the root package.json and will be ignored by npm. Additionally, the major version update for json-logic-engine in flagd-core should be synchronized with the root configuration to maintain consistency across the workspace.
ca0584f to
7646a43
Compare
|
Removing the The following alerts remain open, all tied to dev tooling we don't control:
These will resolve as |
|
The bump to the v3 lockfile makes this PR pretty unreviewable, and it looks like the lockfile isn't actually valid? How was it generated? |
|
@MattIPv4 yea the lock file change does make this hard to review. I'll try and revert that, it seemed fairly safe based on this research: |
|
Yeh, I think v2 -> v3 should be safe, but it doesn't seem like it was done correctly here? I'd probably lean toward just resolving the security alerts in this PR, keeping it v2, and then doing the v2 -> v3 in a dedicated PR with a reproducible command so we can verify it is just a format change and not including any dependency changes. |
ea61101 to
8138b40
Compare
Signed-off-by: Jonathan Norris <jonathan.norris@dynatrace.com>
Signed-off-by: Jonathan Norris <jonathan.norris@dynatrace.com>
…e naturally Signed-off-by: Jonathan Norris <jonathan.norris@dynatrace.com>
Signed-off-by: Jonathan Norris <jonathan.norris@dynatrace.com>
Signed-off-by: Jonathan Norris <jonathan.norris@dynatrace.com>
Signed-off-by: Jonathan Norris <jonathan.norris@dynatrace.com>
…t-uri/fast-xml-builder Signed-off-by: Jonathan Norris <jonathan.norris@dynatrace.com>
Signed-off-by: Jonathan Norris <jonathan.norris@dynatrace.com>
…fjs/qs Signed-off-by: Jonathan Norris <jonathan.norris@dynatrace.com>
4efeb1d to
ea74ab0
Compare
Signed-off-by: Jonathan Norris <jonathan.norris@dynatrace.com>
| "@nx/devkit": "22.7.5", | ||
| "@nx/eslint": "22.7.5", | ||
| "@nx/eslint-plugin": "22.7.5", | ||
| "@nx/jest": "22.7.5", | ||
| "@nx/js": "22.7.5", | ||
| "@nx/plugin": "22.7.5", | ||
| "@nx/rollup": "22.7.5", | ||
| "@nx/web": "22.7.5", | ||
| "@nx/workspace": "22.7.5", |
There was a problem hiding this comment.
Did you use the nx migrate command for this? If not, I would suggest not touching NX deps. They often come with specific migration scripts only run with the above command.
There was a problem hiding this comment.
yea, nx migrate wasn't used here. The intent was to pull in tmp@0.2.6 (alert #264), but nx@20.3.1 already declares tmp: ~0.2.1, which covers 0.2.6 — so a lockfile-only bump would've been enough. I'll revert the NX changes and use npm update tmp instead.
There was a problem hiding this comment.
actually, scratch that — nx@22.7.2 (already on the branch from a prior merge) pins tmp at exactly 0.2.4, so there's no range to exploit with a lockfile-only bump. The 22.7.2 → 22.7.5 bump was necessary to get tmp@0.2.6. I ran nx migrate 22.7.5 locally to verify — no migration scripts exist between those two versions, so the bare version bump is equivalent.
Summary
Dependabot Alerts Resolved
Remaining Open Alerts (dev tooling only)
These will resolve as `nx` and `verdaccio` release updates with patched transitive deps.
Notes