fix(nodeenv): read quoted config values and mirror credentials

CHANGES

@@ -13,6 +13,11 @@ Version [unreleased]
   text files (npm 8) and whose workspaces are missing (npm >= 9); the default
   `--npm=latest` no longer ends in a 404 there
   `#310 <https://github.com/ekalinin/nodeenv/issues/310>`_
+- Quoted values in the config file, as the README shows them, are read
+  without the quotes, and `user:password@` in the `--mirror` URL is sent to
+  the mirror with HTTP Basic authentication instead of being taken for a part
+  of the host; the password stays out of error messages and redirects
+  `#321 <https://github.com/ekalinin/nodeenv/issues/321>`_
 
 Version 1.11.0
 --------------

README.rst

@@ -153,6 +153,11 @@ or a version range, an ``index.json`` next to them::
 
     $ nodeenv --node=22.14.0 --mirror=file:///srv/node-mirror env-22
 
+A mirror that asks for a login takes it in the URL, with special characters
+percent-encoded::
+
+    $ nodeenv --mirror=https://user:p%2Fss@artifactory.example.com/nodejs env
+
 Install the highest node.js release matching a version range::
 
     $ nodeenv --node=22 env-22
@@ -349,7 +354,9 @@ Installation options
 
 ``--mirror=URL``
     Set mirror server of nodejs.org to download from. A ``file://`` URL
-    points nodeenv at a local directory instead of a server.
+    points nodeenv at a local directory instead of a server. The
+    ``user:password@`` part of the URL goes to the mirror with HTTP Basic
+    authentication, and to no other host.
 
 ``-c, --clean-src``
     Remove "src" directory after installation. This is the default.

nodeenv.py

