From cc9761bcfddc170f7d0a93207f537016217ac794 Mon Sep 17 00:00:00 2001 From: Steve Dower Date: Tue, 20 Oct 2026 22:11:55 +0100 Subject: [PATCH 1/2] Adds fallback to newly created environment block for known OS variables. Makes it a hard failure for environment variables in paths to be empty. Fixes #427 --- _msbuild.py | 4 +- _msbuild_test.py | 3 +- src/_native/misc.cpp | 28 +++++++++++++ src/manage/config.py | 99 +++++++++++++++++++++++++++++++++++++++++++- tests/test_config.py | 23 +++++++--- 5 files changed, 148 insertions(+), 9 deletions(-) diff --git a/_msbuild.py b/_msbuild.py index ca920d4..ecf0b0e 100644 --- a/_msbuild.py +++ b/_msbuild.py @@ -67,7 +67,8 @@ class ResourceFile(CSourceFile): PyFile('__init__.py'), CPP_SETTINGS, ItemDefinition("Link", - AdditionalDependencies=Prepend("wintrust.lib;"), + AdditionalDependencies=Prepend("wintrust.lib;userenv.lib;"), + DelayLoadDLLs='userenv.dll', ), IncludeFile('*.h'), CSourceFile('*.cpp'), @@ -100,6 +101,7 @@ class ResourceFile(CSourceFile): CFunction('read_alias_package'), CFunction('broadcast_settings_change'), CFunction('get_processor_architecture'), + CFunction('get_environment_block'), CFunction('verify_trust'), source='src/_native', RootNamespace='_native', diff --git a/_msbuild_test.py b/_msbuild_test.py index 4aebfb5..2a0290b 100644 --- a/_msbuild_test.py +++ b/_msbuild_test.py @@ -24,7 +24,7 @@ LanguageStandard="stdcpp20", ), ItemDefinition("Link", - AdditionalDependencies=Prepend("wintrust.lib;"), + AdditionalDependencies=Prepend("wintrust.lib;userenv.lib;"), ), IncludeFile('*.h'), CSourceFile('*.cpp'), @@ -58,6 +58,7 @@ CFunction('read_alias_package'), CFunction('broadcast_settings_change'), CFunction('get_processor_architecture'), + CFunction('get_environment_block'), CFunction('verify_trust'), source='src/_native', ), diff --git a/src/_native/misc.cpp b/src/_native/misc.cpp index 45cdf1d..2edc641 100644 --- a/src/_native/misc.cpp +++ b/src/_native/misc.cpp @@ -1,6 +1,7 @@ #include #include #include +#include #include "helpers.h" @@ -253,4 +254,31 @@ PyObject *get_processor_architecture(PyObject *, PyObject *, PyObject *) { return PyUnicode_FromString("-64"); } +PyObject *get_environment_block(PyObject *, PyObject *, PyObject *) { + HANDLE token; + wchar_t *env = NULL; + + if (!OpenProcessToken(GetCurrentProcess(), TOKEN_QUERY | TOKEN_DUPLICATE, &token) + || !CreateEnvironmentBlock((LPVOID *)&env, token, FALSE)) { + PyErr_SetFromWindowsErr(0); + return NULL; + } + + // env is double null terminated, so read each string's length until we find + // the zero-length one. Our caller can do the split. + wchar_t *p = env; + size_t cch = wcslen(p); + while (p && cch) { + p += cch + 1; + cch = wcslen(p); + } + + PyObject *r = PyUnicode_FromWideChar(env, p - env); + + DestroyEnvironmentBlock(env); + CloseHandle(token); + + return r; +} + } diff --git a/src/manage/config.py b/src/manage/config.py index 91c3838..2ce2337 100644 --- a/src/manage/config.py +++ b/src/manage/config.py @@ -150,12 +150,107 @@ def load_registry_config(key_path, schema): return cfg +class _EnvironWithFallback(dict): + # Key names must be uppercase + # These are the keys that are filled in from a default environment if they + # are missing in the current process environment. This ensures that a + # broken environment doesn't lead to an invalid loaded configuration. It + # only applies to configuration files, and does not change the environment. + # A launched Python process will probably also fail without some of these, + # at which point the caller still has to fix their setup. + FALLBACK_KEYS = { + "ALLUSERSPROFILE", + "APPDATA", + "COMMONPROGRAMSFILES", + "COMMONPROGRAMSFILES(X86)", + "COMMONPROGRAMW6432", + "COMPUTERNAME", + "COMSPEC", + "HOMEDRIVE", + "HOMEPATH", + "LOCALAPPDATA", + "LOGONSERVER", + "NUMBER_OF_PROCESSORS", + "OS", + "PATH", + "PATHEXT", + "PROCESSOR_ARCHITECTURE", + "PROCESSOR_IDENTIFIER", + "PROCESSOR_LEVEL", + "PROCESSOR_REVISION", + "PROGRAMDATA", + "PROGRAMFILES", + "PROGRAMFILES(X86)", + "PROGRAMW6432", + "PUBLIC", + "SESSIONNAME", + "SYSTEMDRIVE", + "SYSTEMROOT", + "TEMP", + "TMP", + "USERDNSDOMAIN", + "USERDOMAIN", + "USERDOMAIN_ROAMINGPROFILE", + "USERNAME", + "USERPROFILE", + "WINDIR", + } + + _default = None + + @classmethod + def default(cls): + if cls._default is None: + cls._default = cls() + return cls._default + + def __init__(self, d=os.environ): + self._fallback = None + for k, v in (d or {}).items(): + self[k.upper()] = v + + def _get_fallback(self, k): + assert k.isupper() + if k not in self.FALLBACK_KEYS: + return None + if self._fallback is None: + from _native import get_environment_block + self._fallback = {} + for var in get_environment_block().split("\0"): + k2, _, v2 = var.partition("=") + self._fallback[k2.upper()] = v2 + return self._fallback[k] + + def get(self, key, default=None): + k = key.upper() + try: + return self[k] + except KeyError: + pass + + v = self._get_fallback(k) + if v: + LOGGER.warn("Environment variable %%%s%% was not found and so " + "a default was loaded. This usually indicates an " + "issue in your launch conditions and could lead to " + "later failures. Suppress this warning by ensuring " + "system environment variables remain set, or by " + "changing your configuration file to not rely on " + "them.", key) + + return v or default + + + def _expand_vars(v, env): import re def _sub(m): v2 = env.get(m.group(1)) if v2: return v2 + (m.group(2) or "") + if m.group(2): + # Empty values before a path separator are not allowed + raise LookupError(m.group(1)) return "" v2 = re.sub(r"%(.*?)%([\\/])?", _sub, v) return v2 @@ -185,10 +280,12 @@ def resolve_config(cfg, source, relative_to, key_so_far="", schema=None, error_u if "env" in opts and isinstance(v, str): try: orig_v = v - v = _expand_vars(v, os.environ) + v = _expand_vars(v, _EnvironWithFallback.default()) from_env = orig_v != v except TypeError: pass + except LookupError as ex: + raise InvalidConfigurationError(source, key_so_far + k, f'%{ex}%') from ex if not v: del cfg[k] continue diff --git a/tests/test_config.py b/tests/test_config.py index f67f6db..2da54db 100644 --- a/tests/test_config.py +++ b/tests/test_config.py @@ -1,6 +1,7 @@ import pytest from manage.config import ( + _EnvironWithFallback, _expand_vars, config_append, config_bool, @@ -88,16 +89,26 @@ def test_expand_vars(value, variables, expected): assert _expand_vars(value, variables) == expected -def test_expand_vars_removes_missing_variable_and_leading_separator(): - actual = _expand_vars("%MISSING%\\Python", {}) +def test_expand_vars_fails_on_missing_variable(): + with pytest.raises(LookupError): + _expand_vars("%MISSING%\\Python", {}) - assert actual == "Python" +def test_expand_vars_fails_on_empty_variable(): + with pytest.raises(LookupError): + _expand_vars("%EMPTY%\\Python", {"EMPTY": ""}) -def test_expand_vars_removes_empty_variable_and_leading_separator(): - actual = _expand_vars("%EMPTY%\\Python", {"EMPTY": ""}) - assert actual == "Python" +def test_environ_with_fallback(): + env = _EnvironWithFallback({"AllUsersProfile": "test", "APPDATA": ""}) + # Should be the value we provided, despite case + assert env.get("allusersprofile") == "test" + # Should be empty, since it was provided as empty + assert env.get("AppData", "default") == "" + # Should be set, even though it wasn't provided + assert env.get("WinDir", "default") not in (None, "", "default") + # Should be empty, since it's neither set nor has a fallback + assert env.get("PYTHON_MANAGER_DEFAULT", "default") == "default" @pytest.mark.parametrize( From a6fcc15c460208b7ff41fc35c956c7418745d049 Mon Sep 17 00:00:00 2001 From: Steve Dower Date: Tue, 20 Oct 2026 22:54:15 +0100 Subject: [PATCH 2/2] Add test for the env fallback --- src/manage/config.py | 8 +++++--- tests/test_config.py | 42 +++++++++++++++++++++++++++++++++++++++++- 2 files changed, 46 insertions(+), 4 deletions(-) diff --git a/src/manage/config.py b/src/manage/config.py index 2ce2337..8e44a1b 100644 --- a/src/manage/config.py +++ b/src/manage/config.py @@ -256,7 +256,7 @@ def _sub(m): return v2 -def resolve_config(cfg, source, relative_to, key_so_far="", schema=None, error_unknown=False): +def resolve_config(cfg, source, relative_to, key_so_far="", schema=None, error_unknown=False, environ=None): for k, v in list(cfg.items()): if k.startswith("#"): continue @@ -280,7 +280,7 @@ def resolve_config(cfg, source, relative_to, key_so_far="", schema=None, error_u if "env" in opts and isinstance(v, str): try: orig_v = v - v = _expand_vars(v, _EnvironWithFallback.default()) + v = _expand_vars(v, environ or _EnvironWithFallback.default()) from_env = orig_v != v except TypeError: pass @@ -299,7 +299,9 @@ def resolve_config(cfg, source, relative_to, key_so_far="", schema=None, error_u if not from_env: v = relative_to / v else: - v = type(relative_to)(v).absolute() + v = type(relative_to)(v) + if not v.is_absolute(): + v = v.absolute() if v and "uri" in opts: if hasattr(v, "as_uri"): v = v.as_uri() diff --git a/tests/test_config.py b/tests/test_config.py index 2da54db..b17fd07 100644 --- a/tests/test_config.py +++ b/tests/test_config.py @@ -1,5 +1,8 @@ +import os import pytest +from pathlib import Path, PurePath + from manage.config import ( _EnvironWithFallback, _expand_vars, @@ -8,6 +11,7 @@ config_dict_merge, config_split, config_split_append, + resolve_config, ) @@ -211,4 +215,40 @@ 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 + + +@pytest.mark.parametrize( + ("cfg", "schema", "expect", "env"), + [ + ({}, {"a": (int, None), "b": (int, None)}, {}, None), + ({"a": "1"}, {"a": (int, None)}, {"a": 1}, None), + ({"a": 1}, {"a": (str, None)}, {"a": "1"}, None), + ({"a": "abc"}, {"a": (str, None, "path")}, {"a": PurePath("X:/test/abc")}, None), + ({"a": "%X%"}, {"a": (str, None, "path")}, {"a": PurePath("X:/test/%X%")}, {"X": "abc"}), + ({"a": "%X%"}, {"a": (str, None, "path", "env")}, {"a": PurePath("X:/env/abc")}, {"X": "X:/env/abc"}), + ({"a": "%X%"}, {"a": (str, None, "path")}, {"a": PurePath("X:/test/%X%")}, None), + ], + ids=[ + "empty", + "to_int", + "to_str", + "to_path", + "to_path_without_env", + "to_path_with_env", + "to_path_not_in_env", + ], +) +def test_config_resolve(cfg, schema, expect, env, monkeypatch): + resolve_config(cfg, "", PurePath("X:/test/"), schema=schema, environ=env) + assert cfg == expect + + +def test_config_resolve_env_fallback(monkeypatch, assert_log): + cfg = {"a": "%AppData%/Python"} + schema = {"a": (str, None, "path", "env")} + expect = {"a": Path(os.environ["AppData"]) / "Python"} + monkeypatch.delitem(os.environ, "AppData") + resolve_config(cfg, "", Path(), schema=schema) + assert cfg == expect + assert_log(("Environment variable.*not found", ["AppData"]))