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

module is packaged with .pem files #7

Closed
zhaizhai opened this issue Jun 6, 2015 · 14 comments · May be fixed by #19
Closed

module is packaged with .pem files #7

zhaizhai opened this issue Jun 6, 2015 · 14 comments · May be fixed by #19

Comments

@zhaizhai
Copy link

zhaizhai commented Jun 6, 2015

Not sure if this is the right place, but I am using browserify in a chrome extension that I think uses this module somewhere down the chain of dependencies. The presence of .pem files (even though they are just tests) is causing problems, because chrome does not want to pack an extension that has .pem files (in case you accidentally distribute your real keys).

This is largely chrome's fault for being so restrictive and providing no mechanism that I know of to override the .pem file detection, but it would be very helpful if you could somehow get rid of the .pem files or rename them to something else. Thanks!

@dcousens
Copy link
Member

dcousens commented Jun 6, 2015

@zhaizhai ha, that is quite interesting!

@dominictarr thoughts? Haha.
@calvinmetcalf I guess we could just white list in package.json.

"files": [
  "browser.js",
  ...
]

However, maybe renaming the files will be just as effective and maintains a status quo?

@dcousens dcousens added the bug label Jun 6, 2015
@dominictarr
Copy link

Hmm, if you bundle this with browserify then you won't need to keep node_modules at all.
Am I missing something?

@calvinmetcalf
Copy link
Contributor

We can just put *. pem in a .npmignore

On Sat, Jun 6, 2015, 5:32 AM Dominic Tarr [email protected] wrote:

Hmm, if you bundle this with browserify then you won't need to keep
node_modules at all.
Am I missing something?


Reply to this email directly or view it on GitHub
#7 (comment)
.

@dominictarr
Copy link

first we should understand why this is causing @zhaizhai's problem

@zhaizhai
Copy link
Author

zhaizhai commented Jun 6, 2015

@dominictarr Good point, you're right that I don't need to package node_modules in this case. So never mind, it doesn't matter for me. Thanks for responding so quickly!

@dominictarr
Copy link

👍

@dcousens
Copy link
Member

dcousens commented Jun 6, 2015

@zhaizhai how were you packaging node_modules in the first place?

@RobbyChapman
Copy link

I know this is a little late, but I was hitting this problem today. I am developing a chrome extension that depends on browserify. Chrome throws the following errors when trying to unpack the extension:

This extension includes the key file '/app/node_modules/browserify/node_modules/crypto-browserify/node_modules/public-encrypt/test/test_key.pem'. You probably don't want to do that.
This extension includes the key file '/app/node_modules/browserify/node_modules/crypto-browserify/node_modules/public-encrypt/test/test_rsa_privkey.pem'. You probably don't want to do that.
This extension includes the key file 'app/node_modules/browserify/node_modules/crypto-browserify/node_modules/public-encrypt/test/test_rsa_pubkey.pem'. You probably don't want to do that.

I removed the files manually and all was well. However, the next time I run npm install I will hit the same issue. I was thinking maybe adding the test directory to .npmignore would do the trick.

@dcousens
Copy link
Member

@RobbyChapman if you are using browserify, why is the node_modules in the application directory at all?

@dcousens dcousens added help wanted and removed bug labels Sep 11, 2015
@RobbyChapman
Copy link

@dcousens I'm not sure I understand the question. If you are building a chrome extension the entire root directory is the application. Moving node modules wouldn't solve that.

@calvinmetcalf
Copy link
Contributor

@RobbyChapman the contents of node_modules especially node_modules/browserify is a development dependency that should not actually be packed into the chrome extension, though as you are the 2nd one to make this mistake it probably wouldn't hurt to put the .pem files into the npm ignore

@RobbyChapman
Copy link

@dcousens I see what you are saying. I rearrange the app based on your suggestion and all is well. I do agree with the .pem fix though. Thanks for looking into it!

@dcousens
Copy link
Member

@RobbyChapman we could move it, but this sounds like a great way to help people realise you shouldn't pack node_modules, especially since there is no reason to ignore the PEM files otherwise.
If we want to not include the tests folder, that is another issue.

@octref
Copy link

octref commented Jun 29, 2019

@dcousens I'm developing a Chrome extension. I know I shouldn't pack node_modules, but when I'm developing the app I usually have manifest.json, /src and node_modules all on top level, and load the extension from the project root. When I'm publishing I only copy over the built files and manifest.json into a publish folder.

I'll send another PR that just npm-ignores the test/ folder. Hope you can publish a new version. Thanks!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging a pull request may close this issue.

6 participants