fix(config): prevent YAML structure injection via env var substitution#5091
Conversation
…itution Environment variable values containing newlines were substituted verbatim into YAML text before parsing, allowing a value like "legit-service\nmalicious_key: injected" to create extra YAML keys. The spec explicitly requires: "It MUST NOT be possible to inject YAML structures by environment variables." Fix wraps values containing \n or \r in YAML double-quoted scalars. Simple values are returned as-is so that YAML type coercion still applies per "Node types MUST be interpreted after environment variable substitution takes place." Assisted-by: Claude Sonnet 4.6
Assisted-by: Claude Sonnet 4.6
Assisted-by: Claude Sonnet 4.6
|
Check links is failing because of a bad changelog entry PR path, will be fixed in: |
github-project-automation
Bot
moved this from Ready for review
to Approved PRs
in Python PR digest
|
LGTM. By the way, it might be worth looking at: https://github.com/open-telemetry/opentelemetry-collector/tree/main/confmap I'm not sure if declarative config is thinking of adding custom resolvers, but currently the collector does inside-out resolution to allow for things like: |
Good idea, I've created an issue to review vs configmap - thanks @herin049 👍🏻 |
4 participants
Description
Fixes a YAML structure injection vulnerability in declarative file configuration's environment variable substitution (
_env_substitution.py).The
substitute_env_vars()function performed raw string substitution of env var values into YAML text before parsing. A value containing a newline followed by valid YAML (e.g."legit-service\nmalicious_key: injected_value") would inject extra keys at parse time.Example:
Config template:
After substitution, before
yaml.safe_load:The spec explicitly requires: "It MUST NOT be possible to inject YAML structures by environment variables."
Fix
Values containing
\nor\rare now emitted as YAML double-quoted scalars with escaped newlines:Simple values (no newlines) are returned as-is. This is intentional — the spec also states "Node types MUST be interpreted after environment variable substitution takes place", meaning type coercion must be preserved.
Why not
yaml.dump(value)?yaml.dump()was considered but has two problems:Breaks type coercion —
yaml.dump("true")produces'true'(single-quoted string), so env var values liketrue,false,42,3.14,nullwould all parse as strings instead of their native YAML types, violating the spec's type coercion requirement.Produces multi-line output for newline-containing values —
yaml.dump("a\nb")produces a multi-line block scalar that may be structurally invalid when substituted inline into a template. The custom escaping always produces a single-line"a\nb"double-quoted scalar.yaml.dumpresult"true"str 'true'✗bool True✓"42"str '42'✗int 42✓"null"str 'null'✗NoneType None✓"my-service"str 'my-service'✓str 'my-service'✓"a\nb""a\nb"single-line ✓Type of change
How Has This Been Tested?
Added tests in
test_env_substitution.py:test_newline_in_value_prevents_yaml_injection— verifies the exact PoC is blockedtest_newline_in_value_preserved_as_literal— verifies the newline is preserved as a literal character in the parsed valuetest_carriage_return_in_value_is_escaped— covers\rcasetest_type_coercion_preserved_for_simple_values— verifiestrue→bool,42→intstill workDoes This PR Require a Contrib Repo Change?
Checklist: