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

Fix knex vulnerability (update knex to 2.4.0) #313

Open
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

leal32b
Copy link

@leal32b leal32b commented Apr 12, 2023

@sobolev-alexey
Copy link

Did you run tests to check if it works after the update?

@danaument
Copy link

I ran @leal32b 's branch locally:

780 passing (4s)
  25 pending

 MOCHA  Tests completed successfully

I'm using Node 18.16.0 and ran into an issue (exists on master branch) with webpack. I was able to get everything to run with export NODE_OPTIONS=--openssl-legacy-provider per this stack overflow.

@sangaman
Copy link
Contributor

Unfortunately this doesn't fix all the audit issues, there are @mikro-orm dependencies that are vulnerable as well. When I try updating all the vulnerable dependencies the tests fail for me with:

  779 passing (4s)
  25 pending
  1 failing

  1) Mikro ORM
       can perform sample:
     TypeError: builder.generateDdlCommands is not a function
      at SchemaGenerator.dump (node_modules/@mikro-orm/knex/schema/SchemaGenerator.js:479:35)
      at SchemaGenerator.getCreateSchemaSQL (node_modules/@mikro-orm/knex/schema/SchemaGenerator.js:68:31)
      at SchemaGenerator.createSchema (node_modules/@mikro-orm/knex/schema/SchemaGenerator.js:29:32)
      at mikroOrmSample (samples/mikro-orm/simple.ts:48:5)
      at Context.<anonymous> (src/tests/mikro-orm-real.spec.ts:8:20)

It's not clear to me at a glance why this is failing.

Upgrading vulnerable devDependencies as well causes even more tests to fail, but I don't think those are as meaningful. It's the actual dependencies that cause anything that depends on pg-mem to fail npm audit.

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.

4 participants