Skip to content

refactor(core): switch env var interpolation to pre-parse rendering #1204

@christso

Description

@christso

Background

The current interpolateEnv() implementation does post-parse coercion: it replaces ${{ VAR }} references after YAML is parsed, then coerces the resulting string to its native type (boolean, number) based on the resolved value.

This is field-agnostic — the function has no schema knowledge, so coercion applies to every whole-value substitution regardless of the expected type of that field.

Problem

Post-parse coercion is fragile by nature:

  • A string field like branch_prefix: ${{ PREFIX }} will be coerced to a number if PREFIX=42, even though branch_prefix is typed as string.
  • Missing variables resolve to empty string "" rather than null, which can differ semantically from leaving the field unset.
  • Requires PLAIN_NUMBER_PATTERN to exclude edge cases like "1e3", "0x10", "Infinity" — this is treating symptoms, not the root cause.

Better Approach: Pre-parse rendering (Helm/Ansible pattern)

Replace ${{ VAR }} references in the raw YAML string before parsing, so the YAML parser itself handles type inference:

YAML text → replace ${{ VAR }} → "auto_push: true" → parseYamlValue() → boolean true

This is how Helm, Ansible/Jinja2, and Docker Compose work. Types are correct by construction because YAML's own parser understands true as boolean, 42 as integer, etc. No coercion logic required.

Trade-offs

  • Injection risk: same as any shell-based tool (env vars are from the user's own environment — already trusted).
  • Missing vars: unset ${{ AUTOPUSH }} renders to auto_push: → YAML parses as null (falsy), which is more useful than empty string.
  • Migration: the public API of interpolateEnv() changes — it would accept string (raw YAML text) and return unknown (parsed result), or it becomes two functions: renderTemplate(raw: string, env): string + parse. The post-parse recursive object walk becomes unnecessary for env var expansion (though it may still be needed for other reasons like eval case interpolation inside already-parsed objects).

Acceptance Criteria

  • ${{ VAR }} references are expanded in the raw YAML string before parseYamlValue() is called
  • Resolving to a boolean (AUTOPUSH=true) yields a YAML boolean, not a coerced string
  • Resolving to a number (WORKERS=4) yields a YAML integer
  • Unset variables render to empty string (preserving current behavior) OR render to nothing (making the YAML field null) — decide and document
  • Existing behavior for inline/partial substitutions ("prefix-${{ VAR }}") is preserved
  • All existing interpolation tests pass; coercion-specific tests are removed or updated
  • PLAIN_NUMBER_PATTERN and coercePrimitive() are deleted (no longer needed)

Related

Metadata

Metadata

Assignees

No one assigned

    Labels

    No labels
    No labels

    Type

    No type

    Projects

    Status

    Backlog

    Milestone

    No milestone

    Relationships

    None yet

    Development

    No branches or pull requests

    Issue actions