Skip to content

fix(dev): default adk.web.cors.origins to the server's own loopback o… - #1674

Open
behnam1990adel-hash wants to merge 1 commit into
google:mainfrom
behnam1990adel-hash:fix/dev-web-cors-default-to-loopback
Open

behnam1990adel-hash wants to merge 1 commit into
google:mainfrom
behnam1990adel-hash:fix/dev-web-cors-default-to-loopback

Conversation

@behnam1990adel-hash

Copy link
Copy Markdown

Previously, when adk.web.cors.origins was left unset, CORS defaulted to allowing every origin ("*"). Any website a developer's browser visited while 'adk web' was running could call the local dev server cross-origin: list loaded agents, create sessions, and drive full agent execution with the agent's own configured tools and credentials.

This changes the default to the server's own loopback origins (localhost / 127.0.0.1 / [::1] on the configured port) instead. Setting adk.web.cors.origins explicitly still works exactly as before, including the "*" opt-in with its existing warning.

Please ensure you have read the contribution guide before creating a pull request.

Link to Issue or Description of Change

2. No GitHub issue exists for this; describing the change below.

Problem:
The ADK Java adk web dev server has no authentication on any endpoint, and CORS defaults to allowing every origin ("*") when adk.web.cors.origins is not set. This is the documented, by-the-book way of starting the server (mvn google-adk:web -Dagents=<dir>). Because CORS, not network binding, is the only thing standing between a remote page and these endpoints, any website a developer's browser visits while the server is running can read the loaded agent list, create sessions under an arbitrary user ID, and drive full agent execution, reading the response cross-origin. This mirrors a vulnerability already rated Critical (CVSS 9.8) in the Python port of this project (CVE-2026-4810 / GHSA-rg7c-g689-fr3x) and already fixed in the Go port (adk-go#1154).

Solution:
When adk.web.cors.origins is not explicitly configured, default to the server's own loopback origins (http://localhost:<port>, http://127.0.0.1:<port>, http://[::1]:<port>) instead of "*". Developers who need other origins (e.g. a separate frontend dev server) can still set adk.web.cors.origins explicitly, including "*", which keeps its existing startup warning.

Testing Plan

  • mvn -pl dev -am compile — builds clean.
  • mvn -pl dev -am test — all 20 existing tests in the dev module pass, none broken by this change.
  • Manual verification against a running sample server (contrib/samples/configagent):
    • curl -H "Origin: https://evil.example" http://localhost:8000/list-apps → before: 200 OK with the full agent list and Access-Control-Allow-Origin: *; after this change: 403 Invalid CORS request.
    • curl http://localhost:8000/list-apps (no forged Origin) → 200 OK, unchanged, confirms normal usage still works.

…rigins

Previously, when adk.web.cors.origins was left unset, CORS defaulted
to allowing every origin ("*"). Any website a developer's browser
visited while 'adk web' was running could call the local dev server
cross-origin: list loaded agents, create sessions, and drive full
agent execution with the agent's own configured tools and credentials.

This changes the default to the server's own loopback origins
(localhost / 127.0.0.1 / [::1] on the configured port) instead.
Setting adk.web.cors.origins explicitly still works exactly as
before, including the "*" opt-in with its existing warning.

@gemini-code-assist gemini-code-assist Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Code Review

This pull request improves CORS security by defaulting to loopback origins (localhost, 127.0.0.1, [::1]) instead of allowing all origins ("*") when no explicit origins are configured. The reviewer suggests a robust improvement to use setAllowedOriginPatterns with wildcards for the port (e.g., http://localhost:*) instead of hardcoding the server port. This change avoids issues with dynamic ports in test environments, improves developer experience with local frontend servers, and allows removing the unused serverPort field and its import.

import java.util.List;
import org.slf4j.Logger;
import org.slf4j.LoggerFactory;
import org.springframework.beans.factory.annotation.Value;

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

medium

Since we no longer need to inject the server port when using origin patterns, this import can be removed.

Comment on lines +45 to +46
@Value("${server.port:8000}")
private int serverPort;

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

medium

With the transition to using setAllowedOriginPatterns with wildcards for the port, the serverPort field is no longer needed. Removing it avoids potential issues with dynamic ports (e.g., port 0 in test environments).

Comment on lines +50 to +72
List<String> origins = corsProperties.origins();

if (origins.isEmpty()) {
origins =
List.of(
"http://localhost:" + serverPort,
"http://127.0.0.1:" + serverPort,
"http://[::1]:" + serverPort);
logger.info(
"No 'adk.web.cors.origins' configured; defaulting to this server's own loopback"
+ " origins on port {}. Set 'adk.web.cors.origins' explicitly to allow other"
+ " origins.",
serverPort);
} else if (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 (\"*\"), which is insecure and lets any"
+ " website call this server, including the /run_live WebSocket endpoint. Only"
+ " set this if you understand the risk.");
}

CorsConfiguration configuration = new CorsConfiguration();

configuration.setAllowedOrigins(corsProperties.origins());
configuration.setAllowedOrigins(origins);

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

medium

Instead of hardcoding the server port by injecting ${server.port}, we can use setAllowedOriginPatterns with wildcards for the port (e.g., http://localhost:*).

This approach has several advantages:

  1. Robustness: It works seamlessly even if the server is configured to run on a dynamic port (such as port 0 in test environments).
  2. Developer Experience: It automatically supports local frontend development servers running on different ports (e.g., React on 3000, Vite on 5173) without requiring manual CORS configuration.
  3. Security: It still restricts access strictly to the local machine (loopback), maintaining the security fix.
    List<String> origins = corsProperties.origins();
    CorsConfiguration configuration = new CorsConfiguration();

    if (origins.isEmpty()) {
      configuration.setAllowedOriginPatterns(
          List.of(
              "http://localhost:*",
              "http://127.0.0.1:*",
              "http://[::1]:*"));
      logger.info(
          "No 'adk.web.cors.origins' configured; defaulting to loopback origins on any port"
              + " (localhost / 127.0.0.1 / [::1]). Set 'adk.web.cors.origins' explicitly to"
              + " allow other origins.");
    } else {
      configuration.setAllowedOrigins(origins);
      if (origins.contains("*")) {
        logger.warn(
            "CORS is configured to allow all origins (\"*\"), which is insecure and lets any"
                + " website call this server, including the /run_live WebSocket endpoint. Only"
                + " set this if you understand the risk.");
      }
    }

Comment on lines +37 to +39
// Leave unset origins empty rather than defaulting to "*": AdkWebCorsConfig computes a
// same-machine loopback default from the server's own port when this is empty, so every
// origin on the internet is not allowed to call the server out of the box.

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

medium

Update the comment to reflect that the default loopback origins are allowed on any port using origin patterns, rather than being bound to the server's own port.

Suggested change
// Leave unset origins empty rather than defaulting to "*": AdkWebCorsConfig computes a
// same-machine loopback default from the server's own port when this is empty, so every
// origin on the internet is not allowed to call the server out of the box.
// Leave unset origins empty rather than defaulting to "*": AdkWebCorsConfig computes a
// same-machine loopback default on any port when this is empty, so every
// origin on the internet is not allowed to call the server out of the box.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant