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

Upgrade dependency versions to fix audit scanning #210

Merged
merged 4 commits into from
May 20, 2024
Merged

Conversation

BenjiTheC
Copy link
Contributor

@BenjiTheC BenjiTheC commented May 15, 2024

Description

This PR upgrades packages version to address security scanning issues from rush-pnpm audit. It fixes all the issues but one:

┌─────────────────────┬────────────────────────────────────────────────────────┐
│ moderate            │ Regular Expression Denial of Service (ReDoS) in lodash │
├─────────────────────┼────────────────────────────────────────────────────────┤
│ Package             │ lodash.trimend                                         │
├─────────────────────┼────────────────────────────────────────────────────────┤
│ Vulnerable versions │ <=4.5.1                                                │
├─────────────────────┼────────────────────────────────────────────────────────┤
│ Patched versions    │ <0.0.0                                                 │
├─────────────────────┼────────────────────────────────────────────────────────┤
│ Paths               │ ../../packages/services/installer > [email protected]  │
│                     │ > [email protected] > [email protected]                   │
├─────────────────────┼────────────────────────────────────────────────────────┤
│ More info           │ https://github.com/advisories/GHSA-29mw-wpgm-hmr9      │
└─────────────────────┴────────────────────────────────────────────────────────┘
1 vulnerabilities found
Severity: 1 moderate

The package [email protected] is the latest version of package and has not been upgraded since. In my opinion, it's not a security threat because of the way (and the frequency) we use this package in our code base.

Packages with major version upgrade:

Version upgrades are done via running rush upgrade-interactive --make-consistent, when a major version is available, rush will present only major version upgrade option for the package, investigation has been done to make sure there is no breaking change introduced.

Type of change

  • Bug fix (non-breaking change which fixes an issue)
  • New feature (non-breaking change which adds functionality)
  • Breaking change (fix or feature that would cause existing functionality to not work as expected)
  • Refactor (existing code being refactored)
  • This change includes a documentation update

Submission Checklist

  • CI dry-run passing
  • Integration tests passing locally
  • Change logs generated
    • N/A
Additional Notes:

Copy link
Contributor

@pcozzi pcozzi left a comment

Choose a reason for hiding this comment

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

Can you run integration tests on this change?

@@ -6,7 +6,7 @@
"scripts": {
"clean": "npx shx rm -rf dist tsconfig.tsbuildinfo bundle.zip .rush .nyc_output *.log",
"lint": "npx eslint . --ext '.ts'",
"build": "npx tsc -b && npx shx cp -R 'src/config' 'dist/'",
"build": "npx tsc -p tsconfig.json && npx shx cp -R 'src/config' 'dist/'",
Copy link
Contributor

Choose a reason for hiding this comment

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

Can you add a note about why this change exists?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

This is an attempt to make command package pass the build. The rest of the packages use npx tsc -p tsconfig.json for build, so I kept this change in the PR.

@@ -114,6 +116,5 @@ export interface DeploymentPolicy {
};
}

export type AbortConfigFailureType = 'FAILED' | 'REJECTED' | 'TIMED_OUT' | 'ALL';
Copy link
Contributor

Choose a reason for hiding this comment

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

Am I reading this right that we're now getting this type from their library instead of defining it ourselves? Does the same not apply to the other exported types below? If so, can you open an issue to update them to use the library values instead of defining them in our code?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I believe the newer version of the dependent AWS SDK package updated their typing from a general string to more strict enum, which results in this incompatibility. And you are right, this can apply to other types defined in the code base. I have created #211 to track this

@BenjiTheC
Copy link
Contributor Author

BenjiTheC commented May 20, 2024

@pcozzi Can you run integration tests on this change?

Integration tests are passing

@BenjiTheC BenjiTheC merged commit 9fcfc95 into aws:main May 20, 2024
1 check passed
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