diff --git a/Cargo.lock b/Cargo.lock index 394c78b46e..6bc760080b 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -8584,6 +8584,7 @@ dependencies = [ "serde_json", "sha2", "spin-common", + "spin-expressions", "spin-locked-app", "spin-manifest", "spin-outbound-networking-config", diff --git a/crates/expressions/src/lib.rs b/crates/expressions/src/lib.rs index 6a4e843668..2219044372 100644 --- a/crates/expressions/src/lib.rs +++ b/crates/expressions/src/lib.rs @@ -150,7 +150,7 @@ impl Resolver { } /// Resolves the given template. - fn resolve_template(&self, template: &Template) -> Result { + pub fn resolve_template(&self, template: &Template) -> Result { let mut resolved_parts: Vec> = Vec::with_capacity(template.parts().len()); for part in template.parts() { resolved_parts.push(match part { diff --git a/crates/loader/Cargo.toml b/crates/loader/Cargo.toml index c010f62a25..0d85b5f577 100644 --- a/crates/loader/Cargo.toml +++ b/crates/loader/Cargo.toml @@ -16,6 +16,7 @@ serde = { workspace = true } serde_json = { workspace = true } sha2 = { workspace = true } spin-common = { path = "../common" } +spin-expressions = { path = "../expressions" } spin-locked-app = { path = "../locked-app" } spin-manifest = { path = "../manifest" } spin-outbound-networking-config = { path = "../outbound-networking-config" } diff --git a/crates/loader/src/local.rs b/crates/loader/src/local.rs index cd6af7d389..34659fc65b 100644 --- a/crates/loader/src/local.rs +++ b/crates/loader/src/local.rs @@ -4,6 +4,7 @@ use anyhow::{anyhow, bail, ensure, Context, Result}; use futures::{future::try_join_all, StreamExt}; use reqwest::Url; use spin_common::{paths::parent_dir, sloth, ui::quoted_path}; +use spin_expressions::Resolver; use spin_locked_app::{ locked::{ self, ContentPath, ContentRef, LockedApp, LockedComponent, LockedComponentDependency, @@ -87,7 +88,8 @@ impl LocalLoader { let variables = variables .into_iter() .map(|(name, v)| Ok((name.to_string(), locked_variable(v)?))) - .collect::>()?; + .collect::>>()?; + let resolver = Resolver::new(variables.clone())?; let triggers = triggers .into_iter() @@ -102,10 +104,13 @@ impl LocalLoader { let sloth_guard = warn_if_component_load_slothful(); // Load all components concurrently - let components = try_join_all(components.into_iter().map(|(id, c)| async move { - self.load_component(&id, c) - .await - .with_context(|| format!("Failed to load component `{id}`")) + let components = try_join_all(components.into_iter().map(|(id, c)| { + let resolver = &resolver; + async move { + self.load_component(&id, c, resolver) + .await + .with_context(|| format!("Failed to load component `{id}`")) + } })) .await?; @@ -138,11 +143,12 @@ impl LocalLoader { &self, id: &KebabId, component: v2::Component, + resolver: &Resolver, ) -> Result { let allowed_outbound_hosts = component .normalized_allowed_outbound_hosts() .context("`allowed_http_hosts` is malformed")?; - AllowedHostsConfig::validate(&allowed_outbound_hosts) + AllowedHostsConfig::validate(&allowed_outbound_hosts, resolver) .context("`allowed_outbound_hosts` is malformed")?; let component_requires_service_chaining = requires_service_chaining(&component); @@ -872,8 +878,7 @@ mod test { let err_ctx = format!("{err:#}"); assert!( err_ctx.contains(r#""/" is not a valid destination file name"#), - "expected error to show destination file name but got {}", - err_ctx + "expected error to show destination file name but got {err_ctx}", ); Ok(()) } diff --git a/crates/loader/tests/ui/invalid-variable-default-in-allowed-hosts.err b/crates/loader/tests/ui/invalid-variable-default-in-allowed-hosts.err new file mode 100644 index 0000000000..687dc41bb7 --- /dev/null +++ b/crates/loader/tests/ui/invalid-variable-default-in-allowed-hosts.err @@ -0,0 +1,8 @@ +Failed to load Spin app from "/invalid-variable-default-in-allowed-hosts.toml" + +Caused by: + 0: Failed to load component `test` + 1: `allowed_outbound_hosts` is malformed + 2: using default variable value(s) with template "https://{{ invalid_default_host }}" results in invalid config "https://invalid host" + 3: Invalid allowed host "invalid host" + 4: invalid international domain name \ No newline at end of file diff --git a/crates/loader/tests/ui/invalid-variable-default-in-allowed-hosts.toml b/crates/loader/tests/ui/invalid-variable-default-in-allowed-hosts.toml new file mode 100644 index 0000000000..25a1416486 --- /dev/null +++ b/crates/loader/tests/ui/invalid-variable-default-in-allowed-hosts.toml @@ -0,0 +1,14 @@ +spin_manifest_version = 2 + +[application] +name = "invalid-app" + +[variables] +invalid_default_host = { default = "invalid host" } + +[[trigger.fake]] +component = "test" + +[component.test] +source = "dummy.wasm" +allowed_outbound_hosts = ["https://{{ invalid_default_host }}"] \ No newline at end of file diff --git a/crates/outbound-networking-config/src/allowed_hosts.rs b/crates/outbound-networking-config/src/allowed_hosts.rs index 312899557d..6ba73e983a 100644 --- a/crates/outbound-networking-config/src/allowed_hosts.rs +++ b/crates/outbound-networking-config/src/allowed_hosts.rs @@ -3,6 +3,7 @@ use std::sync::Arc; use anyhow::{bail, ensure, Context as _}; use futures_util::future::{BoxFuture, Shared}; +use spin_expressions::Resolver; use url::Host; /// The domain used for service chaining. @@ -433,6 +434,22 @@ impl PartialAllowedHostConfig { Self::Unresolved(t) => AllowedHostConfig::parse(resolver.resolve_template(&t)?), } } + + /// Validates this config. Only templates that can be resolved with default + /// values from the given resolver will be fully validated. + fn validate(&self, resolver: &Resolver) -> anyhow::Result<()> { + if let Self::Unresolved(template) = self { + let Ok(resolved) = resolver.resolve_template(template) else { + // We're missing a default value so we can't validate further + return Ok(()); + }; + AllowedHostConfig::parse(&resolved).with_context(|| { + let template_str = template.to_string(); + format!("using default variable value(s) with template {template_str:?} results in invalid config {resolved:?}") + })?; + } + Ok(()) + } } /// Represents an allowed_outbound_hosts config. @@ -457,10 +474,11 @@ impl AllowedHostsConfig { Ok(Self::SpecificHosts(allowed)) } - /// Validate the given allowed_outbound_hosts values. Templated values are - /// only validated against template syntax. - pub fn validate>(hosts: &[S]) -> anyhow::Result<()> { - _ = Self::parse_partial(hosts)?; + /// Validates the given allowed_outbound_hosts values with the given resolver. + pub fn validate>(hosts: &[S], resolver: &Resolver) -> anyhow::Result<()> { + for partial in Self::parse_partial(hosts)? { + partial.validate(resolver)?; + } Ok(()) }