@@ -10,6 +10,7 @@
     :license: BSD, see LICENSE for more details.
 """
 
+import base64
 import contextlib
 import io
 import json
@@ -37,13 +38,16 @@
     from ConfigParser import SafeConfigParser as ConfigParser  # pyright: ignore[reportMissingImports]  # noqa: E501
     # noinspection PyCompatibility
     import urllib2  # pyright: ignore[reportMissingImports]
+    from urlparse import urlsplit, urlunsplit  # pyright: ignore[reportMissingImports]  # noqa: E501
+    from urllib import unquote  # pyright: ignore[reportAttributeAccessIssue]
     iteritems = operator.methodcaller('iteritems')
     import httplib  # pyright: ignore[reportMissingImports]
     IncompleteRead = httplib.IncompleteRead
 except ImportError:  # pragma: no cover (py3 only)
     from configparser import ConfigParser
     # noinspection PyUnresolvedReferences
     import urllib.request as urllib2
+    from urllib.parse import unquote, urlsplit, urlunsplit
     iteritems = operator.methodcaller('items')
     import http
     IncompleteRead = http.client.IncompleteRead
@@ -62,6 +66,9 @@
 # SSL context backed by the certifi bundle, built once by main()
 # when --with-certifi is given and certifi is importable
 certifi_context = None
+# Authorization header for src_base_url, built by main() from the
+# user:password@ part of --mirror, which is cut off src_base_url
+src_auth = None
 
 # ---------------------------------------------------------
 # Utils
@@ -137,6 +144,9 @@ def _load(cls, configfiles, verbose=False):
                     val = ini_file.getboolean(section, attr)
                 else:
                     val = ini_file.get(section, attr)
+                    # ConfigParser keeps the quotes the README shows, #321
+                    if len(val) > 1 and val[0] == val[-1] and val[0] in '\'"':
+                        val = val[1:-1]
 
                 if verbose:
                     print('CONFIG {0}: {1} = {2}'.format(
@@ -947,10 +957,30 @@ def _urlopen(req):
     return urllib2.urlopen(req)
 
 
+def split_url_auth(url):
+    """
+    Cut user:password@ off the URL, urllib takes it for a part of the host,
+    and return it as a Basic Authorization header.
+    https://github.com/ekalinin/nodeenv/issues/321
+    """
+    parts = urlsplit(url)
+    if parts.username is None:
+        return url, None
+    credentials = '%s:%s' % (unquote(parts.username),
+                             unquote(parts.password or ''))
+    auth = base64.b64encode(credentials.encode('utf-8')).decode('ascii')
+    netloc = parts.netloc.rpartition('@')[2]
+    return urlunsplit(parts._replace(netloc=netloc)), 'Basic ' + auth
+
+
 def urlopen(url):
     home_url = "https://github.com/ekalinin/nodeenv/"
     headers = {'User-Agent': 'nodeenv/%s (%s)' % (nodeenv_version, home_url)}
     req = urllib2.Request(url, None, headers)
+    # the mirror's password is neither for other hosts, like the npm
+    # registry, nor for the ones the mirror redirects to
+    if src_auth and url.startswith(src_base_url):
+        req.add_unredirected_header('Authorization', src_auth)
     try:
         return _urlopen(req)
     except urllib2.HTTPError:
@@ -1574,6 +1604,7 @@ def main():
         exit(1)
 
     global src_base_url
+    global src_auth
     global ignore_ssl_certs
     global certifi_context
 
@@ -1594,6 +1625,7 @@ def main():
         src_domain = 'nodejs.org'
     if src_base_url is None:
         src_base_url = 'https://%s/download/release' % src_domain
+    src_base_url, src_auth = split_url_auth(src_base_url)
 
     # Decide on the system node before any version resolution, so that
     # a found system node never triggers a request for index.json

tests/nodeenv_test.py

@@ -6,16 +6,20 @@
     from pipes import quote as _quote
 else:
     from shlex import quote as _quote
+import contextlib
+import http.server
 import io
 import json
 import os.path
 import pathlib
 import shutil
+import socket
 import subprocess
 import sys
 import platform
 import ssl
 import tarfile
+import threading
 import zipfile
 
 try:
@@ -453,6 +457,140 @@ def test_mirror_option_local_directory(tmpdir):
         mock_logger.assert_called_with('99.0.0')
 
 
+def test_config_strips_quotes(tmpdir):
+    """Values quoted as in the README are read without the quotes, see #321"""
+    rc = tmpdir.join('nodeenvrc')
+    rc.write('[nodeenv]\n'
+             "node = '22.14.0'\n"
+             'mirror = "https://example.com/mirror"\n')
+    try:
+        nodeenv.Config._load([str(rc)])
+        assert nodeenv.Config.node == '22.14.0'
+        assert nodeenv.Config.mirror == 'https://example.com/mirror'
+    finally:
+        nodeenv.Config.node = nodeenv.Config._default['node']
+        nodeenv.Config.mirror = nodeenv.Config._default['mirror']
+
+
+@pytest.fixture
+def auth_mirror(monkeypatch):
+    """
+    A local mirror that serves index.json only to an authorized request.
+    /moved/ redirects to /cdn/, which needs no authorization, like
+    a presigned URL of a cloud storage.
+    Yields its port and the paths with the Authorization headers it has seen.
+    """
+    for name in PROXY_VARS:
+        monkeypatch.delenv(name, raising=False)
+    seen = []
+
+    class Handler(http.server.BaseHTTPRequestHandler):
+        def do_GET(self):
+            auth = self.headers.get('Authorization')
+            seen.append((self.path, auth))
+            if self.path.startswith('/moved/'):
+                self.send_response(302)
+                self.send_header('Location', '/cdn/' + self.path[7:])
+                self.end_headers()
+                return
+            if auth is None and not self.path.startswith('/cdn/'):
+                self.send_response(401)
+                self.send_header('WWW-Authenticate', 'Basic realm="mirror"')
+                self.end_headers()
+                return
+            body = (b'[{"version": "v99.0.0", "date": "2026-01-01",'
+                    b' "lts": false, "files": ["linux-x64",'
+                    b' "linux-x64-musl", "linux-riscv64"]}]')
+            self.send_response(200)
+            self.send_header('Content-Length', str(len(body)))
+            self.end_headers()
+            self.wfile.write(body)
+
+        def log_message(self, *args):
+            pass
+
+    server = http.server.HTTPServer(('127.0.0.1', 0), Handler)
+    thread = threading.Thread(target=server.serve_forever)
+    thread.start()
+    try:
+        yield server.server_address[1], seen
+    finally:
+        server.shutdown()
+        server.server_close()
+        thread.join()
+
+
+@pytest.mark.parametrize('userinfo, header', [
+    ('user:secret', 'Basic dXNlcjpzZWNyZXQ='),
+    # me@corp.com:p/ss, a URL has room for these characters only encoded
+    ('me%40corp.com:p%2Fss', 'Basic bWVAY29ycC5jb206cC9zcw=='),
+])
+def test_mirror_credentials_are_sent_as_basic_auth(auth_mirror,
+                                                   userinfo, header):
+    """user:password@ in --mirror authenticates to the mirror, see #321"""
+    port, seen = auth_mirror
+    mirror = 'http://%s@127.0.0.1:%d' % (userinfo, port)
+    argv = [__file__, '--list', '--mirror=' + mirror]
+    with mock.patch.object(sys, 'argv', argv), \
+            mock.patch.object(nodeenv.logger, 'info') as mock_logger:
+        nodeenv.src_base_url = None
+        nodeenv.main()
+        mock_logger.assert_called_with('99.0.0')
+
+    assert set(seen) == {('/index.json', header)}
+
+
+def test_mirror_credentials_do_not_follow_redirects(auth_mirror):
+    """A redirect may lead to another host, the password stays behind"""
+    port, seen = auth_mirror
+    mirror = 'http://user:secret@127.0.0.1:%d/moved' % port
+    argv = [__file__, '--list', '--mirror=' + mirror]
+    with mock.patch.object(sys, 'argv', argv), \
+            mock.patch.object(nodeenv.logger, 'info') as mock_logger:
+        nodeenv.src_base_url = None
+        nodeenv.main()
+        mock_logger.assert_called_with('99.0.0')
+
+    assert set(seen) == {('/moved/index.json', 'Basic dXNlcjpzZWNyZXQ='),
+                         ('/cdn/index.json', None)}
+
+
+def test_mirror_credentials_stay_out_of_error_messages(monkeypatch):
+    """A failed download must not print the mirror's password"""
+    for name in PROXY_VARS:
+        monkeypatch.delenv(name, raising=False)
+    # a port that was free a moment ago refuses the connection
+    with contextlib.closing(socket.socket()) as sock:
+        sock.bind(('127.0.0.1', 0))
+        port = sock.getsockname()[1]
+    mirror = 'http://user:secret@127.0.0.1:%d' % port
+    argv = [__file__, '--list', '--mirror=' + mirror]
+    with mock.patch.object(sys, 'argv', argv), \
+            mock.patch.object(nodeenv.logger, 'error') as m_error:
+        nodeenv.src_base_url = None
+        with pytest.raises(SystemExit):
+            nodeenv.main()
+
+    errors = _logged_errors(m_error)
+    assert 'http://127.0.0.1:%d/index.json' % port in errors
+    assert 'secret' not in errors
+
+
+def test_mirror_credentials_stay_with_the_mirror():
+    """The npm registry must not get the mirror's password"""
+    with mock.patch.object(nodeenv, 'src_base_url',
+                           'https://mirror.example.com/node'), \
+            mock.patch.object(nodeenv, 'src_auth',
+                              'Basic dXNlcjpzZWNyZXQ='), \
+            mock.patch.object(nodeenv.urllib2, 'urlopen') as m_urlopen:
+        nodeenv.urlopen('https://mirror.example.com/node/index.json')
+        nodeenv.urlopen('https://registry.npmjs.org/npm/latest')
+
+    to_mirror, to_registry = [c[0][0] for c in m_urlopen.call_args_list]
+    assert to_mirror.has_header('Authorization')
+    assert not to_registry.has_header('Authorization')
+
+
 @pytest.mark.usefixtures('mock_index_json', 'mock_host_platform')
 def test_get_latest_node_version():
     assert nodeenv.get_last_stable_node_version() == '13.5.0'