From 7b7bb9c39632effb51c753b7d757a4584af03d1f Mon Sep 17 00:00:00 2001 From: Steve Dower Date: Fri, 16 Oct 2026 11:51:27 +0100 Subject: [PATCH 1/7] Makes NO_PROXY, HTTP_PROXY and HTTPS_PROXY values configurable in settings. Also ignores values of NO_PROXY that aren't a simple 'true'. Fixes #425 --- src/manage/commands.py | 17 ++++++++++++ src/manage/install_command.py | 7 ++--- src/manage/urlutils.py | 52 ++++++++++++++++++++++++----------- src/pymanager.json | 8 ++++++ 4 files changed, 64 insertions(+), 20 deletions(-) diff --git a/src/manage/commands.py b/src/manage/commands.py index 0919ce4..c51f657 100644 --- a/src/manage/commands.py +++ b/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"), + "https_proxy": (str, None, "env"), + "no_proxy": (str, None, "env"), + + # 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"), + "password": (str, None, "env"), + "list": { "format": (str, None, "env"), "unmanaged": (config_bool, None, "env"), @@ -355,6 +369,9 @@ class BaseCommand: default_platform = None automatic_install = True include_unmanaged = True + http_proxy = None + https_proxy = None + no_proxy = None virtual_env = None shebang_can_run_anything = True shebang_can_run_anything_silently = False diff --git a/src/manage/install_command.py b/src/manage/install_command.py index d8b661b..b8beb63 100644 --- a/src/manage/install_command.py +++ b/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 diff --git a/src/manage/urlutils.py b/src/manage/urlutils.py index edfd54a..85172b2 100644 --- a/src/manage/urlutils.py +++ b/src/manage/urlutils.py @@ -102,21 +102,32 @@ def _parse_proxy(value): return proxy, credentials -def _proxy_settings_from_env(): - if os.getenv("NO_PROXY"): - return _ProxySettings(mode=PROXY_MODE_DIRECT) +def _proxy_settings(info): + if not info: + return _ProxySettings() - http_value = os.getenv("HTTP_PROXY") - https_value = os.getenv("HTTPS_PROXY") - if not http_value and not https_value: + if info.no_proxy: + if info.no_proxy.lower() in ("1", "*", "y", "yes", "t", "true"): + return _ProxySettings(mode=PROXY_MODE_DIRECT) + # TODO: Parse and filter based on info.no_proxy IF we can be sure it's + # secure, preferably by delegating responsibility for parsing to the OS. + # Claiming to support proper whitelisting of domains is very likely to + # attract reports of bypasses, none of which we would treat as a + # vulnerability (due to non-default configuration settings), but they + # historically create more noise than they're worth. Let's just avoid + # breaking into jail until we're sure we can do it right. + LOGGER.warn("Values for NO_PROXY other than 1, *, y[es] and t[rue] are " + "not supported. Other proxy settings will be used.") + + if not info.http_proxy and not info.https_proxy: return _ProxySettings() http_proxy = http_credentials = None https_proxy = https_credentials = None - if http_value: - http_proxy, http_credentials = _parse_proxy(http_value) - if https_value: - https_proxy, https_credentials = _parse_proxy(https_value) + if info.http_proxy: + http_proxy, http_credentials = _parse_proxy(info.http_proxy) + if info.https_proxy: + https_proxy, https_credentials = _parse_proxy(info.https_proxy) proxy_list = " ".join( value for value in ( @@ -134,12 +145,19 @@ def _proxy_settings_from_env(): ) +class _EnvProxyInfo: + """Default value for proxy_info arguments from environment variables.""" + no_proxy = os.getenv("NO_PROXY") + http_proxy = os.getenv("HTTP_PROXY") + https_proxy = os.getenv("HTTPS_PROXY") + + class NoInternetError(Exception): pass class _Request: - def __init__(self, url, method="GET", headers={}, outfile=None): + def __init__(self, url, method="GET", headers={}, outfile=None, proxy_info=None): self.url = url self.method = method.upper() self.headers = dict(headers) @@ -147,7 +165,7 @@ def __init__(self, url, method="GET", headers={}, outfile=None): self.username = None self.password = None self.outfile = Path(outfile) if outfile else None - self.proxy_settings = _proxy_settings_from_env() + self.proxy_settings = _proxy_settings(proxy_info) self._on_progress = None self._on_auth_request = None self._on_cancel = None @@ -488,7 +506,8 @@ def _powershell_urlretrieve(request): raise -def urlopen(url, method="GET", headers={}, on_progress=None, on_auth_request=None): +def urlopen(url, method="GET", headers={}, on_progress=None, + on_auth_request=None, proxy_info=_EnvProxyInfo): scheme, sep, path = url.partition("://") if not sep: scheme = "file" @@ -500,7 +519,7 @@ def urlopen(url, method="GET", headers={}, on_progress=None, on_auth_request=Non with open(file_url_to_path(url), "rb") as f: return f.read() - request = _Request(url, method=method, headers=headers) + request = _Request(url, method=method, headers=headers, proxy_info=proxy_info) request._on_progress = on_progress request._on_auth_request = on_auth_request @@ -561,7 +580,8 @@ def urlopen(url, method="GET", headers={}, on_progress=None, on_auth_request=Non def urlretrieve(url, outfile, method="GET", headers={}, chunksize=64 * 1024, - on_progress=None, on_auth_request=None, on_cancel=None): + on_progress=None, on_auth_request=None, on_cancel=None, + proxy_info=_EnvProxyInfo): scheme, sep, path = url.partition("://") if not sep: scheme = "file" @@ -588,7 +608,7 @@ def on_progress(_): pass on_progress(100) return - request = _Request(url, method=method, headers=headers) + request = _Request(url, method=method, headers=headers, proxy_info=proxy_info) request.outfile = Path(outfile) request.chunksize = chunksize request._on_progress = on_progress diff --git a/src/pymanager.json b/src/pymanager.json index fff1efa..b4f24f9 100644 --- a/src/pymanager.json +++ b/src/pymanager.json @@ -12,6 +12,14 @@ "automatic_install": "%PYTHON_MANAGER_AUTOMATIC_INSTALL%", "include_unmanaged": "%PYTHON_MANAGER_INCLUDE_UNMANAGED%", "virtual_env": "%VIRTUAL_ENV%", + + "http_proxy": "%HTTP_PROXY%", + "https_proxy": "%HTTPS_PROXY%", + "no_proxy": "%NO_PROXY", + + "username": "%PYMANAGER_USERNAME%", + "password": "%PYMANAGER_PASSWORD%", + "shebang_can_run_anything": "%PYTHON_MANAGER_SHEBANG_CAN_RUN_ANYTHING%", "shebang_can_run_anything_silently": false, From d2ea37b90c22095812b3862bfb11724cad38e74e Mon Sep 17 00:00:00 2001 From: Steve Dower Date: Thu, 22 Oct 2026 21:21:05 +0100 Subject: [PATCH 2/7] Update tests --- src/manage/commands.py | 10 +- src/manage/urlutils.py | 20 ++-- tests/test_urlutils.py | 208 +++++++++++++++++++++++++---------------- 3 files changed, 146 insertions(+), 92 deletions(-) diff --git a/src/manage/commands.py b/src/manage/commands.py index c51f657..f500896 100644 --- a/src/manage/commands.py +++ b/src/manage/commands.py @@ -369,15 +369,19 @@ class BaseCommand: default_platform = None automatic_install = True include_unmanaged = True - http_proxy = None - https_proxy = None - no_proxy = None virtual_env = None shebang_can_run_anything = True shebang_can_run_anything_silently = False 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 diff --git a/src/manage/urlutils.py b/src/manage/urlutils.py index 85172b2..39ef839 100644 --- a/src/manage/urlutils.py +++ b/src/manage/urlutils.py @@ -109,15 +109,17 @@ def _proxy_settings(info): if info.no_proxy: if info.no_proxy.lower() in ("1", "*", "y", "yes", "t", "true"): return _ProxySettings(mode=PROXY_MODE_DIRECT) - # TODO: Parse and filter based on info.no_proxy IF we can be sure it's - # secure, preferably by delegating responsibility for parsing to the OS. - # Claiming to support proper whitelisting of domains is very likely to - # attract reports of bypasses, none of which we would treat as a - # vulnerability (due to non-default configuration settings), but they - # historically create more noise than they're worth. Let's just avoid - # breaking into jail until we're sure we can do it right. - LOGGER.warn("Values for NO_PROXY other than 1, *, y[es] and t[rue] are " - "not supported. Other proxy settings will be used.") + if info.no_proxy.lower() not in ("0", "n", "no", "f", "false"): + # TODO: Parse and filter based on info.no_proxy IF we can be sure it's + # secure, preferably by delegating responsibility for parsing to the OS. + # Claiming to support proper whitelisting of domains is very likely to + # attract reports of bypasses, none of which we would treat as a + # vulnerability (due to non-default configuration settings), but they + # historically create more noise than they're worth. Let's just avoid + # breaking into jail until we're sure we can do it right. + LOGGER.warn("Values for NO_PROXY other than 1, *, y[es], t[rue], " + "or the equivalent false values are not supported. " + "Other proxy settings will be used.") if not info.http_proxy and not info.https_proxy: return _ProxySettings() diff --git a/tests/test_urlutils.py b/tests/test_urlutils.py index df32e45..8473fed 100644 --- a/tests/test_urlutils.py +++ b/tests/test_urlutils.py @@ -7,14 +7,8 @@ import manage.urlutils as UU -@pytest.fixture -def clean_proxy_env(monkeypatch): - for name in ("NO_PROXY", "HTTP_PROXY", "HTTPS_PROXY"): - monkeypatch.delenv(name, raising=False) - - -def test_proxy_settings_auto(clean_proxy_env): - settings = UU._proxy_settings_from_env() +def test_proxy_settings_auto(): + settings = UU._proxy_settings(None) assert settings.mode == UU.PROXY_MODE_AUTO assert settings.proxy_list is None assert settings.powershell_proxy is None @@ -22,27 +16,62 @@ def test_proxy_settings_auto(clean_proxy_env): assert settings.password is None -def test_proxy_settings_no_proxy(clean_proxy_env, monkeypatch): - monkeypatch.setenv("NO_PROXY", "example.com") - monkeypatch.setenv("HTTP_PROXY", "http://proxy.example:8080") +# These values disable proxy connections for all endpoints +@pytest.mark.parametrize("v", ["1", "*", "y", "Y", "yes", "t", "TRUE"]) +def test_proxy_settings_no_proxy(assert_log, v): + class env: + no_proxy = v + http_proxy = "http://proxy.example:8080" + https_proxy = None - settings = UU._proxy_settings_from_env() + settings = UU._proxy_settings(env) + assert_log(assert_log.end_of_log()) assert settings.mode == UU.PROXY_MODE_DIRECT assert settings.proxy_list is None assert settings.powershell_proxy is None -def test_proxy_settings_override(clean_proxy_env, monkeypatch): - monkeypatch.setenv( - "HTTP_PROXY", - "http://http%40user:http%40password@http-proxy.example:8080/path", - ) - monkeypatch.setenv( - "HTTPS_PROXY", - "https://https%40user:https%40password@https-proxy.example:8443/path", - ) +# These values enable proxy connections for all endpoints, just like leaving +# NO_PROXY empty, and do not produce a warning. +@pytest.mark.parametrize("v", ["0", "n", "N", "no", "f", "FALSE"]) +def test_proxy_settings_no_proxy_false(assert_log, v): + class env: + no_proxy = v + http_proxy = "http://proxy.example:8080" + https_proxy = None - settings = UU._proxy_settings_from_env() + settings = UU._proxy_settings(env) + assert_log(assert_log.end_of_log()) + assert settings.mode == UU.PROXY_MODE_OVERRIDE + assert settings.proxy_list == "http=http://proxy.example:8080" + assert settings.powershell_proxy == "http://proxy.example:8080" + + +# These values are ignored and produce a warning. In the future, they may become +# a list of hosts for which the other proxy settings should be bypassed. +# For now, clearing the variables and relying on the OS configuration is best if +# some hosts need a direct connection. +@pytest.mark.parametrize("v", ["x", "example.com"]) +def test_proxy_settings_no_proxy_url(assert_log, v): + class env: + no_proxy = v + http_proxy = "http://proxy.example:8080" + https_proxy = None + + settings = UU._proxy_settings(env) + assert_log("Values for NO_PROXY.+are not supported.+") + assert settings.mode == UU.PROXY_MODE_OVERRIDE + assert settings.proxy_list == "http=http://proxy.example:8080" + assert settings.powershell_proxy == "http://proxy.example:8080" + + +def test_proxy_settings_override(): + class env: + no_proxy = None + http_proxy = "http://http%40user:http%40password@http-proxy.example:8080/path" + https_proxy = "https://https%40user:https%40password@https-proxy.example:8443/path" + + settings = UU._proxy_settings(env) assert settings.mode == UU.PROXY_MODE_OVERRIDE assert settings.proxy_list == ( "http=http://http-proxy.example:8080 " @@ -55,14 +84,13 @@ def test_proxy_settings_override(clean_proxy_env, monkeypatch): assert "https@password" not in settings.proxy_list -def test_proxy_settings_http_credentials_fallback(clean_proxy_env, monkeypatch): - monkeypatch.setenv( - "HTTP_PROXY", - "proxy%40user:proxy%40password@proxy.example:8080", - ) - monkeypatch.setenv("HTTPS_PROXY", "https://secure-proxy.example:8443") +def test_proxy_settings_http_credentials_fallback(): + class env: + no_proxy = None + http_proxy = "proxy%40user:proxy%40password@proxy.example:8080" + https_proxy = "https://secure-proxy.example:8443" - settings = UU._proxy_settings_from_env() + settings = UU._proxy_settings(env) assert settings.proxy_list == ( "http=http://proxy.example:8080 " "https=https://secure-proxy.example:8443" @@ -72,29 +100,41 @@ def test_proxy_settings_http_credentials_fallback(clean_proxy_env, monkeypatch): assert settings.password == "proxy@password" -def test_winhttp_proxy_env(clean_proxy_env, monkeypatch, localserver): - monkeypatch.setenv("HTTP_PROXY", localserver) - request = UU._Request("http://proxy-target.invalid/through-proxy") +def test_winhttp_proxy_env(localserver): + class env: + no_proxy = None + http_proxy = localserver + https_proxy = None + + request = UU._Request("http://proxy-target.invalid/through-proxy", proxy_info=env) assert UU._winhttp_urlopen(request) == b"Proxy OK" -def test_winhttp_proxy_env_auth(clean_proxy_env, monkeypatch, localserver): - proxy = localserver.replace( - "http://", - "http://proxy-user:proxy-password@", - ) - monkeypatch.setenv("HTTP_PROXY", proxy) - request = UU._Request("http://proxy-target.invalid/through-auth-proxy") +def test_winhttp_proxy_env_auth(localserver): + class env: + no_proxy = None + http_proxy = localserver.replace( + "http://", + "http://proxy-user:proxy-password@", + ) + https_proxy = None + + request = UU._Request("http://proxy-target.invalid/through-auth-proxy", proxy_info=env) assert UU._winhttp_urlopen(request) == b"Proxy Basic proxy-user:proxy-password" -def test_powershell_proxy_env(clean_proxy_env, monkeypatch, localserver, tmp_path): - monkeypatch.setenv("HTTPS_PROXY", localserver) +def test_powershell_proxy_env(localserver, tmp_path): + class env: + no_proxy = None + http_proxy = None + https_proxy = localserver + request = UU._Request( "http://proxy-target.invalid/through-proxy", outfile=tmp_path / "proxy.txt", + proxy_info=env, ) UU._powershell_urlretrieve(request) @@ -102,20 +142,18 @@ def test_powershell_proxy_env(clean_proxy_env, monkeypatch, localserver, tmp_pat assert request.outfile.read_bytes() == b"Proxy OK" -def test_powershell_proxy_env_auth( - clean_proxy_env, - monkeypatch, - localserver, - tmp_path, -): - proxy = localserver.replace( - "http://", - "http://proxy-user:proxy-password@", - ) - monkeypatch.setenv("HTTPS_PROXY", proxy) +def test_powershell_proxy_env_auth(localserver, tmp_path): + class env: + no_proxy = None + http_proxy = None + https_proxy = localserver.replace( + "http://", + "http://proxy-user:proxy-password@", + ) request = UU._Request( "http://proxy-target.invalid/through-auth-proxy", outfile=tmp_path / "proxy-auth.txt", + proxy_info=env, ) UU._powershell_urlretrieve(request) @@ -123,17 +161,16 @@ def test_powershell_proxy_env_auth( assert request.outfile.read_bytes() == b"Proxy Basic proxy-user:proxy-password" -def test_powershell_no_proxy_env( - clean_proxy_env, - monkeypatch, - localserver, - tmp_path, -): - monkeypatch.setenv("HTTP_PROXY", "http://127.0.0.1:1") - monkeypatch.setenv("NO_PROXY", "anything") +def test_powershell_no_proxy_env(localserver, tmp_path): + class env: + no_proxy = "*" + http_proxy = "http://127.0.0.1:1" + https_proxy = None + request = UU._Request( localserver + "/1kb", outfile=tmp_path / "direct.txt", + proxy_info=env, ) UU._powershell_urlretrieve(request) @@ -141,11 +178,16 @@ def test_powershell_no_proxy_env( assert len(request.outfile.read_bytes()) == 1024 -def test_bits_proxy_env(clean_proxy_env, monkeypatch, localserver, tmp_path): - monkeypatch.setenv("HTTP_PROXY", localserver) +def test_bits_proxy_env(localserver, tmp_path): + class env: + no_proxy = None + http_proxy = localserver + https_proxy = None + request = UU._Request( "http://proxy-target.invalid/through-proxy", outfile=tmp_path / "proxy.txt", + proxy_info=env, ) UU._bits_urlretrieve(request) @@ -153,15 +195,19 @@ def test_bits_proxy_env(clean_proxy_env, monkeypatch, localserver, tmp_path): assert request.outfile.read_bytes() == b"Proxy OK" -def test_bits_proxy_env_auth(clean_proxy_env, monkeypatch, localserver, tmp_path): - proxy = localserver.replace( - "http://", - "http://proxy-user:proxy-password@", - ) - monkeypatch.setenv("HTTP_PROXY", proxy) +def test_bits_proxy_env_auth(localserver, tmp_path): + class env: + no_proxy = None + http_proxy = localserver.replace( + "http://", + "http://proxy-user:proxy-password@", + ) + https_proxy = None + request = UU._Request( "http://proxy-target.invalid/through-auth-proxy", outfile=tmp_path / "proxy-auth.txt", + proxy_info=env, ) UU._bits_urlretrieve(request) @@ -169,25 +215,27 @@ def test_bits_proxy_env_auth(clean_proxy_env, monkeypatch, localserver, tmp_path assert request.outfile.read_bytes() == b"Proxy Basic proxy-user:proxy-password" -def test_winhttp_no_proxy_env(clean_proxy_env, monkeypatch, localserver): - monkeypatch.setenv("HTTP_PROXY", "http://127.0.0.1:1") - monkeypatch.setenv("NO_PROXY", "anything") - request = UU._Request(localserver + "/1kb") +def test_winhttp_no_proxy_env(localserver): + class env: + no_proxy = "1" + http_proxy = "http://127.0.0.1:1" + https_proxy = None + + request = UU._Request(localserver + "/1kb", proxy_info=env) assert UU._winhttp_urlopen(request) -def test_bits_no_proxy_env( - clean_proxy_env, - monkeypatch, - localserver, - tmp_path, -): - monkeypatch.setenv("HTTP_PROXY", "http://127.0.0.1:1") - monkeypatch.setenv("NO_PROXY", "anything") +def test_bits_no_proxy_env(localserver, tmp_path): + class env: + no_proxy = "1" + http_proxy = "http://127.0.0.1:1" + https_proxy = None + request = UU._Request( localserver + "/1kb", outfile=tmp_path / "direct.txt", + proxy_info=env, ) UU._bits_urlretrieve(request) From 1637832c16cebd4899ab98dbf90cf24e39e260b4 Mon Sep 17 00:00:00 2001 From: Steve Dower Date: Fri, 23 Oct 2026 12:12:42 +0100 Subject: [PATCH 3/7] Handle sensitive values in config better --- src/manage/commands.py | 6 +++--- src/manage/config.py | 26 +++++++++++++++----------- src/manage/exceptions.py | 11 ++++++++++- src/manage/install_command.py | 1 + src/manage/urlutils.py | 2 ++ tests/test_config.py | 2 +- 6 files changed, 32 insertions(+), 16 deletions(-) diff --git a/src/manage/commands.py b/src/manage/commands.py index f500896..af73f1a 100644 --- a/src/manage/commands.py +++ b/src/manage/commands.py @@ -259,8 +259,8 @@ def execute(self): # 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"), - "https_proxy": (str, None, "env"), + "http_proxy": (str, None, "env", "uri"), + "https_proxy": (str, None, "env", "uri"), "no_proxy": (str, None, "env"), # Global auth for accessing sources, typically set to '%PYMANAGER_USERNAME%' @@ -268,7 +268,7 @@ def execute(self): # '%PYTHON_MANAGER_USERNAME%' rather than '%PYMANAGER' - the latter is meant # for debugging/hidden options, not user-visible ones. "username": (str, None, "env"), - "password": (str, None, "env"), + "password": (str, None, "env", "sensitive"), "list": { "format": (str, None, "env"), diff --git a/src/manage/config.py b/src/manage/config.py index 91c3838..ebe078f 100644 --- a/src/manage/config.py +++ b/src/manage/config.py @@ -176,13 +176,13 @@ 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) 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, os.environ) @@ -195,7 +195,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 @@ -209,7 +209,7 @@ 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 @@ -242,11 +242,15 @@ def merge_config(into_cfg, from_cfg, schema, *, source="", overwrite=Fa LOGGER.warn("Invalid configuration key %s in %s", k, source) continue - _, merge, *_ = subschema - if not merge or overwrite: - LOGGER.debug("Updating config %s from %r to %r", k, into, v) + _, merge, *opts = subschema + if merge and not overwrite: + v = merge(into, v) + if v != into: + if "sensitive" in opts: + LOGGER.debug("Updating config %s", k) + elif "uri" in opts: + from .urlutils import sanitise_url + LOGGER.debug("Updating config %s to %r", k, sanitise_url(v)) + else: + LOGGER.debug("Updating config %s to %r", k, v) into_cfg[k] = v - else: - v2 = merge(into, v) - LOGGER.debug("Updating config %s from %r to %r", k, into, v2) - into_cfg[k] = v2 diff --git a/src/manage/exceptions.py b/src/manage/exceptions.py index 3f0c86c..3d7cc46 100644 --- a/src/manage/exceptions.py +++ b/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: diff --git a/src/manage/install_command.py b/src/manage/install_command.py index b8beb63..93bfe56 100644 --- a/src/manage/install_command.py +++ b/src/manage/install_command.py @@ -142,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 diff --git a/src/manage/urlutils.py b/src/manage/urlutils.py index 39ef839..a48a11f 100644 --- a/src/manage/urlutils.py +++ b/src/manage/urlutils.py @@ -820,6 +820,7 @@ def urlopen_index(self, url): "GET", {"Accept": "application/json"}, on_auth_request=self.on_auth, + proxy_info=self.cmd, ) except FileNotFoundError: # includes 404 (LOGGER.verbose if self.quiet else LOGGER.error)( @@ -870,6 +871,7 @@ def verify(self, url, data, params, show_settings=False): "GET", {"Accept": "application/octet-stream"}, on_auth_request=self.on_auth, + proxy_info=self.cmd, ) self._cache[url + ".cat"] = cat except OSError as ex: diff --git a/tests/test_config.py b/tests/test_config.py index f67f6db..dd61698 100644 --- a/tests/test_config.py +++ b/tests/test_config.py @@ -200,4 +200,4 @@ def test_config_split_append_does_not_mutate_existing_list(): assert actual == ["first", ["second", "third"]] assert existing == ["first"] - assert actual is not existing \ No newline at end of file + assert actual is not existing From 15ddc46a9eef7bd91e544aea560efcb3c1edb13c Mon Sep 17 00:00:00 2001 From: Steve Dower Date: Fri, 23 Oct 2026 12:27:45 +0100 Subject: [PATCH 4/7] Fix setting --- src/pymanager.json | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/pymanager.json b/src/pymanager.json index b4f24f9..12d93e8 100644 --- a/src/pymanager.json +++ b/src/pymanager.json @@ -15,7 +15,7 @@ "http_proxy": "%HTTP_PROXY%", "https_proxy": "%HTTPS_PROXY%", - "no_proxy": "%NO_PROXY", + "no_proxy": "%NO_PROXY%", "username": "%PYMANAGER_USERNAME%", "password": "%PYMANAGER_PASSWORD%", From b0b9c7203711d6bc79422e897dc2d35cbca18352 Mon Sep 17 00:00:00 2001 From: Steve Dower Date: Fri, 23 Oct 2026 12:46:11 +0100 Subject: [PATCH 5/7] Fix leak of initial config values --- src/manage/commands.py | 8 ++++---- src/manage/config.py | 21 +++++++++++++++------ 2 files changed, 19 insertions(+), 10 deletions(-) diff --git a/src/manage/commands.py b/src/manage/commands.py index af73f1a..bfe4f82 100644 --- a/src/manage/commands.py +++ b/src/manage/commands.py @@ -259,15 +259,15 @@ def execute(self): # 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", "uri"), - "https_proxy": (str, None, "env", "uri"), - "no_proxy": (str, None, "env"), + "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"), + "username": (str, None, "env", "sensitive"), "password": (str, None, "env", "sensitive"), "list": { diff --git a/src/manage/config.py b/src/manage/config.py index ebe078f..205a10d 100644 --- a/src/manage/config.py +++ b/src/manage/config.py @@ -217,12 +217,6 @@ def merge_config(into_cfg, from_cfg, schema, *, source="", overwrite=Fa 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] @@ -233,6 +227,7 @@ def merge_config(into_cfg, from_cfg, schema, *, source="", overwrite=Fa 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) @@ -242,6 +237,20 @@ def merge_config(into_cfg, from_cfg, schema, *, source="", overwrite=Fa LOGGER.warn("Invalid configuration key %s in %s", k, source) continue + try: + into = into_cfg[k] + except KeyError: + # No existing value, so just assign + if "sensitive" in opts: + LOGGER.debug("Setting config %s", k) + elif "uri" in opts: + from .urlutils import sanitise_url + LOGGER.debug("Setting config %s to %r", k, sanitise_url(v)) + else: + LOGGER.debug("Setting config %s to %r", k, v) + into_cfg[k] = v + continue + _, merge, *opts = subschema if merge and not overwrite: v = merge(into, v) From 67e18ae4dc9028fe1c98a9549d572bdab5de7c5d Mon Sep 17 00:00:00 2001 From: Steve Dower Date: Fri, 23 Oct 2026 13:48:50 +0100 Subject: [PATCH 6/7] Fix schema read --- src/manage/config.py | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/src/manage/config.py b/src/manage/config.py index 205a10d..e16086e 100644 --- a/src/manage/config.py +++ b/src/manage/config.py @@ -237,6 +237,8 @@ def merge_config(into_cfg, from_cfg, schema, *, source="", overwrite=Fa LOGGER.warn("Invalid configuration key %s in %s", k, source) continue + _, merge, *opts = subschema + try: into = into_cfg[k] except KeyError: @@ -251,7 +253,6 @@ def merge_config(into_cfg, from_cfg, schema, *, source="", overwrite=Fa into_cfg[k] = v continue - _, merge, *opts = subschema if merge and not overwrite: v = merge(into, v) if v != into: From 91be83864daace66e981bee35c52d5f7dad90910 Mon Sep 17 00:00:00 2001 From: Steve Dower Date: Fri, 23 Oct 2026 14:39:25 +0100 Subject: [PATCH 7/7] Added test --- src/manage/config.py | 36 +++++++++++++---------------- tests/test_config.py | 55 ++++++++++++++++++++++++++++++++++++++++++++ 2 files changed, 71 insertions(+), 20 deletions(-) diff --git a/src/manage/config.py b/src/manage/config.py index 63044c6..ace95ec 100644 --- a/src/manage/config.py +++ b/src/manage/config.py @@ -316,7 +316,10 @@ def resolve_config(cfg, source, relative_to, key_so_far="", schema=None, error_u cfg[k] = v -def merge_config(into_cfg, from_cfg, schema, *, source="", overwrite=False): +def merge_config(into_cfg, from_cfg, schema, *, key_so_far="", source="", 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 @@ -325,19 +328,20 @@ def merge_config(into_cfg, from_cfg, schema, *, source="", overwrite=Fa 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, *opts = subschema @@ -345,25 +349,17 @@ def merge_config(into_cfg, from_cfg, schema, *, source="", overwrite=Fa try: into = into_cfg[k] except KeyError: - # No existing value, so just assign - if "sensitive" in opts: - LOGGER.debug("Setting config %s", k) - elif "uri" in opts: - from .urlutils import sanitise_url - LOGGER.debug("Setting config %s to %r", k, sanitise_url(v)) - else: - LOGGER.debug("Setting config %s to %r", k, v) - into_cfg[k] = v - continue + into = None + else: + if merge and not overwrite: + v = merge(into, v) - if merge and not overwrite: - v = merge(into, v) if v != into: if "sensitive" in opts: - LOGGER.debug("Updating config %s", k) + 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", k, sanitise_url(v)) + LOGGER.debug("Updating config %s to %r", key_so_far + k, sanitise_url(v)) else: - LOGGER.debug("Updating config %s to %r", k, v) + LOGGER.debug("Updating config %s to %r", key_so_far + k, v) into_cfg[k] = v diff --git a/tests/test_config.py b/tests/test_config.py index 788180a..ac917ce 100644 --- a/tests/test_config.py +++ b/tests/test_config.py @@ -11,6 +11,7 @@ config_dict_merge, config_split, config_split_append, + merge_config, resolve_config, ) @@ -252,3 +253,57 @@ def test_config_resolve_env_fallback(assert_log): resolve_config(cfg, "", Path(), schema=schema, environ=env) assert cfg == expect assert_log(("Environment variable.*not found", ["AppData"])) + + +@pytest.mark.parametrize( + ("cfg", "schema", "expect"), + [ + ({"a": "abc"}, {"a": (str, None, "sensitive")}, ["a"]), + # No redaction without "sensitive" + ({"a": "abc"}, {"a": (str, None)}, ["a", "abc"]), + ({"a": "http://example.com/"}, {"a": (str, None, "uri")}, ["a", "http://example.com/"]), + ({"a": "http://user:pass@example.com/"}, {"a": (str, None, "uri")}, ["a", "http://example.com/"]), + ({"a": "NOT A URI"}, {"a": (str, None, "uri")}, ["a", "NOT A URI"]), + # No redaction without "uri" + ({"a": "http://user:pass@example.com/"}, {"a": (str, None)}, ["a", "http://user:pass@example.com/"]), + ], + ids=[ + "sensitive-str", + "str", + "uri", + "uri-with-creds", + "bad-uri", + "str-with-creds", + ], +) +def test_config_redact_single(cfg, schema, expect, assert_log): + into = {} + merge_config(into, cfg, schema) + assert into == cfg + + into = {k: None for k in into} + merge_config(into, cfg, schema) + assert into == cfg + + assert_log(("Updating config.+", expect), ("Updating config.+", expect)) + +def test_config_redact_nested(assert_log): + cfg = { + "str": dict(a="abc", x="xyz"), + "uri": dict(a="http://example.com/", x="http://user:pass@example.com/"), + } + schema = { + "str": dict(a=(str, None), x=(str, None, "sensitive")), + "uri": dict(a=(str, None, "uri"), x=(str, None, "uri")), + } + into = {} + merge_config(into, cfg, schema) + assert into == cfg + assert_log( + ("Recursively updating.+", ["str"]), + ("Updating config.+to.+", ["str.a", "abc"]), + ("Updating config.+", ["str.x"]), + ("Recursively updating.+", ["uri"]), + ("Updating config.+to.+", ["uri.a", "http://example.com/"]), + ("Updating config.+to.+", ["uri.x", "http://example.com/"]), + )