From f97aac2cb60ef460ab825897397bc72c07e3b12f Mon Sep 17 00:00:00 2001 From: Mateusz Krawiec Date: Mon, 12 Oct 2026 02:42:00 -0700 Subject: [PATCH] fix(dev): allow only same-origin requests by default for CORS and /run_live - A UI served from another origin now needs `adk.web.cors.origins`, for example `adk.web.cors.origins=http://localhost:4200`. - The server now applies `Forwarded` and `X-Forwarded-*` headers by default. Make sure your proxy strips them from untrusted clients, or set `server.forward-headers-strategy=none`. PiperOrigin-RevId: 997788515 --- dev/README.md | 34 ++- .../adk/web/config/AdkWebCorsConfig.java | 39 ++- .../adk/web/config/AdkWebCorsProperties.java | 5 +- .../google/adk/web/AdkWebServerCorsTest.java | 272 ++++++++++++++++++ .../web/AdkWebServerProxyRedirectTest.java | 10 +- .../adk/web/BackendUrlRedirectTest.java | 23 +- .../RuntimeConfigControllerTest.java | 11 +- 7 files changed, 358 insertions(+), 36 deletions(-) create mode 100644 dev/src/test/java/com/google/adk/web/AdkWebServerCorsTest.java diff --git a/dev/README.md b/dev/README.md index 97c49ea36..a1ac809db 100644 --- a/dev/README.md +++ b/dev/README.md @@ -35,7 +35,33 @@ a relative path like `/my-app` cannot be used. If the application also sets instead of prepending the context path or `X-Forwarded-Prefix`. When `adk.web.backend-url` is unset, `/` and `/dev-ui` redirect to `/dev-ui/` -and the bundled `backendUrl` value is preserved. If you instead enable -`server.forward-headers-strategy=framework`, make sure your reverse proxy strips -or overwrites incoming `Forwarded` and `X-Forwarded-*` headers from untrusted -clients. +and the bundled `backendUrl` value is preserved. + +The server applies `Forwarded` and `X-Forwarded-*` headers to the request URL by +default, as with `server.forward-headers-strategy=framework`. Make sure your +reverse proxy strips or overwrites these headers from untrusted clients. Set +`server.forward-headers-strategy=none` to ignore them, or `native` to let the +web server handle them instead. Behind a proxy that terminates TLS, keep them +on: with `none`, a page served over plain HTTP under the same host name passes +the same-origin check. + +## Cross-origin requests + +By default, the REST API and the `/run_live` WebSocket accept only same-origin +requests. The dev UI served by the server itself is same-origin and needs no +configuration. + +To allow other origins, for example a UI served by a separate development +server, set `adk.web.cors.origins` to a comma-separated list of origins: + +```properties +adk.web.cors.origins=http://localhost:4200 +``` + +Behind a reverse proxy, the same-origin check uses the URL that the proxy +reports in forwarded headers, so the dev UI keeps working behind a proxy that +terminates TLS. A proxy on a non-default port must include the port in its +forwarded headers. List the external origin in `adk.web.cors.origins` if the +proxy does not report the external URL, or if it reports `ws` or `wss` as the +scheme of WebSocket requests (as Traefik does). Avoid `*`: it lets any website +open in your browser call the server. diff --git a/dev/src/main/java/com/google/adk/web/config/AdkWebCorsConfig.java b/dev/src/main/java/com/google/adk/web/config/AdkWebCorsConfig.java index 5e54f54cb..364586e5b 100644 --- a/dev/src/main/java/com/google/adk/web/config/AdkWebCorsConfig.java +++ b/dev/src/main/java/com/google/adk/web/config/AdkWebCorsConfig.java @@ -16,14 +16,21 @@ package com.google.adk.web.config; +import jakarta.servlet.DispatcherType; import org.slf4j.Logger; import org.slf4j.LoggerFactory; +import org.springframework.beans.factory.ObjectProvider; +import org.springframework.boot.autoconfigure.condition.ConditionalOnProperty; +import org.springframework.boot.web.server.autoconfigure.servlet.ForwardedHeaderFilterCustomizer; +import org.springframework.boot.web.servlet.FilterRegistrationBean; import org.springframework.context.annotation.Bean; import org.springframework.context.annotation.Configuration; +import org.springframework.core.Ordered; import org.springframework.web.cors.CorsConfiguration; import org.springframework.web.cors.CorsConfigurationSource; import org.springframework.web.cors.UrlBasedCorsConfigurationSource; import org.springframework.web.filter.CorsFilter; +import org.springframework.web.filter.ForwardedHeaderFilter; /** * Configuration class for setting up Cross-Origin Resource Sharing (CORS) in the ADK Web @@ -39,6 +46,8 @@ *
  • {@link CorsConfigurationSource}: Configures CORS settings such as allowed origins, methods, * headers, credentials, and max age. *
  • {@link CorsFilter}: Applies the CORS configuration to incoming requests. + *
  • {@link ForwardedHeaderFilter}: Applies reverse proxy headers to the URL of every request, + * so the same-origin checks see the URL the browser used. * */ @Configuration @@ -50,10 +59,10 @@ public class AdkWebCorsConfig { public CorsConfigurationSource corsConfigurationSource(AdkWebCorsProperties corsProperties) { if (corsProperties.origins().contains("*")) { logger.warn( - "CORS is configured to allow all origins (\"*\"), which is insecure and intended for" - + " local development only. This also applies to the /run_live WebSocket endpoint." - + " Set 'adk.web.cors.origins' to an explicit allowlist to restrict which origins" - + " may call the server."); + "CORS is configured to allow all origins (\"*\"), so any website open in a browser can" + + " call this server, including the /run_live WebSocket endpoint. Set" + + " 'adk.web.cors.origins' to an explicit allowlist to restrict which origins may" + + " call the server."); } CorsConfiguration configuration = new CorsConfiguration(); @@ -74,4 +83,26 @@ public CorsConfigurationSource corsConfigurationSource(AdkWebCorsProperties cors public CorsFilter corsFilter(CorsConfigurationSource corsConfigurationSource) { return new CorsFilter(corsConfigurationSource); } + + /** + * Makes {@code server.forward-headers-strategy=framework} the default, so same-origin checks see + * the URL the browser used behind a TLS-terminating proxy. Registered like Spring Boot's own + * filter, which then backs off; set the property to {@code native} or {@code none} to opt out. + */ + @Bean + @ConditionalOnProperty( + name = "server.forward-headers-strategy", + havingValue = "framework", + matchIfMissing = true) + FilterRegistrationBean adkForwardedHeaderFilter( + ObjectProvider customizers) { + ForwardedHeaderFilter filter = new ForwardedHeaderFilter(); + customizers.orderedStream().forEach(customizer -> customizer.customize(filter)); + FilterRegistrationBean registration = + new FilterRegistrationBean<>(filter); + registration.setDispatcherTypes( + DispatcherType.REQUEST, DispatcherType.ASYNC, DispatcherType.ERROR); + registration.setOrder(Ordered.HIGHEST_PRECEDENCE); + return registration; + } } diff --git a/dev/src/main/java/com/google/adk/web/config/AdkWebCorsProperties.java b/dev/src/main/java/com/google/adk/web/config/AdkWebCorsProperties.java index c96dce91e..234b98f4b 100644 --- a/dev/src/main/java/com/google/adk/web/config/AdkWebCorsProperties.java +++ b/dev/src/main/java/com/google/adk/web/config/AdkWebCorsProperties.java @@ -21,7 +21,8 @@ /** * Properties for configuring CORS in ADK Web. This class is used to load CORS settings from - * application properties. + * application properties. {@code origins} defaults to an empty list, which allows only same-origin + * requests. */ @ConfigurationProperties(prefix = "adk.web.cors") public record AdkWebCorsProperties( @@ -34,7 +35,7 @@ public record AdkWebCorsProperties( public AdkWebCorsProperties { mapping = mapping != null ? mapping : "/**"; - origins = origins != null && !origins.isEmpty() ? origins : List.of("*"); + origins = origins != null ? origins : List.of(); methods = methods != null && !methods.isEmpty() ? methods diff --git a/dev/src/test/java/com/google/adk/web/AdkWebServerCorsTest.java b/dev/src/test/java/com/google/adk/web/AdkWebServerCorsTest.java new file mode 100644 index 000000000..deb359803 --- /dev/null +++ b/dev/src/test/java/com/google/adk/web/AdkWebServerCorsTest.java @@ -0,0 +1,272 @@ +/* + * Copyright 2026 Google LLC + * + * Licensed under the Apache License, Version 2.0 (the "License"); + * you may not use this file except in compliance with the License. + * You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ + +package com.google.adk.web; + +import static com.google.common.truth.Truth.assertThat; +import static org.junit.jupiter.api.Assertions.assertThrows; +import static org.springframework.http.HttpHeaders.ACCESS_CONTROL_ALLOW_ORIGIN; +import static org.springframework.http.HttpHeaders.ACCESS_CONTROL_REQUEST_METHOD; +import static org.springframework.http.HttpHeaders.ORIGIN; +import static org.springframework.test.web.servlet.request.MockMvcRequestBuilders.get; +import static org.springframework.test.web.servlet.request.MockMvcRequestBuilders.options; +import static org.springframework.test.web.servlet.result.MockMvcResultMatchers.header; +import static org.springframework.test.web.servlet.result.MockMvcResultMatchers.status; + +import com.google.common.base.Throwables; +import java.net.URI; +import java.net.http.HttpClient; +import java.net.http.WebSocket; +import java.net.http.WebSocketHandshakeException; +import java.util.concurrent.CompletionException; +import org.junit.jupiter.api.Nested; +import org.junit.jupiter.api.Test; +import org.junit.jupiter.params.ParameterizedTest; +import org.junit.jupiter.params.provider.ValueSource; +import org.springframework.beans.factory.annotation.Autowired; +import org.springframework.boot.test.context.SpringBootTest; +import org.springframework.boot.test.context.SpringBootTest.WebEnvironment; +import org.springframework.boot.test.web.server.LocalServerPort; +import org.springframework.boot.webmvc.test.autoconfigure.AutoConfigureMockMvc; +import org.springframework.test.web.servlet.MockMvc; + +/** + * Tests for the browser origins allowed to call the REST API and open the {@code /run_live} + * WebSocket. + */ +final class AdkWebServerCorsTest { + + private static final String REMOTE_ORIGIN = "https://evil.example"; + private static final HttpClient httpClient = HttpClient.newHttpClient(); + + @Nested + @SpringBootTest(webEnvironment = WebEnvironment.RANDOM_PORT) + @AutoConfigureMockMvc + class DefaultOrigins { + + @ParameterizedTest + @ValueSource(strings = {REMOTE_ORIGIN, "http://localhost:4200"}) + public void restApi_fromOtherOrigin_isRejected(String origin, @Autowired MockMvc mockMvc) + throws Exception { + mockMvc + .perform(get("/list-apps").header(ORIGIN, origin)) + .andExpect(status().isForbidden()) + .andExpect(header().doesNotExist(ACCESS_CONTROL_ALLOW_ORIGIN)); + } + + @Test + public void restApi_fromSameOrigin_isAllowed(@Autowired MockMvc mockMvc) throws Exception { + // Same-origin requests skip the origin list, so the bundled UI works from any host name. + mockMvc + .perform(get("http://devbox:8000/list-apps").header(ORIGIN, "http://devbox:8000")) + .andExpect(status().isOk()); + } + + @Test + public void restApi_behindTlsProxy_isAllowed(@Autowired MockMvc mockMvc) throws Exception { + // The proxy forwards plain HTTP; its X-Forwarded-Proto restores the browser's https origin. + mockMvc + .perform( + get("http://app.example.com/list-apps") + .header("X-Forwarded-Proto", "https") + .header(ORIGIN, "https://app.example.com")) + .andExpect(status().isOk()); + } + + @Test + public void restApi_behindTlsProxy_fromRemoteOrigin_isRejected(@Autowired MockMvc mockMvc) + throws Exception { + mockMvc + .perform( + get("http://app.example.com/list-apps") + .header("X-Forwarded-Proto", "https") + .header(ORIGIN, REMOTE_ORIGIN)) + .andExpect(status().isForbidden()); + } + + @Test + public void restApi_behindTlsProxy_fromPlainHttpOrigin_isRejected(@Autowired MockMvc mockMvc) + throws Exception { + // Matches the raw request URL, but the browser reached the server over https. + mockMvc + .perform( + get("http://app.example.com/list-apps") + .header("X-Forwarded-Proto", "https") + .header(ORIGIN, "http://app.example.com")) + .andExpect(status().isForbidden()); + } + + @Test + public void preflight_fromRemoteOrigin_isRejected(@Autowired MockMvc mockMvc) throws Exception { + mockMvc + .perform( + options("/apps/test-app/users/test-user/sessions/test-session") + .header(ORIGIN, REMOTE_ORIGIN) + .header(ACCESS_CONTROL_REQUEST_METHOD, "DELETE")) + .andExpect(status().isForbidden()) + .andExpect(header().doesNotExist(ACCESS_CONTROL_ALLOW_ORIGIN)); + } + + @Test + public void runLive_fromSameOrigin_completesHandshake(@LocalServerPort int port) { + openRunLive(port, "http://localhost:" + port).abort(); + } + + @ParameterizedTest + @ValueSource(strings = {REMOTE_ORIGIN, "http://localhost:4200"}) + public void runLive_fromOtherOrigin_isRejected(String origin, @LocalServerPort int port) { + assertHandshakeForbidden(port, origin); + } + + @Test + public void runLive_behindTlsProxy_completesHandshake(@LocalServerPort int port) { + WebSocket.Builder handshake = + httpClient + .newWebSocketBuilder() + .header(ORIGIN, "https://app.example.com") + .header("X-Forwarded-Proto", "https") + .header("X-Forwarded-Host", "app.example.com"); + + connect(port, handshake).abort(); + } + + @Test + public void runLive_behindTlsProxy_fromPlainHttpOrigin_isRejected(@LocalServerPort int port) { + WebSocket.Builder handshake = + httpClient + .newWebSocketBuilder() + .header(ORIGIN, "http://localhost:" + port) + .header("X-Forwarded-Proto", "https"); + + assertHandshakeForbidden(port, handshake); + } + } + + @Nested + @SpringBootTest(properties = "server.forward-headers-strategy=none") + @AutoConfigureMockMvc + class ForwardedHeadersDisabled { + + @Test + public void restApi_behindTlsProxy_isRejected(@Autowired MockMvc mockMvc) throws Exception { + mockMvc + .perform( + get("http://app.example.com/list-apps") + .header("X-Forwarded-Proto", "https") + .header(ORIGIN, "https://app.example.com")) + .andExpect(status().isForbidden()); + } + } + + /** With the CORS filter mapped away from /run_live, only the handshake's origin check runs. */ + @Nested + @SpringBootTest( + webEnvironment = WebEnvironment.RANDOM_PORT, + properties = "adk.web.cors.mapping=/apps/**") + class CorsFilterNotOnRunLive { + + @Test + public void runLive_fromSameOrigin_completesHandshake(@LocalServerPort int port) { + openRunLive(port, "http://localhost:" + port).abort(); + } + + @Test + public void runLive_fromRemoteOrigin_isRejected(@LocalServerPort int port) { + assertHandshakeForbidden(port, REMOTE_ORIGIN); + } + } + + @Nested + @SpringBootTest( + webEnvironment = WebEnvironment.RANDOM_PORT, + properties = "adk.web.cors.origins=https://ui.example.com") + @AutoConfigureMockMvc + class ConfiguredOrigins { + + @Test + public void restApi_fromConfiguredOrigin_isAllowed(@Autowired MockMvc mockMvc) + throws Exception { + mockMvc + .perform(get("/list-apps").header(ORIGIN, "https://ui.example.com")) + .andExpect(status().isOk()) + .andExpect(header().string(ACCESS_CONTROL_ALLOW_ORIGIN, "https://ui.example.com")); + } + + @Test + public void restApi_behindTlsProxy_isAllowed(@Autowired MockMvc mockMvc) throws Exception { + mockMvc + .perform( + get("http://app.example.com/list-apps") + .header("X-Forwarded-Proto", "https") + .header(ORIGIN, "https://app.example.com")) + .andExpect(status().isOk()); + } + + @Test + public void runLive_fromConfiguredOrigin_completesHandshake(@LocalServerPort int port) { + openRunLive(port, "https://ui.example.com").abort(); + } + + @Test + public void runLive_fromUnlistedOrigin_isRejected(@LocalServerPort int port) { + assertHandshakeForbidden(port, REMOTE_ORIGIN); + } + } + + @Nested + @SpringBootTest( + webEnvironment = WebEnvironment.RANDOM_PORT, + properties = "adk.web.cors.origins=*") + @AutoConfigureMockMvc + class AllOriginsOptIn { + + @Test + public void restApi_fromRemoteOrigin_isAllowed(@Autowired MockMvc mockMvc) throws Exception { + mockMvc + .perform(get("/list-apps").header(ORIGIN, REMOTE_ORIGIN)) + .andExpect(status().isOk()) + .andExpect(header().string(ACCESS_CONTROL_ALLOW_ORIGIN, "*")); + } + + @Test + public void runLive_fromRemoteOrigin_completesHandshake(@LocalServerPort int port) { + openRunLive(port, REMOTE_ORIGIN).abort(); + } + } + + private static WebSocket openRunLive(int port, String origin) { + return connect(port, httpClient.newWebSocketBuilder().header(ORIGIN, origin)); + } + + private static WebSocket connect(int port, WebSocket.Builder handshake) { + return handshake + .buildAsync(URI.create("ws://localhost:" + port + "/run_live"), new WebSocket.Listener() {}) + .join(); + } + + private static void assertHandshakeForbidden(int port, String origin) { + assertHandshakeForbidden(port, httpClient.newWebSocketBuilder().header(ORIGIN, origin)); + } + + private static void assertHandshakeForbidden(int port, WebSocket.Builder handshake) { + CompletionException thrown = + assertThrows(CompletionException.class, () -> connect(port, handshake)); + + WebSocketHandshakeException cause = + Throwables.getCauseAs(thrown, WebSocketHandshakeException.class); + assertThat(cause.getResponse().statusCode()).isEqualTo(403); + } +} diff --git a/dev/src/test/java/com/google/adk/web/AdkWebServerProxyRedirectTest.java b/dev/src/test/java/com/google/adk/web/AdkWebServerProxyRedirectTest.java index 3a323d431..b1626371d 100644 --- a/dev/src/test/java/com/google/adk/web/AdkWebServerProxyRedirectTest.java +++ b/dev/src/test/java/com/google/adk/web/AdkWebServerProxyRedirectTest.java @@ -29,12 +29,12 @@ import org.springframework.test.web.servlet.MockMvc; /** - * Verifies that when {@code adk.web.backend-url} is unset and {@code - * server.forward-headers-strategy=framework} is enabled, Spring's {@code ForwardedHeaderFilter} - * applies {@code Forwarded} and {@code X-Forwarded-*} headers to the entry redirects. When {@code - * adk.web.backend-url} is set, its path prefix takes precedence over {@code X-Forwarded-Prefix}. + * Verifies that when {@code adk.web.backend-url} is unset, Spring's {@code ForwardedHeaderFilter}, + * on by default, applies {@code Forwarded} and {@code X-Forwarded-*} headers to the entry + * redirects. When {@code adk.web.backend-url} is set, its path prefix takes precedence over {@code + * X-Forwarded-Prefix}. */ -@SpringBootTest(properties = "server.forward-headers-strategy=framework") +@SpringBootTest @AutoConfigureMockMvc final class AdkWebServerProxyRedirectTest { diff --git a/dev/src/test/java/com/google/adk/web/BackendUrlRedirectTest.java b/dev/src/test/java/com/google/adk/web/BackendUrlRedirectTest.java index ce6d61381..01ea83f23 100644 --- a/dev/src/test/java/com/google/adk/web/BackendUrlRedirectTest.java +++ b/dev/src/test/java/com/google/adk/web/BackendUrlRedirectTest.java @@ -32,18 +32,13 @@ /** * With {@code adk.web.backend-url} set, the entry redirect carries the gateway's path prefix on its * own, so a deployment behind a path-stripping proxy needs nothing forwarded from the proxy. The - * property governs the path only: host and scheme still come from forwarded headers wherever the - * operator has enabled {@code server.forward-headers-strategy}. + * property governs the path only: host and scheme still come from forwarded headers unless the + * operator sets {@code server.forward-headers-strategy=none}. */ public class BackendUrlRedirectTest { @Nested - @SpringBootTest( - properties = { - "adk.web.backend-url=https://gw.example.com/my-app", - // Enable ForwardedHeaderFilter so X-Forwarded-* headers reach the servlet request. - "server.forward-headers-strategy=framework" - }) + @SpringBootTest(properties = "adk.web.backend-url=https://gw.example.com/my-app") @AutoConfigureMockMvc class ConfiguredWithTheFilter { @@ -83,14 +78,17 @@ public void devUiEntryPoint_forwardedHostAndProto_stillSetTheRedirectsOrigin( } @Nested - @SpringBootTest(properties = "adk.web.backend-url=https://gw.example.com/my-app") + @SpringBootTest( + properties = { + "adk.web.backend-url=https://gw.example.com/my-app", + "server.forward-headers-strategy=none" + }) @AutoConfigureMockMvc class ConfiguredWithoutTheFilter { @Test - public void devUiEntryPoint_forwardedHeaders_reachNothingUnderTheShippedStrategy( + public void devUiEntryPoint_forwardedHeaders_reachNothingWithTheFilterOff( @Autowired MockMvc mockMvc) throws Exception { - // With forward-headers-strategy unset (default), forwarded headers are ignored. mockMvc .perform( get("/") @@ -137,7 +135,7 @@ public void devUiEntryPoints_shouldRedirectWithoutAPrefix( } @Nested - @SpringBootTest + @SpringBootTest(properties = "server.forward-headers-strategy=none") @AutoConfigureMockMvc class UnconfiguredWithoutTheFilter { @@ -155,7 +153,6 @@ public void devUiEntryPoints_shouldRedirectWithoutAPrefix( @ValueSource(strings = {"/", "/dev-ui"}) public void devUiEntryPoints_withForwardedHeaders_shouldStillRedirectWithoutAPrefix( String path, @Autowired MockMvc mockMvc) throws Exception { - // By default forward-headers-strategy is unset, so forwarded headers are ignored. mockMvc .perform( get(path) diff --git a/dev/src/test/java/com/google/adk/web/controller/RuntimeConfigControllerTest.java b/dev/src/test/java/com/google/adk/web/controller/RuntimeConfigControllerTest.java index 1bbf58718..da2101282 100644 --- a/dev/src/test/java/com/google/adk/web/controller/RuntimeConfigControllerTest.java +++ b/dev/src/test/java/com/google/adk/web/controller/RuntimeConfigControllerTest.java @@ -41,12 +41,7 @@ public class RuntimeConfigControllerTest { private static final String CONFIG = "/dev-ui/assets/config/runtime-config.json"; @Nested - @SpringBootTest( - properties = { - "adk.web.backend-url=https://gw.example.com/my-app", - // Enable ForwardedHeaderFilter so X-Forwarded-* headers reach the servlet request. - "server.forward-headers-strategy=framework" - }) + @SpringBootTest(properties = "adk.web.backend-url=https://gw.example.com/my-app") @AutoConfigureMockMvc class ConfiguredWithTheFilter { @@ -92,7 +87,7 @@ public void runtimeConfig_shouldNotServeTheCredentials(@Autowired MockMvc mockMv } @Nested - @SpringBootTest + @SpringBootTest(properties = "server.forward-headers-strategy=none") @AutoConfigureMockMvc class UnconfiguredWithoutTheFilter { @@ -111,7 +106,7 @@ public void runtimeConfig_forwardedHeaders_areNotReadFromTheRequest(@Autowired M } @Nested - @SpringBootTest(properties = "server.forward-headers-strategy=framework") + @SpringBootTest @AutoConfigureMockMvc class UnconfiguredWithTheFilter {