-
Notifications
You must be signed in to change notification settings - Fork 182
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
USWDS-Site - POAM: October '24 #2867
Conversation
"fancy-log": "^2.0.0", | ||
"gulp": "^4.0.2", | ||
"gulp-cli": "^2.3.0", | ||
"gulp": "^5.0.0", |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@mahoneycm have we tested and confirmed fonts/assets corruption issue here?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
No perceived changes from my initial testing but let me give this another sweep to make sure 👍
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Looks like there was an error with some blog post images being encoding in the copyDocImages
function!
Resolved in fdc8882 and I haven't found any additional gulp.src()
functions that need to be updated.
Builds currently failing due to Node LTS updgrade
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@mahoneycm build issues should be resolved now, but we have a new snyk error being flagged.
Issues with no direct upgrade or patch:
✗ Missing Release of Resource after Effective Lifetime [Medium Severity][https://security.snyk.io/vuln/SNYK-JS-INFLIGHT-6095116] in [email protected]
introduced by @uswds/[email protected] > [email protected] > [email protected] > [email protected] > [email protected]
No upgrade or patch available
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@mejiaj thanks for flagging! I looked into seeing if we can resolve the issue in compile. There is already a compile issue automatically created by Snyk but it's a breaking change. Del v7 and up requires ESM syntax.
Added the issue back to our snyk ignore for now but perhaps we should consider updating to ESM in the future?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
question (non-blocking): I don't think this needs to block this PR, but I wonder if the gulp dependency is needed here since the same version is included in uswds compile.
Looks like #2934's getting blocked by this but I'm not confident I can appropriately review this one. @heymatthenry, could you take a look? |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM!
- Confirmed I can run a clean
npm install
,npm start
,npm run build
, andnpm run test
without error. - Confirmed that the dependabot alert requirements have been addressed
A couple things:
- I had one non-blocking question about needing the gulp dependency below.
- Can you update the dependency and gem tables in the PR description? They don't seem to match what I see in the changed files
"fancy-log": "^2.0.0", | ||
"gulp": "^4.0.2", | ||
"gulp-cli": "^2.3.0", | ||
"gulp": "^5.0.0", |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
question (non-blocking): I don't think this needs to block this PR, but I wonder if the gulp dependency is needed here since the same version is included in uswds compile.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM! Thanks, @mahoneycm!
Summary
Installed available minor and patch updates for direct dependencies.
Related issue
USWDS-Team - POAM: October 2024
Preview link
Preview link →
Resolves https://github.com/uswds/uswds-site/security/dependabot/82
Resolves https://github.com/uswds/uswds-site/security/dependabot/81
Resolves https://github.com/uswds/uswds-site/security/dependabot/66
Major changes
Dependency updates
Before:
After:
Package updates
Gem updates
Testing and review
npm install.
npm run build
and confirm there are no build errors.npm start
and confirm there are no build errors.npm test
and confirm there are no errors.