Respect 'allowed_redis_hosts' even in v1 components if manifest key set

Signed-off-by: Ryan Levick <[email protected]>

Author: rylev

crates/loader/src/local.rs

@@ -114,7 +114,7 @@ impl LocalLoader {
         let metadata = ValuesMapBuilder::new()
             .string("description", component.description)
             .string_array("allowed_http_hosts", component.allowed_http_hosts)
-            .string_array("allowed_redis_hosts", component.allowed_redis_hosts)
+            .string_array_option("allowed_redis_hosts", component.allowed_redis_hosts)
             .string_array("key_value_stores", component.key_value_stores)
             .string_array("databases", component.sqlite_databases)
             .string_array("ai_models", component.ai_models)

crates/locked-app/src/values.rs

@@ -58,6 +58,20 @@ impl ValuesMapBuilder {
         self.entry(key, entries)
     }
 
+    /// Inserts an optional list of strings
+    pub fn string_array_option(
+        &mut self,
+        key: impl Into<String>,
+        value: Option<impl IntoIterator<Item = impl Into<String>>>,
+    ) -> &mut Self {
+        if let Some(value) = value {
+            let entries = value.into_iter().map(|s| s.into()).collect::<Vec<_>>();
+            self.entry(key, entries)
+        } else {
+            self.entry(key, Value::Null)
+        }
+    }
+
     /// Inserts an entry into the map using the value's `impl Into<Value>`.
     pub fn entry(&mut self, key: impl Into<String>, value: impl Into<Value>) -> &mut Self {
         self.0.insert(key.into(), value.into());

crates/manifest/src/schema/v1.rs

@@ -80,7 +80,7 @@ pub struct ComponentV1 {
     pub allowed_http_hosts: Vec<String>,
     /// `allowed_redis_hosts` = ["redis://redis.com:6379"]`
     #[serde(default)]
-    pub allowed_redis_hosts: Vec<String>,
+    pub allowed_redis_hosts: Option<Vec<String>>,
     /// `key_value_stores = ["default"]`
     #[serde(default)]
     pub key_value_stores: Vec<String>,

crates/manifest/src/schema/v2.rs

@@ -103,8 +103,8 @@ pub struct Component {
     #[serde(default, skip_serializing_if = "Vec::is_empty")]
     pub allowed_http_hosts: Vec<String>,
     /// `allowed_redis_hosts = ["myredishost.com"]`
-    #[serde(default, skip_serializing_if = "Vec::is_empty")]
-    pub allowed_redis_hosts: Vec<String>,
+    #[serde(default, skip_serializing_if = "is_none_or_empty")]
+    pub allowed_redis_hosts: Option<Vec<String>>,
     /// `key_value_stores = ["default"]`
     #[serde(default, skip_serializing_if = "Vec::is_empty")]
     pub key_value_stores: Vec<SnakeId>,
@@ -119,6 +119,11 @@ pub struct Component {
     pub build: Option<ComponentBuildConfig>,
 }
 
+/// Used to skip serializing if the value is either `None` or `Some(v)` where `v` is empty
+fn is_none_or_empty<T>(value: &Option<Vec<T>>) -> bool {
+    value.as_ref().map(|s| !s.is_empty()).unwrap_or_default()
+}
+
 mod one_or_many {
     use serde::{Deserialize, Deserializer, Serialize, Serializer};
 

crates/outbound-redis/src/host_component.rs

@@ -29,7 +29,7 @@ impl DynamicHostComponent for OutboundRedisComponent {
         let hosts = component
             .get_metadata(crate::ALLOWED_REDIS_HOSTS_KEY)?
             .unwrap_or_default();
-        data.allowed_hosts.extend(hosts);
+        data.allowed_hosts = hosts;
         Ok(())
     }
 }

crates/outbound-redis/src/lib.rs

@@ -9,7 +9,7 @@ use spin_locked_app::MetadataKey;
 use spin_world::v1::redis::{self as v1, RedisParameter, RedisResult};
 use spin_world::v2::redis::{self as v2, Connection as RedisConnection, Error};
 
-pub const ALLOWED_REDIS_HOSTS_KEY: MetadataKey<Vec<String>> =
+pub const ALLOWED_REDIS_HOSTS_KEY: MetadataKey<Option<HashSet<String>>> =
     MetadataKey::new("allowed_redis_hosts");
 
 pub use host_component::OutboundRedisComponent;
@@ -35,7 +35,7 @@ impl FromRedisValue for RedisResults {
 }
 
 pub struct OutboundRedis {
-    allowed_hosts: HashSet<String>,
+    allowed_hosts: Option<HashSet<String>>,
     connections: table::Table<Connection>,
 }
 
@@ -49,6 +49,24 @@ impl Default for OutboundRedis {
 }
 
 impl OutboundRedis {
+    fn is_address_allowed(&self, address: &str, default: bool) -> bool {
+        fn do_check(allowed_hosts: Option<&HashSet<String>>, address: &str, default: bool) -> bool {
+            let Some(allowed_hosts) = allowed_hosts else {
+                return default;
+            };
+            allowed_hosts.contains(address)
+        }
+
+        let response = do_check(self.allowed_hosts.as_ref(), address, default);
+        if !response {
+            terminal::warn!(
+                "A component tried to make a HTTP request to non-allowed address {address:?}."
+            );
+            eprintln!("To allow requests, add 'allowed_redis_hosts = [{address:?}]' to the manifest component section.");
+        }
+        response
+    }
+
     async fn establish_connection(
         &mut self,
         address: String,
@@ -73,11 +91,7 @@ impl v2::Host for OutboundRedis {}
 #[async_trait]
 impl v2::HostConnection for OutboundRedis {
     async fn open(&mut self, address: String) -> Result<Result<Resource<RedisConnection>, Error>> {
-        if !self.allowed_hosts.contains(&address) {
-            terminal::warn!(
-                "A component tried to make a HTTP request to non-allowed address '{address}'."
-            );
-            eprintln!("To allow requests, add 'allowed_redis_hosts = [\"{address}\"]' to the manifest component section.");
+        if !self.is_address_allowed(&address, false) {
             return Ok(Err(Error::InvalidAddress));
         }
 
@@ -239,6 +253,9 @@ fn other_error(e: impl std::fmt::Display) -> Error {
 /// Delegate a function call to the v2::HostConnection implementation
 macro_rules! delegate {
     ($self:ident.$name:ident($address:expr, $($arg:expr),*)) => {{
+        if !$self.is_address_allowed(&$address, true) {
+            return Ok(Err(v1::Error::Error));
+        }
         let connection = match $self.establish_connection($address).await? {
             Ok(c) => c,
             Err(_) => return Ok(Err(v1::Error::Error)),