diff --git a/src/specify_cli/extensions/__init__.py b/src/specify_cli/extensions/__init__.py index b84b836545..856f13d124 100644 --- a/src/specify_cli/extensions/__init__.py +++ b/src/specify_cli/extensions/__init__.py @@ -2758,15 +2758,24 @@ def _get_env_config(self) -> Dict[str, Any]: # Remove prefix and split into parts config_path = key[len(prefix) :].lower().split("_") - # Build nested dict + # Build nested dict. Two env vars can collide on a prefix, e.g. + # SPECKIT_X_CONNECTION=a and SPECKIT_X_CONNECTION_URL=b. Guard the + # walk so a colliding scalar is replaced by a dict (deeper/more + # specific vars win) instead of being indexed into — which raised + # TypeError ('str' object does not support item assignment) — and + # guard the leaf so a scalar processed after the nested var does + # not clobber the nested dict. Order-independent: both insertion + # orders yield {'connection': {'url': ...}}. Nested-wins mirrors + # _merge_configs' dict-preserving semantics. current = env_config for part in config_path[:-1]: - if part not in current: + if not isinstance(current.get(part), dict): current[part] = {} current = current[part] - # Set the final value - current[config_path[-1]] = value + # Set the final value, unless a nested dict already occupies it. + if not isinstance(current.get(config_path[-1]), dict): + current[config_path[-1]] = value return env_config diff --git a/tests/test_extensions.py b/tests/test_extensions.py index 9dae5fefda..c19f7c703b 100644 --- a/tests/test_extensions.py +++ b/tests/test_extensions.py @@ -7586,3 +7586,46 @@ def test_hook_condition_returns_false_without_raising(self, tmp_path): (ext_dir / "jira-config.yml").write_text("just a string\n", encoding="utf-8") executor = HookExecutor(tmp_path) assert executor._evaluate_condition("config.x is set", "jira") is False + + +class TestConfigManagerEnvPrefixCollision: + """Prefix-colliding env vars must not crash or clobber nested config.""" + + def test_scalar_then_nested_yields_nested(self, tmp_path, monkeypatch): + """SPECKIT_X_CONNECTION=x then SPECKIT_X_CONNECTION_URL=y. + + The scalar-first order previously raised TypeError ('str' object + does not support item assignment) when the walk indexed into 'x'. + """ + monkeypatch.setenv("SPECKIT_TESTEXT_CONNECTION", "x") + monkeypatch.setenv("SPECKIT_TESTEXT_CONNECTION_URL", "y") + cm = ConfigManager(tmp_path, "testext") + assert cm._get_env_config() == {"connection": {"url": "y"}} + + def test_nested_then_scalar_does_not_clobber(self, tmp_path, monkeypatch): + """Reverse order previously returned {'connection': 'x'}, losing url.""" + monkeypatch.setenv("SPECKIT_TESTEXT_CONNECTION_URL", "y") + monkeypatch.setenv("SPECKIT_TESTEXT_CONNECTION", "x") + cm = ConfigManager(tmp_path, "testext") + assert cm._get_env_config() == {"connection": {"url": "y"}} + + def test_colliding_env_does_not_disable_hook_condition(self, tmp_path, monkeypatch): + """`config.connection.url is set` must stay True under colliding env. + + Before the fix the TypeError propagated into should_execute_hook's + blanket `except Exception: return False`, silently disabling the hook. + """ + ext_dir = tmp_path / ".specify" / "extensions" / "testext" + ext_dir.mkdir(parents=True) + (ext_dir / "testext-config.yml").write_text( + "connection:\n url: https://example.com\n", encoding="utf-8" + ) + monkeypatch.setenv("SPECKIT_TESTEXT_CONNECTION", "x") + monkeypatch.setenv("SPECKIT_TESTEXT_CONNECTION_URL", "y") + executor = HookExecutor(tmp_path) + # Exercise the public API: before the fix the TypeError was swallowed + # by should_execute_hook's `except Exception: return False`, so the + # hook was silently disabled (False); after the fix it returns True. + assert executor.should_execute_hook( + {"condition": "config.connection.url is set", "extension": "testext"} + ) is True