Skip to content

Type scalar ArgsContainer fields strictly so env/config strings cannot bypass flags (v0.1.21) - #64

Merged
Volv-G merged 1 commit into
masterfrom
piforge/tangle-pipeline-crud/tangle-cli-0-1-21-typed-coercion-d1c4f42
Sep 25, 2026
Merged

Volv-G merged 1 commit into
masterfrom
piforge/tangle-pipeline-crud/tangle-cli-0-1-21-typed-coercion-d1c4f42

Conversation

@Volv-G

@Volv-G Volv-G commented Sep 25, 2026

Copy link
Copy Markdown
Collaborator

(AI-assisted)

What

This is a security fix for a safety-gate bypass that 0.1.20's _env introduced (found by Binks on Shopify/discovery#35126).

_env values are always strings, and so are their stringified defaults. A boolean ArgsContainer field without a converter therefore received the truthy string "false".

Example: tangle sdk secrets delete loads force=(force, False) and prompts unless args.force is truthy. So a config with force: {_env: DELETE_FORCE, default: false} deleted without confirmation. skip_validation, no_wait, dry_run, trusted_hydration_cli and other flags have the same shape.

A quoted YAML literal (force: "false") had the same latent bug long before _env existed.

How

Scalar fields are typed strictly at the single field-resolution point, ArgsContainer._resolve.

  • Type source
    • An explicit converter in the spec wins. New public strict converters are strict_bool, strict_int and strict_float, in tangle_cli.args_container.
    • Otherwise the type is inferred from the default's type (bool / int / float).
    • An inferred type parses config and env values only; CLI values are already typed by the parser.
  • bool accepts a bool, 0/1, or exactly true/false, yes/no, 1/0. Matching is case-insensitive, untrimmed, and ASCII-only. Everything else is rejected: "", on/off, true, 2, lists, maps.
  • int accepts an int, or a string of ASCII digits with an optional sign. 1.0, 1e3, 0x1f, 1_000, full-width digits and bools are rejected.
  • float accepts a number, or a decimal string with an optional exponent. nan and inf are rejected.
  • Fail closed, no echo. A rejection raises ConfigFileError naming the field and its source, never the value, e.g. Invalid value for force from environment variable DELETE_FORCE (_env at config key 'force'): expected a boolean: …. Getting this wording required a small provenance side channel: _load_config_entries returns (config, env_sources), and _load_config_file is an unchanged wrapper around it.
  • Unchanged
    • String, JSON, repeatable and enum fields.
    • _env values nested in maps or lists (they stay strings; typing applies to scalar fields only).
    • The raw args._config.
    • The pipeline cfg path, which never goes through _resolve and keeps its own typing rules. A test confirms this.
  • Precedence CLI > config > env > default is unchanged, with a matrix test.

YAML string literals: parsed strictly too

The same parser applies to a plain config string (force: "false" → False), not only to _env / EnvField values. Reasons:

  1. The truthy-string bug is identical for literals, and upstream already treated it as a safety issue: submit-from-python rejected a string trusted_hydration_cli because "a truthy string silently enables allow-all hydration".
  2. Strict parsing closes every truthy-string misread while keeping configs that already worked, such as "true" flags and numeric strings (e.g. tangle api … limit: "10"), which used to pass through as strings.
  3. It gives one rule for all sources.

The submit-from-python test is updated accordingly: "false" is now a valid False, and "maybe" or ["yes"] are rejected by the typed field.

Built-in fields wired

bool | None / int | None options with a None default can't be inferred, so their specs now name the converter:

  • pipeline_runs_cli: dry_run, trusted_hydration_cli, include_*, stream, local_time, include_pipeline_names, include_execution_stats, dehydrate, limit (strict_int)
  • published_components_cli: dry_run, allow_downgrade, include_deprecated, all_versions, follow_deprecated, full_spec
  • components_cli: strip_code, use_legacy_naming, update_timestamp

Fields with typed defaults (force, hydrate, exit_on_first_failure, allow_failure, max_wait, poll_interval, submit_recovery_attempts, and generic tangle api parameters with bool/int defaults) are covered by inference.

Downstream audit (Discovery oasis/tangle-deploy/src, origin/main)

  • Covered by inference. The safety gates and toggles there use typed defaults, so they are covered on bump. Examples: force in secrets_manager, quota_group_manager, subscription_manager and airflow_converter; skip_validation / no_wait / graceful_shutdown / hydrate in runner and pipeline_run_from_python; dry_run in publisher, quota_group_manager and subscription_manager; no_update_tangle_auth_secret, skip_layout / force_layout, enable / disable / unpin, retry, the mutex_* fields, concurrency_limit, port / timeout.
  • Need strict_int / strict_float in their specs downstream (they have None defaults):
    • capacity, page_size (quota_group_manager)
    • expire_seconds, page_size (subscription_manager)
    • semantic_k (component_search_v2)
    • mutex_overlap_start_percent / mutex_previous_run_min_progress_percent_to_wait (pipeline_scheduler, runner)
  • The vendored submodule is upstream tangle-cli itself (pinned at 0.1.19 on main) and is covered by this change.

Failure modes

  • Breaking (intentional): a config that relied on a garbage string being truthy (e.g. force: "on", force: "y"), or a non-integer string in an int field, now fails with a clear error instead of silently misbehaving.
  • strict_bool and ints: it accepts only 0/1 for an int value, so force: 2 is now rejected.

Review focus

  • The inference guard: inferred types parse config/env only, while explicit converters apply to every source.
  • That no path echoes a value (tests assert on the rendered traceback).
  • The _defaults provenance merge.

Tophatting

uv run --frozen pytest tests/test_args_container_typed.py

82 tests:

  • The Binks scenario end to end through tangle sdk secrets delete --config: DELETE_FORCE unset / false / FALSE / no / 0 still prompts, true / yes / 1 skips, and invalid values are rejected without a prompt, a delete, or an echo.
  • bool / int / float accept and reject sets, from env, _env and YAML literals.
  • Explicit converters on None-default fields.
  • Untouched string / JSON / repeatable / enum / nested fields.
  • Raw config, the cfg path, the precedence matrix, and _defaults provenance.
  • Mutation-checked: 14 deliberate breaks, all caught. They cover dropping inference; truthy, empty, case-sensitive and on/off bool parsing; loose int/float parsing; lost provenance; value echo; and un-wiring trusted_hydration_cli.

Checklist

  • Full suite green locally on Python 3.12 and 3.13: 1784 passed.
  • pyright: no new errors (the 4 in published_components_cli.py exist on master). ruff clean on the changed lines. git diff --check clean.
  • uv build for both packages. The wheel is 0.1.21 and test_packaging passes.
  • Version bumped to 0.1.21 in pyproject.toml, __init__.py, tests/test_packaging.py, and the uv.lock editable self-entry (a two-line lock diff; uv lock not run).
  • README "Scalar field typing" section, cross-linked from _env and EnvField.
  • ArgsContainer.origin() remains the provisional, unused surface flagged in Read config values from the environment: _env, EnvField, and _select/_env in pipeline cfg (v0.1.20) #63.

@Volv-G
Volv-G requested a review from Ark-kun as a code owner September 25, 2026 04:35
@Volv-G
Volv-G merged commit cf53254 into master Sep 25, 2026
6 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant