feat(config)!: parse configuration before interpolation

changelog.d/parse_first_interpolation.breaking.md

@@ -0,0 +1,52 @@
+# Configuration is parsed before interpolation
+
+## Summary
+
+Vector now parses configuration files before substituting environment variables and
+`SECRET[backend.key]` references. Substitution applies only to string values, followed
+by conversion to the field's declared type. Quotes, newlines, and other configuration
+syntax in substituted values cannot add keys, components, or array elements.
+
+This removes structural interpolation, deprecated in 0.57.0. Environment-variable
+interpolation remains disabled by default and still requires
+`--dangerously-allow-env-var-interpolation` or
+`VECTOR_DANGEROUSLY_ALLOW_ENV_VAR_INTERPOLATION=true`.
+
+## Migration
+
+Configuration must be valid YAML, TOML, or JSON before substitution. Quote placeholders
+in TOML and JSON values, including numeric and boolean fields. For example, change
+`count = ${MY_COUNT}` to `count = "${MY_COUNT}"`. The same applies to secret references.
+
+YAML string placeholders remain supported, including in numeric fields:
+
+```yaml
+sources:
+  demo:
+    type: demo_logs
+    format: json
+    count: "${MY_COUNT}"
+```
+
+Replace interpolated map keys, component names, and table headers with literal names.
+Placeholders in keys and comments are no longer substituted or collected as secrets.
+Values cannot expand into configuration fragments or multiple array elements. For
+example, replace a comma-separated `${VECTOR_INPUTS}` expansion with an explicit list:
+
+```yaml
+inputs:
+  - "${FIRST_INPUT}"
+  - "${SECOND_INPUT}"
+```
+
+Environment-variable and secret values are now inserted as literal strings. Remove
+escaping that was needed only for the outer configuration format. Multiline environment
+values are supported within a string field, but cannot inject configuration blocks.
+Escaping required by an embedded language, such as VRL, is still your responsibility.
+
+For configurations that rely on structural environment-variable expansion, preprocess
+the file with a tool such as `envsubst`, inspect the resulting configuration, and pass
+that static file to Vector. `envsubst` does not resolve `SECRET[...]` references or
+Vector's extended default/required-variable syntax.
+
+authors: pront

src/config/loading/config_builder.rs

@@ -1,15 +1,10 @@
 use std::{collections::HashMap, io::Read};
 
-use indexmap::IndexMap;
-
 use super::{
-    ComponentHint, Process, deserialize_config_map, loader, prepare_input,
-    representation::ConfigMap, secret,
-};
-use crate::config::{
-    ComponentKey, ConfigBuilder, EnrichmentTableOuter, SinkOuter, SourceOuter, TestDefinition,
-    TransformOuter,
+    ComponentHint, Process, deserialize_component_map, deserialize_config_map,
+    interpolate_config_map_with_secrets, loader, representation::ConfigMap,
 };
