-
Notifications
You must be signed in to change notification settings - Fork 227
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
Copy private NPM credentials if exists #104
base: master
Are you sure you want to change the base?
Conversation
Thank you for your contribution. I've just checked and your commit doesn't appear to be signed-off. |
Signed-off-by: Feynman Liang <[email protected]>
* 'patch-1' of github.com:feynmanliang/templates: Update Dockerfile Signed-off-by: Feynman Liang <[email protected]>
Hi @feynmanliang thanks for your interest in the project. I see you're having a bit of trouble with the bot and sign-off process. You'd be better off resetting all those commits and then adding your change on the top of master. I am not sure this is a good change though since this could lead to leaking private npm credentials within your Docker image. cc @padiazg @pyramation Maybe you could pass it via a build-arg or similar? Alex |
Great idea and thanks for helping out! However, this is honestly a bad security practice to put in the main repo IMHO. Not secure. The pattern I use that works today using Dockerfile is this: place a
|
Thanks for catching that security flaw; that would have been bad >.< I will revise this to accept a build arg. Not 100% sure if copying |
Yes @feynmanliang looks like it does error out:
https://travis-ci.org/openfaas/templates/builds/469855203#L1348 |
Description
Motivation and Context
Some projects have dependencies in private NPM registries. Currently,
faas-cli build
will release an incomplete image which dependency errors in production.This change will copy a
.npmrc
containing the NPM registry auth token, if it is present.Which issue(s) this PR fixes
Fixes #103