Summary
renderConfigTemplate() (cmd/action/main.go) builds the .Env map verbatim from os.Environ() and renders the stack/services/volumes input as a plain text/template. The template engine has no awareness of YAML structure, so a referenced variable whose value contains \n or \r is spliced in byte-for-byte — the remainder of the value is then parsed as new YAML at top level.
Follow-up to the analysis in #151 / PR #152 (template rendering vs. YAML comments); this is the more severe sibling of that bug.
Confirmed behavior
With X = "nginx\nvolumes:\n evil:\n name: pwned" and
services:
app:
image: {{ .Env.X }}
the render succeeds without error and the parsed result contains a fabricated top-level key:
services:
app:
image: nginx
volumes:
evil:
name: pwned
Since the action sends the parsed body to UpdateStack, which replaces the full stack state, the corrupted definition is applied verbatim.
Further confirmed variants:
- Quoting does not protect:
image: "{{ .Env.Z }}" with a value containing " + newline breaks out of the quotes and injects top-level keys, error-free.
\r\n and even a lone \r are treated as line breaks by yaml.v3 — same injection. A trailing \r is silently swallowed.
- Legitimate multi-line secrets are mangled: an unquoted PEM key fails with a misleading
yaml: line 6: could not find expected ':'; a quoted PEM key is silently line-folded into a single space-joined string — a broken private key gets deployed with no warning at all.
Unreferenced multi-line env vars (common on GitHub runners) render fine today and must keep doing so after any fix.
Impact
- Correctness / data loss: multi-line secrets (PEM/SSH keys, JSON blobs, certificates) are silently mangled or fail with misleading errors.
- Silent stack corruption: injected top-level
services:/volumes: keys change what UpdateStack declares.
- Injection vector (secondary, requires an unsafe workflow): a workflow exporting untrusted data (PR titles, branch names) into env and referencing it from the stack template lets that data add or alter services/volumes.
Proposed fix
Probe-render with sentinel substitution: render once with every line-break-carrying env value replaced by a unique nonce token; if a token appears in the output, fail with an error naming the variable ("environment variable X contains a line break and cannot be used in the configuration template"); otherwise re-render with the real values.
This rejects exactly the values that actually reach the output — complete by construction (also covers {{ index .Env "X" }} and {{ range .Env }}), zero false positives for unreferenced vars or untaken {{ if }} branches, and {{ .Env.NAME }} syntax stays unchanged.
Rejected alternatives:
- Skip newline vars from the
.Env map — missingkey=error then produces a misleading "map has no entry" message, and {{ index .Env "X" }} / {{ range }} / {{ or ... }} silently render an empty string instead — a new silent-corruption path.
- Error-returning wrapper types / FuncMap —
String() cannot return an error, a panic inside String() is swallowed by fmt (renders a %!v(PANIC=…) marker error-free), and wrapper types break len/eq on .Env values. Breaking the documented {{ .Env.FOO }} syntax is a non-starter.
- Post-render line-count check — cannot name the offending variable and breaks legitimate multi-line template constructs.
- Auto YAML-escaping — impossible; the engine does not know whether the action sits in a plain scalar, quoted scalar, comment, or key position, and each demands different escaping.
The behavior change (multi-line secrets now fail hard instead of "working" badly) is intended and should be called out in the release notes; the README should state that {{ .Env.NAME }} values must be single-line.
Summary
renderConfigTemplate()(cmd/action/main.go) builds the.Envmap verbatim fromos.Environ()and renders the stack/services/volumes input as a plaintext/template. The template engine has no awareness of YAML structure, so a referenced variable whose value contains\nor\ris spliced in byte-for-byte — the remainder of the value is then parsed as new YAML at top level.Follow-up to the analysis in #151 / PR #152 (template rendering vs. YAML comments); this is the more severe sibling of that bug.
Confirmed behavior
With
X = "nginx\nvolumes:\n evil:\n name: pwned"andthe render succeeds without error and the parsed result contains a fabricated top-level key:
Since the action sends the parsed body to
UpdateStack, which replaces the full stack state, the corrupted definition is applied verbatim.Further confirmed variants:
image: "{{ .Env.Z }}"with a value containing"+ newline breaks out of the quotes and injects top-level keys, error-free.\r\nand even a lone\rare treated as line breaks by yaml.v3 — same injection. A trailing\ris silently swallowed.yaml: line 6: could not find expected ':'; a quoted PEM key is silently line-folded into a single space-joined string — a broken private key gets deployed with no warning at all.Unreferenced multi-line env vars (common on GitHub runners) render fine today and must keep doing so after any fix.
Impact
services:/volumes:keys change whatUpdateStackdeclares.Proposed fix
Probe-render with sentinel substitution: render once with every line-break-carrying env value replaced by a unique nonce token; if a token appears in the output, fail with an error naming the variable ("environment variable X contains a line break and cannot be used in the configuration template"); otherwise re-render with the real values.
This rejects exactly the values that actually reach the output — complete by construction (also covers
{{ index .Env "X" }}and{{ range .Env }}), zero false positives for unreferenced vars or untaken{{ if }}branches, and{{ .Env.NAME }}syntax stays unchanged.Rejected alternatives:
.Envmap —missingkey=errorthen produces a misleading "map has no entry" message, and{{ index .Env "X" }}/{{ range }}/{{ or ... }}silently render an empty string instead — a new silent-corruption path.String()cannot return an error, a panic insideString()is swallowed byfmt(renders a%!v(PANIC=…)marker error-free), and wrapper types breaklen/eqon.Envvalues. Breaking the documented{{ .Env.FOO }}syntax is a non-starter.The behavior change (multi-line secrets now fail hard instead of "working" badly) is intended and should be called out in the release notes; the README should state that
{{ .Env.NAME }}values must be single-line.