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

[Snyk] Security upgrade sequelize from 6.19.0 to 6.21.2 #188

Closed
wants to merge 3 commits into from

Conversation

mikicho
Copy link
Collaborator

@mikicho mikicho commented Jul 25, 2022

This PR was automatically created by Snyk using the credentials of a real user.


Snyk has created this PR to fix one or more vulnerable packages in the `npm` dependencies of this project.

merge advice

Changes included in this PR

  • Changes to the following files to upgrade the vulnerable dependencies to a fixed version:
    • src/code-templates/services/order-service/package.json
    • src/code-templates/services/order-service/package-lock.json

Vulnerabilities that will be fixed

With an upgrade:
Severity Priority Score (*) Issue Breaking Change Exploit Maturity
high severity 636/1000
Why? Recently disclosed, Has a fix available, CVSS 7
SQL Injection
SNYK-JS-SEQUELIZE-2959225
No No Known Exploit

(*) Note that the real score may have changed since the PR was raised.

Commit messages
Package name: sequelize The new version differs by 11 commits.
  • 7bb60e3 fix: properly escaoe multiple `$` in `fn` args (#14678)
  • 86d35b1 docs: added nest option inside findAll query (#14683)
  • 2f3b924 fix(postgres): use schema set in sequelize config by default (#14665)
  • cbdf73e feat: exports types to support typescript >= 4.5 nodenext module (#14620)
  • a333862 docs(readme): update README to be more like main (#14626)
  • e1a9c28 fix: kill connection on commit/rollback error (#14535)
  • b37df96 feat: support cyclic foreign keys (#14499)
  • e37c572 fix: accept replacements in `ARRAY[]` & followed by `;` (#14518)
  • 6c5f8ec test: disable mysql/mariadb deadlock test (#14514)
  • 87655eb build: fix esdoc (#14513)
  • ccaa399 fix: do not replace `:replacements` inside of strings (#14472)

See the full diff

Check the changes in this PR to ensure they won't cause issues with your project.


Note: You are seeing this because you or someone else with access to this repository has authorized Snyk to open fix PRs.

For more information:
🧐 View latest project report

🛠 Adjust project settings

📚 Read more about Snyk's upgrade and patch logic


Learn how to fix vulnerabilities with free interactive lessons:

🦉 SQL Injection

…e-templates/services/order-service/package-lock.json to reduce vulnerabilities

The following vulnerabilities are fixed with an upgrade:
- https://snyk.io/vuln/SNYK-JS-SEQUELIZE-2959225
@codecov
Copy link

codecov bot commented Jul 25, 2022

Codecov Report

Merging #188 (b9c7b5a) into main (11a8d1d) will not change coverage.
The diff coverage is n/a.

@@           Coverage Diff           @@
##             main     #188   +/-   ##
=======================================
  Coverage   89.91%   89.91%           
=======================================
  Files          14       14           
  Lines         456      456           
  Branches       40       40           
=======================================
  Hits          410      410           
  Misses         45       45           
  Partials        1        1           
Flag Coverage Δ
app 91.47% <ø> (ø)
generator 59.09% <ø> (ø)

Flags with carried forward coverage won't be shown. Click here to find out more.


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 11a8d1d...b9c7b5a. Read the comment docs.

@goldbergyoni
Copy link
Contributor

@lirantal @mikicho If the tests pass, can't we merge it automatically? What added value does a human being bring here - I know nothing about sequelize v19 vs v21

@lirantal
Copy link

lirantal commented Jul 28, 2022

I got asked that recently too and was thinking about integrating a workflow that automerges. Seems that you'd be in favor of that happening.

Not everyone has tests, and some developers I assume might still want to vet what changes were exactly introduced. I've heard of developers going through the changelog and READMEs of updated packages.

@goldbergyoni
Copy link
Contributor

@mikicho Isn't it better to let Snyk act as an app instead of impersonation (real user token)?

@mikicho
Copy link
Collaborator Author

mikicho commented Aug 5, 2022

@goldbergyoni I'd love to.
@lirantal Is it possible?

@lirantal
Copy link

lirantal commented Aug 5, 2022

What's the actual ask?

@mikicho
Copy link
Collaborator Author

mikicho commented Aug 5, 2022

What's the actual ask?

@lirantal is it possible to open the PR on behalf of snyk app instead of my user name?

@lirantal
Copy link

lirantal commented Aug 5, 2022

Ahh I see. This is somewhat explained here but here's how to fix it.

@goldbergyoni
Copy link
Contributor

@mikicho In the article that Liran provided:

"For public repositories, the fix PR will be opened by Snyk-bot"

Should be a place in Snyk's config to set this

@mikicho
Copy link
Collaborator Author

mikicho commented Aug 21, 2022

@goldbergyoni I couldn't find this option. You are also to org owner in Snyk if you want to take a look

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.

None yet

4 participants