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
    No fields configured for issues without a type.

    Projects

    Status

    Backlog

    Milestone

    No milestone

    Relationships

    None yet

    Development

    No branches or pull requests

    Issue actions