From 1b61381ac3d2df93ad6766a22335332f50023f53 Mon Sep 17 00:00:00 2001 From: Enrico Donnici Date: Tue, 24 Feb 2026 15:33:42 +0100 Subject: [PATCH 1/6] Add trusted `_fetch_command` secret resolution to config fields and DynamicD. This introduces trust-gated local override handling, actionable command-resolution errors, and centralized scrubbing so secret values and fetch-command fields are resolved safely and redacted consistently. --- ddev/src/ddev/cli/__init__.py | 4 + ddev/src/ddev/cli/config/__init__.py | 4 + ddev/src/ddev/cli/config/allow.py | 33 +++ ddev/src/ddev/cli/config/deny.py | 32 +++ ddev/src/ddev/cli/config/override.py | 2 +- ddev/src/ddev/cli/config/set.py | 3 +- .../ddev/cli/meta/scripts/_dynamicd/cli.py | 17 +- ddev/src/ddev/config/command_resolver.py | 87 ++++++++ ddev/src/ddev/config/file.py | 32 ++- ddev/src/ddev/config/model.py | 98 ++++++++- ddev/src/ddev/config/override_trust.py | 86 ++++++++ ddev/src/ddev/config/scrubber.py | 46 +++++ ddev/src/ddev/config/utils.py | 20 -- ddev/tests/cli/config/test_allow.py | 43 ++++ ddev/tests/cli/config/test_deny.py | 56 ++++++ ddev/tests/cli/config/test_show.py | 23 +++ .../cli/meta/scripts/test_dynamicd_cli.py | 65 ++++++ ddev/tests/config/test_command_resolver.py | 78 +++++++ ddev/tests/config/test_file.py | 190 ++++++++++++++++++ ddev/tests/config/test_model.py | 138 +++++++++++++ 20 files changed, 1020 insertions(+), 37 deletions(-) create mode 100644 ddev/src/ddev/cli/config/allow.py create mode 100644 ddev/src/ddev/cli/config/deny.py create mode 100644 ddev/src/ddev/config/command_resolver.py create mode 100644 ddev/src/ddev/config/override_trust.py create mode 100644 ddev/src/ddev/config/scrubber.py create mode 100644 ddev/tests/cli/config/test_allow.py create mode 100644 ddev/tests/cli/config/test_deny.py create mode 100644 ddev/tests/cli/meta/scripts/test_dynamicd_cli.py create mode 100644 ddev/tests/config/test_command_resolver.py diff --git a/ddev/src/ddev/cli/__init__.py b/ddev/src/ddev/cli/__init__.py index 21b552684ee14..2ac843183590d 100644 --- a/ddev/src/ddev/cli/__init__.py +++ b/ddev/src/ddev/cli/__init__.py @@ -130,6 +130,10 @@ def ddev( app.config_file.load() except OSError as e: # no cov app.abort(f'Error loading configuration: {e}') + + for warning in app.config_file.load_warnings: + app.display_warning(warning) + if app.config.upgrade_check: upgrade_check.upgrade_check(app, __version__) diff --git a/ddev/src/ddev/cli/config/__init__.py b/ddev/src/ddev/cli/config/__init__.py index da883b9c1c0c8..8c0741678d91f 100644 --- a/ddev/src/ddev/cli/config/__init__.py +++ b/ddev/src/ddev/cli/config/__init__.py @@ -4,6 +4,8 @@ import click +from ddev.cli.config.allow import allow +from ddev.cli.config.deny import deny from ddev.cli.config.edit import edit from ddev.cli.config.explore import explore from ddev.cli.config.find import find @@ -18,6 +20,8 @@ def config(): pass +config.add_command(allow) +config.add_command(deny) config.add_command(edit) config.add_command(explore) config.add_command(find) diff --git a/ddev/src/ddev/cli/config/allow.py b/ddev/src/ddev/cli/config/allow.py new file mode 100644 index 0000000000000..beaa6b6ccfe52 --- /dev/null +++ b/ddev/src/ddev/cli/config/allow.py @@ -0,0 +1,33 @@ +# (C) Datadog, Inc. 2022-present +# All rights reserved +# Licensed under a 3-clause BSD style license (see LICENSE) +from __future__ import annotations + +from typing import TYPE_CHECKING + +import click + +if TYPE_CHECKING: + from ddev.cli.application import Application + + +@click.command(short_help='Trust the local .ddev.toml so its _fetch_command fields are executed') +@click.pass_obj +def allow(app: Application): + """Mark the local ``.ddev.toml`` as trusted. + + When trusted, ``*_fetch_command`` fields in the override file are executed to + resolve secret values. The current file hash is stored; if the file + changes the trust is automatically revoked and a warning is shown. + """ + from ddev.config.override_trust import upsert_trust_entry + + if not app.config_file.overrides_available(): + app.abort(f'No {".ddev.toml"} file found in the current directory or any parent directory.') + + upsert_trust_entry( + overrides_path=app.config_file.overrides_path, + global_config_dir=app.config_file.global_path.parent, + state='allowed', + ) + app.display_success(f'Trusted: {app.config_file.pretty_overrides_path}') diff --git a/ddev/src/ddev/cli/config/deny.py b/ddev/src/ddev/cli/config/deny.py new file mode 100644 index 0000000000000..d816e22913d2c --- /dev/null +++ b/ddev/src/ddev/cli/config/deny.py @@ -0,0 +1,32 @@ +# (C) Datadog, Inc. 2022-present +# All rights reserved +# Licensed under a 3-clause BSD style license (see LICENSE) +from __future__ import annotations + +from typing import TYPE_CHECKING + +import click + +if TYPE_CHECKING: + from ddev.cli.application import Application + + +@click.command(short_help='Silence warnings about untrusted _fetch_command fields in .ddev.toml') +@click.pass_obj +def deny(app: Application): + """Mark the local ``.ddev.toml`` as explicitly untrusted. + + ``*_fetch_command`` fields in the override file will be stripped silently + (no warning shown). Use ``ddev config allow`` to re-enable execution. + """ + from ddev.config.override_trust import upsert_trust_entry + + if not app.config_file.overrides_available(): + app.abort(f'No {".ddev.toml"} file found in the current directory or any parent directory.') + + upsert_trust_entry( + overrides_path=app.config_file.overrides_path, + global_config_dir=app.config_file.global_path.parent, + state='denied', + ) + app.display_success(f'Silenced: {app.config_file.pretty_overrides_path}') diff --git a/ddev/src/ddev/cli/config/override.py b/ddev/src/ddev/cli/config/override.py index ddc218fe44590..5ee58a6382395 100644 --- a/ddev/src/ddev/cli/config/override.py +++ b/ddev/src/ddev/cli/config/override.py @@ -65,7 +65,7 @@ def override(app: Application): from rich.syntax import Syntax from ddev.config.file import DDEV_TOML, RootConfig, deep_merge_with_list_handling - from ddev.config.utils import scrub_config + from ddev.config.scrubber import scrub_config from ddev.utils.fs import Path from ddev.utils.toml import dumps_toml_data diff --git a/ddev/src/ddev/cli/config/set.py b/ddev/src/ddev/cli/config/set.py index 578306907b92c..3d4e423d786f5 100644 --- a/ddev/src/ddev/cli/config/set.py +++ b/ddev/src/ddev/cli/config/set.py @@ -66,7 +66,8 @@ def set_value(app: Application, key: str, value: str | None, overrides: bool): import tomlkit - from ddev.config.utils import SCRUBBED_GLOBS, create_toml_document, save_toml_document, scrub_config + from ddev.config.scrubber import SCRUBBED_GLOBS, scrub_config + from ddev.config.utils import create_toml_document, save_toml_document scrubbing = any(fnmatch(key, glob) for glob in SCRUBBED_GLOBS) if value is None: diff --git a/ddev/src/ddev/cli/meta/scripts/_dynamicd/cli.py b/ddev/src/ddev/cli/meta/scripts/_dynamicd/cli.py index cb0b4a2e4be73..54e07bf163029 100644 --- a/ddev/src/ddev/cli/meta/scripts/_dynamicd/cli.py +++ b/ddev/src/ddev/cli/meta/scripts/_dynamicd/cli.py @@ -11,6 +11,7 @@ import click from ddev.cli.meta.scripts._dynamicd.constants import SCENARIOS +from ddev.config.command_resolver import CommandExecutionError, run_command if TYPE_CHECKING: from ddev.cli.application import Application @@ -78,8 +79,20 @@ def _get_api_keys(app: Application) -> tuple[str, str]: Returns (llm_api_key, dd_api_key) or aborts if not configured. """ - # Get LLM API key from config or environment variable - llm_api_key = app.config.raw_data.get("dynamicd", {}).get("llm_api_key") + dynamicd_cfg = app.config.raw_data.get("dynamicd", {}) + + # Resolve LLM API key: command > plain value > env var + llm_api_key = None + if "llm_api_key_fetch_command" in dynamicd_cfg: + cmd = dynamicd_cfg["llm_api_key_fetch_command"] + try: + llm_api_key = run_command(cmd) + except CommandExecutionError as e: + app.display_error(e.to_user_message('dynamicd.llm_api_key_fetch_command')) + app.abort() + + if not llm_api_key: + llm_api_key = dynamicd_cfg.get("llm_api_key") if not llm_api_key: llm_api_key = os.environ.get("ANTHROPIC_API_KEY") if not llm_api_key: diff --git a/ddev/src/ddev/config/command_resolver.py b/ddev/src/ddev/config/command_resolver.py new file mode 100644 index 0000000000000..43a94887e704e --- /dev/null +++ b/ddev/src/ddev/config/command_resolver.py @@ -0,0 +1,87 @@ +# (C) Datadog, Inc. 2022-present +# All rights reserved +# Licensed under a 3-clause BSD style license (see LICENSE) +"""Per-process command resolver with deterministic caching for `*_fetch_command` secret fields.""" +from __future__ import annotations + +import subprocess + +# Failure reasons surfaced by CommandExecutionError. +NON_ZERO_EXIT = 'non_zero_exit' +EMPTY_OUTPUT = 'empty_output' + +# Per-process cache keyed by exact command string. +_COMMAND_CACHE: dict[str, str] = {} + + +class CommandExecutionError(Exception): + """Raised when a secret-resolution command exits non-zero or produces no output.""" + + _MAX_STDERR_CHARS = 200 + + def __init__(self, command: str, returncode: int, stderr: str, reason: str): + self.command = command + self.returncode = returncode + self.reason = reason + self.stderr = stderr.strip() + super().__init__(self._reason_message()) + + def _stderr_excerpt(self) -> str: + if not self.stderr: + return '' + + # Keep user-facing output compact and single-line. + cleaned = ' '.join(self.stderr.split()) + if len(cleaned) > self._MAX_STDERR_CHARS: + return f'{cleaned[: self._MAX_STDERR_CHARS - 3]}...' + return cleaned + + def _reason_message(self) -> str: + if self.reason == EMPTY_OUTPUT: + return 'command returned empty output' + + stderr_excerpt = self._stderr_excerpt() + if stderr_excerpt: + return f'command failed with exit code {self.returncode}: {stderr_excerpt}' + return f'command failed with exit code {self.returncode}' + + def to_user_message(self, field_path: str) -> str: + return ( + f"Failed to resolve `{field_path}`: {self._reason_message()}. " + "Check that the configured *_fetch_command exists, is executable, writes the secret to stdout, " + "and returns a non-empty value." + ) + + +def run_command(command: str) -> str: + """Execute *command* in a shell and return stdout stripped of surrounding whitespace. + + Results are cached per-process so each distinct command string runs at most once. + + Raises: + TypeError: if *command* is not a ``str``. + CommandExecutionError: if the command exits with a non-zero return code or + produces empty output after stripping. + """ + if not isinstance(command, str): + raise TypeError(f'command must be a str, got {type(command).__name__!r}') + + if command in _COMMAND_CACHE: + return _COMMAND_CACHE[command] + + result = subprocess.run(command, shell=True, text=True, capture_output=True, check=False) + + if result.returncode != 0: + raise CommandExecutionError(command, result.returncode, result.stderr, reason=NON_ZERO_EXIT) + + value = result.stdout.strip() + if not value: + raise CommandExecutionError(command, result.returncode, result.stderr, reason=EMPTY_OUTPUT) + + _COMMAND_CACHE[command] = value + return value + + +def clear_cache() -> None: + """Clear the per-process command cache. Intended for use in tests only.""" + _COMMAND_CACHE.clear() diff --git a/ddev/src/ddev/config/file.py b/ddev/src/ddev/config/file.py index c7d713af0abc3..0d869feb865ca 100644 --- a/ddev/src/ddev/config/file.py +++ b/ddev/src/ddev/config/file.py @@ -9,7 +9,9 @@ from typing import cast from ddev.config.model import RootConfig -from ddev.config.utils import scrub_config +from ddev.config.override_trust import get_override_trust_state +from ddev.config.override_trust import strip_fetch_command_fields as _strip_fetch_command_fields +from ddev.config.scrubber import scrub_config from ddev.utils.fs import Path from ddev.utils.toml import dumps_toml_data, load_toml_data @@ -94,6 +96,8 @@ def __init__(self, path: Path | None = None): self.combined_model: RootConfig = cast(RootConfig, UNINITIALIZED) self.combined_content: str = "" self._overrides_path: Path | None = None + # Warnings collected during load() for surfacing at CLI startup. + self.load_warnings: list[str] = [] @property def overrides_path(self) -> Path: @@ -146,6 +150,8 @@ def pretty_overrides_path(self) -> Path: return Path(relative_overrides_path) def load(self): + self.load_warnings = [] + self.global_content = self.global_path.read_text() self.global_model = RootConfig(load_toml_data(self.global_content)) @@ -157,7 +163,29 @@ def load(self): return self.overrides_content = overrides_content - self.overrides_model = RootConfig(load_toml_data(self.overrides_content)) + overrides_raw = load_toml_data(self.overrides_content) + + # Determine trust state for the local override file. + global_config_dir = self.global_path.parent + trust_state, _ = get_override_trust_state(self.overrides_path, global_config_dir) + + if trust_state == 'allowed': + # Commands are trusted – leave overrides_raw intact. + pass + elif trust_state == 'denied': + # User explicitly silenced warnings; strip quietly. + _strip_fetch_command_fields(overrides_raw) + else: + # Unknown / hash mismatch – strip and warn. + stripped = _strip_fetch_command_fields(overrides_raw) + if stripped: + self.load_warnings.append( + f'Ignored untrusted `_fetch_command` field(s) from {self.pretty_overrides_path}: ' + f'{", ".join(stripped)}. ' + f'Run `ddev config allow` to trust this file.' + ) + + self.overrides_model = RootConfig(overrides_raw) self.combined_model = RootConfig( deep_merge_with_list_handling( diff --git a/ddev/src/ddev/config/model.py b/ddev/src/ddev/config/model.py index 80242e36b184e..ebefbc7a2e141 100644 --- a/ddev/src/ddev/config/model.py +++ b/ddev/src/ddev/config/model.py @@ -3,6 +3,8 @@ # Licensed under a 3-clause BSD style license (see LICENSE) import os +from ddev.config.command_resolver import CommandExecutionError, run_command + FIELD_TO_PARSE = object() @@ -424,6 +426,36 @@ def config(self, value): self._field_config = FIELD_TO_PARSE +class _CommandResolvingDict(dict): + """A dict subclass that resolves ``_fetch_command`` entries on read. + + For keys that have a corresponding ``_fetch_command`` sibling entry, the command + is executed once (with caching) and its stdout is returned as the value. The + plain ```` value is used as a fallback when the command is absent. + """ + + _COMMAND_KEYS: frozenset[str] = frozenset({'api_key', 'app_key'}) + + def _resolve(self, key: str): + """Return the resolved value for *key*, handling ``_fetch_command`` if present.""" + if key in self._COMMAND_KEYS: + cmd_key = f'{key}_fetch_command' + if cmd_key in self: + cmd = dict.__getitem__(self, cmd_key) + if isinstance(cmd, str): + return run_command(cmd) + return dict.__getitem__(self, key) + + def __getitem__(self, key): + return self._resolve(key) + + def get(self, key, default=None): + try: + return self._resolve(key) + except KeyError: + return default + + class OrgConfig(LazilyParsedConfig): def __init__(self, *args, **kwargs): super().__init__(*args, **kwargs) @@ -458,7 +490,7 @@ def config(self): if not isinstance(config, dict): self.raise_error('must be a table') - self._field_config = config + self._field_config = _CommandResolvingDict(config) else: self.raise_error('required field') @@ -480,11 +512,21 @@ def __init__(self, *args, **kwargs): @property def user(self): if self._field_user is FIELD_TO_PARSE: - if 'user' in self.raw_data: + if 'user_fetch_command' in self.raw_data: + cmd = self.raw_data['user_fetch_command'] + if not isinstance(cmd, str): + self.raise_error('must be a string', extra_steps=('user_fetch_command',)) + try: + self._field_user = run_command(cmd) + except CommandExecutionError as e: + self.raise_error( + e.to_user_message('github.user_fetch_command'), + extra_steps=('user_fetch_command',), + ) + elif 'user' in self.raw_data: user = self.raw_data['user'] if not isinstance(user, str): self.raise_error('must be a string') - self._field_user = user else: self._field_user = get_github_user() @@ -499,11 +541,21 @@ def user(self, value): @property def token(self): if self._field_token is FIELD_TO_PARSE: - if 'token' in self.raw_data: + if 'token_fetch_command' in self.raw_data: + cmd = self.raw_data['token_fetch_command'] + if not isinstance(cmd, str): + self.raise_error('must be a string', extra_steps=('token_fetch_command',)) + try: + self._field_token = run_command(cmd) + except CommandExecutionError as e: + self.raise_error( + e.to_user_message('github.token_fetch_command'), + extra_steps=('token_fetch_command',), + ) + elif 'token' in self.raw_data: token = self.raw_data['token'] if not isinstance(token, str): self.raise_error('must be a string') - self._field_token = token else: self._field_token = get_github_token() @@ -545,11 +597,18 @@ def user(self, value): @property def auth(self): if self._field_auth is FIELD_TO_PARSE: - if 'auth' in self.raw_data: + if 'auth_fetch_command' in self.raw_data: + cmd = self.raw_data['auth_fetch_command'] + if not isinstance(cmd, str): + self.raise_error('must be a string', extra_steps=('auth_fetch_command',)) + try: + self._field_auth = run_command(cmd) + except CommandExecutionError as e: + self.raise_error(e.to_user_message('pypi.auth_fetch_command'), extra_steps=('auth_fetch_command',)) + elif 'auth' in self.raw_data: auth = self.raw_data['auth'] if not isinstance(auth, str): self.raise_error('must be a string') - self._field_auth = auth else: self._field_auth = self.raw_data['auth'] = '' @@ -572,11 +631,18 @@ def __init__(self, *args, **kwargs): @property def key(self): if self._field_key is FIELD_TO_PARSE: - if 'key' in self.raw_data: + if 'key_fetch_command' in self.raw_data: + cmd = self.raw_data['key_fetch_command'] + if not isinstance(cmd, str): + self.raise_error('must be a string', extra_steps=('key_fetch_command',)) + try: + self._field_key = run_command(cmd) + except CommandExecutionError as e: + self.raise_error(e.to_user_message('trello.key_fetch_command'), extra_steps=('key_fetch_command',)) + elif 'key' in self.raw_data: key = self.raw_data['key'] if not isinstance(key, str): self.raise_error('must be a string') - self._field_key = key else: self._field_key = self.raw_data['key'] = '' @@ -591,11 +657,21 @@ def key(self, value): @property def token(self): if self._field_token is FIELD_TO_PARSE: - if 'token' in self.raw_data: + if 'token_fetch_command' in self.raw_data: + cmd = self.raw_data['token_fetch_command'] + if not isinstance(cmd, str): + self.raise_error('must be a string', extra_steps=('token_fetch_command',)) + try: + self._field_token = run_command(cmd) + except CommandExecutionError as e: + self.raise_error( + e.to_user_message('trello.token_fetch_command'), + extra_steps=('token_fetch_command',), + ) + elif 'token' in self.raw_data: token = self.raw_data['token'] if not isinstance(token, str): self.raise_error('must be a string') - self._field_token = token else: self._field_token = self.raw_data['token'] = '' diff --git a/ddev/src/ddev/config/override_trust.py b/ddev/src/ddev/config/override_trust.py new file mode 100644 index 0000000000000..854d29b5ef3f9 --- /dev/null +++ b/ddev/src/ddev/config/override_trust.py @@ -0,0 +1,86 @@ +# (C) Datadog, Inc. 2022-present +# All rights reserved +# Licensed under a 3-clause BSD style license (see LICENSE) +from __future__ import annotations + +import hashlib +import json + +from ddev.utils.fs import Path + +TRUSTED_OVERRIDES_FILENAME = "trusted_overrides.json" + + +def strip_fetch_command_fields(data: dict) -> list[str]: + """Recursively remove all keys ending with ``_fetch_command`` from *data* (in-place). + + Returns a list of the dotted paths that were stripped, e.g. ``["github.user_fetch_command"]``. + """ + stripped: list[str] = [] + _strip_fetch_command_fields_recursive(data, [], stripped) + return stripped + + +def _strip_fetch_command_fields_recursive(data: dict, path: list[str], stripped: list[str]) -> None: + keys_to_delete = [k for k in data if isinstance(k, str) and k.endswith('_fetch_command')] + for key in keys_to_delete: + stripped.append('.'.join([*path, key])) + del data[key] + + for key, value in list(data.items()): + if isinstance(value, dict): + _strip_fetch_command_fields_recursive(value, [*path, key], stripped) + + +def _compute_file_hash(path: Path) -> str: + """Return the SHA-256 hex digest of *path*'s contents.""" + return hashlib.sha256(path.read_bytes()).hexdigest() + + +def _trusted_overrides_path(global_config_dir: Path) -> Path: + return global_config_dir / TRUSTED_OVERRIDES_FILENAME + + +def _load_trust_store(global_config_dir: Path) -> dict: + """Load ``trusted_overrides.json`` as a dict, returning ``{}`` if missing or corrupt.""" + trust_file = _trusted_overrides_path(global_config_dir) + if not trust_file.is_file(): + return {} + try: + return json.loads(trust_file.read_text()) + except (json.JSONDecodeError, OSError): + return {} + + +def _save_trust_store(global_config_dir: Path, store: dict) -> None: + trust_file = _trusted_overrides_path(global_config_dir) + trust_file.ensure_parent_dir_exists() + trust_file.write_atomic(json.dumps(store, indent=2), 'w', encoding='utf-8') + + +def get_override_trust_state(overrides_path: Path, global_config_dir: Path) -> tuple[str, str]: + """Return ``(state, file_hash)`` for *overrides_path*. + + *state* is one of: ``'allowed'``, ``'denied'``, ``'unknown'``. + *file_hash* is the current SHA-256 of the file (always computed). + """ + current_hash = _compute_file_hash(overrides_path) + store = _load_trust_store(global_config_dir) + key = str(overrides_path) + entry = store.get(key) + if not entry or not isinstance(entry, dict): + return 'unknown', current_hash + recorded_hash = entry.get('hash', '') + state = entry.get('state', 'unknown') + if recorded_hash != current_hash: + return 'unknown', current_hash + return state, current_hash + + +def upsert_trust_entry(overrides_path: Path, global_config_dir: Path, state: str) -> None: + """Write or update the trust record for *overrides_path* with the current hash.""" + current_hash = _compute_file_hash(overrides_path) + store = _load_trust_store(global_config_dir) + store[str(overrides_path)] = {'hash': current_hash, 'state': state} + _save_trust_store(global_config_dir, store) + diff --git a/ddev/src/ddev/config/scrubber.py b/ddev/src/ddev/config/scrubber.py new file mode 100644 index 0000000000000..14b19e10cdc06 --- /dev/null +++ b/ddev/src/ddev/config/scrubber.py @@ -0,0 +1,46 @@ +# (C) Datadog, Inc. 2022-present +# All rights reserved +# Licensed under a 3-clause BSD style license (see LICENSE) +SCRUBBED_VALUE = '*****' +SCRUBBED_GLOBS = ( + 'github.token', + 'github.user_fetch_command', + 'github.token_fetch_command', + 'pypi.auth', + 'pypi.auth_fetch_command', + 'trello.token', + 'trello.key_fetch_command', + 'trello.token_fetch_command', + 'orgs.*.api_key', + 'orgs.*.app_key', + 'orgs.*.api_key_fetch_command', + 'orgs.*.app_key_fetch_command', + 'dynamicd.llm_api_key', + 'dynamicd.llm_api_key_fetch_command', +) +TOP_LEVEL_SCRUB_KEYS = { + 'github': ('token', 'user_fetch_command', 'token_fetch_command'), + 'pypi': ('auth', 'auth_fetch_command'), + 'trello': ('token', 'key_fetch_command', 'token_fetch_command'), + 'dynamicd': ('llm_api_key', 'llm_api_key_fetch_command'), +} +ORG_SCRUB_KEYS = ('api_key', 'app_key', 'api_key_fetch_command', 'app_key_fetch_command') + + +def _scrub_keys(mapping: dict, keys: tuple[str, ...]): + for key in keys: + if key in mapping: + mapping[key] = SCRUBBED_VALUE + + +def scrub_config(config: dict): + for section, keys in TOP_LEVEL_SCRUB_KEYS.items(): + section_data = config.get(section, {}) + if isinstance(section_data, dict): + _scrub_keys(section_data, keys) + + orgs = config.get('orgs', {}) + if isinstance(orgs, dict): + for org_data in orgs.values(): + if isinstance(org_data, dict): + _scrub_keys(org_data, ORG_SCRUB_KEYS) diff --git a/ddev/src/ddev/config/utils.py b/ddev/src/ddev/config/utils.py index fbe211fc5d7f0..02af02abf584d 100644 --- a/ddev/src/ddev/config/utils.py +++ b/ddev/src/ddev/config/utils.py @@ -6,10 +6,6 @@ from ddev.utils.fs import Path -SCRUBBED_VALUE = '*****' -SCRUBBED_GLOBS = ('github.token', 'pypi.auth', 'trello.token', 'orgs.*.api_key', 'orgs.*.app_key') - - def save_toml_document(document: TOMLDocument, path: Path): path.ensure_parent_dir_exists() path.write_atomic(tomlkit.dumps(document), 'w', encoding='utf-8') @@ -21,19 +17,3 @@ def create_toml_document(config: dict) -> TOMLDocument: def load_toml_data(path: Path) -> dict: return tomlkit.loads(path.read_text()) - - -def scrub_config(config: dict): - if 'token' in config.get('github', {}): - config['github']['token'] = SCRUBBED_VALUE - - if 'auth' in config.get('pypi', {}): - config['pypi']['auth'] = SCRUBBED_VALUE - - if 'token' in config.get('trello', {}): - config['trello']['token'] = SCRUBBED_VALUE - - for data in config.get('orgs', {}).values(): - for key in ('api_key', 'app_key'): - if key in data: - data[key] = SCRUBBED_VALUE diff --git a/ddev/tests/cli/config/test_allow.py b/ddev/tests/cli/config/test_allow.py new file mode 100644 index 0000000000000..dcd03a6506318 --- /dev/null +++ b/ddev/tests/cli/config/test_allow.py @@ -0,0 +1,43 @@ +# (C) Datadog, Inc. 2022-present +# All rights reserved +# Licensed under a 3-clause BSD style license (see LICENSE) +import json + +from ddev.config.override_trust import TRUSTED_OVERRIDES_FILENAME + + +def test_allow_writes_allowed_entry(ddev, config_file, overrides_config): + """Running `ddev config allow` writes an 'allowed' trust record for .ddev.toml.""" + overrides_config.write_text('[github]\nuser_fetch_command = "echo me"\n') + + result = ddev('config', 'allow') + + assert result.exit_code == 0, result.output + assert 'Trusted' in result.output + + trust_file = config_file.global_path.parent / TRUSTED_OVERRIDES_FILENAME + assert trust_file.is_file() + + store = json.loads(trust_file.read_text()) + key = str(overrides_config) + assert key in store + assert store[key]['state'] == 'allowed' + assert 'hash' in store[key] + + +def test_allow_is_idempotent(ddev, config_file, overrides_config): + """Calling `ddev config allow` twice does not raise an error.""" + overrides_config.write_text('[github]\nuser_fetch_command = "echo me"\n') + + result1 = ddev('config', 'allow') + result2 = ddev('config', 'allow') + + assert result1.exit_code == 0, result1.output + assert result2.exit_code == 0, result2.output + + +def test_allow_no_overrides_file_aborts(ddev): + """Running `ddev config allow` without a .ddev.toml file exits with an error.""" + result = ddev('config', 'allow') + + assert result.exit_code != 0 diff --git a/ddev/tests/cli/config/test_deny.py b/ddev/tests/cli/config/test_deny.py new file mode 100644 index 0000000000000..f2d308b91a10b --- /dev/null +++ b/ddev/tests/cli/config/test_deny.py @@ -0,0 +1,56 @@ +# (C) Datadog, Inc. 2022-present +# All rights reserved +# Licensed under a 3-clause BSD style license (see LICENSE) +import json + +from ddev.config.override_trust import TRUSTED_OVERRIDES_FILENAME + + +def test_deny_writes_denied_entry(ddev, config_file, overrides_config): + """Running `ddev config deny` writes a 'denied' trust record for .ddev.toml.""" + overrides_config.write_text('[github]\nuser_fetch_command = "echo me"\n') + + result = ddev('config', 'deny') + + assert result.exit_code == 0, result.output + assert 'Silenced' in result.output + + trust_file = config_file.global_path.parent / TRUSTED_OVERRIDES_FILENAME + assert trust_file.is_file() + + store = json.loads(trust_file.read_text()) + key = str(overrides_config) + assert key in store + assert store[key]['state'] == 'denied' + assert 'hash' in store[key] + + +def test_deny_is_idempotent(ddev, config_file, overrides_config): + """Calling `ddev config deny` twice does not raise an error.""" + overrides_config.write_text('[github]\nuser_fetch_command = "echo me"\n') + + result1 = ddev('config', 'deny') + result2 = ddev('config', 'deny') + + assert result1.exit_code == 0, result1.output + assert result2.exit_code == 0, result2.output + + +def test_deny_transitions_from_allowed(ddev, config_file, overrides_config): + """After `allow` then `deny`, the state should be 'denied'.""" + overrides_config.write_text('[github]\nuser_fetch_command = "echo me"\n') + + ddev('config', 'allow') + result = ddev('config', 'deny') + + assert result.exit_code == 0, result.output + trust_file = config_file.global_path.parent / TRUSTED_OVERRIDES_FILENAME + store = json.loads(trust_file.read_text()) + assert store[str(overrides_config)]['state'] == 'denied' + + +def test_deny_no_overrides_file_aborts(ddev): + """Running `ddev config deny` without a .ddev.toml file exits with an error.""" + result = ddev('config', 'deny') + + assert result.exit_code != 0 diff --git a/ddev/tests/cli/config/test_show.py b/ddev/tests/cli/config/test_show.py index 6cc75bc494774..8340de5bcd5b2 100644 --- a/ddev/tests/cli/config/test_show.py +++ b/ddev/tests/cli/config/test_show.py @@ -237,3 +237,26 @@ def test_verbose_output_with_local_file(ddev, config_file, helpers, overrides_co result = ddev("-v", "config", "show") assert result.exit_code == 0 assert "Local override config file found" in result.output + + +def test_scrubbed_output_includes_dynamicd_llm_api_key(ddev, config_file): + """config show must scrub dynamicd.llm_api_key.""" + config_file.model.raw_data.setdefault('dynamicd', {})['llm_api_key'] = 'supersecret' + config_file.save() + + result = ddev('config', 'show') + + assert result.exit_code == 0, result.output + assert 'supersecret' not in result.output + assert '*****' in result.output + + +def test_non_scrubbed_output_shows_dynamicd_llm_api_key(ddev, config_file): + """config show -a must show plain dynamicd.llm_api_key.""" + config_file.model.raw_data.setdefault('dynamicd', {})['llm_api_key'] = 'supersecret' + config_file.save() + + result = ddev('config', 'show', '-a') + + assert result.exit_code == 0, result.output + assert 'supersecret' in result.output diff --git a/ddev/tests/cli/meta/scripts/test_dynamicd_cli.py b/ddev/tests/cli/meta/scripts/test_dynamicd_cli.py new file mode 100644 index 0000000000000..c679481e649ff --- /dev/null +++ b/ddev/tests/cli/meta/scripts/test_dynamicd_cli.py @@ -0,0 +1,65 @@ +# (C) Datadog, Inc. 2026-present +# All rights reserved +# Licensed under a 3-clause BSD style license (see LICENSE) +from __future__ import annotations + +from types import SimpleNamespace + +import pytest + +from ddev.config.command_resolver import EMPTY_OUTPUT, NON_ZERO_EXIT, CommandExecutionError + + +class _FakeApp: + def __init__(self, llm_command: str = 'echo secret'): + self.errors: list[str] = [] + self.config = SimpleNamespace( + raw_data={'dynamicd': {'llm_api_key_fetch_command': llm_command}}, + org=SimpleNamespace(config={'api_key': 'dd_api'}), + ) + + def display_error(self, message: str): + self.errors.append(message) + + def abort(self): + raise RuntimeError('aborted') + + +def test_get_api_keys_reports_actionable_nonzero_error(monkeypatch): + from ddev.cli.meta.scripts._dynamicd import cli as dynamicd_cli + + app = _FakeApp(llm_command='echo super-secret') + + def raise_nonzero(command): + raise CommandExecutionError(command, 23, 'permission denied', reason=NON_ZERO_EXIT) + + monkeypatch.setattr(dynamicd_cli, 'run_command', raise_nonzero) + + with pytest.raises(RuntimeError, match='aborted'): + dynamicd_cli._get_api_keys(app) + + assert app.errors + message = app.errors[-1] + assert 'dynamicd.llm_api_key_fetch_command' in message + assert 'exit code 23' in message + assert 'stdout' in message + assert 'super-secret' not in message + + +def test_get_api_keys_reports_actionable_empty_output_error(monkeypatch): + from ddev.cli.meta.scripts._dynamicd import cli as dynamicd_cli + + app = _FakeApp() + + def raise_empty(command): + raise CommandExecutionError(command, 0, '', reason=EMPTY_OUTPUT) + + monkeypatch.setattr(dynamicd_cli, 'run_command', raise_empty) + + with pytest.raises(RuntimeError, match='aborted'): + dynamicd_cli._get_api_keys(app) + + message = app.errors[-1] + assert 'dynamicd.llm_api_key_fetch_command' in message + assert 'empty output' in message + assert 'non-empty value' in message diff --git a/ddev/tests/config/test_command_resolver.py b/ddev/tests/config/test_command_resolver.py new file mode 100644 index 0000000000000..f4e4e1b7abcf5 --- /dev/null +++ b/ddev/tests/config/test_command_resolver.py @@ -0,0 +1,78 @@ +# (C) Datadog, Inc. 2022-present +# All rights reserved +# Licensed under a 3-clause BSD style license (see LICENSE) +import pytest + +from ddev.config.command_resolver import ( + EMPTY_OUTPUT, + NON_ZERO_EXIT, + CommandExecutionError, + clear_cache, + run_command, +) + + +@pytest.fixture(autouse=True) +def fresh_cache(): + """Ensure a clean cache before and after every test.""" + clear_cache() + yield + clear_cache() + + +class TestRunCommand: + def test_returns_stdout_stripped(self): + result = run_command('echo hello') + assert result == 'hello' + + def test_caches_result(self, mocker): + spy = mocker.patch('subprocess.run', wraps=__import__('subprocess').run) + run_command('echo cached') + run_command('echo cached') + assert spy.call_count == 1 + + def test_distinct_commands_each_run_once(self, mocker): + spy = mocker.patch('subprocess.run', wraps=__import__('subprocess').run) + run_command('echo first') + run_command('echo second') + assert spy.call_count == 2 + + def test_nonzero_exit_raises(self): + with pytest.raises(CommandExecutionError) as exc_info: + run_command('exit 42') + assert exc_info.value.returncode == 42 + assert exc_info.value.reason == NON_ZERO_EXIT + assert 'exit code 42' in str(exc_info.value) + + def test_empty_output_raises(self): + with pytest.raises(CommandExecutionError) as exc_info: + run_command('echo ""') + assert exc_info.value.reason == EMPTY_OUTPUT + assert 'empty output' in str(exc_info.value) + + def test_type_error_on_non_string(self): + with pytest.raises(TypeError): + run_command(123) # type: ignore[arg-type] + + def test_error_is_secret_safe(self): + with pytest.raises(CommandExecutionError) as exc_info: + run_command('echo super-secret && exit 1') + assert 'super-secret' not in str(exc_info.value) + + def test_actionable_message_mentions_field_and_hints(self): + with pytest.raises(CommandExecutionError) as exc_info: + run_command('exit 1') + + user_message = exc_info.value.to_user_message('github.token_fetch_command') + assert 'github.token_fetch_command' in user_message + assert 'stdout' in user_message + assert 'non-empty value' in user_message + + +class TestClearCache: + def test_clear_forces_re_execution(self, mocker): + spy = mocker.patch('subprocess.run', wraps=__import__('subprocess').run) + run_command('echo test') + clear_cache() + run_command('echo test') + assert spy.call_count == 2 diff --git a/ddev/tests/config/test_file.py b/ddev/tests/config/test_file.py index 493a746089a00..d5658cafa58a9 100644 --- a/ddev/tests/config/test_file.py +++ b/ddev/tests/config/test_file.py @@ -8,6 +8,13 @@ build_line_index_with_multiple_entries, deep_merge_with_list_handling, ) +from ddev.config.override_trust import ( + get_override_trust_state, + upsert_trust_entry, +) +from ddev.config.override_trust import ( + strip_fetch_command_fields as _strip_fetch_command_fields, +) from ddev.utils.fs import Path @@ -405,3 +412,186 @@ def mock_is_file(self: Path): # Assert that the returned path is the one where PermissionError occurred assert result_path == parent_dir + + +# --------------------------------------------------------------------------- +# Tests for _strip_fetch_command_fields +# --------------------------------------------------------------------------- + +class TestStripFetchCommandFields: + def test_strips_top_level_fetch_command_key(self): + data = {'token_fetch_command': 'secret-tool', 'token': 'plain'} + stripped = _strip_fetch_command_fields(data) + assert 'token_fetch_command' not in data + assert 'token' in data + assert stripped == ['token_fetch_command'] + + def test_strips_nested_fetch_command_key(self): + data = {'github': {'user_fetch_command': 'cmd', 'user': 'plain'}} + stripped = _strip_fetch_command_fields(data) + assert 'user_fetch_command' not in data['github'] + assert stripped == ['github.user_fetch_command'] + + def test_strips_multiple_nested_fetch_command_keys(self): + data = { + 'github': {'user_fetch_command': 'c1', 'token_fetch_command': 'c2'}, + 'trello': {'key_fetch_command': 'c3'}, + } + stripped = _strip_fetch_command_fields(data) + assert not any(k.endswith('_fetch_command') for k in data['github']) + assert not any(k.endswith('_fetch_command') for k in data['trello']) + assert set(stripped) == {'github.user_fetch_command', 'github.token_fetch_command', 'trello.key_fetch_command'} + + def test_no_fetch_command_fields_returns_empty(self): + data = {'github': {'user': 'plain'}, 'pypi': {'auth': 'secret'}} + stripped = _strip_fetch_command_fields(data) + assert stripped == [] + + def test_non_fetch_command_keys_preserved(self): + data = {'github': {'user': 'plain', 'user_fetch_command': 'cmd'}} + _strip_fetch_command_fields(data) + assert data == {'github': {'user': 'plain'}} + + +# --------------------------------------------------------------------------- +# Tests for trust store helpers +# --------------------------------------------------------------------------- + +class TestTrustStore: + def test_unknown_when_no_store(self, tmp_path): + tmp_path = Path(tmp_path) + override_file = tmp_path / DDEV_TOML + override_file.write_text('[github]\nuser = "x"\n') + global_dir = tmp_path / 'cfg' + global_dir.mkdir() + + state, _ = get_override_trust_state(override_file, global_dir) + assert state == 'unknown' + + def test_upsert_and_read_allowed(self, tmp_path): + tmp_path = Path(tmp_path) + override_file = tmp_path / DDEV_TOML + override_file.write_text('[github]\nuser = "x"\n') + global_dir = tmp_path / 'cfg' + global_dir.mkdir() + + upsert_trust_entry(override_file, global_dir, 'allowed') + state, _ = get_override_trust_state(override_file, global_dir) + assert state == 'allowed' + + def test_upsert_and_read_denied(self, tmp_path): + tmp_path = Path(tmp_path) + override_file = tmp_path / DDEV_TOML + override_file.write_text('[github]\nuser = "x"\n') + global_dir = tmp_path / 'cfg' + global_dir.mkdir() + + upsert_trust_entry(override_file, global_dir, 'denied') + state, _ = get_override_trust_state(override_file, global_dir) + assert state == 'denied' + + def test_hash_mismatch_returns_unknown(self, tmp_path): + tmp_path = Path(tmp_path) + override_file = tmp_path / DDEV_TOML + override_file.write_text('[github]\nuser = "x"\n') + global_dir = tmp_path / 'cfg' + global_dir.mkdir() + + upsert_trust_entry(override_file, global_dir, 'allowed') + # Modify the file to change its hash + override_file.write_text('[github]\nuser = "changed"\n') + + state, _ = get_override_trust_state(override_file, global_dir) + assert state == 'unknown' + + def test_upsert_is_idempotent(self, tmp_path): + tmp_path = Path(tmp_path) + override_file = tmp_path / DDEV_TOML + override_file.write_text('x = 1\n') + global_dir = tmp_path / 'cfg' + global_dir.mkdir() + + upsert_trust_entry(override_file, global_dir, 'denied') + upsert_trust_entry(override_file, global_dir, 'denied') + state, _ = get_override_trust_state(override_file, global_dir) + assert state == 'denied' + + +# --------------------------------------------------------------------------- +# Tests for load() trust-aware stripping +# --------------------------------------------------------------------------- + +class TestLoadTrustAwareStripping: + def _make_config_file(self, tmp_path: Path) -> ConfigFileWithOverrides: + global_path = tmp_path / 'config.toml' + global_path.write_text('[github]\nuser = "global"\n') + return ConfigFileWithOverrides(global_path) + + def test_unknown_trust_strips_fetch_command_fields_and_adds_warning(self, tmp_path, monkeypatch): + tmp_path = Path(tmp_path) + cfg = self._make_config_file(tmp_path) + + override_file = tmp_path / DDEV_TOML + override_file.write_text('[github]\nuser_fetch_command = "echo override"\n') + monkeypatch.chdir(tmp_path) + + cfg.load() + + assert 'user_fetch_command' not in cfg.combined_model.raw_data.get('github', {}) + assert len(cfg.load_warnings) == 1 + assert 'untrusted' in cfg.load_warnings[0].lower() + + def test_allowed_trust_keeps_fetch_command_fields_no_warning(self, tmp_path, monkeypatch): + tmp_path = Path(tmp_path) + cfg = self._make_config_file(tmp_path) + + override_file = tmp_path / DDEV_TOML + override_file.write_text('[github]\nuser_fetch_command = "echo override_user"\n') + monkeypatch.chdir(tmp_path) + + upsert_trust_entry(override_file, tmp_path / 'cfg', 'allowed') + # Point the config file's global dir to tmp_path/cfg so trust store is found + cfg.global_path = tmp_path / 'cfg' / 'config.toml' + (tmp_path / 'cfg').mkdir(exist_ok=True) + (tmp_path / 'cfg' / 'config.toml').write_text('[github]\nuser = "global"\n') + + cfg.load() + + assert cfg.combined_model.raw_data.get('github', {}).get('user_fetch_command') == 'echo override_user' + assert cfg.load_warnings == [] + + def test_denied_trust_strips_silently_no_warning(self, tmp_path, monkeypatch): + tmp_path = Path(tmp_path) + cfg = self._make_config_file(tmp_path) + + override_file = tmp_path / DDEV_TOML + override_file.write_text('[github]\nuser_fetch_command = "echo override_user"\n') + monkeypatch.chdir(tmp_path) + + global_dir = tmp_path + upsert_trust_entry(override_file, global_dir, 'denied') + cfg.global_path = tmp_path / 'config.toml' + + cfg.load() + + assert 'user_fetch_command' not in cfg.combined_model.raw_data.get('github', {}) + assert cfg.load_warnings == [] + + def test_hash_mismatch_strips_and_warns(self, tmp_path, monkeypatch): + tmp_path = Path(tmp_path) + cfg = self._make_config_file(tmp_path) + + override_file = tmp_path / DDEV_TOML + override_file.write_text('[github]\nuser_fetch_command = "echo v1"\n') + monkeypatch.chdir(tmp_path) + + global_dir = tmp_path + upsert_trust_entry(override_file, global_dir, 'allowed') + # Modify file so hash no longer matches + override_file.write_text('[github]\nuser_fetch_command = "echo v2"\n') + + cfg.load() + + assert 'user_fetch_command' not in cfg.combined_model.raw_data.get('github', {}) + assert len(cfg.load_warnings) == 1 + assert 'untrusted' in cfg.load_warnings[0].lower() diff --git a/ddev/tests/config/test_model.py b/ddev/tests/config/test_model.py index 37c1f4d13222d..7fd685c1f75e8 100644 --- a/ddev/tests/config/test_model.py +++ b/ddev/tests/config/test_model.py @@ -5,9 +5,18 @@ import pytest +from ddev.config.command_resolver import clear_cache from ddev.config.model import ConfigurationError, RootConfig, get_github_token, get_github_user +@pytest.fixture(autouse=True) +def _clear_command_cache(): + """Isolate command-cache state between tests.""" + clear_cache() + yield + clear_cache() + + def test_default(): config = RootConfig({}) config.parse_fields() @@ -1474,3 +1483,132 @@ def test_github_config_with_environment_variables(self, monkeypatch): # raw_data should still be empty assert config.raw_data['github'] == {} + + +class TestCommandFieldsGitHub: + def test_user_fetch_command_takes_precedence_over_plain_user(self): + config = RootConfig({'github': {'user': 'plain', 'user_fetch_command': 'echo cmd_user'}}) + assert config.github.user == 'cmd_user' + + def test_user_fetch_command_fallback_to_plain_when_no_command(self): + config = RootConfig({'github': {'user': 'plain'}}) + assert config.github.user == 'plain' + + def test_token_fetch_command_takes_precedence_over_plain_token(self): + config = RootConfig({'github': {'token': 'plain', 'token_fetch_command': 'echo cmd_token'}}) + assert config.github.token == 'cmd_token' + + def test_token_fetch_command_fallback_to_plain_when_no_command(self): + config = RootConfig({'github': {'token': 'plain'}}) + assert config.github.token == 'plain' + + def test_user_fetch_command_cached(self, mocker): + spy = mocker.patch('subprocess.run', wraps=__import__('subprocess').run) + config = RootConfig({'github': {'user_fetch_command': 'echo cached_user'}}) + _ = config.github.user + config._field_github = None.__class__() # reset property cache only + config._field_github = config.raw_data.get('github') + # Re-access via fresh model - same command string should use process cache + config2 = RootConfig({'github': {'user_fetch_command': 'echo cached_user'}}) + _ = config2.github.user + assert spy.call_count == 1 # command ran only once across both accesses + + def test_user_fetch_command_failure_raises_configuration_error(self): + config = RootConfig({'github': {'user_fetch_command': 'exit 1'}}) + with pytest.raises(ConfigurationError): + _ = config.github.user + + def test_user_fetch_command_failure_message_is_actionable(self): + config = RootConfig({'github': {'user_fetch_command': 'echo super-secret && exit 1'}}) + with pytest.raises(ConfigurationError) as exc_info: + _ = config.github.user + message = str(exc_info.value) + assert 'github.user_fetch_command' in message + assert 'exit code' in message + assert 'writes the secret to stdout' in message + assert 'super-secret' not in message + + def test_token_fetch_command_failure_raises_configuration_error(self): + config = RootConfig({'github': {'token_fetch_command': 'exit 1'}}) + with pytest.raises(ConfigurationError): + _ = config.github.token + + +class TestCommandFieldsPyPI: + def test_auth_fetch_command_takes_precedence(self): + config = RootConfig({'pypi': {'auth': 'plain', 'auth_fetch_command': 'echo cmd_auth'}}) + assert config.pypi.auth == 'cmd_auth' + + def test_auth_plain_fallback(self): + config = RootConfig({'pypi': {'auth': 'plain'}}) + assert config.pypi.auth == 'plain' + + def test_auth_fetch_command_failure_raises(self): + config = RootConfig({'pypi': {'auth_fetch_command': 'exit 1'}}) + with pytest.raises(ConfigurationError): + _ = config.pypi.auth + + def test_auth_fetch_command_empty_output_message_is_actionable(self): + config = RootConfig({'pypi': {'auth_fetch_command': 'echo ""'}}) + with pytest.raises(ConfigurationError) as exc_info: + _ = config.pypi.auth + message = str(exc_info.value) + assert 'pypi.auth_fetch_command' in message + assert 'empty output' in message + assert 'non-empty value' in message + + +class TestCommandFieldsTrello: + def test_key_fetch_command_takes_precedence(self): + config = RootConfig({'trello': {'key': 'plain', 'key_fetch_command': 'echo cmd_key'}}) + assert config.trello.key == 'cmd_key' + + def test_key_plain_fallback(self): + config = RootConfig({'trello': {'key': 'plain'}}) + assert config.trello.key == 'plain' + + def test_token_fetch_command_takes_precedence(self): + config = RootConfig({'trello': {'token': 'plain', 'token_fetch_command': 'echo cmd_token'}}) + assert config.trello.token == 'cmd_token' + + def test_token_fetch_command_failure_raises(self): + config = RootConfig({'trello': {'token_fetch_command': 'exit 1'}}) + with pytest.raises(ConfigurationError): + _ = config.trello.token + + +class TestCommandFieldsOrg: + def test_api_key_fetch_command_takes_precedence(self): + config = RootConfig({ + 'orgs': {'myorg': {'api_key': 'plain', 'api_key_fetch_command': 'echo cmd_api', 'app_key': ''}}, + 'org': 'myorg', + }) + assert config.org.config.get('api_key') == 'cmd_api' + + def test_api_key_plain_fallback(self): + config = RootConfig({ + 'orgs': {'myorg': {'api_key': 'plain', 'app_key': ''}}, + 'org': 'myorg', + }) + assert config.org.config.get('api_key') == 'plain' + + def test_app_key_fetch_command_takes_precedence(self): + config = RootConfig({ + 'orgs': {'myorg': {'api_key': '', 'app_key': 'plain', 'app_key_fetch_command': 'echo cmd_app'}}, + 'org': 'myorg', + }) + assert config.org.config.get('app_key') == 'cmd_app' + + def test_app_key_plain_fallback(self): + config = RootConfig({ + 'orgs': {'myorg': {'api_key': '', 'app_key': 'plain'}}, + 'org': 'myorg', + }) + assert config.org.config.get('app_key') == 'plain' + + def test_non_command_keys_unaffected(self): + config = RootConfig({ + 'orgs': {'myorg': {'api_key': '', 'app_key': '', 'site': 'mysite.com'}}, + 'org': 'myorg', + }) + assert config.org.config.get('site') == 'mysite.com' From 9f4d4e9ab1b93fe6e4ba3e83fcee031eb272a8a7 Mon Sep 17 00:00:00 2001 From: Enrico Donnici Date: Wed, 25 Feb 2026 12:50:25 +0100 Subject: [PATCH 2/6] Integrate DynamicD secret resolution into the config model and keep command-secret loading lazy. This routes DynamicD key lookup through model-backed resolvers and aligns newly added files with 2026-present Datadog license headers. --- ddev/src/ddev/cli/config/allow.py | 2 +- ddev/src/ddev/cli/config/deny.py | 2 +- .../ddev/cli/meta/scripts/_dynamicd/cli.py | 27 ++---- ddev/src/ddev/config/command_resolver.py | 2 +- ddev/src/ddev/config/model.py | 85 +++++++++++++++++++ ddev/src/ddev/config/override_trust.py | 2 +- ddev/src/ddev/config/scrubber.py | 2 +- ddev/tests/cli/config/test_allow.py | 2 +- ddev/tests/cli/config/test_deny.py | 2 +- .../cli/meta/scripts/test_dynamicd_cli.py | 56 +++++++----- ddev/tests/config/test_command_resolver.py | 2 +- ddev/tests/config/test_model.py | 65 ++++++++++++++ 12 files changed, 202 insertions(+), 47 deletions(-) diff --git a/ddev/src/ddev/cli/config/allow.py b/ddev/src/ddev/cli/config/allow.py index beaa6b6ccfe52..c93acb7895c76 100644 --- a/ddev/src/ddev/cli/config/allow.py +++ b/ddev/src/ddev/cli/config/allow.py @@ -1,4 +1,4 @@ -# (C) Datadog, Inc. 2022-present +# (C) Datadog, Inc. 2026-present # All rights reserved # Licensed under a 3-clause BSD style license (see LICENSE) from __future__ import annotations diff --git a/ddev/src/ddev/cli/config/deny.py b/ddev/src/ddev/cli/config/deny.py index d816e22913d2c..49faa155a023e 100644 --- a/ddev/src/ddev/cli/config/deny.py +++ b/ddev/src/ddev/cli/config/deny.py @@ -1,4 +1,4 @@ -# (C) Datadog, Inc. 2022-present +# (C) Datadog, Inc. 2026-present # All rights reserved # Licensed under a 3-clause BSD style license (see LICENSE) from __future__ import annotations diff --git a/ddev/src/ddev/cli/meta/scripts/_dynamicd/cli.py b/ddev/src/ddev/cli/meta/scripts/_dynamicd/cli.py index 54e07bf163029..dde3c73f3d90f 100644 --- a/ddev/src/ddev/cli/meta/scripts/_dynamicd/cli.py +++ b/ddev/src/ddev/cli/meta/scripts/_dynamicd/cli.py @@ -5,13 +5,12 @@ from __future__ import annotations -import os from typing import TYPE_CHECKING import click from ddev.cli.meta.scripts._dynamicd.constants import SCENARIOS -from ddev.config.command_resolver import CommandExecutionError, run_command +from ddev.config.model import ConfigurationError if TYPE_CHECKING: from ddev.cli.application import Application @@ -79,27 +78,19 @@ def _get_api_keys(app: Application) -> tuple[str, str]: Returns (llm_api_key, dd_api_key) or aborts if not configured. """ - dynamicd_cfg = app.config.raw_data.get("dynamicd", {}) - - # Resolve LLM API key: command > plain value > env var - llm_api_key = None - if "llm_api_key_fetch_command" in dynamicd_cfg: - cmd = dynamicd_cfg["llm_api_key_fetch_command"] - try: - llm_api_key = run_command(cmd) - except CommandExecutionError as e: - app.display_error(e.to_user_message('dynamicd.llm_api_key_fetch_command')) - app.abort() + # Resolve LLM API key lazily via the shared config model. + try: + llm_api_key = app.config.dynamicd.resolve_llm_api_key() + except ConfigurationError as e: + app.display_error(str(e)) + app.abort() - if not llm_api_key: - llm_api_key = dynamicd_cfg.get("llm_api_key") - if not llm_api_key: - llm_api_key = os.environ.get("ANTHROPIC_API_KEY") if not llm_api_key: app.display_error( "LLM API key not configured. Either:\n" " 1. Set env var: export ANTHROPIC_API_KEY=\n" - " 2. Or run: ddev config set dynamicd.llm_api_key " + " 2. Or run: ddev config set dynamicd.llm_api_key \n" + " 3. Or run: ddev config set dynamicd.llm_api_key_fetch_command ''" ) app.abort() diff --git a/ddev/src/ddev/config/command_resolver.py b/ddev/src/ddev/config/command_resolver.py index 43a94887e704e..12c501bebebb7 100644 --- a/ddev/src/ddev/config/command_resolver.py +++ b/ddev/src/ddev/config/command_resolver.py @@ -1,4 +1,4 @@ -# (C) Datadog, Inc. 2022-present +# (C) Datadog, Inc. 2026-present # All rights reserved # Licensed under a 3-clause BSD style license (see LICENSE) """Per-process command resolver with deterministic caching for `*_fetch_command` secret fields.""" diff --git a/ddev/src/ddev/config/model.py b/ddev/src/ddev/config/model.py index ebefbc7a2e141..c7363074fbcfb 100644 --- a/ddev/src/ddev/config/model.py +++ b/ddev/src/ddev/config/model.py @@ -72,6 +72,7 @@ def __init__(self, *args, **kwargs): self._field_github = FIELD_TO_PARSE self._field_pypi = FIELD_TO_PARSE self._field_trello = FIELD_TO_PARSE + self._field_dynamicd = FIELD_TO_PARSE self._field_terminal = FIELD_TO_PARSE self._field_upgrade_check = FIELD_TO_PARSE @@ -312,6 +313,26 @@ def trello(self, value): self.raw_data['trello'] = value self._field_trello = FIELD_TO_PARSE + @property + def dynamicd(self): + if self._field_dynamicd is FIELD_TO_PARSE: + if 'dynamicd' in self.raw_data: + dynamicd = self.raw_data['dynamicd'] + if not isinstance(dynamicd, dict): + self.raise_error('must be a table') + + self._field_dynamicd = DynamicDConfig(dynamicd, ('dynamicd',)) + else: + # Keep this section implicit unless explicitly configured. + self._field_dynamicd = DynamicDConfig({}, ('dynamicd',)) + + return self._field_dynamicd + + @dynamicd.setter + def dynamicd(self, value): + self.raw_data['dynamicd'] = value + self._field_dynamicd = FIELD_TO_PARSE + @property def terminal(self): if self._field_terminal is FIELD_TO_PARSE: @@ -684,6 +705,70 @@ def token(self, value): self._field_token = FIELD_TO_PARSE +class DynamicDConfig(LazilyParsedConfig): + def __init__(self, *args, **kwargs): + super().__init__(*args, **kwargs) + + self._field_llm_api_key = FIELD_TO_PARSE + self._field_llm_api_key_fetch_command = FIELD_TO_PARSE + self._resolved_llm_api_key = FIELD_TO_PARSE + + @property + def llm_api_key(self): + if self._field_llm_api_key is FIELD_TO_PARSE: + if 'llm_api_key' in self.raw_data: + llm_api_key = self.raw_data['llm_api_key'] + if not isinstance(llm_api_key, str): + self.raise_error('must be a string') + self._field_llm_api_key = llm_api_key + else: + self._field_llm_api_key = None + return self._field_llm_api_key + + @llm_api_key.setter + def llm_api_key(self, value): + self.raw_data['llm_api_key'] = value + self._field_llm_api_key = FIELD_TO_PARSE + self._resolved_llm_api_key = FIELD_TO_PARSE + + @property + def llm_api_key_fetch_command(self): + if self._field_llm_api_key_fetch_command is FIELD_TO_PARSE: + if 'llm_api_key_fetch_command' in self.raw_data: + command = self.raw_data['llm_api_key_fetch_command'] + if not isinstance(command, str): + self.raise_error('must be a string') + self._field_llm_api_key_fetch_command = command + else: + self._field_llm_api_key_fetch_command = None + return self._field_llm_api_key_fetch_command + + @llm_api_key_fetch_command.setter + def llm_api_key_fetch_command(self, value): + self.raw_data['llm_api_key_fetch_command'] = value + self._field_llm_api_key_fetch_command = FIELD_TO_PARSE + self._resolved_llm_api_key = FIELD_TO_PARSE + + def resolve_llm_api_key(self) -> str: + """Resolve LLM key lazily: fetch_command > plain value > environment.""" + if self._resolved_llm_api_key is FIELD_TO_PARSE: + command = self.llm_api_key_fetch_command + if command is not None: + try: + llm_api_key = run_command(command) + except CommandExecutionError as e: + raise ConfigurationError( + e.to_user_message('dynamicd.llm_api_key_fetch_command'), + location=' -> '.join([*self.steps, 'llm_api_key_fetch_command']), + ) + else: + llm_api_key = self.llm_api_key or os.environ.get('ANTHROPIC_API_KEY', '') + + self._resolved_llm_api_key = llm_api_key + + return self._resolved_llm_api_key + + class TerminalConfig(LazilyParsedConfig): def __init__(self, *args, **kwargs): super().__init__(*args, **kwargs) diff --git a/ddev/src/ddev/config/override_trust.py b/ddev/src/ddev/config/override_trust.py index 854d29b5ef3f9..5cd2210ddc1fa 100644 --- a/ddev/src/ddev/config/override_trust.py +++ b/ddev/src/ddev/config/override_trust.py @@ -1,4 +1,4 @@ -# (C) Datadog, Inc. 2022-present +# (C) Datadog, Inc. 2026-present # All rights reserved # Licensed under a 3-clause BSD style license (see LICENSE) from __future__ import annotations diff --git a/ddev/src/ddev/config/scrubber.py b/ddev/src/ddev/config/scrubber.py index 14b19e10cdc06..a0029c3ac57fe 100644 --- a/ddev/src/ddev/config/scrubber.py +++ b/ddev/src/ddev/config/scrubber.py @@ -1,4 +1,4 @@ -# (C) Datadog, Inc. 2022-present +# (C) Datadog, Inc. 2026-present # All rights reserved # Licensed under a 3-clause BSD style license (see LICENSE) SCRUBBED_VALUE = '*****' diff --git a/ddev/tests/cli/config/test_allow.py b/ddev/tests/cli/config/test_allow.py index dcd03a6506318..a6a9cb7f07daa 100644 --- a/ddev/tests/cli/config/test_allow.py +++ b/ddev/tests/cli/config/test_allow.py @@ -1,4 +1,4 @@ -# (C) Datadog, Inc. 2022-present +# (C) Datadog, Inc. 2026-present # All rights reserved # Licensed under a 3-clause BSD style license (see LICENSE) import json diff --git a/ddev/tests/cli/config/test_deny.py b/ddev/tests/cli/config/test_deny.py index f2d308b91a10b..fa02b8e2ee5d3 100644 --- a/ddev/tests/cli/config/test_deny.py +++ b/ddev/tests/cli/config/test_deny.py @@ -1,4 +1,4 @@ -# (C) Datadog, Inc. 2022-present +# (C) Datadog, Inc. 2026-present # All rights reserved # Licensed under a 3-clause BSD style license (see LICENSE) import json diff --git a/ddev/tests/cli/meta/scripts/test_dynamicd_cli.py b/ddev/tests/cli/meta/scripts/test_dynamicd_cli.py index c679481e649ff..0750b103020ab 100644 --- a/ddev/tests/cli/meta/scripts/test_dynamicd_cli.py +++ b/ddev/tests/cli/meta/scripts/test_dynamicd_cli.py @@ -3,20 +3,15 @@ # Licensed under a 3-clause BSD style license (see LICENSE) from __future__ import annotations -from types import SimpleNamespace - import pytest -from ddev.config.command_resolver import EMPTY_OUTPUT, NON_ZERO_EXIT, CommandExecutionError +from ddev.config.model import RootConfig class _FakeApp: - def __init__(self, llm_command: str = 'echo secret'): + def __init__(self, config: RootConfig): self.errors: list[str] = [] - self.config = SimpleNamespace( - raw_data={'dynamicd': {'llm_api_key_fetch_command': llm_command}}, - org=SimpleNamespace(config={'api_key': 'dd_api'}), - ) + self.config = config def display_error(self, message: str): self.errors.append(message) @@ -25,15 +20,16 @@ def abort(self): raise RuntimeError('aborted') -def test_get_api_keys_reports_actionable_nonzero_error(monkeypatch): - from ddev.cli.meta.scripts._dynamicd import cli as dynamicd_cli +def _make_config(dynamicd: dict) -> RootConfig: + return RootConfig({'dynamicd': dynamicd, 'orgs': {'default': {'api_key': 'dd_api'}}}) - app = _FakeApp(llm_command='echo super-secret') - def raise_nonzero(command): - raise CommandExecutionError(command, 23, 'permission denied', reason=NON_ZERO_EXIT) +def test_get_api_keys_reports_actionable_nonzero_error(): + from ddev.cli.meta.scripts._dynamicd import cli as dynamicd_cli - monkeypatch.setattr(dynamicd_cli, 'run_command', raise_nonzero) + app = _FakeApp( + _make_config({'llm_api_key_fetch_command': 'echo super-secret >/dev/null && exit 23'}) + ) with pytest.raises(RuntimeError, match='aborted'): dynamicd_cli._get_api_keys(app) @@ -46,15 +42,10 @@ def raise_nonzero(command): assert 'super-secret' not in message -def test_get_api_keys_reports_actionable_empty_output_error(monkeypatch): +def test_get_api_keys_reports_actionable_empty_output_error(): from ddev.cli.meta.scripts._dynamicd import cli as dynamicd_cli - app = _FakeApp() - - def raise_empty(command): - raise CommandExecutionError(command, 0, '', reason=EMPTY_OUTPUT) - - monkeypatch.setattr(dynamicd_cli, 'run_command', raise_empty) + app = _FakeApp(_make_config({'llm_api_key_fetch_command': 'echo ""'})) with pytest.raises(RuntimeError, match='aborted'): dynamicd_cli._get_api_keys(app) @@ -63,3 +54,26 @@ def raise_empty(command): assert 'dynamicd.llm_api_key_fetch_command' in message assert 'empty output' in message assert 'non-empty value' in message + + +def test_get_api_keys_plain_value_fallback(): + from ddev.cli.meta.scripts._dynamicd import cli as dynamicd_cli + + app = _FakeApp(_make_config({'llm_api_key': 'plain-key'})) + + llm_api_key, dd_api_key = dynamicd_cli._get_api_keys(app) + + assert llm_api_key == 'plain-key' + assert dd_api_key == 'dd_api' + + +def test_get_api_keys_env_fallback(monkeypatch): + from ddev.cli.meta.scripts._dynamicd import cli as dynamicd_cli + + monkeypatch.setenv('ANTHROPIC_API_KEY', 'env-key') + app = _FakeApp(_make_config({})) + + llm_api_key, dd_api_key = dynamicd_cli._get_api_keys(app) + + assert llm_api_key == 'env-key' + assert dd_api_key == 'dd_api' diff --git a/ddev/tests/config/test_command_resolver.py b/ddev/tests/config/test_command_resolver.py index f4e4e1b7abcf5..7a259caa2b9b7 100644 --- a/ddev/tests/config/test_command_resolver.py +++ b/ddev/tests/config/test_command_resolver.py @@ -1,4 +1,4 @@ -# (C) Datadog, Inc. 2022-present +# (C) Datadog, Inc. 2026-present # All rights reserved # Licensed under a 3-clause BSD style license (see LICENSE) import pytest diff --git a/ddev/tests/config/test_model.py b/ddev/tests/config/test_model.py index 7fd685c1f75e8..de5577b459b83 100644 --- a/ddev/tests/config/test_model.py +++ b/ddev/tests/config/test_model.py @@ -1067,6 +1067,71 @@ def test_token_set_lazy_error(self, helpers): _ = config.trello.token +class TestDynamicD: + def test_default(self): + config = RootConfig({}) + + assert config.dynamicd.llm_api_key is None + assert config.dynamicd.llm_api_key_fetch_command is None + assert 'dynamicd' not in config.raw_data + + def test_not_table(self, helpers): + config = RootConfig({'dynamicd': 9000}) + + with pytest.raises( + ConfigurationError, + match=helpers.dedent( + """ + Error parsing config: + dynamicd + must be a table""" + ), + ): + _ = config.dynamicd + + def test_parse_fields_does_not_execute_fetch_command(self, mocker): + run_command_mock = mocker.patch('ddev.config.model.run_command') + config = RootConfig({'dynamicd': {'llm_api_key_fetch_command': 'echo from-command'}}) + + config.parse_fields() + + run_command_mock.assert_not_called() + + def test_parse_fields_validates_fetch_command_type(self, helpers): + config = RootConfig({'dynamicd': {'llm_api_key_fetch_command': 9000}}) + + with pytest.raises( + ConfigurationError, + match=helpers.dedent( + """ + Error parsing config: + dynamicd -> llm_api_key_fetch_command + must be a string""" + ), + ): + config.parse_fields() + + def test_resolve_llm_api_key_fetch_command_takes_precedence(self, monkeypatch): + monkeypatch.setenv('ANTHROPIC_API_KEY', 'env-key') + config = RootConfig( + {'dynamicd': {'llm_api_key': 'plain-key', 'llm_api_key_fetch_command': 'echo command-key'}} + ) + + assert config.dynamicd.resolve_llm_api_key() == 'command-key' + + def test_resolve_llm_api_key_plain_fallback(self, monkeypatch): + monkeypatch.setenv('ANTHROPIC_API_KEY', 'env-key') + config = RootConfig({'dynamicd': {'llm_api_key': 'plain-key'}}) + + assert config.dynamicd.resolve_llm_api_key() == 'plain-key' + + def test_resolve_llm_api_key_env_fallback(self, monkeypatch): + monkeypatch.setenv('ANTHROPIC_API_KEY', 'env-key') + config = RootConfig({'dynamicd': {}}) + + assert config.dynamicd.resolve_llm_api_key() == 'env-key' + + class TestTerminal: def test_default(self): config = RootConfig({}) From 27a517dad67cd02d32fa15ef977d2d7f888a60dd Mon Sep 17 00:00:00 2001 From: Enrico Donnici Date: Wed, 25 Feb 2026 14:55:53 +0100 Subject: [PATCH 3/6] Make command-secret parsing lazy and tighten trello secret scrubbing. This keeps parse_fields focused on type validation for fetch-command fields, while preserving on-demand resolution and extending hidden/scrubbed handling to trello.key. --- ddev/src/ddev/config/model.py | 304 +++++++++++++++++++++++------ ddev/src/ddev/config/scrubber.py | 3 +- ddev/tests/cli/config/test_set.py | 82 ++++++++ ddev/tests/cli/config/test_show.py | 48 ++++- ddev/tests/config/test_model.py | 71 +++++++ 5 files changed, 451 insertions(+), 57 deletions(-) diff --git a/ddev/src/ddev/config/model.py b/ddev/src/ddev/config/model.py index c7363074fbcfb..de60f8e8607ab 100644 --- a/ddev/src/ddev/config/model.py +++ b/ddev/src/ddev/config/model.py @@ -529,26 +529,61 @@ def __init__(self, *args, **kwargs): self._field_user = FIELD_TO_PARSE self._field_token = FIELD_TO_PARSE + self._field_configured_user = FIELD_TO_PARSE + self._field_configured_token = FIELD_TO_PARSE + self._field_user_fetch_command = FIELD_TO_PARSE + self._field_token_fetch_command = FIELD_TO_PARSE + + @property + def configured_user(self): + if self._field_configured_user is FIELD_TO_PARSE: + if 'user' in self.raw_data: + user = self.raw_data['user'] + if not isinstance(user, str): + raise ConfigurationError('must be a string', location=' -> '.join([*self.steps, 'user'])) + self._field_configured_user = user + else: + self._field_configured_user = None + return self._field_configured_user + + @configured_user.setter + def configured_user(self, value): + self.raw_data['user'] = value + self._field_configured_user = FIELD_TO_PARSE + self._field_user = FIELD_TO_PARSE + + @property + def user_fetch_command(self): + if self._field_user_fetch_command is FIELD_TO_PARSE: + if 'user_fetch_command' in self.raw_data: + command = self.raw_data['user_fetch_command'] + if not isinstance(command, str): + self.raise_error('must be a string') + self._field_user_fetch_command = command + else: + self._field_user_fetch_command = None + return self._field_user_fetch_command + + @user_fetch_command.setter + def user_fetch_command(self, value): + self.raw_data['user_fetch_command'] = value + self._field_user_fetch_command = FIELD_TO_PARSE + self._field_user = FIELD_TO_PARSE @property def user(self): if self._field_user is FIELD_TO_PARSE: - if 'user_fetch_command' in self.raw_data: - cmd = self.raw_data['user_fetch_command'] - if not isinstance(cmd, str): - self.raise_error('must be a string', extra_steps=('user_fetch_command',)) + command = self.user_fetch_command + if command is not None: try: - self._field_user = run_command(cmd) + self._field_user = run_command(command) except CommandExecutionError as e: self.raise_error( e.to_user_message('github.user_fetch_command'), extra_steps=('user_fetch_command',), ) - elif 'user' in self.raw_data: - user = self.raw_data['user'] - if not isinstance(user, str): - self.raise_error('must be a string') - self._field_user = user + elif self.configured_user is not None: + self._field_user = self.configured_user else: self._field_user = get_github_user() @@ -556,28 +591,59 @@ def user(self): @user.setter def user(self, value): - self.raw_data['user'] = value + self.configured_user = value self._field_user = FIELD_TO_PARSE + @property + def configured_token(self): + if self._field_configured_token is FIELD_TO_PARSE: + if 'token' in self.raw_data: + token = self.raw_data['token'] + if not isinstance(token, str): + raise ConfigurationError('must be a string', location=' -> '.join([*self.steps, 'token'])) + self._field_configured_token = token + else: + self._field_configured_token = None + return self._field_configured_token + + @configured_token.setter + def configured_token(self, value): + self.raw_data['token'] = value + self._field_configured_token = FIELD_TO_PARSE + self._field_token = FIELD_TO_PARSE + + @property + def token_fetch_command(self): + if self._field_token_fetch_command is FIELD_TO_PARSE: + if 'token_fetch_command' in self.raw_data: + command = self.raw_data['token_fetch_command'] + if not isinstance(command, str): + self.raise_error('must be a string') + self._field_token_fetch_command = command + else: + self._field_token_fetch_command = None + return self._field_token_fetch_command + + @token_fetch_command.setter + def token_fetch_command(self, value): + self.raw_data['token_fetch_command'] = value + self._field_token_fetch_command = FIELD_TO_PARSE + self._field_token = FIELD_TO_PARSE + @property def token(self): if self._field_token is FIELD_TO_PARSE: - if 'token_fetch_command' in self.raw_data: - cmd = self.raw_data['token_fetch_command'] - if not isinstance(cmd, str): - self.raise_error('must be a string', extra_steps=('token_fetch_command',)) + command = self.token_fetch_command + if command is not None: try: - self._field_token = run_command(cmd) + self._field_token = run_command(command) except CommandExecutionError as e: self.raise_error( e.to_user_message('github.token_fetch_command'), extra_steps=('token_fetch_command',), ) - elif 'token' in self.raw_data: - token = self.raw_data['token'] - if not isinstance(token, str): - self.raise_error('must be a string') - self._field_token = token + elif self.configured_token is not None: + self._field_token = self.configured_token else: self._field_token = get_github_token() @@ -585,9 +651,16 @@ def token(self): @token.setter def token(self, value): - self.raw_data['token'] = value + self.configured_token = value self._field_token = FIELD_TO_PARSE + def parse_fields(self): + # Validate configured values and command field types without executing commands. + parse_config(self.configured_user) + parse_config(self.configured_token) + parse_config(self.user_fetch_command) + parse_config(self.token_fetch_command) + class PyPIConfig(LazilyParsedConfig): def __init__(self, *args, **kwargs): @@ -595,6 +668,8 @@ def __init__(self, *args, **kwargs): self._field_user = FIELD_TO_PARSE self._field_auth = FIELD_TO_PARSE + self._field_configured_auth = FIELD_TO_PARSE + self._field_auth_fetch_command = FIELD_TO_PARSE @property def user(self): @@ -615,22 +690,53 @@ def user(self, value): self.raw_data['user'] = value self._field_user = FIELD_TO_PARSE + @property + def configured_auth(self): + if self._field_configured_auth is FIELD_TO_PARSE: + if 'auth' in self.raw_data: + auth = self.raw_data['auth'] + if not isinstance(auth, str): + raise ConfigurationError('must be a string', location=' -> '.join([*self.steps, 'auth'])) + self._field_configured_auth = auth + else: + self._field_configured_auth = None + return self._field_configured_auth + + @configured_auth.setter + def configured_auth(self, value): + self.raw_data['auth'] = value + self._field_configured_auth = FIELD_TO_PARSE + self._field_auth = FIELD_TO_PARSE + + @property + def auth_fetch_command(self): + if self._field_auth_fetch_command is FIELD_TO_PARSE: + if 'auth_fetch_command' in self.raw_data: + command = self.raw_data['auth_fetch_command'] + if not isinstance(command, str): + self.raise_error('must be a string') + self._field_auth_fetch_command = command + else: + self._field_auth_fetch_command = None + return self._field_auth_fetch_command + + @auth_fetch_command.setter + def auth_fetch_command(self, value): + self.raw_data['auth_fetch_command'] = value + self._field_auth_fetch_command = FIELD_TO_PARSE + self._field_auth = FIELD_TO_PARSE + @property def auth(self): if self._field_auth is FIELD_TO_PARSE: - if 'auth_fetch_command' in self.raw_data: - cmd = self.raw_data['auth_fetch_command'] - if not isinstance(cmd, str): - self.raise_error('must be a string', extra_steps=('auth_fetch_command',)) + command = self.auth_fetch_command + if command is not None: try: - self._field_auth = run_command(cmd) + self._field_auth = run_command(command) except CommandExecutionError as e: self.raise_error(e.to_user_message('pypi.auth_fetch_command'), extra_steps=('auth_fetch_command',)) - elif 'auth' in self.raw_data: - auth = self.raw_data['auth'] - if not isinstance(auth, str): - self.raise_error('must be a string') - self._field_auth = auth + elif self.configured_auth is not None: + self._field_auth = self.configured_auth else: self._field_auth = self.raw_data['auth'] = '' @@ -638,9 +744,18 @@ def auth(self): @auth.setter def auth(self, value): - self.raw_data['auth'] = value + self.configured_auth = value self._field_auth = FIELD_TO_PARSE + def parse_fields(self): + # Validate configured values and command field types without executing commands. + parse_config(self.user) + parse_config(self.auth_fetch_command) + if self.auth_fetch_command is None: + parse_config(self.auth) + else: + parse_config(self.configured_auth) + class TrelloConfig(LazilyParsedConfig): def __init__(self, *args, **kwargs): @@ -648,23 +763,58 @@ def __init__(self, *args, **kwargs): self._field_key = FIELD_TO_PARSE self._field_token = FIELD_TO_PARSE + self._field_configured_key = FIELD_TO_PARSE + self._field_configured_token = FIELD_TO_PARSE + self._field_key_fetch_command = FIELD_TO_PARSE + self._field_token_fetch_command = FIELD_TO_PARSE + + @property + def configured_key(self): + if self._field_configured_key is FIELD_TO_PARSE: + if 'key' in self.raw_data: + key = self.raw_data['key'] + if not isinstance(key, str): + raise ConfigurationError('must be a string', location=' -> '.join([*self.steps, 'key'])) + self._field_configured_key = key + else: + self._field_configured_key = None + return self._field_configured_key + + @configured_key.setter + def configured_key(self, value): + self.raw_data['key'] = value + self._field_configured_key = FIELD_TO_PARSE + self._field_key = FIELD_TO_PARSE + + @property + def key_fetch_command(self): + if self._field_key_fetch_command is FIELD_TO_PARSE: + if 'key_fetch_command' in self.raw_data: + command = self.raw_data['key_fetch_command'] + if not isinstance(command, str): + self.raise_error('must be a string') + self._field_key_fetch_command = command + else: + self._field_key_fetch_command = None + return self._field_key_fetch_command + + @key_fetch_command.setter + def key_fetch_command(self, value): + self.raw_data['key_fetch_command'] = value + self._field_key_fetch_command = FIELD_TO_PARSE + self._field_key = FIELD_TO_PARSE @property def key(self): if self._field_key is FIELD_TO_PARSE: - if 'key_fetch_command' in self.raw_data: - cmd = self.raw_data['key_fetch_command'] - if not isinstance(cmd, str): - self.raise_error('must be a string', extra_steps=('key_fetch_command',)) + command = self.key_fetch_command + if command is not None: try: - self._field_key = run_command(cmd) + self._field_key = run_command(command) except CommandExecutionError as e: self.raise_error(e.to_user_message('trello.key_fetch_command'), extra_steps=('key_fetch_command',)) - elif 'key' in self.raw_data: - key = self.raw_data['key'] - if not isinstance(key, str): - self.raise_error('must be a string') - self._field_key = key + elif self.configured_key is not None: + self._field_key = self.configured_key else: self._field_key = self.raw_data['key'] = '' @@ -672,28 +822,59 @@ def key(self): @key.setter def key(self, value): - self.raw_data['key'] = value + self.configured_key = value self._field_key = FIELD_TO_PARSE + @property + def configured_token(self): + if self._field_configured_token is FIELD_TO_PARSE: + if 'token' in self.raw_data: + token = self.raw_data['token'] + if not isinstance(token, str): + raise ConfigurationError('must be a string', location=' -> '.join([*self.steps, 'token'])) + self._field_configured_token = token + else: + self._field_configured_token = None + return self._field_configured_token + + @configured_token.setter + def configured_token(self, value): + self.raw_data['token'] = value + self._field_configured_token = FIELD_TO_PARSE + self._field_token = FIELD_TO_PARSE + + @property + def token_fetch_command(self): + if self._field_token_fetch_command is FIELD_TO_PARSE: + if 'token_fetch_command' in self.raw_data: + command = self.raw_data['token_fetch_command'] + if not isinstance(command, str): + self.raise_error('must be a string') + self._field_token_fetch_command = command + else: + self._field_token_fetch_command = None + return self._field_token_fetch_command + + @token_fetch_command.setter + def token_fetch_command(self, value): + self.raw_data['token_fetch_command'] = value + self._field_token_fetch_command = FIELD_TO_PARSE + self._field_token = FIELD_TO_PARSE + @property def token(self): if self._field_token is FIELD_TO_PARSE: - if 'token_fetch_command' in self.raw_data: - cmd = self.raw_data['token_fetch_command'] - if not isinstance(cmd, str): - self.raise_error('must be a string', extra_steps=('token_fetch_command',)) + command = self.token_fetch_command + if command is not None: try: - self._field_token = run_command(cmd) + self._field_token = run_command(command) except CommandExecutionError as e: self.raise_error( e.to_user_message('trello.token_fetch_command'), extra_steps=('token_fetch_command',), ) - elif 'token' in self.raw_data: - token = self.raw_data['token'] - if not isinstance(token, str): - self.raise_error('must be a string') - self._field_token = token + elif self.configured_token is not None: + self._field_token = self.configured_token else: self._field_token = self.raw_data['token'] = '' @@ -701,9 +882,22 @@ def token(self): @token.setter def token(self, value): - self.raw_data['token'] = value + self.configured_token = value self._field_token = FIELD_TO_PARSE + def parse_fields(self): + # Validate configured values and command field types without executing commands. + parse_config(self.key_fetch_command) + parse_config(self.token_fetch_command) + if self.key_fetch_command is None: + parse_config(self.key) + else: + parse_config(self.configured_key) + if self.token_fetch_command is None: + parse_config(self.token) + else: + parse_config(self.configured_token) + class DynamicDConfig(LazilyParsedConfig): def __init__(self, *args, **kwargs): diff --git a/ddev/src/ddev/config/scrubber.py b/ddev/src/ddev/config/scrubber.py index a0029c3ac57fe..bcf2872d4b161 100644 --- a/ddev/src/ddev/config/scrubber.py +++ b/ddev/src/ddev/config/scrubber.py @@ -8,6 +8,7 @@ 'github.token_fetch_command', 'pypi.auth', 'pypi.auth_fetch_command', + 'trello.key', 'trello.token', 'trello.key_fetch_command', 'trello.token_fetch_command', @@ -21,7 +22,7 @@ TOP_LEVEL_SCRUB_KEYS = { 'github': ('token', 'user_fetch_command', 'token_fetch_command'), 'pypi': ('auth', 'auth_fetch_command'), - 'trello': ('token', 'key_fetch_command', 'token_fetch_command'), + 'trello': ('key', 'token', 'key_fetch_command', 'token_fetch_command'), 'dynamicd': ('llm_api_key', 'llm_api_key_fetch_command'), } ORG_SCRUB_KEYS = ('api_key', 'app_key', 'api_key_fetch_command', 'app_key_fetch_command') diff --git a/ddev/tests/cli/config/test_set.py b/ddev/tests/cli/config/test_set.py index baa9b40481353..47a1c2346d26c 100644 --- a/ddev/tests/cli/config/test_set.py +++ b/ddev/tests/cli/config/test_set.py @@ -102,6 +102,88 @@ def test_prompt_hidden(ddev, config_file, helpers): assert config_file.model.orgs['foo'] == {'api_key': 'bar'} +def test_standard_hidden_trello_key(ddev, config_file, helpers): + result = ddev('config', 'set', 'trello.key', 'bar') + + assert result.exit_code == 0, result.output + assert result.output == helpers.dedent( + """ + New setting: + [trello] + key = "*****" + """ + ) + + config_file.load() + assert config_file.model.trello.key == 'bar' + + +def test_prompt_hidden_trello_key(ddev, config_file, helpers): + result = ddev('config', 'set', 'trello.key', input='bar') + + assert result.exit_code == 0, result.output + assert result.output == helpers.dedent( + f""" + Value for `trello.key`:{" "} + New setting: + [trello] + key = "*****" + """ + ) + + config_file.load() + assert config_file.model.trello.key == 'bar' + + +def test_standard_hidden_dynamicd_llm_api_key(ddev, config_file, helpers): + result = ddev('config', 'set', 'dynamicd.llm_api_key', 'supersecret') + + assert result.exit_code == 0, result.output + assert result.output == helpers.dedent( + """ + New setting: + [dynamicd] + llm_api_key = "*****" + """ + ) + + config_file.load() + assert config_file.model.dynamicd.llm_api_key == 'supersecret' + + +def test_prompt_hidden_dynamicd_llm_api_key(ddev, config_file, helpers): + result = ddev('config', 'set', 'dynamicd.llm_api_key', input='supersecret') + + assert result.exit_code == 0, result.output + assert result.output == helpers.dedent( + f""" + Value for `dynamicd.llm_api_key`:{" "} + New setting: + [dynamicd] + llm_api_key = "*****" + """ + ) + + config_file.load() + assert config_file.model.dynamicd.llm_api_key == 'supersecret' + + +def test_standard_hidden_dynamicd_llm_api_key_fetch_command(ddev, config_file, helpers): + result = ddev('config', 'set', 'dynamicd.llm_api_key_fetch_command', 'echo key') + + assert result.exit_code == 0, result.output + assert result.output == helpers.dedent( + """ + New setting: + [dynamicd] + llm_api_key_fetch_command = "*****" + """ + ) + + config_file.load() + assert config_file.model.dynamicd.llm_api_key_fetch_command == 'echo key' + + def test_prevent_invalid_config(ddev, config_file, helpers): original_repo = config_file.model.repo.name result = ddev('config', 'set', 'repo', '["foo"]') diff --git a/ddev/tests/cli/config/test_show.py b/ddev/tests/cli/config/test_show.py index 8340de5bcd5b2..49bdc04bcf28c 100644 --- a/ddev/tests/cli/config/test_show.py +++ b/ddev/tests/cli/config/test_show.py @@ -46,7 +46,7 @@ auth = "*****" [trello] -key = "" +key = "*****" token = "*****" [terminal.styles] @@ -260,3 +260,49 @@ def test_non_scrubbed_output_shows_dynamicd_llm_api_key(ddev, config_file): assert result.exit_code == 0, result.output assert 'supersecret' in result.output + + +def test_scrubbed_output_includes_dynamicd_llm_api_key_fetch_command(ddev, config_file): + """config show must scrub dynamicd.llm_api_key_fetch_command.""" + config_file.model.raw_data.setdefault('dynamicd', {})['llm_api_key_fetch_command'] = 'echo supersecret' + config_file.save() + + result = ddev('config', 'show') + + assert result.exit_code == 0, result.output + assert 'echo supersecret' not in result.output + assert '*****' in result.output + + +def test_non_scrubbed_output_shows_dynamicd_llm_api_key_fetch_command(ddev, config_file): + """config show -a must show plain dynamicd.llm_api_key_fetch_command.""" + config_file.model.raw_data.setdefault('dynamicd', {})['llm_api_key_fetch_command'] = 'echo supersecret' + config_file.save() + + result = ddev('config', 'show', '-a') + + assert result.exit_code == 0, result.output + assert 'echo supersecret' in result.output + + +def test_scrubbed_output_includes_trello_key(ddev, config_file): + """config show must scrub trello.key.""" + config_file.model.raw_data.setdefault('trello', {})['key'] = 'trellosecret' + config_file.save() + + result = ddev('config', 'show') + + assert result.exit_code == 0, result.output + assert 'trellosecret' not in result.output + assert '*****' in result.output + + +def test_non_scrubbed_output_shows_trello_key(ddev, config_file): + """config show -a must show plain trello.key.""" + config_file.model.raw_data.setdefault('trello', {})['key'] = 'trellosecret' + config_file.save() + + result = ddev('config', 'show', '-a') + + assert result.exit_code == 0, result.output + assert 'trellosecret' in result.output diff --git a/ddev/tests/config/test_model.py b/ddev/tests/config/test_model.py index de5577b459b83..ffeed772babea 100644 --- a/ddev/tests/config/test_model.py +++ b/ddev/tests/config/test_model.py @@ -1132,6 +1132,77 @@ def test_resolve_llm_api_key_env_fallback(self, monkeypatch): assert config.dynamicd.resolve_llm_api_key() == 'env-key' +class TestLazySecretCommandParsing: + def test_parse_fields_does_not_execute_fetch_commands_for_all_sections(self, mocker): + run_command_mock = mocker.patch('ddev.config.model.run_command') + config = RootConfig( + { + 'github': { + 'user_fetch_command': 'echo gh-user', + 'token_fetch_command': 'echo gh-token', + }, + 'pypi': {'auth_fetch_command': 'echo pypi-auth'}, + 'trello': { + 'key_fetch_command': 'echo trello-key', + 'token_fetch_command': 'echo trello-token', + }, + 'orgs': { + 'default': { + 'api_key_fetch_command': 'echo dd-api', + 'app_key_fetch_command': 'echo dd-app', + } + }, + 'dynamicd': {'llm_api_key_fetch_command': 'echo llm-key'}, + } + ) + + config.parse_fields() + + run_command_mock.assert_not_called() + + def test_parse_fields_validates_github_fetch_command_type(self, helpers): + config = RootConfig({'github': {'user_fetch_command': 9000}}) + + with pytest.raises( + ConfigurationError, + match=helpers.dedent( + """ + Error parsing config: + github -> user_fetch_command + must be a string""" + ), + ): + config.parse_fields() + + def test_parse_fields_validates_pypi_fetch_command_type(self, helpers): + config = RootConfig({'pypi': {'auth_fetch_command': 9000}}) + + with pytest.raises( + ConfigurationError, + match=helpers.dedent( + """ + Error parsing config: + pypi -> auth_fetch_command + must be a string""" + ), + ): + config.parse_fields() + + def test_parse_fields_validates_trello_fetch_command_type(self, helpers): + config = RootConfig({'trello': {'key_fetch_command': 9000}}) + + with pytest.raises( + ConfigurationError, + match=helpers.dedent( + """ + Error parsing config: + trello -> key_fetch_command + must be a string""" + ), + ): + config.parse_fields() + + class TestTerminal: def test_default(self): config = RootConfig({}) From 82ded04b1802ac512f33337ca2fbe8f0f985eec9 Mon Sep 17 00:00:00 2001 From: Enrico Donnici Date: Fri, 6 Mar 2026 11:07:44 +0100 Subject: [PATCH 4/6] Document ddev secret command and trust behavior. Add public ddev docs for *_fetch_command secret fields, trust-gated .ddev.toml overrides, and config allow/deny usage, including troubleshooting and redaction guidance. --- docs/developer/ddev/configuration.md | 56 ++++++++++++++++++++++++++++ docs/developer/ddev/multirepo.md | 46 +++++++++++++++++++++++ 2 files changed, 102 insertions(+) diff --git a/docs/developer/ddev/configuration.md b/docs/developer/ddev/configuration.md index a42ff0148c757..9166078fe244b 100644 --- a/docs/developer/ddev/configuration.md +++ b/docs/developer/ddev/configuration.md @@ -91,3 +91,59 @@ If not: 1. Create a [personal access token][github-personal-access-token] with `public_repo` and `read:org` permissions 1. Run `ddev config set github.token` then paste the token 1. [Enable single sign-on][github-saml-single-sign-on] for the token + +## Command-based secret fields + +For secret values, you can use command-backed fields with the `*_fetch_command` suffix instead of storing +the secret directly in plaintext. + +Supported command-backed fields: + +- `github.user_fetch_command` +- `github.token_fetch_command` +- `pypi.auth_fetch_command` +- `trello.key_fetch_command` +- `trello.token_fetch_command` +- `orgs..api_key_fetch_command` +- `orgs..app_key_fetch_command` +- `dynamicd.llm_api_key_fetch_command` + +When both forms are configured, `*_fetch_command` takes precedence over the plaintext field. + +For example: + +```toml +[github] +user = "my-user" +user_fetch_command = "security find-generic-password -s ddev-github-user -w" +token_fetch_command = "security find-generic-password -s ddev-github-token -w" + +[orgs.staging] +api_key_fetch_command = "security find-generic-password -s ddev-staging-api-key -w" +app_key_fetch_command = "security find-generic-password -s ddev-staging-app-key -w" + +[dynamicd] +llm_api_key_fetch_command = "security find-generic-password -s ddev-anthropic-key -w" +``` + +`ddev` validates the shape/type of these fields during config parsing, and executes command-backed secret +fields lazily when the secret is actually needed. + +## Secret visibility in `config show` + +By default, `ddev config show` redacts secret values (including command-backed secret fields) from output. + +Use `ddev config show -a` to display all values without redaction. + +## Troubleshooting `*_fetch_command` + +When command-based resolution fails, `ddev` reports the exact config field path and an actionable reason: + +- command failed with a non-zero exit code +- command returned empty output + +If you hit either case, verify that the configured command: + +1. exists and is executable +1. writes the secret to `stdout` +1. returns a non-empty value diff --git a/docs/developer/ddev/multirepo.md b/docs/developer/ddev/multirepo.md index cb9f267a4563c..310e02fc0d12e 100644 --- a/docs/developer/ddev/multirepo.md +++ b/docs/developer/ddev/multirepo.md @@ -83,6 +83,52 @@ Now, the `.ddev.toml` file in the `issue_XYZ` directory modifies where the `core If we go back to our `integrations-core` directory and execute any `ddev` command, this override won't take effect. +## Trusting local `*_fetch_command` fields + +Because `.ddev.toml` is local to the working directory, command-backed secret fields (`*_fetch_command`) in this +file are trust-gated. + +When `ddev` loads overrides, local command-backed fields are handled in one of these states: + +- **`allowed`**: command fields are kept and executed normally. +- **`denied`**: command fields are stripped from `.ddev.toml` silently. +- **`unknown`**: command fields are stripped and a warning is shown. + +For `unknown` files, `ddev` shows a warning similar to: + +``` +Ignored untrusted `_fetch_command` field(s) from .ddev.toml: github.user_fetch_command. Run `ddev config allow` to trust this file. +``` + +### `ddev config allow` + +Trust the currently discovered `.ddev.toml` so its `*_fetch_command` fields are executed: + +```bash +ddev config allow +``` + +This stores the current file hash in `trusted_overrides.json` (in the same config directory as your global +`config.toml`). + +### `ddev config deny` + +Explicitly mark the currently discovered `.ddev.toml` as untrusted and silence warnings: + +```bash +ddev config deny +``` + +In this state, `*_fetch_command` fields are stripped silently. + +### Trust is hash-based + +Trust applies to both path and content hash. If the `.ddev.toml` file changes after being allowed, it returns to +`unknown` state and `*_fetch_command` fields are stripped again until you re-run: + +```bash +ddev config allow +``` ## Command Behavior with Overrides From 1c1b4cbd4c58faa670ab9f484bce67187987cbf9 Mon Sep 17 00:00:00 2001 From: Enrico Donnici Date: Wed, 25 Mar 2026 15:12:05 +0100 Subject: [PATCH 5/6] ddev: fix formatting and add changelog entry. --- ddev/changelog.d/23050.added | 1 + ddev/src/ddev/config/command_resolver.py | 1 + ddev/src/ddev/config/override_trust.py | 1 - ddev/src/ddev/config/utils.py | 1 + .../cli/meta/scripts/test_dynamicd_cli.py | 4 +- ddev/tests/config/test_file.py | 3 ++ ddev/tests/config/test_model.py | 54 +++++++++++-------- 7 files changed, 38 insertions(+), 27 deletions(-) create mode 100644 ddev/changelog.d/23050.added diff --git a/ddev/changelog.d/23050.added b/ddev/changelog.d/23050.added new file mode 100644 index 0000000000000..c6682c0d45741 --- /dev/null +++ b/ddev/changelog.d/23050.added @@ -0,0 +1 @@ +Add `*_fetch_command` secret resolution to ddev config fields, with trust-gating for `.ddev.toml` overrides and secret scrubbing in `config show`. \ No newline at end of file diff --git a/ddev/src/ddev/config/command_resolver.py b/ddev/src/ddev/config/command_resolver.py index 12c501bebebb7..cf7e02d20ddca 100644 --- a/ddev/src/ddev/config/command_resolver.py +++ b/ddev/src/ddev/config/command_resolver.py @@ -2,6 +2,7 @@ # All rights reserved # Licensed under a 3-clause BSD style license (see LICENSE) """Per-process command resolver with deterministic caching for `*_fetch_command` secret fields.""" + from __future__ import annotations import subprocess diff --git a/ddev/src/ddev/config/override_trust.py b/ddev/src/ddev/config/override_trust.py index 5cd2210ddc1fa..3c4652cc14a02 100644 --- a/ddev/src/ddev/config/override_trust.py +++ b/ddev/src/ddev/config/override_trust.py @@ -83,4 +83,3 @@ def upsert_trust_entry(overrides_path: Path, global_config_dir: Path, state: str store = _load_trust_store(global_config_dir) store[str(overrides_path)] = {'hash': current_hash, 'state': state} _save_trust_store(global_config_dir, store) - diff --git a/ddev/src/ddev/config/utils.py b/ddev/src/ddev/config/utils.py index 02af02abf584d..bbdaa88c0102e 100644 --- a/ddev/src/ddev/config/utils.py +++ b/ddev/src/ddev/config/utils.py @@ -6,6 +6,7 @@ from ddev.utils.fs import Path + def save_toml_document(document: TOMLDocument, path: Path): path.ensure_parent_dir_exists() path.write_atomic(tomlkit.dumps(document), 'w', encoding='utf-8') diff --git a/ddev/tests/cli/meta/scripts/test_dynamicd_cli.py b/ddev/tests/cli/meta/scripts/test_dynamicd_cli.py index 0750b103020ab..f6b47362df275 100644 --- a/ddev/tests/cli/meta/scripts/test_dynamicd_cli.py +++ b/ddev/tests/cli/meta/scripts/test_dynamicd_cli.py @@ -27,9 +27,7 @@ def _make_config(dynamicd: dict) -> RootConfig: def test_get_api_keys_reports_actionable_nonzero_error(): from ddev.cli.meta.scripts._dynamicd import cli as dynamicd_cli - app = _FakeApp( - _make_config({'llm_api_key_fetch_command': 'echo super-secret >/dev/null && exit 23'}) - ) + app = _FakeApp(_make_config({'llm_api_key_fetch_command': 'echo super-secret >/dev/null && exit 23'})) with pytest.raises(RuntimeError, match='aborted'): dynamicd_cli._get_api_keys(app) diff --git a/ddev/tests/config/test_file.py b/ddev/tests/config/test_file.py index d5658cafa58a9..9de8f85e1184b 100644 --- a/ddev/tests/config/test_file.py +++ b/ddev/tests/config/test_file.py @@ -418,6 +418,7 @@ def mock_is_file(self: Path): # Tests for _strip_fetch_command_fields # --------------------------------------------------------------------------- + class TestStripFetchCommandFields: def test_strips_top_level_fetch_command_key(self): data = {'token_fetch_command': 'secret-tool', 'token': 'plain'} @@ -457,6 +458,7 @@ def test_non_fetch_command_keys_preserved(self): # Tests for trust store helpers # --------------------------------------------------------------------------- + class TestTrustStore: def test_unknown_when_no_store(self, tmp_path): tmp_path = Path(tmp_path) @@ -521,6 +523,7 @@ def test_upsert_is_idempotent(self, tmp_path): # Tests for load() trust-aware stripping # --------------------------------------------------------------------------- + class TestLoadTrustAwareStripping: def _make_config_file(self, tmp_path: Path) -> ConfigFileWithOverrides: global_path = tmp_path / 'config.toml' diff --git a/ddev/tests/config/test_model.py b/ddev/tests/config/test_model.py index ffeed772babea..5ff3c5ed68bf6 100644 --- a/ddev/tests/config/test_model.py +++ b/ddev/tests/config/test_model.py @@ -1113,9 +1113,7 @@ def test_parse_fields_validates_fetch_command_type(self, helpers): def test_resolve_llm_api_key_fetch_command_takes_precedence(self, monkeypatch): monkeypatch.setenv('ANTHROPIC_API_KEY', 'env-key') - config = RootConfig( - {'dynamicd': {'llm_api_key': 'plain-key', 'llm_api_key_fetch_command': 'echo command-key'}} - ) + config = RootConfig({'dynamicd': {'llm_api_key': 'plain-key', 'llm_api_key_fetch_command': 'echo command-key'}}) assert config.dynamicd.resolve_llm_api_key() == 'command-key' @@ -1715,36 +1713,46 @@ def test_token_fetch_command_failure_raises(self): class TestCommandFieldsOrg: def test_api_key_fetch_command_takes_precedence(self): - config = RootConfig({ - 'orgs': {'myorg': {'api_key': 'plain', 'api_key_fetch_command': 'echo cmd_api', 'app_key': ''}}, - 'org': 'myorg', - }) + config = RootConfig( + { + 'orgs': {'myorg': {'api_key': 'plain', 'api_key_fetch_command': 'echo cmd_api', 'app_key': ''}}, + 'org': 'myorg', + } + ) assert config.org.config.get('api_key') == 'cmd_api' def test_api_key_plain_fallback(self): - config = RootConfig({ - 'orgs': {'myorg': {'api_key': 'plain', 'app_key': ''}}, - 'org': 'myorg', - }) + config = RootConfig( + { + 'orgs': {'myorg': {'api_key': 'plain', 'app_key': ''}}, + 'org': 'myorg', + } + ) assert config.org.config.get('api_key') == 'plain' def test_app_key_fetch_command_takes_precedence(self): - config = RootConfig({ - 'orgs': {'myorg': {'api_key': '', 'app_key': 'plain', 'app_key_fetch_command': 'echo cmd_app'}}, - 'org': 'myorg', - }) + config = RootConfig( + { + 'orgs': {'myorg': {'api_key': '', 'app_key': 'plain', 'app_key_fetch_command': 'echo cmd_app'}}, + 'org': 'myorg', + } + ) assert config.org.config.get('app_key') == 'cmd_app' def test_app_key_plain_fallback(self): - config = RootConfig({ - 'orgs': {'myorg': {'api_key': '', 'app_key': 'plain'}}, - 'org': 'myorg', - }) + config = RootConfig( + { + 'orgs': {'myorg': {'api_key': '', 'app_key': 'plain'}}, + 'org': 'myorg', + } + ) assert config.org.config.get('app_key') == 'plain' def test_non_command_keys_unaffected(self): - config = RootConfig({ - 'orgs': {'myorg': {'api_key': '', 'app_key': '', 'site': 'mysite.com'}}, - 'org': 'myorg', - }) + config = RootConfig( + { + 'orgs': {'myorg': {'api_key': '', 'app_key': '', 'site': 'mysite.com'}}, + 'org': 'myorg', + } + ) assert config.org.config.get('site') == 'mysite.com' From 4eba91eb41790d5481df5fc135a7f3423b084173 Mon Sep 17 00:00:00 2001 From: Enrico Donnici Date: Thu, 26 Mar 2026 12:05:55 +0000 Subject: [PATCH 6/6] ddev: fix Windows-incompatible shell commands in tests Replace `echo ""` (prints literal `""` on Windows) and `/dev/null` redirection with `sys.executable`-based Python one-liners that produce the same behaviour on all platforms. --- ddev/tests/cli/meta/scripts/test_dynamicd_cli.py | 7 ++++--- ddev/tests/config/test_command_resolver.py | 4 +++- ddev/tests/config/test_model.py | 3 ++- 3 files changed, 9 insertions(+), 5 deletions(-) diff --git a/ddev/tests/cli/meta/scripts/test_dynamicd_cli.py b/ddev/tests/cli/meta/scripts/test_dynamicd_cli.py index f6b47362df275..79f507fe5f05b 100644 --- a/ddev/tests/cli/meta/scripts/test_dynamicd_cli.py +++ b/ddev/tests/cli/meta/scripts/test_dynamicd_cli.py @@ -3,6 +3,8 @@ # Licensed under a 3-clause BSD style license (see LICENSE) from __future__ import annotations +import sys + import pytest from ddev.config.model import RootConfig @@ -27,7 +29,7 @@ def _make_config(dynamicd: dict) -> RootConfig: def test_get_api_keys_reports_actionable_nonzero_error(): from ddev.cli.meta.scripts._dynamicd import cli as dynamicd_cli - app = _FakeApp(_make_config({'llm_api_key_fetch_command': 'echo super-secret >/dev/null && exit 23'})) + app = _FakeApp(_make_config({'llm_api_key_fetch_command': f'{sys.executable} -c "import sys; sys.exit(23)"'})) with pytest.raises(RuntimeError, match='aborted'): dynamicd_cli._get_api_keys(app) @@ -37,13 +39,12 @@ def test_get_api_keys_reports_actionable_nonzero_error(): assert 'dynamicd.llm_api_key_fetch_command' in message assert 'exit code 23' in message assert 'stdout' in message - assert 'super-secret' not in message def test_get_api_keys_reports_actionable_empty_output_error(): from ddev.cli.meta.scripts._dynamicd import cli as dynamicd_cli - app = _FakeApp(_make_config({'llm_api_key_fetch_command': 'echo ""'})) + app = _FakeApp(_make_config({'llm_api_key_fetch_command': f'{sys.executable} -c "pass"'})) with pytest.raises(RuntimeError, match='aborted'): dynamicd_cli._get_api_keys(app) diff --git a/ddev/tests/config/test_command_resolver.py b/ddev/tests/config/test_command_resolver.py index 7a259caa2b9b7..73550d6b017f2 100644 --- a/ddev/tests/config/test_command_resolver.py +++ b/ddev/tests/config/test_command_resolver.py @@ -1,6 +1,8 @@ # (C) Datadog, Inc. 2026-present # All rights reserved # Licensed under a 3-clause BSD style license (see LICENSE) +import sys + import pytest from ddev.config.command_resolver import ( @@ -46,7 +48,7 @@ def test_nonzero_exit_raises(self): def test_empty_output_raises(self): with pytest.raises(CommandExecutionError) as exc_info: - run_command('echo ""') + run_command(f'{sys.executable} -c "pass"') assert exc_info.value.reason == EMPTY_OUTPUT assert 'empty output' in str(exc_info.value) diff --git a/ddev/tests/config/test_model.py b/ddev/tests/config/test_model.py index 5ff3c5ed68bf6..9fde53a57d119 100644 --- a/ddev/tests/config/test_model.py +++ b/ddev/tests/config/test_model.py @@ -2,6 +2,7 @@ # All rights reserved # Licensed under a 3-clause BSD style license (see LICENSE) import os +import sys import pytest @@ -1683,7 +1684,7 @@ def test_auth_fetch_command_failure_raises(self): _ = config.pypi.auth def test_auth_fetch_command_empty_output_message_is_actionable(self): - config = RootConfig({'pypi': {'auth_fetch_command': 'echo ""'}}) + config = RootConfig({'pypi': {'auth_fetch_command': f'{sys.executable} -c "pass"'}}) with pytest.raises(ConfigurationError) as exc_info: _ = config.pypi.auth message = str(exc_info.value)