Skip to content

Commit f8465c6

Browse files
MikeGoldsmithxrmx
andauthored
fix(config): prevent YAML structure injection via env var substitution (#5091)
* fix YAML structure injection in declarative file config env var substitution 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 * update CHANGELOG with PR number #5091 Assisted-by: Claude Sonnet 4.6 * fix ruff: move yaml import to top level Assisted-by: Claude Sonnet 4.6 --------- Co-authored-by: Riccardo Magliocchetti <riccardo.magliocchetti@gmail.com>
1 parent 5fa8b98 commit f8465c6

File tree

3 files changed

+63
-0
lines changed

3 files changed

+63
-0
lines changed

CHANGELOG.md

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -12,6 +12,8 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0
1212

1313
## Unreleased
1414

15+
- `opentelemetry-sdk`: fix YAML structure injection via environment variable substitution in declarative file configuration; values containing newlines are now emitted as quoted YAML scalars per spec requirement
16+
([#5091](https://github.com/open-telemetry/opentelemetry-python/pull/5091))
1517
- `opentelemetry-sdk`: Add `create_logger_provider`/`configure_logger_provider` to declarative file configuration, enabling LoggerProvider instantiation from config files without reading env vars
1618
([#4990](https://github.com/open-telemetry/opentelemetry-python/pull/4990))
1719
- `opentelemetry-sdk`: Add `service` resource detector support to declarative file configuration via `detection_development.detectors[].service`

opentelemetry-sdk/src/opentelemetry/sdk/_configuration/file/_env_substitution.py

Lines changed: 17 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -81,6 +81,23 @@ def replace_var(match) -> str:
8181
f"Environment variable '{var_name}' not found and no default provided"
8282
)
8383

84+
# Per spec: "It MUST NOT be possible to inject YAML structures by
85+
# environment variables." Newlines are the primary injection vector —
86+
# a value like "legit\nmalicious_key: val" would create extra YAML
87+
# keys if substituted verbatim. Wrap such values in a YAML
88+
# double-quoted scalar so the newline is treated as literal text.
89+
# Simple values (no newlines) are returned as-is so that YAML type
90+
# coercion still applies per spec ("Node types MUST be interpreted
91+
# after environment variable substitution takes place").
92+
if "\n" in value or "\r" in value:
93+
escaped = (
94+
value.replace("\\", "\\\\")
95+
.replace('"', '\\"')
96+
.replace("\n", "\\n")
97+
.replace("\r", "\\r")
98+
.replace("\t", "\\t")
99+
)
100+
return f'"{escaped}"'
84101
return value
85102

86103
return re.sub(pattern, replace_var, text)

opentelemetry-sdk/tests/_configuration/file/test_env_substitution.py

Lines changed: 44 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -16,6 +16,8 @@
1616
import unittest
1717
from unittest.mock import patch
1818

19+
import yaml
20+
1921
from opentelemetry.sdk._configuration.file import (
2022
EnvSubstitutionError,
2123
substitute_env_vars,
@@ -115,3 +117,45 @@ def test_only_dollar_signs(self):
115117
"""Test string with only escaped dollar signs."""
116118
result = substitute_env_vars("$$$$")
117119
self.assertEqual(result, "$$")
120+
121+
def test_newline_in_value_prevents_yaml_injection(self):
122+
"""Values containing newlines must not inject YAML structure.
123+
124+
Per spec: "It MUST NOT be possible to inject YAML structures by
125+
environment variables." A value like "legit\\nmalicious_key: val"
126+
must be emitted as a quoted scalar, not raw YAML.
127+
"""
128+
with patch.dict(
129+
os.environ,
130+
{"SERVICE_NAME": "legit-service\nmalicious_key: injected_value"},
131+
):
132+
result = substitute_env_vars(
133+
"file_format: '0.1'\nservice_name: ${SERVICE_NAME}"
134+
)
135+
parsed = yaml.safe_load(result)
136+
self.assertNotIn("malicious_key", parsed)
137+
self.assertIn("legit-service", parsed["service_name"])
138+
139+
def test_newline_in_value_preserved_as_literal(self):
140+
"""Newline within a value is preserved as a literal newline character."""
141+
with patch.dict(os.environ, {"MULTI": "line1\nline2"}):
142+
result = substitute_env_vars("key: ${MULTI}")
143+
parsed = yaml.safe_load(result)
144+
self.assertEqual(parsed["key"], "line1\nline2")
145+
146+
def test_carriage_return_in_value_is_escaped(self):
147+
"""Carriage return in value is escaped, not injected."""
148+
with patch.dict(os.environ, {"VAL": "text\r\nmore"}):
149+
result = substitute_env_vars("key: ${VAL}")
150+
parsed = yaml.safe_load(result)
151+
self.assertIsInstance(parsed["key"], str)
152+
153+
def test_type_coercion_preserved_for_simple_values(self):
154+
"""Simple values without newlines still undergo YAML type coercion per spec."""
155+
with patch.dict(os.environ, {"BOOL_VAL": "true", "INT_VAL": "42"}):
156+
bool_result = yaml.safe_load(
157+
substitute_env_vars("key: ${BOOL_VAL}")
158+
)
159+
int_result = yaml.safe_load(substitute_env_vars("key: ${INT_VAL}"))
160+
self.assertIs(bool_result["key"], True)
161+
self.assertEqual(int_result["key"], 42)

0 commit comments

Comments
 (0)