fix(config): prevent YAML structure injection via env var substitution#5091
Open
MikeGoldsmith wants to merge 4 commits intoopen-telemetry:mainfrom
Open
fix(config): prevent YAML structure injection via env var substitution#5091MikeGoldsmith wants to merge 4 commits intoopen-telemetry:mainfrom
MikeGoldsmith wants to merge 4 commits intoopen-telemetry:mainfrom
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
Member
Author
|
Check links is failing because of a bad changelog entry PR path, will be fixed in: |
xrmx
approved these changes
Apr 14, 2026
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
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: