VirusTotalNet icon indicating copy to clipboard operation
VirusTotalNet copied to clipboard

Copy private NPM credentials if exists

Open feynmanliang opened this issue 5 years ago • 5 comments

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.

  • [x] I have raised an issue to propose this change (required)

Which issue(s) this PR fixes

Fixes #103

feynmanliang avatar Dec 19 '18 04:12 feynmanliang

Thank you for your contribution. I've just checked and your commit doesn't appear to be signed-off. That's something we need before your Pull Request can be merged. Please see our contributing guide.

derek[bot] avatar Dec 19 '18 04:12 derek[bot]

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

alexellis avatar Dec 20 '18 21:12 alexellis

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 .npmrc locally

contents of .npmrc:

//registry.npmjs.org/:_authToken=${NPM_TOKEN}

edit your Dockerfile to use ARG:

ARG NPM_TOKEN  
COPY .npmrc .npmrc 

ENV NODE_ENV production
RUN npm install -g @yourNpmAccount/[email protected]
RUN rm .npmrc
  • Notice it removes the .npmrc!

build image

Then make sure you have NPM_TOKEN in your env, and run

faas build -f production-stack.yml --build-arg NPM_TOKEN=${NPM_TOKEN}

pyramation avatar Dec 20 '18 21:12 pyramation

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 .npmrcwill work (Docker might complain if none of the files in a COPY command exist) but will test it out shortly.

feynmanliang avatar Dec 20 '18 23:12 feynmanliang

Yes @feynmanliang looks like it does error out:

COPY failed: stat /var/lib/docker/tmp/docker-builder325671983/function/.npmrc: no such file or directory

https://travis-ci.org/openfaas/templates/builds/469855203#L1348

pyramation avatar Dec 21 '18 00:12 pyramation