Makes NO_PROXY, HTTP_PROXY, HTTPS_PROXY, PYMANAGER_USERNAME and _PASSWORD values configurable in settings (#430)

Ignore values of NO_PROXY that aren't a simple 'true'.
Sanitise configuration output for variables known to potentially contain sensitive info.

Fixes #425

Author: zooba

src/manage/commands.py

@@ -256,6 +256,20 @@ def execute(self):
     # Typically configured to '%VIRTUAL_ENV%' to pick up the active environment
     "virtual_env": (str, None, "env", "path"),
 
+    # Overrides for the OS proxy configuration, typically set to '%HTTP_PROXY%'
+    # by default to pick up environment settings, but can be overridden to empty
+    # to disable use of the process environment.
+    "http_proxy": (str, None, "env", "sensitive"),
+    "https_proxy": (str, None, "env", "sensitive"),
+    "no_proxy": (str, None, "env", "sensitive"),
+
+    # Global auth for accessing sources, typically set to '%PYMANAGER_USERNAME%'
+    # and '%PYMANAGER_PASSWORD%'. These should really have been
+    # '%PYTHON_MANAGER_USERNAME%' rather than '%PYMANAGER' - the latter is meant
+    # for debugging/hidden options, not user-visible ones.
+    "username": (str, None, "env", "sensitive"),
+    "password": (str, None, "env", "sensitive"),
+
     "list": {
         "format": (str, None, "env"),
         "unmanaged": (config_bool, None, "env"),
@@ -361,6 +375,13 @@ class BaseCommand:
     shebang_templates = {}
     welcome_on_update = False
 
+    # These variables are mainly for capturing optional environment variables.
+    http_proxy = None
+    https_proxy = None
+    no_proxy = None
+    username = None
+    password = None
+
     log_file = None
     _create_log_file = True
     keep_log = True

src/manage/config.py

@@ -243,16 +243,29 @@ def get(self, key, default=None):
 
 
 
-def _expand_vars(v, env):
+def _expand_vars(k, v, env):
     import re
     def _sub(m):
+        # Double %% turns into literal %
+        if not m.group(1):
+            return "%"
         v2 = env.get(m.group(1))
         if v2:
             return v2 + (m.group(2) or "")
+        # Extra warning for undefined environment variables that are not shaped
+        # like identifiers (typical, but not strictly required).
+        if not m.group(1).isidentifier():
+            LOGGER.warn("The environment variable %%%s%% in configuration "
+                        "setting %s looks unusual. Ensure you have defined "
+                        "your configuration file correctly. This value may "
+                        "require literal percents to be escaped as %%%%.",
+                        m.group(1), k)
         if m.group(2):
             # Empty values before a path separator are not allowed
             raise LookupError(m.group(1))
         return ""
+    # Ignore URL encoded characters. If you really want your environment
+    # variable to start with two digits, you'll just have to change it.
     v2 = re.sub(r"%(.*?)%([\\/])?", _sub, v)
     return v2
 
@@ -275,16 +288,16 @@ def resolve_config(cfg, source, relative_to, key_so_far="", schema=None, error_u
 
         if isinstance(subschema, dict):
             if not isinstance(v, dict):
-                raise InvalidConfigurationError(source, key_so_far + k, v)
+                raise InvalidConfigurationError(source, key_so_far + k)
             resolve_config(v, source, relative_to, f"{key_so_far}{k}.", subschema, environ=environ)
             continue
 
         kind, merge, *opts = subschema
         from_env = False
-        if "env" in opts and isinstance(v, str):
+        if "env" in opts and v and isinstance(v, str):
             try:
                 orig_v = v
-                v = _expand_vars(v, environ)
+                v = _expand_vars(key_so_far + k, v, environ)
                 from_env = orig_v != v
             except TypeError:
                 pass
@@ -296,7 +309,7 @@ def resolve_config(cfg, source, relative_to, key_so_far="", schema=None, error_u
         try:
             v = kind(v)
         except (TypeError, ValueError):
-            raise InvalidConfigurationError(source, key_so_far + k, v)
+            raise InvalidConfigurationError(source, key_so_far + k, v, opts)
         if v and "path" in opts and not _is_valid_url(v):
             # Paths from the config file are relative to the config file.
             # Paths from the environment are relative to the current working dir
@@ -312,44 +325,83 @@ def resolve_config(cfg, source, relative_to, key_so_far="", schema=None, error_u
             else:
                 v = str(v)
             if not _is_valid_url(v):
-                raise InvalidConfigurationError(source, key_so_far + k, v)
+                raise InvalidConfigurationError(source, key_so_far + k, v, opts)
         cfg[k] = v
 
 
-def merge_config(into_cfg, from_cfg, schema, *, source="<unknown>", overwrite=False):
+def merge_config(into_cfg, from_cfg, schema, *, key_so_far="", source="<unknown>", overwrite=False):
+    if key_so_far and not key_so_far.endswith("."):
+        key_so_far = f"{key_so_far}."
+
     for k, v in from_cfg.items():
         if k.startswith("#"):
             continue
-        try:
-            into = into_cfg[k]
-        except LookupError:
-            LOGGER.debug("Setting config %s to %r", k, v)
-            into_cfg[k] = v
-            continue
 
         try:
             subschema = schema[k]
         except LookupError:
             # No schema information, so let's just replace
-            LOGGER.warn("Unknown configuration key %s in %s", k, source)
+            LOGGER.warn("Unknown configuration key %s in %s", key_so_far + k, source)
             into_cfg[k] = v
             continue
 
         if isinstance(subschema, dict):
+            into = into_cfg.setdefault(k, {})
             if isinstance(into, dict) and isinstance(v, dict):
-                LOGGER.debug("Recursively updating config %s", k)
-                merge_config(into, v, subschema, source=source, overwrite=overwrite)
+                LOGGER.debug("Recursively updating config %s", key_so_far + k)
+                merge_config(into, v, subschema, key_so_far=f"{key_so_far}{k}.",
+                             source=source, overwrite=overwrite)
             else:
                 # Source isn't recursing, so let's ignore
                 # Should have been validated earlier
-                LOGGER.warn("Invalid configuration key %s in %s", k, source)
+                LOGGER.warn("Invalid configuration key %s in %s", key_so_far + k, source)
             continue
 
-        _, merge, *_ = subschema
-        if not merge or overwrite:
-            LOGGER.debug("Updating config %s from %r to %r", k, into, v)
-            into_cfg[k] = v
+        _, merge, *opts = subschema
+
+        try:
+            into = into_cfg[k]
+        except KeyError:
+            into = None
         else:
-            v2 = merge(into, v)
-            LOGGER.debug("Updating config %s from %r to %r", k, into, v2)
-            into_cfg[k] = v2
+            if merge and not overwrite:
+                v = merge(into, v)
+
+        if v != into:
+            if "sensitive" in opts:
+                LOGGER.debug("Updating config %s", key_so_far + k)
+            elif "uri" in opts:
+                from .urlutils import sanitise_url
+                LOGGER.debug("Updating config %s to %r", key_so_far + k, sanitise_url(v))
+            else:
+                LOGGER.debug("Updating config %s to %r", key_so_far + k, v)
+            into_cfg[k] = v
+
+
+def sanitise_config(config, schema):
+    clean = {}
+    for k, v in config.items():
+        subschema = schema.get(k)
+        if not subschema:
+            LOGGER.debug("Omitting %s from config output as it is not "
+                         "in the schema", k)
+            continue
+        if isinstance(subschema, dict):
+            if not isinstance(v, dict):
+                LOGGER.debug("Omitting %s from config output as the schema "
+                             "wants an object and the config has a value.", k)
+                continue
+            clean[k] = sanitise_config(v, subschema)
+            continue
+        kind, _, *opts = subschema
+        if "sensitive" in opts:
+            LOGGER.debug("Omitting %s from config output as it is "
+                         "marked sensitive.", k)
+            continue
+        if kind:
+            v = kind(v)
+        if "uri" in opts:
+            from .urlutils import sanitise_url
+            v = sanitise_url(v)
+        clean[k] = v
+    return clean

src/manage/exceptions.py

@@ -51,7 +51,16 @@ def __init__(self, message, prefix=None):
 
 
 class InvalidConfigurationError(ValueError):
-    def __init__(self, file=None, argument=None, value=None):
+    def __init__(self, file=None, argument=None, value=None, opts=None):
+        if value and opts:
+            if "sensitive" in opts:
+                value = "*" * len(value)
+            elif "uri" in opts:
+                try:
+                    from .urlutils import sanitise_url
+                    value = sanitise_url(value)
+                except Exception:
+                    value = ""
         if value:
             msg = f"Invalid configuration value {value!r} for key {argument} in {file}"
         elif argument:

src/manage/install_command.py

@@ -15,7 +15,6 @@
 from .tagutils import install_matches_any, tag_or_range
 from .urlutils import (
     sanitise_url,
-    urlopen as _urlopen,
     urlretrieve as _urlretrieve,
     IndexDownloader,
 )
@@ -48,7 +47,7 @@ def _expand_versions_by_tag(versions):
                 yield {**v, "tag": t}
 
 
-def select_package(index_downloader, tag, platform=None, *, urlopen=_urlopen, by_id=False, allow_pre=True):
+def select_package(index_downloader, tag, platform=None, *, by_id=False, allow_pre=True):
     """Finds suitable package from index.json that looks like:
     {"versions": [
       {"id": ..., "company": ..., "tag": ..., "url": ..., "hash": {"sha256": hexdigest}},
@@ -96,7 +95,7 @@ def select_package(index_downloader, tag, platform=None, *, urlopen=_urlopen, by
     raise RuntimeError("End of select_package reached")
 
 
-def download_package(cmd, install, dest, cache, *, on_progress=None, urlopen=_urlopen, urlretrieve=_urlretrieve):
+def download_package(cmd, install, dest, cache, *, on_progress=None, urlretrieve=_urlretrieve):
     LOGGER.debug("Starting download package %s to %s", sanitise_url(install["url"]), dest)
 
     if not cmd.force and dest.is_file():
@@ -130,7 +129,7 @@ def _find_creds(url):
         if auth:
             LOGGER.debug("Found credentials in URL or configured source.")
             return auth
-        auth = os.getenv("PYMANAGER_USERNAME", ""), os.getenv("PYMANAGER_PASSWORD", "")
+        auth = cmd.username, (cmd.password or "")
         if auth[0]:
             LOGGER.debug("Found credentials in environment.")
             return auth
@@ -143,6 +142,7 @@ def _find_creds(url):
         on_progress=on_progress,
         on_auth_request=_find_creds,
         on_cancel=lambda: cmd.ask_yn("Abort download?"),
+        proxy_info=cmd,
     )
     LOGGER.debug("Downloaded to %s", dest)
     return dest

src/manage/list_command.py

@@ -225,9 +225,16 @@ def list_formats(cmd, installs):
         LOGGER.print(f"{k:<{max_key_width}} {doc}", always=True)
 
 
-def list_config(cmd, installs):
+def list_config(cmd, installs, schema=None):
     "List the current config"
-    LOGGER.print(json.dumps(cmd.config, default=str), always=True)
+    from .config import sanitise_config
+    if schema is None:
+        from .commands import CONFIG_SCHEMA as schema
+
+    LOGGER.print(
+        json.dumps(sanitise_config(cmd.config, schema), default=str),
+        always=True
+    )
 
 
 def format_legacy(cmd, installs, paths=False):