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

@@ -3,13 +3,11 @@ 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, deserialize_config_value},
 };
+use crate::config::{ConfigBuilder, TestDefinition};
 
 #[derive(Debug)]
 pub struct ConfigBuilderLoader {
@@ -66,48 +64,50 @@ 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)
+        }
     }
 
     /// 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),
+                    deserialize_config_value::<IndexMap<String, TestDefinition<String>>>(
+                        serde_json::Value::Object(map),
+                    )?
+                    .into_iter()
+                    .map(|(_, test)| test),
                 );
             }
             None => {

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})
+            );
+        }
     }
 }

src/config/loading/loader.rs

@@ -1,12 +1,20 @@
-use std::path::{Path, PathBuf};
+use std::{
+    collections::HashMap,
+    io::Read,
+    path::{Path, PathBuf},
+};
 
 use serde_json::Value;
 
 use super::{
-    Format, component_name, open_file, read_dir,
+    Format, component_name, interpolate_config_map_with_env_vars,
+    interpolation::ENVIRONMENT_VARIABLE_INTERPOLATION_REGEX,
+    open_file, read_dir,
     representation::{
-        ConfigMap, deserialize_config, deserialize_config_value, merge_into_map, merge_values,
+        ConfigMap, deserialize_config_value, merge_into_map, merge_values, parse_config_value,
     },
+    schema_coercion::ValueCoercer,
+    secret::COLLECTOR,
 };
 
 /// Provides a hint to the loading system of the type of components that should be found
@@ -23,7 +31,7 @@ pub enum ComponentHint {
 impl ComponentHint {
     /// Returns the component string field that should host a component -- e.g. sources,
     /// transforms, etc.
-    const fn as_component_field(&self) -> &str {
+    pub(super) const fn as_component_field(self) -> &'static str {
         match self {
             ComponentHint::Source => "sources",
             ComponentHint::Transform => "transforms",
@@ -47,26 +55,42 @@ impl ComponentHint {
 // because there are numerous internal functions for dealing with (non)recursive loading that
 // rely on `&self` but don't need overriding and would be confusingly named in a public API.
 pub(super) mod process {
-    use std::io::Read;
-
     use super::*;
 
     /// This trait contains methods that deserialize files/folders. There are a few methods
     /// in here with subtly different names that can be hidden from public view, hence why
     /// this is nested in a private mod.
     pub trait Process {
-        /// Prepares input for serialization. This can be a useful step to interpolate
-        /// environment variables or perform some other pre-processing on the input.
-        fn prepare<R: Read>(&mut self, input: R) -> Result<String, Vec<String>>;
-
-        /// Calls into the `prepare` method, and deserializes a `Read` to a `T`.
-        fn load<R: std::io::Read, T>(&mut self, input: R, format: Format) -> Result<T, Vec<String>>
-        where
-            T: serde::de::DeserializeOwned,
-        {
-            let content = self.prepare(input)?;
-
-            deserialize_config(&content, format)
+        /// Returns whether environment variable interpolation should be applied.
+        fn should_interpolate_env(&self) -> bool;
+
+        /// Runs loader-specific processing on the parsed, environment-interpolated map.
+        fn postprocess(&mut self, map: ConfigMap) -> Result<ConfigMap, Vec<String>>;
+
+        /// Parses the document before substituting any environment variables or secrets.
+        fn load<R: Read>(&mut self, input: R, format: Format) -> Result<ConfigMap, Vec<String>> {
+            let source = string_from_input(input)?;
+            let value = parse_config_value(&source, format).map_err(|mut errors| {
+                if matches!(format, Format::Toml | Format::Json)
+                    && (ENVIRONMENT_VARIABLE_INTERPOLATION_REGEX.is_match(&source)
+                        || COLLECTOR.is_match(&source))
+                {
+                    errors.push(
+                        "Configuration is parsed before interpolation. Quote placeholders in \
+                         TOML and JSON values; they will be coerced to the field's declared \
+                         type after substitution."
+                            .to_string(),
+                    );
+                }
+                errors
+            })?;
+            let map = deserialize_config_value(value)?;
+            let map = if self.should_interpolate_env() {
+                resolve_environment_variables(map)?
+            } else {
+                map
+            };
+            self.postprocess(map)
         }
 
         /// Helper method used by other methods to recursively handle file/dir loading, merging
@@ -222,10 +246,8 @@ where
         input: R,
         format: Format,
     ) -> Result<(), Vec<String>> {
-        if let Some(map) = self.load(input, format)? {
-            self.merge(map, None)?;
-        }
-        Ok(())
+        let map = self.load(input, format)?;
+        self.merge(map, None)
     }
 
     /// Deserializes a file with the provided format, and makes the result available via `take`.
@@ -298,9 +320,51 @@ fn merge_with_value(res: &mut ConfigMap, name: String, value: Value) -> Result<(
     Ok(())
 }
 
-/// Deserialize a configuration map into a `T`.
+/// Coerces a root configuration before serde performs authoritative deserialization.
 pub(super) fn deserialize_config_map<T: serde::de::DeserializeOwned>(
     map: ConfigMap,
 ) -> Result<T, Vec<String>> {
-    deserialize_config_value(Value::Object(map))
+    let mut value = Value::Object(map);
+    coerce_config(&mut value)?;
+    deserialize_config_value(value)
+}
+
+/// Coerces a namespaced component map using its schema at the root configuration field.
+pub(super) fn deserialize_component_map<T: serde::de::DeserializeOwned>(
+    map: ConfigMap,
+    hint: ComponentHint,
+) -> Result<indexmap::IndexMap<crate::config::ComponentKey, T>, Vec<String>> {
+    let key = hint.as_component_field();
+    let mut value = serde_json::json!({key: map});
+    coerce_config(&mut value)?;
+    deserialize_config_value(value[key].take())
+}
+
+fn coerce_config(value: &mut Value) -> Result<(), Vec<String>> {
+    let schema = vector_config::schema::generate_root_schema::<crate::config::ConfigBuilder>()
+        .map_err(|error| vec![format!("{error:?}")])?;
+    let schema = serde_json::to_value(schema).map_err(|error| vec![error.to_string()])?;
+    ValueCoercer::new(&schema)
+        .coerce(value)
+        .map_err(|error| vec![error.to_string()])
+}
+
+pub(super) fn string_from_input<R: Read>(mut input: R) -> Result<String, Vec<String>> {
+    let mut source = String::new();
+    input
+        .read_to_string(&mut source)
+        .map_err(|error| vec![error.to_string()])?;
+    Ok(source)
+}
+
+fn resolve_environment_variables(map: ConfigMap) -> Result<ConfigMap, Vec<String>> {
+    let mut vars = std::env::vars_os()
+        .filter_map(|(key, value)| Some((key.into_string().ok()?, value.into_string().ok()?)))
+        .collect::<HashMap<_, _>>();
+    if !vars.contains_key("HOSTNAME")
+        && let Ok(hostname) = crate::get_hostname()
+    {
+        vars.insert("HOSTNAME".into(), hostname);
+    }
+    interpolate_config_map_with_env_vars(&map, &vars)
 }