[v2,1/1] fetch2/npm: Adding npmrc support for private registry authentication

Signed-off-by: Eric Meyers <eric.meyers@arthrex.com>
Cc: Geoff Parker <geoffrey.parker@arthrex.com>
Cc: Alex Kanavin <alex.kanavin@gmail.com>
Cc: Chuck Wolber <chuckwolber@gmail.com>
Cc: Stefan Herbrechtsmeier <stefan.herbrechtsmeier-oss@weidmueller.com>
---
 lib/bb/fetch2/npm.py | 15 +++++++++++++++
 1 file changed, 15 insertions(+)

I'm broadly ok with this (except the question below), but it needs to
be lgtm'd by real npm fetcher users.

Does npmrc contain secrets? Would copying it to some tmpdir put the
secrets at risk?

Alex

On Thu, 20 Feb 2025 at 15:30, Eric Meyers <eric.meyers15310@gmail.com> wrote:
>
> Signed-off-by: Eric Meyers <eric.meyers@arthrex.com>
> Cc: Geoff Parker <geoffrey.parker@arthrex.com>
> Cc: Alex Kanavin <alex.kanavin@gmail.com>
> Cc: Chuck Wolber <chuckwolber@gmail.com>
> Cc: Stefan Herbrechtsmeier <stefan.herbrechtsmeier-oss@weidmueller.com>
> ---
>  lib/bb/fetch2/npm.py | 15 +++++++++++++++
>  1 file changed, 15 insertions(+)
>
> diff --git a/lib/bb/fetch2/npm.py b/lib/bb/fetch2/npm.py
> index c09f05044..23fb5029d 100644
> --- a/lib/bb/fetch2/npm.py
> +++ b/lib/bb/fetch2/npm.py
> @@ -107,6 +107,21 @@ class NpmEnvironment(object):
>          """Run npm command in a controlled environment"""
>          with tempfile.TemporaryDirectory() as tmpdir:
>              d = bb.data.createCopy(self.d)
> +
> +            # If an .npmrc file exists in the HOME directory, then use it in the NPM environment
> +            # (along with the user provided configs). Otherwise, leave it blank.
> +            home_npmrc_file = os.path.join(os.environ.get("HOME"), ".npmrc")
> +            tmp_npmrc_file = os.path.join(tmpdir, ".npmrc")
> +            if os.path.exists(home_npmrc_file):
> +                bb.utils.copyfile(home_npmrc_file, tmp_npmrc_file)
> +                # Still need to append the contents of the user config created in __init__
> +                with open(tmp_npmrc_file, 'a') as t:
> +                    with open(self.user_config.name, 'r') as u:
> +                        t.write(u.read())
> +                        t.flush()
> +
> +                self.user_config.name = tmp_npmrc_file
> +
>              d.setVar("PATH", d.getVar("PATH"))  # PATH might contain $HOME - evaluate it before patching
>              d.setVar("HOME", tmpdir)
>

On Thu, Feb 20, 2025 at 9:48 AM Alexander Kanavin <alex.kanavin@gmail.com>
wrote:
>
> I'm broadly ok with this (except the question below), but it needs to
> be lgtm'd by real npm fetcher users.
>
> Does npmrc contain secrets? Would copying it to some tmpdir put the
> secrets at risk?
>
> Alex
>

It *could* have authentication credentials in plaintext, but ultimately
this is up to the user if they provide them in their .npmrc file within
their home directory. See:


https://docs.npmjs.com/cli/v9/configuring-npm/npmrc#auth-related-configuration

The way I'm using it now requires me to create a base64 encrypted string
based on the input string of "<username>:<API_KEY>" and insert this as the
"_auth" value in the .npmrc file, but there are multiple ways to provide
credentials.

The only reason I copied the .npmrc file into the tmp directory was to
avoid losing any user-provided configs passed into the environment (I could
have taken the opposite approach and modified the user's .npmrc file
instead, but didn't think that was desirable) - an example of this is
within the create_npm.py function upstream in the poky repo:


https://github.com/yoctoproject/poky/blob/ca289e5d853c5ef980ad070eab0d20f973fcdb7c/scripts/lib/recipetool/create_npm.py#L98

FWIW, the wget fetcher relies on a netrc file in a user's home directory in
a similar fashion, which uses username/password in plaintext to insert a
basic auth header to a URL:


https://github.com/yoctoproject/poky/blob/ca289e5d853c5ef980ad070eab0d20f973fcdb7c/bitbake/lib/bb/fetch2/wget.py#L399

On Thu, Feb 20, 2025 at 8:33 AM Eric Meyers <eric.meyers15310@gmail.com>
wrote:

> On Thu, Feb 20, 2025 at 9:48 AM Alexander Kanavin <alex.kanavin@gmail.com>
> wrote:
> >
> > I'm broadly ok with this (except the question below), but it needs to
> > be lgtm'd by real npm fetcher users.
> >
> > Does npmrc contain secrets? Would copying it to some tmpdir put the
> > secrets at risk?
>

If the secrets are already in $HOME, copying them into a build is not going
to
expose things any more than they already are.

That being said, as a general rule I would prefer that we be more explicit
when elements found in $HOME are being used in a build.

I say this from a build consistency standpoint more than anything else. As
someone
who is responsible for consistent results across a wide range of developers
and
platforms, environment creep is a huge time-sink.

Can we disable this behavior by default and allow it to be enabled in the
distro
or local config with a flag?



> The only reason I copied the .npmrc file into the tmp directory was to
> avoid losing any user-provided configs passed into the environment
>

I think this is very problematic from a build consistency standpoint. As I
noted above, I see no problem with reusing credentials. But I have
significant
concerns with reusing anything else.

I believe it is possible to configure an .npmrc in a way that can trigger
behavior variations that affect build results. Build variation should be
controlled by the distro level configs and the recipe. All other sources
of variation should be disallowed or, at the very least, disabled by
default with the option to enable if you are willing to accept the
consequences.



> (I could have taken the opposite approach and modified the user's .npmrc
> file instead, but didn't think that was desirable)
>

Correct, it is definitely undesirable. Can you simply grab the credentials
from a
user's npmrc if they happen to be available? Or does that fall into the
"easier
said than done" bucket?



> FWIW, the wget fetcher relies on a netrc file in a user's home directory
> in a similar fashion, which uses username/password in plaintext to insert a
> basic auth header to a URL
>

I am unaware of any netrc behavior that could introduce uncontrolled build
variation. But I learn something new every day, so maybe it is possible?

..Ch:W..