Snyk July Review #7895
Snyk July Review #7895
Conversation
|
It does look like the smartystreets upgrade might have broken address validation and suggestions On dev5, this is what it looks like for this address On dev4, it says it cannot be verified |
|
I'll do some research to see if I can fix it. Or I may remove smartystreets for this review and add it onto the next one |
|
After further research the |
|
Reverted
|
| implementation 'org.apache.tomcat.embed:tomcat-embed-core:10.1.25' | ||
| implementation 'org.apache.tomcat.embed:tomcat-embed-websocket:10.1.19' | ||
| implementation 'org.springframework.security:spring-security-web:6.1.7' |
There was a problem hiding this comment.
I haven't done a Snyk PR before so just to make sure I understand: are we pinning these dependencies to specific versions because others have vulnerabilities? or do these just need to be upgraded?
There was a problem hiding this comment.
Based on the Snyk scans the previous version of these packages introduced vulnerabilities, so we are upgrading them to to fix the vulnerability. See references below that explain each vulnerability:
There was a problem hiding this comment.
Moving forward I'll add the reference to the vulnerability in the description to provide more context
frontend/package.json
Outdated
| @@ -22,6 +22,8 @@ | |||
| "@types/testing-library__jest-dom": "^5.14.5", | |||
| "@uswds/uswds": "^3.8.0", | |||
| "apollo-upload-client": "^17.0.0", | |||
| "axios": "^1.7.2", | |||
There was a problem hiding this comment.
same question here - what's axios for?
There was a problem hiding this comment.
See reference below that explains vulnerability:
There was a problem hiding this comment.
Not sure if this is what you were asking @mehansen but did a yarn why axios and looks like smartystreets-javascript-sdk depends on it
frontend/package.json
Outdated
| @@ -254,6 +256,7 @@ | |||
| ".stories.tsx", | |||
| "<rootDir>/src/app/testQueue/constants.ts", | |||
| "<rootDir>/src/patientApp/timeOfTest/constants.ts" | |||
| ] | |||
| ], | |||
| "transformIgnorePatterns": ["node_modules/(?!axios)/"] | |||
There was a problem hiding this comment.
Do we need this? 🤔 I ran jest without this and it looked to work for me and I don't think we are importing axios anywhere
There was a problem hiding this comment.
This was for the updated version of smartystreets-javascript-sdk, its no longer needed since we aren't updating that package in this PR. I removed it
frontend/package.json
Outdated
| @@ -22,6 +22,8 @@ | |||
| "@types/testing-library__jest-dom": "^5.14.5", | |||
| "@uswds/uswds": "^3.8.0", | |||
| "apollo-upload-client": "^17.0.0", | |||
| "axios": "^1.7.2", | |||
| "babel-jest": "^29.7.0", | |||
There was a problem hiding this comment.
nit: can we move this under the devDependencies?
There was a problem hiding this comment.
Change made
There was a problem hiding this comment.
Smoke test on dev4 looked good -- left a couple comments/questions for you, @shanice-skylight! Thank you for your work on this!
|
BACKEND PULL REQUEST
Related Issue
Snyk Review - July 2024
Changes Proposed
Update packages in package.json and yarn.lock (frontend)
Update packages in build.gradle and gradle.lockfile (backend).
Testing
A smoke test of basic functionality should be accomplished before merging.
This PR has already been verified on
dev4