kelektiv / node.bcrypt.js

bcrypt for NodeJs
MIT License
7.4k stars 512 forks source link

Problem with bcrypt's 'node-pre-gyp' dependency #964

Closed eau-de-la-seine closed 8 months ago

eau-de-la-seine commented 1 year ago

Encountered error

When I execute this esbuild command:

esbuild index.ts --bundle --platform=node --target=es2020 --outdir=${npm_package_config_buildpath}

I encounter the following error:

[ERROR] Could not resolve "mock-aws-s3"
    node_modules/@mapbox/node-pre-gyp/lib/util/s3_setup.js:43:28:
      43 │     const AWSMock = require('mock-aws-s3');

[ERROR] Could not resolve "aws-sdk"
    node_modules/@mapbox/node-pre-gyp/lib/util/s3_setup.js:76:22:
      76 │   const AWS = require('aws-sdk');

Analysis

As you can see, the bcrypt lib is using the node-pre-gyp:1.0.10 dependency that has a s3_setup.js production file that contains mock-aws-s3 (a testing library) and aws-sdk ... does bcrypt need that? Isn't this a security issue?

Question

Am I really supposed to embbed mock-aws-s3 and aws-sdk dependencies in order to use bcrypt?

My env: NodeJS: v16.17.1 on Ubuntu 20.04 LTS, I'm using npm + esbuild

Thanks

recrsn commented 1 year ago

These are dev dependencies, bcrypt doesn't need mock-aws-sdk to run.

Bcrypt however needs node-pre-gyp to install itself, node-pre-gyp requires AWS-sdk. It should be possible to exclude certain modules from embedding.

PS: bcrypt and other native addons don't like to be embedded and may not work due to differences in paths for their native counterparts.

eau-de-la-seine commented 1 year ago

Yes mock-aws-s3 and even aws-sdk are declared as dev dependencies but they're used in a production file.

When I import bcrypt, bcrypt itself imports var nodePreGyp = require('@mapbox/node-pre-gyp');, and if I check node_modules/@mapbox/node-pre-gyp/lib/node-pre-gyp.js it imports exports.mockS3Http = require('./util/s3_setup').get_mockS3Http(); that contains const AWSMock = require('mock-aws-s3');

Maybe these AWS dependencies are never used and I just encounter them because I use esbuild (very powerful but not very configurable) that recursively analyze imported dependencies. Anyway in order to make bcrypt work, I had to install aws-sdk and mock-aws-s3 explicitely, but these indirect requirements from @mapbox/node-pre-gyp are not mentioned in bcrypt's README

eau-de-la-seine commented 1 year ago

Just noticed there's a node-pre-gyp issue about this problem that exists since July 6!

recrsn commented 1 year ago

Yeah, we don't have control over what node-pre-gyp is doing.

We are trying to move away from node-pre-gyp and use prebuildify, should land in v6 which is due soon.

Joepolymath commented 1 year ago

Yeah, we don't have control over what node-pre-gyp is doing.

We are trying to move away from node-pre-gyp and use prebuildify, should land in v6 which is due soon.

How soon will this happen?

Aid19801 commented 1 year ago

No work around for this, no?

joshuagraber commented 1 year ago

Yeah, we don't have control over what node-pre-gyp is doing. We are trying to move away from node-pre-gyp and use prebuildify, should land in v6 which is due soon.

Any word on a fix? I'm having the most frustrating time bundling my app with esbuild because of this. Anybody find a workaround that does not require changing my dependencies, etc.?

eau-de-la-seine commented 1 year ago

No work around for this, no?

Hi @Aid19801 , for me the solution was to use bcryptjs (it's bcrypt with pure JS implementation)

Advantages:

Drawbacks:

cptflammin commented 1 year ago

Sorry for my naivety, but is it ok to use a lib that has not been updated for 6 years ?? Last update of bcryptjs 6 years!

Joepolymath commented 1 year ago

I noticed some of the difficulty in installation happens when trying to install bcrypt on a nodejs 19 engine. I had to downgrade my nodejs before it worked.

franktronics commented 1 year ago

the same problem

✘ [ERROR] Could not resolve "mock-aws-s3"

    node_modules/@mapbox/node-pre-gyp/lib/util/s3_setup.js:43:28:
      43 │     const AWSMock = require('mock-aws-s3');
         ╵                             ~~~~~~~~~~~~~

  You can mark the path "mock-aws-s3" as external to
  exclude it from the bundle, which will remove this
  error. You can also surround this "require" call with
  a try/catch block to handle this failure at run-time
  instead of bundle-time.

✘ [ERROR] Could not resolve "aws-sdk"

    node_modules/@mapbox/node-pre-gyp/lib/util/s3_setup.js:76:22:
      76 │   const AWS = require('aws-sdk');
         ╵                       ~~~~~~~~~

  You can mark the path "aws-sdk" as external to
  exclude it from the bundle, which will remove this
  error. You can also surround this "require" call with
  a try/catch block to handle this failure at run-time
  instead of bundle-time.

✘ [ERROR] Could not resolve "nock"

    node_modules/@mapbox/node-pre-gyp/lib/util/s3_setup.js:112:23:
      112 │   const nock = require('nock');
          ╵

I use nx and node.s( v18.12.1)

JClackett commented 1 year ago

Still getting this issue!

bete7512 commented 1 year ago

The Same is here

navalega0109 commented 1 year ago

I'm facing the same issue even after installing https://www.npmjs.com/package/node-gyp.

bugrata commented 11 months ago

Still getting this issue.

Maxou44 commented 11 months ago

Same issue on my side 😅

sangyookm commented 11 months ago

Same here

ali-restock commented 9 months ago

Same here

ezeamin commented 9 months ago

... and same here

MaurovicCachiaSE commented 7 months ago

still have this issue, not sure if we still are waiting for v6 for this to be fixed

Intevel commented 7 months ago

Still having this issue :D

cemk31 commented 7 months ago

catastrophical exception. Are there any other solution?

Intevel commented 6 months ago

catastrophical exception. Are there any other solution?

You can exclude it from the bundle.

enmanuelmag commented 6 months ago

Sen help please, I'm stuck too.

vamman commented 3 months ago

Still got it. 2024...

Nethsara commented 3 months ago

The fix for the issue for me was excluding those packages when building;

"build": "esbuild src/index.ts --bundle --minify --sourcemap --platform=node --target=es2022 --outfile=dist/index.js --external:aws-sdk --external:nock --external:mock-aws-s3 --loader:.html=text",

I added these lines in the build code and worked fine for me.

acf77 commented 1 month ago

No work around for this, no?

Hi @Aid19801 , for me the solution was to use bcryptjs (it's bcrypt with pure JS implementation)

Advantages:

  • Completely fits bcrypt contract, I just had to change the dependency from bcrypt to bcryptjs in package.json, zero code to change
  • Implementation is pure JS, there's no problem related to third party dependencies

Drawbacks:

  • bcryptjs is slower than bcrypt, but the bcrypt algorithm is slow in general. That drawback wasn't really a problem for me because I already execute the bcrypt algorithm in a AWS Lambda Authorizer which has a 5 minutes cache

This solved my problem! Just replaced bcrypt with bcryptjs and it just worked. No coding changes required. Even though is 6-7 years without updates, seem to work fine.