diff --git a/README.md b/README.md index 63889ab..5be41d2 100644 --- a/README.md +++ b/README.md @@ -296,7 +296,7 @@ limit: {_env: RUN_LIMIT, default: 10} - The directive is exactly `{_env: NAME}` or `{_env: NAME, default: }`. Any other key beside `_env` — including underscore-prefixed keys and aliases such as `fallback` — is rejected, and `NAME` follows the same rule as `_select.env` (`[A-Za-z_][A-Za-z0-9_]*`). - It may appear anywhere a value may appear: under a config key, in `_defaults`, in `configs` entries, and inside nested maps and lists. A document or config-entry mapping is never itself a directive, so an `_env:` helper/anchor key at the top level of a config object is unaffected. - A missing variable without `default` fails closed with the variable name, the key path (for example `configs[1].token`), and the config file. An empty string counts as set and is used as-is. -- The resolved value is always a string, and a `default` is stringified the same way: numbers and booleans use their JSON spelling (`10`, `1.5`, `true`), dates use ISO format, and `null`/maps/lists are rejected (quote `''` for an empty default). A field therefore has one type whether or not the variable is set, and the command's usual conversion (JSON fields, repeatable options, enums, typed converters) applies downstream in both cases. +- The resolved value is always a string, and a `default` is stringified the same way: numbers and booleans use their JSON spelling (`10`, `1.5`, `true`), dates use ISO format, and `null`/maps/lists are rejected (quote `''` for an empty default). A field therefore has one type whether or not the variable is set, and the command's field typing (below) applies in both cases. - Diagnostics name the variable, never its value, and no directive value is logged. There is no `${VAR}` string interpolation. With `_select`, selection happens first, and `_env` applies to the selected document: @@ -315,6 +315,17 @@ Every `_env` directive — in every case and `default` branch, and in helper sec Precedence per field is **CLI > config > environment > default**: an explicit CLI value wins, then the config key (including a value read through `_env`), then an environment variable the command has opted that field into (see [`EnvField`](#shared-cli-helpers-and-logging)), then the default. As before, a CLI value equal to the option default is indistinguishable from an omitted one. +#### Scalar field typing + +Command flags and numeric options are typed strictly, so an environment or config string can never reach them as the wrong type. This matters most for flags: `force: {_env: DELETE_FORCE, default: false}` yields the string `"false"`, which Python treats as truthy, so without typing it would skip the delete confirmation. + +- A field is typed when its default is a `bool`, `int`, or `float`, or when its spec names a strict converter (`strict_bool`, `strict_int`, `strict_float` from `tangle_cli.args_container`). The latter is how a `bool | None` / `int | None` option with a `None` default is typed; the built-in commands use it for their flags, e.g. `dry_run`, `trusted_hydration_cli`, `allow_downgrade`, and `limit`. +- **bool** accepts a YAML/JSON boolean, `0`/`1`, or exactly one of `true`/`false`, `yes`/`no`, `1`/`0` (case-insensitive, no surrounding whitespace). Anything else is rejected, including `""`, `on`/`off`, lists, and maps. +- **int** accepts an integer or a string of ASCII digits with an optional sign. `1.0`, `1e3`, `0x1f`, `1_000`, and booleans are rejected. **float** accepts a number or a decimal string with an optional exponent; `nan`/`inf` are rejected. +- The same rule applies to a config **string literal** (`force: "false"` → `False`), a value read through `_env`, and an `EnvField` variable. Before 0.1.21 a quoted `"false"` also reached a flag as a truthy string. CLI values are already typed by the parser. +- A rejection names the field and its source — config key, variable, or `_env` at a config key — never the value. +- Only scalar fields are typed. String fields, JSON fields, repeatable options, enums, and values nested inside maps or lists are unchanged: an `_env` inside a nested structure stays a string. Pipeline `cfg` files keep their own typing rules (native YAML values; `_env` strings are not coerced). + ## API schema cache and dynamic commands Refresh the local schema cache for a live backend with: @@ -1021,7 +1032,7 @@ Use these for generic downstream behavior such as alternate storage, extra annot `cli_options.py` centralizes shared Cyclopts annotations such as `BaseUrlOption`, `TokenOption`, `AuthHeaderOption`, `HeaderOption`, `ConfigOption`, and `LogTypeOption`. `cli_helpers.py` centralizes config loading, JSON printing, credential-isolation helpers, and the native-safe `LazyTangleApiClient` proxy. `logger.py` provides `ConsoleLogger`, `NullLogger`, `CaptureLogger`, `logger_for_log_type(...)`, and `run_with_logging(...)`. -`ArgsContainer.load(...)` field specs are tuples (see `ArgsContainer._resolve`). To give one field an environment tier, wrap its unchanged spec: `token=EnvField("TANGLE_PROD_TOKEN", (token, None))`. The field then resolves CLI > config > `os.environ["TANGLE_PROD_TOKEN"]` > default; the raw string (empty counts as set) goes through the spec's usual converter, and a conversion error names the variable without echoing its value. Nothing is mapped automatically: fields that are not wrapped never read the environment. `args.origin(name)` reports where each field came from — `cli`, `config`, `env:NAME`, or `default` — without the value. +`ArgsContainer.load(...)` field specs are tuples (see `ArgsContainer._resolve`). To give one field an environment tier, wrap its unchanged spec: `token=EnvField("TANGLE_PROD_TOKEN", (token, None))`. The field then resolves CLI > config > `os.environ["TANGLE_PROD_TOKEN"]` > default; the raw string (empty counts as set) goes through the spec's converter or the [scalar typing](#scalar-field-typing), and a conversion error names the variable without echoing its value. Nothing is mapped automatically: fields that are not wrapped never read the environment. `args.origin(name)` reports where each field came from — `cli`, `config`, `env:NAME`, or `default` — without the value. Use these helpers for new SDK commands so top-level imports remain native-free, `--config` behavior stays consistent, credentials from config do not accidentally mix with ambient environment auth, and progress logs stay off structured stdout. diff --git a/packages/tangle-cli/src/tangle_cli/__init__.py b/packages/tangle-cli/src/tangle_cli/__init__.py index 375f8b8..ceba2aa 100644 --- a/packages/tangle-cli/src/tangle_cli/__init__.py +++ b/packages/tangle-cli/src/tangle_cli/__init__.py @@ -14,6 +14,6 @@ try: __version__ = metadata_version("tangle-cli") except PackageNotFoundError: - __version__ = "0.1.20" + __version__ = "0.1.21" __all__ = ["TangleDynamicDiscoveryClient", "__version__"] diff --git a/packages/tangle-cli/src/tangle_cli/args_container.py b/packages/tangle-cli/src/tangle_cli/args_container.py index b169832..67f9f6e 100644 --- a/packages/tangle-cli/src/tangle_cli/args_container.py +++ b/packages/tangle-cli/src/tangle_cli/args_container.py @@ -7,6 +7,12 @@ Precedence per field is CLI > config > environment (only for fields wrapped in :class:`EnvField`) > default. Config values may themselves be read from the environment with the ``{_env: NAME}`` value directive. + +Scalar fields are typed strictly: a field whose default is a bool/int/float +(or whose spec names :func:`strict_bool` / :func:`strict_int` / +:func:`strict_float`) parses a config or environment string into that type +and rejects anything else, so ``"false"`` can never reach a flag as a truthy +string. """ from __future__ import annotations @@ -274,6 +280,72 @@ def __post_init__(self) -> None: raise ValueError("EnvField.spec must be an ArgsContainer field-spec tuple") +class _ScalarValueError(ConfigFileError): + """A strict scalar converter's value-free rejection reason.""" + + +#: The only accepted boolean spellings, matched case-insensitively and untrimmed. +_BOOL_STRINGS = {"true": True, "false": False, "yes": True, "no": False, "1": True, "0": False} +_INT_PATTERN = re.compile(r"[+-]?[0-9]+") +_FLOAT_PATTERN = re.compile(r"[+-]?(?:[0-9]+(?:\.[0-9]*)?|\.[0-9]+)(?:[eE][+-]?[0-9]+)?") + + +def strict_bool(value: Any) -> bool: + """Field converter: a bool, the integer 0/1, or one of ``true``/``false``, + ``yes``/``no``, ``1``/``0`` (case-insensitive). Anything else -- the empty + string included -- is rejected without echoing the value.""" + + if isinstance(value, bool): + return value + if isinstance(value, int) and value in (0, 1): + return bool(value) + if isinstance(value, str) and value.isascii() and value.lower() in _BOOL_STRINGS: + return _BOOL_STRINGS[value.lower()] + raise _ScalarValueError("expected a boolean: true/false, yes/no, or 1/0 (case-insensitive)") + + +def strict_int(value: Any) -> int: + """Field converter: an int, or a string of ASCII digits with an optional sign.""" + + if isinstance(value, int) and not isinstance(value, bool): + return value + if isinstance(value, str) and _INT_PATTERN.fullmatch(value): + return int(value) + raise _ScalarValueError("expected an integer") + + +def strict_float(value: Any) -> float: + """Field converter: an int/float, or a decimal string (optional exponent). + + ``nan``/``inf`` spellings are rejected. + """ + + if isinstance(value, (int, float)) and not isinstance(value, bool): + return value + if isinstance(value, str) and _FLOAT_PATTERN.fullmatch(value): + return float(value) + raise _ScalarValueError("expected a number") + + +_STRICT_SCALARS: dict[type, Callable[[Any], Any]] = { + bool: strict_bool, + int: strict_int, + float: strict_float, +} + + +def _direct_env_sources(entry: dict[str, Any]) -> dict[str, str]: + """Map each top-level key whose value is an ``_env`` directive to its variable.""" + + sources: dict[str, str] = {} + for key, value in entry.items(): + if isinstance(value, dict) and ENV_KEY in value: + name = cast(dict[Any, Any], value)[ENV_KEY] + if _is_env_name(name): + sources[key] = name + return sources + + class ArgsContainer: """Container for resolved CLI arguments with config-file defaults.""" @@ -665,6 +737,21 @@ def _load_config_file( ) -> list[dict[str, Any]]: """Load a YAML/JSON config file as a list of config dictionaries. + See :meth:`_load_config_entries`, which this wraps without provenance. + """ + + return [entry for entry, _ in ArgsContainer._load_config_entries(config_path, logger)] + + @staticmethod + def _load_config_entries( + config_path: str | Path | None, + logger: Logger | None = None, + ) -> list[tuple[dict[str, Any], dict[str, str]]]: + """Load a YAML/JSON config file as ``(config, env_sources)`` pairs. + + ``env_sources`` maps each top-level key read through an ``_env`` + directive to its variable name, for diagnostics only. + Supported shapes are a single object, a list of objects, or an object with ``_defaults`` and ``configs`` where defaults are applied to each config entry. Other top-level keys are ignored, which lets YAML files @@ -681,7 +768,7 @@ def _load_config_file( log = logger or get_default_logger() if config_path is None: - return [{}] + return [({}, {})] path = Path(config_path) if not path.exists(): @@ -692,7 +779,7 @@ def _load_config_file( if path.suffix in (".yaml", ".yml"): parsed = yaml.safe_load(f) if parsed is None: - return [{}] + return [({}, {})] else: parsed = json.load(f) except (OSError, json.JSONDecodeError, yaml.YAMLError) as exc: @@ -700,7 +787,11 @@ def _load_config_file( resolution = resolve_config_document(parsed, path) parsed = resolution.document - resolve_entry = resolution.resolve_entry + + def resolve_entry( + entry: dict[str, Any], entry_path: _ConfigPath + ) -> tuple[dict[str, Any], dict[str, str]]: + return resolution.resolve_entry(entry, entry_path), _direct_env_sources(entry) if isinstance(parsed, dict): parsed_dict = cast(dict[str, Any], parsed) @@ -721,15 +812,26 @@ def _load_config_file( "configs entry " f"{index} must be an object, got {type(item).__name__}" ) - defaults = resolve_entry(cast(dict[str, Any], defaults), ("_defaults",)) - configs_list = [ + defaults, defaults_sources = resolve_entry( + cast(dict[str, Any], defaults), ("_defaults",) + ) + entries = [ resolve_entry(item, ("configs", index)) for index, item in enumerate(cast(list[dict[str, Any]], configs_list)) ] - merged = apply_defaults(configs_list, defaults) + merged = apply_defaults([entry for entry, _ in entries], defaults) assert isinstance(merged, list) log.info(f"Loaded config: {path} ({len(merged)} configs with defaults)") - return merged + return [ + ( + merged_entry, + { + **{k: v for k, v in defaults_sources.items() if k not in entry}, + **sources, + }, + ) + for merged_entry, (entry, sources) in zip(merged, entries, strict=True) + ] log.info(f"Loaded config: {path} (1 config)") return [resolve_entry(parsed_dict, ())] @@ -796,7 +898,12 @@ def convert(value: Any) -> Any: return convert @staticmethod - def _resolve(config: dict[str, Any], **kwargs: Any) -> ArgsContainer: + def _resolve( + config: dict[str, Any], + env_sources: dict[str, str] | None = None, + /, + **kwargs: Any, + ) -> ArgsContainer: """Resolve CLI args against a single config dict. Field specs can be: @@ -811,6 +918,13 @@ def _resolve(config: dict[str, Any], **kwargs: Any) -> ArgsContainer: Precedence: an explicit CLI value (one differing from the spec default), then the config key, then the ``EnvField`` variable, then the CLI/default value. + + Without a converter, a bool/int/float default types the field: config + and environment values go through :func:`strict_bool` / + :func:`strict_int` / :func:`strict_float` (CLI values are already + typed by the parser). A rejection names the field and its source -- + config key or variable -- never the value. *env_sources* maps config + keys read through ``_env`` to their variables, for those messages. """ resolved: dict[str, Any] = {} @@ -853,8 +967,12 @@ def _resolve(config: dict[str, Any], **kwargs: Any) -> ArgsContainer: if required: required_fields.append(param_name) + inferred_scalar = False if converter is None and isinstance(default_value, Enum): converter = ArgsContainer._make_enum_converter(param_name, type(default_value)) + elif converter is None and type(default_value) in _STRICT_SCALARS: + converter = _STRICT_SCALARS[type(default_value)] + inferred_scalar = True if cli_value is not None and cli_value != default_value: value, origin = cli_value, "cli" @@ -865,18 +983,26 @@ def _resolve(config: dict[str, Any], **kwargs: Any) -> ArgsContainer: else: value, origin = cli_value, "default" - if converter and value is not None: - if env_name is not None and origin == f"env:{env_name}": - try: - value = converter(value) - except (ConfigFileError, ValueError, TypeError): - # A converter message may quote the raw value. - raise ConfigFileError( - f"Invalid value for {param_name} from environment " - f"variable {env_name}" - ) from None - else: + # An inferred type only parses config/env values; CLI values are typed. + if converter and value is not None and not (inferred_scalar and origin in ("cli", "default")): + from_env_tier = env_name is not None and origin == f"env:{env_name}" + try: value = converter(value) + except _ScalarValueError as exc: + source = ArgsContainer._describe_source( + origin, config_key, env_name, env_sources or {} + ) + raise ConfigFileError( + f"Invalid value for {param_name} from {source}: {exc}" + ) from None + except (ConfigFileError, ValueError, TypeError): + if not from_env_tier: + raise + # A converter message may quote the raw value. + raise ConfigFileError( + f"Invalid value for {param_name} from environment " + f"variable {env_name}" + ) from None resolved[param_name] = value origins[param_name] = origin @@ -893,6 +1019,24 @@ def _resolve(config: dict[str, Any], **kwargs: Any) -> ArgsContainer: return ArgsContainer(resolved, config, origins) + @staticmethod + def _describe_source( + origin: str, config_key: Any, env_name: str | None, env_sources: dict[str, str] + ) -> str: + """Name where a rejected value came from, without the value.""" + + if origin == "config": + key = _render_config_key(config_key) + if config_key in env_sources: + return ( + f"environment variable {env_sources[config_key]} " + f"({ENV_KEY} at config key {key})" + ) + return f"config key {key}" + if env_name is not None and origin == f"env:{env_name}": + return f"environment variable {env_name}" + return "the CLI argument" if origin == "cli" else "the default" + @staticmethod def load( config_path: str | Path | None, @@ -901,8 +1045,8 @@ def load( ) -> list[ArgsContainer]: """Load a config file and resolve CLI args against each config entry.""" - configs = ArgsContainer._load_config_file(config_path, logger=logger) - return [ArgsContainer._resolve(config, **kwargs) for config in configs] + entries = ArgsContainer._load_config_entries(config_path, logger=logger) + return [ArgsContainer._resolve(config, sources, **kwargs) for config, sources in entries] @dataclass(frozen=True) @@ -969,4 +1113,7 @@ def resolve_config_document( "EnvField", "ResolvedConfigDocument", "resolve_config_document", + "strict_bool", + "strict_float", + "strict_int", ] diff --git a/packages/tangle-cli/src/tangle_cli/components_cli.py b/packages/tangle-cli/src/tangle_cli/components_cli.py index 3fcf692..0aa7a02 100644 --- a/packages/tangle-cli/src/tangle_cli/components_cli.py +++ b/packages/tangle-cli/src/tangle_cli/components_cli.py @@ -4,6 +4,7 @@ from cyclopts import App, Parameter +from .args_container import strict_bool from .cli_helpers import load_args_or_exit, optional_path from .cli_options import ConfigOption, LogTypeOption from .logger import logger_for_log_type @@ -115,8 +116,8 @@ def _components_generate_from_python_impl( function_name=("function", function_name, None, False), image=(image, None), dependencies_from=(dependencies_from, None, optional_path), - strip_code=(strip_code, None), - use_legacy_naming=(use_legacy_naming, None), + strip_code=(strip_code, None, strict_bool), + use_legacy_naming=(use_legacy_naming, None, strict_bool), mode=(mode, None), resolve_root=(resolve_root, None, optional_path), log_type=(log_type, "console"), @@ -246,7 +247,7 @@ def components_bump_version( config, yaml_file=("yaml_file", yaml_file, None, False, True, optional_path), set_version=(set_version, None), - update_timestamp=(update_timestamp, None), + update_timestamp=(update_timestamp, None, strict_bool), log_type=(log_type, "console"), ) result: dict[str, Any] = {} diff --git a/packages/tangle-cli/src/tangle_cli/pipeline_runs_cli.py b/packages/tangle-cli/src/tangle_cli/pipeline_runs_cli.py index c607bca..f869d3d 100644 --- a/packages/tangle-cli/src/tangle_cli/pipeline_runs_cli.py +++ b/packages/tangle-cli/src/tangle_cli/pipeline_runs_cli.py @@ -14,7 +14,7 @@ from cyclopts import App, Parameter -from .args_container import ArgsContainer +from .args_container import ArgsContainer, strict_bool, strict_int from .cli_helpers import ( LazyTangleApiClient, api_arg_specs, @@ -276,10 +276,10 @@ def pipeline_runs_submit( "arg_secrets_config": ("arg_secrets", None, None, True), "annotation": (annotation, None), "hydrate": (hydrate, True), - "dry_run": (dry_run, None), + "dry_run": (dry_run, None, strict_bool), "run_as": (run_as, None), "trusted_source": (trusted_source, None), - "trusted_hydration_cli": ("trusted_hydration_cli", trusted_hydration, None, False), + "trusted_hydration_cli": ("trusted_hydration_cli", trusted_hydration, None, False, False, strict_bool), "submit_recovery_attempts": (submit_recovery_attempts, _DEFAULT_SUBMIT_RECOVERY_ATTEMPTS), "log_type": (log_type, "console"), **api_arg_specs(base_url=base_url, token=token, auth_header=auth_header, header=header), @@ -590,10 +590,10 @@ def pipeline_runs_submit_from_python( "arg_secret": (arg_secret, None), "arg_secrets_config": ("arg_secrets", None, None, True), "annotation": (annotation, None), - "dry_run": (dry_run, None), + "dry_run": (dry_run, None, strict_bool), "run_as": (run_as, None), "trusted_source": (trusted_source, None), - "trusted_hydration_cli": ("trusted_hydration_cli", trusted_hydration, None, False), + "trusted_hydration_cli": ("trusted_hydration_cli", trusted_hydration, None, False, False, strict_bool), "submit_recovery_attempts": (submit_recovery_attempts, _DEFAULT_SUBMIT_RECOVERY_ATTEMPTS), "log_type": (log_type, "console"), **api_arg_specs(base_url=base_url, token=token, auth_header=auth_header, header=header), @@ -676,9 +676,9 @@ def pipeline_runs_details( specs = { "run_id": (run_id,), "execution_id": (execution_id, None), - "include_implementations": (include_implementations, None), - "include_annotations": (include_annotations, None), - "include_execution_state": (include_execution_state, None), + "include_implementations": (include_implementations, None, strict_bool), + "include_annotations": (include_annotations, None, strict_bool), + "include_execution_state": (include_execution_state, None, strict_bool), "log_type": (log_type, "console"), **api_arg_specs(base_url=base_url, token=token, auth_header=auth_header, header=header), } @@ -896,7 +896,7 @@ def pipeline_runs_logs( """Print Tangle API container logs for an execution id.""" specs = { "execution_id": (execution_id,), - "stream": (stream, None), + "stream": (stream, None, strict_bool), "log_type": (log_type, "console"), **api_arg_specs(base_url=base_url, token=token, auth_header=auth_header, header=header), } @@ -964,12 +964,12 @@ def pipeline_runs_search( "annotations_json": (annotations_json, None), "start_date": (start_date, None), "end_date": (end_date, None), - "local_time": (local_time, None), + "local_time": (local_time, None, strict_bool), "raw_query": (raw_query, None), - "limit": (limit, None), + "limit": (limit, None, strict_int), "page_token": (page_token, None), - "include_pipeline_names": (include_pipeline_names, None), - "include_execution_stats": (include_execution_stats, None), + "include_pipeline_names": (include_pipeline_names, None, strict_bool), + "include_execution_stats": (include_execution_stats, None, strict_bool), "output": (output, "json"), "log_type": (log_type, "console"), **api_arg_specs(base_url=base_url, token=token, auth_header=auth_header, header=header), @@ -1052,7 +1052,7 @@ def pipeline_runs_export( specs = { "run_id": (run_id,), "output": (output, None, optional_path), - "dehydrate": (dehydrate, None), + "dehydrate": (dehydrate, None, strict_bool), "log_type": (log_type, "console"), **api_arg_specs(base_url=base_url, token=token, auth_header=auth_header, header=header), } diff --git a/packages/tangle-cli/src/tangle_cli/published_components_cli.py b/packages/tangle-cli/src/tangle_cli/published_components_cli.py index 0744278..c083920 100644 --- a/packages/tangle-cli/src/tangle_cli/published_components_cli.py +++ b/packages/tangle-cli/src/tangle_cli/published_components_cli.py @@ -7,6 +7,7 @@ from cyclopts import App, Parameter +from .args_container import strict_bool from .cli_helpers import ( LazyTangleApiClient, api_arg_specs, @@ -79,7 +80,7 @@ def published_components_search( for args in load_args_or_exit( config, name=(name, None), - include_deprecated=(include_deprecated, None), + include_deprecated=(include_deprecated, None, strict_bool), published_by=(published_by, None), digest=(digest, None), log_type=(log_type, "console"), @@ -141,10 +142,10 @@ def published_components_inspect( config, name=(name, None), digest=(digest, None), - all_versions=(all_versions, None), - include_deprecated=(include_deprecated, None), - follow_deprecated=(follow_deprecated, None), - full_spec=(full_spec, None), + all_versions=(all_versions, None, strict_bool), + include_deprecated=(include_deprecated, None, strict_bool), + follow_deprecated=(follow_deprecated, None, strict_bool), + full_spec=(full_spec, None, strict_bool), published_by=(published_by, None), log_type=(log_type, "console"), **api_arg_specs( @@ -278,8 +279,8 @@ def published_components_publish( name=(name, None), description=(description, None), annotations=("annotations", annotations, None, True), - dry_run=(dry_run, None), - allow_downgrade=(allow_downgrade, None), + dry_run=(dry_run, None, strict_bool), + allow_downgrade=(allow_downgrade, None, strict_bool), git_remote_sha=(git_remote_sha, None), git_remote_branch=(git_remote_branch, None), git_remote_url=(git_remote_url, None), diff --git a/pyproject.toml b/pyproject.toml index 1426452..013e106 100644 --- a/pyproject.toml +++ b/pyproject.toml @@ -1,6 +1,6 @@ [project] name = "tangle-cli" -version = "0.1.20" +version = "0.1.21" description = "CLI for Tangle, the open-source ML pipeline orchestration platform" readme = "README.md" authors = [ diff --git a/tests/test_args_container_typed.py b/tests/test_args_container_typed.py new file mode 100644 index 0000000..d577f49 --- /dev/null +++ b/tests/test_args_container_typed.py @@ -0,0 +1,374 @@ +"""Strict typing of scalar ArgsContainer fields fed by config or environment strings.""" + +from __future__ import annotations + +import builtins +import traceback +from enum import Enum +from types import SimpleNamespace +from typing import Any + +import pytest + +from tangle_cli import cli, secrets_cli +from tangle_cli.args_container import ( + ArgsContainer, + ConfigFileError, + EnvField, + strict_bool, + strict_float, + strict_int, +) +from tangle_cli.python_pipeline.cfg import load_cfg + +SECRET = "FAKE-SECRET-do-not-echo-7f3a9c2e1b" +ENV_VARS = ("DELETE_FORCE", "FLAG", "COUNT", "RATIO", "NAME_VAR", "OTHER") + + +@pytest.fixture(autouse=True) +def _clean_env(monkeypatch): + for name in ENV_VARS: + monkeypatch.delenv(name, raising=False) + + +def _write(tmp_path, text: str, name: str = "config.yaml"): + path = tmp_path / name + path.write_text(text, encoding="utf-8") + return path + + +def _rendered(excinfo) -> str: + return "".join(traceback.format_exception(excinfo.value)) + + +# --- the Binks scenario: `tangle sdk secrets delete` with force from _env ------------------ + + +class _FakeClient: + instances: list[_FakeClient] = [] + + def __init__(self, **kwargs: Any) -> None: + self.calls: list[str] = [] + _FakeClient.instances.append(self) + + def secrets_delete(self, secret_name: str) -> SimpleNamespace: + self.calls.append(secret_name) + return SimpleNamespace() + + +@pytest.fixture +def delete_app(monkeypatch, tmp_path): + _FakeClient.instances = [] + monkeypatch.setattr(secrets_cli, "LazyTangleApiClient", _FakeClient) + prompts: list[str] = [] + + def fake_input() -> str: + prompts.append("asked") + return "n" + + monkeypatch.setattr(builtins, "input", fake_input) + config = _write( + tmp_path, + "secret_name: API_TOKEN\nlog_type: none\nforce: {_env: DELETE_FORCE, default: false}\n", + ) + + def run() -> tuple[list[str], list[str], str]: + exit_message = "" + try: + cli.build_app()(["sdk", "secrets", "delete", "--config", str(config)]) + except SystemExit as exc: + exit_message = str(exc.code) if exc.code not in (0, None) else "" + deleted = [name for client in _FakeClient.instances for name in client.calls] + return prompts, deleted, exit_message + + return run + + +@pytest.mark.parametrize("value", [None, "false", "FALSE", "no", "0"]) +def test_secrets_delete_env_force_false_still_confirms(delete_app, monkeypatch, value) -> None: + if value is not None: + monkeypatch.setenv("DELETE_FORCE", value) + + prompts, deleted, exit_message = delete_app() + + assert prompts == ["asked"] + assert deleted == [] + assert "Delete cancelled" in exit_message + + +@pytest.mark.parametrize("value", ["true", "True", "yes", "1"]) +def test_secrets_delete_env_force_true_skips_confirmation(delete_app, monkeypatch, value) -> None: + monkeypatch.setenv("DELETE_FORCE", value) + + prompts, deleted, exit_message = delete_app() + + assert prompts == [] + assert deleted == ["API_TOKEN"] + assert exit_message == "" + + +@pytest.mark.parametrize("value", ["", "maybe", " true", "false ", "on", SECRET]) +def test_secrets_delete_env_force_invalid_is_rejected_without_echo( + delete_app, monkeypatch, value +) -> None: + monkeypatch.setenv("DELETE_FORCE", value) + + prompts, deleted, exit_message = delete_app() + + assert prompts == [] and deleted == [] + assert exit_message == ( + "Config error: Invalid value for force from environment variable DELETE_FORCE " + "(_env at config key 'force'): expected a boolean: true/false, yes/no, or 1/0 " + "(case-insensitive)" + ) + if value == SECRET: + assert SECRET not in exit_message + + +# --- bool fields ----------------------------------------------------------------------------- + + +@pytest.mark.parametrize( + ("text", "expected"), + [ + ("true", True), ("TRUE", True), ("yes", True), ("Yes", True), ("1", True), + ("false", False), ("False", False), ("no", False), ("NO", False), ("0", False), + ], +) +def test_bool_field_accepts_documented_spellings_from_env(monkeypatch, text, expected) -> None: + monkeypatch.setenv("FLAG", text) + + [args] = ArgsContainer.load(None, flag=EnvField("FLAG", (False, False))) + + assert args.flag is expected + assert args.origin("flag") == "env:FLAG" + + +@pytest.mark.parametrize("text", ["", "maybe", "on", "off", "t", "2", " yes", "tru\u0435", SECRET]) +def test_bool_field_rejects_other_env_strings_without_echo(monkeypatch, text) -> None: + monkeypatch.setenv("FLAG", text) + + with pytest.raises(ConfigFileError) as excinfo: + ArgsContainer.load(None, flag=EnvField("FLAG", (False, False))) + + assert str(excinfo.value).startswith( + "Invalid value for flag from environment variable FLAG: expected a boolean" + ) + if text == SECRET: + assert SECRET not in _rendered(excinfo) + + +def test_bool_yaml_string_literals_are_parsed_strictly(tmp_path) -> None: + config = _write(tmp_path, "a: 'false'\nb: 'yes'\nc: true\nd: 0\n") + + [args] = ArgsContainer.load( + config, a=(False, False), b=(False, False), c=(False, False), d=(True, True) + ) + + assert (args.a, args.b, args.c, args.d) == (False, True, True, False) + + +@pytest.mark.parametrize("literal", ["'maybe'", "''", "[x]", "{k: v}", "2", "1.0"]) +def test_bool_config_literal_of_wrong_type_is_rejected(tmp_path, literal) -> None: + config = _write(tmp_path, f"force: {literal}\n") + + with pytest.raises( + ConfigFileError, match=r"^Invalid value for force from config key 'force': expected a boolean" + ): + ArgsContainer.load(config, force=(False, False)) + + +def test_explicit_strict_bool_types_a_none_default_field(tmp_path, monkeypatch) -> None: + monkeypatch.setenv("FLAG", "false") + config = _write(tmp_path, "dry_run: {_env: FLAG}\nallow: 'no'\n") + + [args] = ArgsContainer.load( + config, + dry_run=(None, None, strict_bool), + allow=("allow", None, None, False, False, strict_bool), + unset=(None, None, strict_bool), + ) + + assert (args.dry_run, args.allow, args.unset) == (False, False, None) + + +def test_explicit_strict_bool_accepts_cli_bools(tmp_path) -> None: + [args] = ArgsContainer.load(None, dry_run=(True, None, strict_bool)) + + assert args.dry_run is True + assert args.origin("dry_run") == "cli" + + +# --- int / float fields ---------------------------------------------------------------------- + + +@pytest.mark.parametrize(("text", "expected"), [("0", 0), ("42", 42), ("-3", -3), ("+7", 7)]) +def test_int_field_parses_env_strings(monkeypatch, text, expected) -> None: + monkeypatch.setenv("COUNT", text) + + [args] = ArgsContainer.load(None, count=EnvField("COUNT", (6, 6))) + + assert args.count == expected and type(args.count) is int + + +@pytest.mark.parametrize("text", ["", "1.0", "1e3", "0x1f", "1_000", " 5", "５", "true", SECRET]) +def test_int_field_rejects_non_integers_without_echo(tmp_path, monkeypatch, text) -> None: + monkeypatch.setenv("COUNT", text) + config = _write(tmp_path, "count: {_env: COUNT}\n") + + with pytest.raises(ConfigFileError) as excinfo: + ArgsContainer.load(config, count=(6, 6)) + + assert str(excinfo.value) == ( + "Invalid value for count from environment variable COUNT (_env at config key 'count'): " + "expected an integer" + ) + if text == SECRET: + assert SECRET not in _rendered(excinfo) + + +@pytest.mark.parametrize("literal", ["true", "1.5", "[1]"]) +def test_int_field_rejects_wrong_yaml_types(tmp_path, literal) -> None: + config = _write(tmp_path, f"count: {literal}\n") + + with pytest.raises(ConfigFileError, match="from config key 'count': expected an integer"): + ArgsContainer.load(config, count=(6, 6)) + + +@pytest.mark.parametrize( + ("text", "expected"), [("1.5", 1.5), ("10", 10.0), ("-.5", -0.5), ("2e3", 2000.0), ("3.", 3.0)] +) +def test_float_field_parses_env_strings(monkeypatch, text, expected) -> None: + monkeypatch.setenv("RATIO", text) + + [args] = ArgsContainer.load(None, ratio=EnvField("RATIO", (600.0, 600.0))) + + assert args.ratio == expected and type(args.ratio) is float + + +@pytest.mark.parametrize("text", ["", "nan", "inf", "1,5", "abc", "true"]) +def test_float_field_rejects_non_numbers(monkeypatch, text) -> None: + monkeypatch.setenv("RATIO", text) + + with pytest.raises(ConfigFileError, match="from environment variable RATIO: expected a number"): + ArgsContainer.load(None, ratio=EnvField("RATIO", (600.0, 600.0))) + + +def test_numeric_yaml_values_pass_through(tmp_path) -> None: + config = _write(tmp_path, "count: 3\nratio: 2\nlimit: '10'\n") + + [args] = ArgsContainer.load( + config, count=(6, 6), ratio=(1.0, 1.0), limit=(None, None, strict_int) + ) + + assert (args.count, args.ratio, args.limit) == (3, 2, 10) + + +def test_strict_converters_directly() -> None: + assert strict_bool("Yes") is True and strict_bool(0) is False + assert strict_int("-12") == -12 and strict_float("1.25") == 1.25 + for converter, value in ((strict_bool, 2), (strict_int, True), (strict_float, False)): + with pytest.raises(ConfigFileError): + converter(value) + + +# --- untouched: strings, JSON, repeatables, enums, nested, cfg ------------------------------ + + +class _Mode(Enum): + FAST = "fast" + SLOW = "slow" + + +def test_string_json_repeatable_and_enum_fields_are_not_coerced(tmp_path, monkeypatch) -> None: + monkeypatch.setenv("FLAG", "false") + monkeypatch.setenv("COUNT", "10") + config = _write( + tmp_path, + "name: {_env: FLAG}\n" + "none_default: {_env: COUNT}\n" + "payload: {_env: COUNT}\n" + "header: [{_env: FLAG}]\n" + "mode: slow\n" + "nested: {flag: {_env: FLAG}, n: {_env: COUNT}}\n", + ) + + [args] = ArgsContainer.load( + config, + name=("default-name", "default-name"), + none_default=(None, None), + payload=("payload", None, None, True), + header=(None, None), + mode=(_Mode.FAST, _Mode.FAST), + nested=(None, None), + ) + + assert args.name == "false" + assert args.none_default == "10" + assert args.payload == 10 + assert args.header == ["false"] + assert args.mode is _Mode.SLOW + # Typing applies to scalar fields only; nested _env values stay strings. + assert args.nested == {"flag": "false", "n": "10"} + + +def test_raw_config_keeps_strings(tmp_path, monkeypatch) -> None: + monkeypatch.setenv("FLAG", "false") + config = _write(tmp_path, "force: {_env: FLAG}\n") + + [args] = ArgsContainer.load(config, force=(False, False)) + + assert args.force is False + assert args._config == {"force": "false"} + assert ArgsContainer._load_config_file(config) == [{"force": "false"}] + + +def test_cfg_load_path_keeps_its_own_typing(tmp_path, monkeypatch) -> None: + monkeypatch.setenv("FLAG", "false") + path = _write(tmp_path, "force: {_env: FLAG}\nnative: false\n") + + cfg = load_cfg(path, {"cli_flag": "false"}) + + assert cfg.force == "false" # _env stays a string in cfg + assert cfg.native is False + assert cfg.cli_flag is False # --override strings keep YAML coercion + + +# --- precedence unchanged -------------------------------------------------------------------- + + +@pytest.mark.parametrize( + ("cli", "config", "env", "expected", "origin"), + [ + (True, "false", "false", True, "cli"), + (False, "true", "false", True, "config"), + (False, None, "yes", True, "env:FLAG"), + (False, None, None, False, "default"), + (True, None, "no", True, "cli"), + (False, "no", "yes", False, "config"), + ], +) +def test_precedence_matrix_with_typing(tmp_path, monkeypatch, cli, config, env, expected, origin) -> None: + path = _write(tmp_path, f"flag: '{config}'\n" if config is not None else "other: 1\n") + if env is not None: + monkeypatch.setenv("FLAG", env) + + [args] = ArgsContainer.load(path, flag=EnvField("FLAG", (cli, False))) + + assert args.flag is expected + assert args.origin("flag") == origin + + +def test_defaults_env_source_is_named(tmp_path, monkeypatch) -> None: + monkeypatch.setenv("FLAG", "maybe") + config = _write( + tmp_path, + "_defaults:\n force: {_env: FLAG}\nconfigs:\n - {}\n - force: 'nope'\n", + ) + + with pytest.raises(ConfigFileError, match=r"environment variable FLAG \(_env at config key 'force'\)"): + ArgsContainer.load(config, force=(False, False)) + + monkeypatch.setenv("FLAG", "yes") + with pytest.raises(ConfigFileError, match=r"from config key 'force': expected a boolean"): + ArgsContainer.load(config, force=(False, False)) diff --git a/tests/test_packaging.py b/tests/test_packaging.py index 9a5bcee..4c91c75 100644 --- a/tests/test_packaging.py +++ b/tests/test_packaging.py @@ -183,7 +183,7 @@ def test_tangle_cli_wheel_supports_expert_no_deps_import_path_without_tangle_api requires_dist = [line for line in metadata.splitlines() if line.startswith("Requires-Dist: ")] assert not any(name.startswith("tangle_api/") for name in names) assert "tangle_cli/openapi/openapi.json" not in names - assert "Version: 0.1.20" in metadata + assert "Version: 0.1.21" in metadata assert "Requires-Dist: tangle-api==0.1.1" in requires_dist assert not any("extra == 'native'" in line for line in requires_dist) assert "Provides-Extra: native" in metadata diff --git a/tests/test_pipeline_runs_cli.py b/tests/test_pipeline_runs_cli.py index ce86fb5..557c8ad 100644 --- a/tests/test_pipeline_runs_cli.py +++ b/tests/test_pipeline_runs_cli.py @@ -4691,10 +4691,20 @@ def test_submit_from_python_rejects_a_misspelled_config_key_in_a_later_entry( [ # A non-int budget crashes an ambiguous submit before its recovery # lookup, so a possibly-created run is never reconciled. - ({"submit_recovery_attempts": "oops"}, "submit_recovery_attempts must be"), + ( + {"submit_recovery_attempts": "oops"}, + "Invalid value for submit_recovery_attempts from config key " + "'submit_recovery_attempts': expected an integer", + ), ({"submit_recovery_attempts": -1}, "submit_recovery_attempts must be"), - # bool("false") is True, which would silently allow all hydration. - ({"trusted_hydration_cli": "false"}, "trusted_hydration_cli must be a boolean"), + # bool("maybe") is True, which would silently allow all hydration; the + # field is strictly typed, so only an explicit boolean spelling passes. + ( + {"trusted_hydration_cli": "maybe"}, + "Invalid value for trusted_hydration_cli from config key " + "'trusted_hydration_cli': expected a boolean", + ), + ({"trusted_hydration_cli": ["yes"]}, "'trusted_hydration_cli': expected a boolean"), ], ) def test_submit_from_python_rejects_unsafe_config_types( diff --git a/uv.lock b/uv.lock index 2e7a4cd..55cefa8 100644 --- a/uv.lock +++ b/uv.lock @@ -2083,7 +2083,7 @@ requires-dist = [{ name = "pydantic", specifier = ">=2.0" }] [[package]] name = "tangle-cli" -version = "0.1.20" +version = "0.1.21" source = { editable = "." } dependencies = [ { name = "cloud-pipelines" },