Merge pull request #3016 from alexcrichton/gate-service-chained-requests

Check allowed hosts before checking the `request_interceptor`
spinframework · Feb 19, 2025 · 28ca31c · 28ca31c
2 parents ee2f691 + 8fc4c03
commit 28ca31c
Show file tree

Hide file tree

Showing 3 changed files with 40 additions and 22 deletions.
diff --git a/crates/factor-outbound-http/src/wasi.rs b/crates/factor-outbound-http/src/wasi.rs
@@ -132,23 +132,6 @@ async fn send_request_impl(
 
     spin_telemetry::inject_trace_context(&mut request);
 
-    if let Some(interceptor) = request_interceptor {
-        let intercept_request = std::mem::take(&mut request).into();
-        match interceptor.intercept(intercept_request).await? {
-            InterceptOutcome::Continue(req) => {
-                request = req.into_hyper_request();
-            }
-            InterceptOutcome::Complete(resp) => {
-                let resp = IncomingResponse {
-                    resp,
-                    worker: None,
-                    between_bytes_timeout: config.between_bytes_timeout,
-                };
-                return Ok(Ok(resp));
-            }
-        }
-    }
-
     let host = request.uri().host().unwrap_or_default();
     let tls_client_config = component_tls_configs.get_client_config(host).clone();
 
@@ -189,6 +172,23 @@ async fn send_request_impl(
         *request.uri_mut() = origin.into_uri(path_and_query);
     }
 
+    if let Some(interceptor) = request_interceptor {
+        let intercept_request = std::mem::take(&mut request).into();
+        match interceptor.intercept(intercept_request).await? {
+            InterceptOutcome::Continue(req) => {
+                request = req.into_hyper_request();
+            }
+            InterceptOutcome::Complete(resp) => {
+                let resp = IncomingResponse {
+                    resp,
+                    worker: None,
+                    between_bytes_timeout: config.between_bytes_timeout,
+                };
+                return Ok(Ok(resp));
+            }
+        }
+    }
+
     let authority = request.uri().authority().context("authority not set")?;
     span.record("server.address", authority.host());
     if let Some(port) = authority.port() {

diff --git a/tests/test-components/components/internal-http-front/src/lib.rs b/tests/test-components/components/internal-http-front/src/lib.rs
@@ -1,5 +1,5 @@
 use anyhow::anyhow;
-use helper::{ensure, ensure_eq, ensure_ok, ensure_some};
+use helper::{ensure, ensure_eq, ensure_matches, ensure_ok, ensure_some};
 use spin_sdk::{
     http::{IntoResponse, Request},
     http_component,
@@ -32,5 +32,17 @@ async fn handle_front_impl(_req: Request) -> Result<impl IntoResponse, String> {
 
     res.headers_mut()
         .append("spin-component", ensure_ok!("internal-http-front-component".try_into()));
+
+    // Double-check that we can only send requests to allowed hosts.
+    ensure_matches!(
+        spin_sdk::http::send::<_, spin_sdk::http::Response>(spin_sdk::http::Request::new(
+            spin_sdk::http::Method::Get,
+            "http://back.spin.internal/hello/from/middle"
+        ))
+        .await,
+        Err(spin_sdk::http::SendError::Http(
+            spin_sdk::http::Error::UnexpectedError(_)
+        )),
+    );
     Ok(res)
 }
diff --git a/tests/test-components/helper/src/lib.rs b/tests/test-components/helper/src/lib.rs
@@ -159,11 +159,17 @@ macro_rules! ensure_matches {
 
 #[macro_export]
 macro_rules! ensure_eq {
-    ($expr1:expr, $expr2:expr) => {
-        if $expr1 != $expr2 {
-            $crate::bail!("`{}` != `{}`", stringify!($expr1), stringify!($expr2));
+    ($expr1:expr, $expr2:expr) => {{
+        let a = $expr1;
+        let b = $expr2;
+        if a != b {
+            $crate::bail!(
+                "`{}` != `{}`\n{a:?} != {b:?}",
+                stringify!($expr1),
+                stringify!($expr2),
+            );
         }
-    };
+    }};
 }
 
 #[macro_export]