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.

Sign up for GitHub

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

Jump to bottom

[CVE] Bump tsd to 0.21.0 #1770

Merged
merged 2 commits into from
Jun 23, 2022
Merged

[CVE] Bump tsd to 0.21.0 #1770

merged 2 commits into from
Jun 23, 2022

Conversation

joshuarrrr
Copy link
Member

@joshuarrrr joshuarrrr commented Jun 20, 2022 •
edited
Loading

Description

  • Partially addresses: CVE-2022-33987
  • bumps tsd dependencies from 0.16.0 to 0.21.0 (latest)
    • Release change log
    • Breaking changes:
      • 0.17.0 includes "Require Node.js 12" - this is not a breaking change as we've already update to node 14.19.1

tsd v0.17.0 removes dependency chain:

  • update-notifier
  • latest-version
  • package-json
  • got

Issues Resolved

Partial fix for #1764

Check List

  • New functionality includes testing.
    • All tests pass
      • yarn test:jest
      • yarn test:jest_integration
      • yarn test:ftr
  • New functionality has been documented.
  • Commits are signed per the DCO using --signoff

@joshuarrrr
[CVE] Bump tsd to 0.21.0
f04eea0 
which removes dependency chain:
- `update-notifier`
- `latest-version`
- `package-json`
- `got`

Signed-off-by: Josh Romero <[email protected]>
@joshuarrrr joshuarrrr requested a review from a team as a code owner June 20, 2022 22:12
@codecov-commenter
Copy link

Codecov Report

Merging #1770 (f04eea0) into main (1dda730) will increase coverage by 0.00%.
The diff coverage is n/a.

@@           Coverage Diff           @@
##             main    #1770   +/-   ##
=======================================
  Coverage   67.50%   67.51%           
=======================================
  Files        3073     3073           
  Lines       59068    59069    +1     
  Branches     8963     8963           
=======================================
+ Hits        39876    39879    +3     
+ Misses      17008    17007    -1     
+ Partials     2184     2183    -1
Impacted Files Coverage Δ
src/cli/cluster/worker.ts 82.35% <0.00%> (ø)
src/cli/cluster/cluster.mock.ts 100.00% <0.00%> (ø)
...ic/application/models/sense_editor/sense_editor.ts 64.88% <0.00%> (+0.88%) ⬆️

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 1dda730...f04eea0. Read the comment docs.

@tmarkley
Copy link
Contributor

See #1369 for an example of all of the details that should be included in a CVE PR, including adding a link to the changelog and calling out any breaking changes there.

@joshuarrrr
Copy link
Member Author

See #1369 for an example of all of the details that should be included in a CVE PR, including adding a link to the changelog and calling out any breaking changes there.

Updated in PR description; will copy to commit message when merging.

@joshuarrrr joshuarrrr requested a review from tmarkley June 22, 2022 19:38
tmarkley
tmarkley approved these changes Jun 22, 2022
View reviewed changes
Copy link
Contributor

@tmarkley tmarkley left a comment

Choose a reason for hiding this comment

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

Are we targeting v2.1?

@joshuarrrr
Copy link
Member Author

Are we targeting v2.1?

If we can, I think that'd be best? Or are you saying that this may need to wait?

@kavilla
Copy link
Member

kavilla commented Jun 22, 2022

Are we targeting v2.1?

If we can, I think that'd be best? Or are you saying that this may need to wait?

Do we see this causing conflicts with plugins? If just for testing purposes at least under OpenSearch Project they use Cypress.

@joshuarrrr
Copy link
Member Author

Are we targeting v2.1?

If we can, I think that'd be best? Or are you saying that this may need to wait?

Do we see this causing conflicts with plugins? If just for testing purposes at least under OpenSearch Project they use Cypress.

I would imagine this affects more than tests - in the dashboards core, we use packages/opensearch-safer-lodash-set all over the place. But I'm not familiar about how/whether plugins take explicit dependencies on these internal overridden packages updated by this PR, so I don't have enough info to determine the potential impact.