+use crate::config::ConfigBuilder;
 
 #[derive(Debug)]
 pub struct ConfigBuilderLoader {
@@ -66,49 +61,55 @@ impl Default for ConfigBuilderLoader {
 }
 
 impl Process for ConfigBuilderLoader {
-    /// Prepares input for a `ConfigBuilder` by interpolating environment variables.
-    fn prepare<R: Read>(&mut self, input: R) -> Result<String, Vec<String>> {
-        let prepared_input = prepare_input(input, self.interpolate_env)?;
-        Ok(if self.secrets.is_empty() {
-            prepared_input
+    fn should_interpolate_env(&self) -> bool {
+        self.interpolate_env
+    }
+
+    fn postprocess(&mut self, map: ConfigMap) -> Result<ConfigMap, Vec<String>> {
+        if self.secrets.is_empty() {
+            Ok(map)
         } else {
-            secret::interpolate(&prepared_input, &self.secrets)?
-        })
+            interpolate_config_map_with_secrets(&map, &self.secrets)
+        }
+    }
+
+    fn merge_root(&mut self, files: ConfigMap) -> Result<(), Vec<String>> {
+        self.merge(loader::merge_root_config(files)?, None)
     }
 
     /// Merge a configuration map with a `ConfigBuilder`. Component types extend specific keys.
     fn merge(&mut self, map: ConfigMap, hint: Option<ComponentHint>) -> Result<(), Vec<String>> {
         match hint {
-            Some(ComponentHint::Source) => {
+            Some(hint @ ComponentHint::Source) => {
                 self.builder
                     .sources
-                    .extend(deserialize_config_map::<IndexMap<ComponentKey, SourceOuter>>(map)?);
+                    .extend(deserialize_component_map(map, hint)?);
             }
-            Some(ComponentHint::Sink) => {
-                self.builder.sinks.extend(deserialize_config_map::<
-                    IndexMap<ComponentKey, SinkOuter<_>>,
-                >(map)?);
+            Some(hint @ ComponentHint::Sink) => {
+                self.builder
+                    .sinks
+                    .extend(deserialize_component_map(map, hint)?);
             }
-            Some(ComponentHint::Transform) => {
-                self.builder.transforms.extend(deserialize_config_map::<
-                    IndexMap<ComponentKey, TransformOuter<_>>,
-                >(map)?);
+            Some(hint @ ComponentHint::Transform) => {
+                self.builder
+                    .transforms
+                    .extend(deserialize_component_map(map, hint)?);
             }
-            Some(ComponentHint::EnrichmentTable) => {
+            Some(hint @ ComponentHint::EnrichmentTable) => {
                 self.builder
                     .enrichment_tables
-                    .extend(deserialize_config_map::<
-                        IndexMap<ComponentKey, EnrichmentTableOuter<_>>,
-                    >(map)?);
+                    .extend(deserialize_component_map(map, hint)?);
             }
-            Some(ComponentHint::Test) => {
-                // This serializes to a `Vec<TestDefinition<_>>`, so we need to first expand
-                // it to an ordered map, and then pull out the value, ignoring the keys.
-                self.builder.tests.extend(
-                    deserialize_config_map::<IndexMap<String, TestDefinition<String>>>(map)?
-                        .into_iter()
-                        .map(|(_, test)| test),
-                );
+            Some(hint @ ComponentHint::Test) => {
+                // Tests use a root array, not a component map. Discard filenames while
+                // preserving their order, then use the same coercion as top-level tests.
+                let map = ConfigMap::from_iter([(
+                    hint.as_component_field().to_owned(),
+                    serde_json::Value::Array(map.into_values().collect()),
+                )]);
+                self.builder
+                    .tests
+                    .extend(deserialize_config_map::<ConfigBuilder>(map)?.tests);
             }
             None => {
                 self.builder.append(deserialize_config_map(map)?)?;

src/config/loading/interpolation.rs

@@ -84,17 +84,9 @@ pub fn interpolate(input: &str, vars: &HashMap<String, String>) -> Result<String
                 .or_else(|| caps.get(2))
                 .map(|m| m.as_str())
                 .map(|name| {
-                    // Get the value and check for newlines (LF or CR)
-                    let val = vars.get(name).and_then(|v| {
-                        if v.contains(['\n', '\r']) {
-                            errors.push(format!(
-                                "Environment variable contains newline character. name = {name:?}",
-                            ));
-                            None
-                        } else {
-                            Some(v.as_str())
-                        }
-                    });
+                    // Parsing has already fixed the configuration's structure. Newlines and
+                    // other syntax characters remain part of this string value.
+                    let val = vars.get(name).map(String::as_str);
 
                     match flags {
                         ":-" => match val {
@@ -225,8 +217,8 @@ mod test {
     }
 
     #[test]
-    fn test_multiline_expansion_prevented() {
-        let vars = vec![
+    fn multiline_values_cannot_change_the_parsed_structure() {
+        let vars: HashMap<String, String> = vec![
             ("SAFE_VAR".into(), "single line value".into()),
             ("MULTILINE_VAR".into(), "line1\nline2\nline3".into()),
             ("WITH_NEWLINE".into(), "before\nafter".into()),
@@ -236,30 +228,14 @@ mod test {
         .into_iter()
         .collect();
 
-        // Test that multiline values are treated as missing
-        let result = interpolate("$MULTILINE_VAR", &vars);
-        assert!(result.is_err(), "Multiline var should be rejected");
-
-        let result = interpolate("$WITH_NEWLINE", &vars);
-        assert!(result.is_err(), "Newline var should be rejected");
-
-        let result = interpolate("$WITH_CR", &vars);
-        assert!(result.is_err(), "CR var should be rejected");
-
-        let result = interpolate("$WITH_CRLF", &vars);
-        assert!(result.is_err(), "CRLF var should be rejected");
-
-        // Test that safe values still work
-        let result = interpolate("$SAFE_VAR", &vars).unwrap();
-        assert_eq!("single line value", result);
-
-        // Test with default values - multiline vars should still error
-        let result = interpolate("${MULTILINE_VAR:-safe default}", &vars);
-        assert!(result.is_err(), "Should error even with default");
-
-        // Verify error messages are helpful
-        let err = interpolate("$MULTILINE_VAR", &vars).unwrap_err();
-        assert!(err.iter().any(|e| e.contains("newline character")));
-        assert!(err.iter().any(|e| e.contains("MULTILINE_VAR")));
+        for (name, value) in &vars {
+            let input = serde_json::json!({"key": format!("${{{name}:-default}}")});
+            let result =
+                interpolate_config_map_with_env_vars(input.as_object().unwrap(), &vars).unwrap();
+            assert_eq!(
+                serde_json::Value::Object(result),
+                serde_json::json!({"key": value})
+            );
+        }
     }
 }