Validate variable and config key names at app load

crates/loader/Cargo.toml

@@ -25,6 +25,7 @@ serde = { version = "1.0", features = ["derive"] }
 serde_json = "1.0"
 shellexpand = "3.1"
 spin-common = { path = "../common" }
+spin-config = { path = "../config" }
 spin-manifest = { path = "../manifest" }
 tempfile = "3.3.0"
 terminal = { path = "../terminal" }

crates/loader/src/local/mod.rs

@@ -27,7 +27,10 @@ use spin_manifest::{
 };
 use tokio::{fs::File, io::AsyncReadExt};
 
-use crate::{cache::Cache, validation::validate_key_value_stores};
+use crate::{
+    cache::Cache,
+    validation::{validate_config_keys, validate_key_value_stores, validate_variable_names},
+};
 use config::{
     FileComponentUrlSource, RawAppInformation, RawAppManifest, RawAppManifestAnyVersion,
     RawAppManifestAnyVersionPartial, RawComponentManifest, RawComponentManifestPartial,
@@ -117,6 +120,9 @@ fn error_on_duplicate_ids(components: Vec<RawComponentManifest>) -> Result<()> {
 /// Validate fields in raw app manifest
 pub fn validate_raw_app_manifest(raw: &RawAppManifestAnyVersion) -> Result<()> {
     let manifest = raw.as_v1();
+
+    validate_variable_names(&manifest.variables)?;
+
     manifest
         .components
         .iter()
@@ -125,6 +131,10 @@ pub fn validate_raw_app_manifest(raw: &RawAppManifestAnyVersion) -> Result<()> {
         .components
         .iter()
         .try_for_each(|c| validate_key_value_stores(&c.wasm.key_value_stores))?;
+    manifest
+        .components
+        .iter()
+        .try_for_each(|c| validate_config_keys(&c.config))?;
 
     Ok(())
 }
@@ -474,6 +484,33 @@ source = "nonexistent.wasm"
         manifest
     }
 
+    fn load_config_test_manifest(var_name: &str) -> anyhow::Result<RawAppManifestAnyVersion> {
+        let manifest_toml = format!(
+            r#"
+spin_version = "1"
+name = "test"
+trigger = {{ type = "http", base = "/" }}
+version = "0.0.1"
+
+[variables]
+{var_name} = {{ required = true }}
+
+[[component]]
+id = "test"
+source = "nonexistent.wasm"
+[component.trigger]
+route = "/"
+[component.config]
+{var_name} = "hello"
+"#
+        );
+
+        let manifest = raw_manifest_from_slice(manifest_toml.as_bytes()).unwrap();
+        validate_raw_app_manifest(&manifest)?;
+
+        Ok(manifest)
+    }
+
     #[test]
     fn can_parse_http_trigger() {
         let m = load_test_manifest(r#"{ type = "http", base = "/" }"#, r#"route = "/...""#);
@@ -522,4 +559,15 @@ source = "nonexistent.wasm"
         assert!(matches!(t, ApplicationTrigger::External(_)));
         assert!(matches!(ct, TriggerConfig::External(_)));
     }
+
+    #[test]
+    fn rejects_bad_variable_names() {
+        load_config_test_manifest("hello").expect("hello should validate");
+
+        load_config_test_manifest("hello_123").expect("hello_123 should validate");
+
+        load_config_test_manifest("HELLO").expect_err("HELLO should not validate");
+
+        load_config_test_manifest("hell-o").expect_err("hell-o should not validate");
+    }
 }

crates/loader/src/validation.rs

@@ -1,5 +1,27 @@
+use std::collections::HashMap;
+
 use anyhow::{ensure, Context, Result};
 
+use crate::common::RawVariable;
+
+pub(crate) fn validate_variable_names(variables: &HashMap<String, RawVariable>) -> Result<()> {
+    for name in variables.keys() {
+        if let Err(spin_config::Error::InvalidKey(m)) = spin_config::Key::new(name) {
+            anyhow::bail!("Invalid variable name {name}: {m}. Variable names and config keys may contain only lower-case letters, numbers, and underscores.");
+        };
+    }
+    Ok(())
+}
+
+pub(crate) fn validate_config_keys(config: &Option<HashMap<String, String>>) -> Result<()> {
+    for name in config.iter().flat_map(|c| c.keys()) {
+        if let Err(spin_config::Error::InvalidKey(m)) = spin_config::Key::new(name) {
+            anyhow::bail!("Invalid config key {name}: {m}. Variable names and config keys may contain only lower-case letters, numbers, and underscores.");
+        };
+    }
+    Ok(())
+}
+
 pub(crate) fn validate_key_value_stores(key_value_stores: &Option<Vec<String>>) -> Result<()> {
     for store in key_value_stores.iter().flatten() {
         validate_component_like_label(store)