@joshuarrrr
Copy link
Member Author

I did a bit more digging. I see no evidence that any of the internal packages updated by this PR are used in any other repo (either in the org or in GitHub globally). Unless there's some non-standard mechanism I'm not aware of for plugins to take explicit dependencies on these packages, this shouldn't cause any conflicts.

org searches for each of the packages getting a tsd bump:

  1. @elastic/safer-lodash-set
  2. @osd/apm-config-loader
  3. @osd/config-schema
  4. @osd/config
  5. @osd/std
  6. @osd/utility-types

(As a side note, it appears we chose not to update the package name from @elastic/safer-lodash-set to @osd/safer-lodash-set in this PR, but that seems inconsistent with some of the other package names).

I also did an org search for any other explicit tsd dependencies. I'll create an issue to also bump this in opensearch-js

@kavilla kavilla added medium severity Medium severity CVE cve Security vulnerabilities detected by Dependabot or Mend labels Jun 23, 2022
kavilla
kavilla approved these changes Jun 23, 2022
View reviewed changes
Copy link
Member

@kavilla kavilla left a comment

Choose a reason for hiding this comment

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

LGTM! I'll defer to your decision if to backport or not. I don't see a particular issue with backporting to 2.x for 2.1.

If you do, can you add the v2.1.0 label.

Thanks!

@joshuarrrr joshuarrrr merged commit d1cfe78 into opensearch-project:main Jun 23, 2022
@joshuarrrr joshuarrrr deleted the cve/bump-tsd branch June 23, 2022 19:09
opensearch-trigger-bot bot pushed a commit that referenced this pull request Jun 23, 2022
@joshuarrrr @github-actions
[CVE] Bump tsd from 0.16.0 to 0.21.0 (#1770)
3c18084 
- Partially addresses: CVE-2022-33987
- bumps `tsd` dependencies from `0.16.0` to `0.21.0` (latest)
  - [`tsd` release changelog](https://github.com/SamVerschueren/tsd/releases)
  - Breaking changes:
    - `0.17.0` includes "Require Node.js 12" - this is not a breaking change as we've already update to node `14.19.1`

`tsd v0.17.0` removes dependency chain:
- `update-notifier`
- `latest-version`
- `package-json`
- `got`

Partial fix for #1764

Signed-off-by: Josh Romero <[email protected]>
(cherry picked from commit d1cfe78)
@opensearch-trigger-bot opensearch-trigger-bot bot mentioned this pull request Jun 23, 2022
Merged
joshuarrrr added a commit that referenced this pull request Jun 24, 2022
@opensearch-trigger-bot @joshuarrrr
[CVE] Bump tsd from 0.16.0 to 0.21.0 (#1770) (#1786)
53bd77b 
- Partially addresses: CVE-2022-33987
- bumps `tsd` dependencies from `0.16.0` to `0.21.0` (latest)
  - [`tsd` release changelog](https://github.com/SamVerschueren/tsd/releases)
  - Breaking changes:
    - `0.17.0` includes "Require Node.js 12" - this is not a breaking change as we've already update to node `14.19.1`

`tsd v0.17.0` removes dependency chain:
- `update-notifier`
- `latest-version`
- `package-json`
- `got`

Partial fix for #1764

Signed-off-by: Josh Romero <[email protected]>
(cherry picked from commit d1cfe78)

Co-authored-by: Josh Romero <[email protected]>
@ananzh ananzh mentioned this pull request Oct 24, 2022
Closed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@kavilla kavilla kavilla approved these changes

@tmarkley tmarkley tmarkley approved these changes

Assignees
No one assigned
Labels
backport 2.x cve Security vulnerabilities detected by Dependabot or Mend medium severity Medium severity CVE v2.1.0
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
4 participants
@joshuarrrr @codecov-commenter @tmarkley @kavilla