diff --git a/rust/src/cmd/src/cli.rs b/rust/src/cmd/src/cli.rs index 14b949c87d..eeb458c86e 100644 --- a/rust/src/cmd/src/cli.rs +++ b/rust/src/cmd/src/cli.rs @@ -401,9 +401,9 @@ pub struct SharedRuntimeArgs { #[serde(default)] pub served_model_name: Vec, - /// CORS allowed origins as a JSON list. `["*"]` allows any origin. - #[arg(long, value_parser = parse_json::, value_name = "JSON", default_value = r#"["*"]"#)] - #[serde(default = "default_cors_wildcard")] + /// CORS allowed origins as a JSON list. Defaults to `[]` (no cross-origin access). + #[arg(long, value_parser = parse_json::, value_name = "JSON", default_value = "[]")] + #[serde(default = "default_cors_origins")] pub allowed_origins: JsonStringList, /// CORS allowed methods as a JSON list. `["*"]` allows the standard set. @@ -664,6 +664,10 @@ fn default_cors_wildcard() -> JsonStringList { JsonStringList(vec!["*".to_string()]) } +fn default_cors_origins() -> JsonStringList { + JsonStringList(Vec::new()) +} + fn default_py_bootstrap_parser_selection() -> ParserSelection { ParserSelection::None } diff --git a/rust/src/cmd/src/cli/tests.rs b/rust/src/cmd/src/cli/tests.rs index 57780027de..cc545ce33e 100644 --- a/rust/src/cmd/src/cli/tests.rs +++ b/rust/src/cmd/src/cli/tests.rs @@ -205,9 +205,7 @@ fn serve_args_forward_python_flags_with_separator() { disable_log_stats: false, served_model_name: [], allowed_origins: JsonStringList( - [ - "*", - ], + [], ), allowed_methods: JsonStringList( [ @@ -1075,9 +1073,7 @@ fn frontend_args_accept_json() { disable_log_stats: false, served_model_name: [], allowed_origins: JsonStringList( - [ - "*", - ], + [], ), allowed_methods: JsonStringList( [ @@ -1382,13 +1378,13 @@ fn serve_args_parse_cors_flags() { } #[test] -fn serve_args_cors_defaults_are_permissive() { +fn serve_args_cors_defaults_restrict_origins() { let cli = Cli::try_parse_from(["vllm-rs", "serve", "Qwen/Qwen3-0.6B"]).unwrap(); let Command::Serve(serve) = cli.command else { panic!("expected serve args"); }; - assert_eq!(serve.runtime.allowed_origins.0, ["*"]); + assert!(serve.runtime.allowed_origins.0.is_empty()); assert_eq!(serve.runtime.allowed_methods.0, ["*"]); assert_eq!(serve.runtime.allowed_headers.0, ["*"]); assert!(!serve.runtime.allow_credentials); @@ -1748,9 +1744,7 @@ fn serve_args_accept_handshake_aliases() { disable_log_stats: false, served_model_name: [], allowed_origins: JsonStringList( - [ - "*", - ], + [], ), allowed_methods: JsonStringList( [ @@ -1906,9 +1900,7 @@ fn serve_frontend_config_uses_dp_address_as_advertised_host() { sse_keep_alive_interval: None, }, cors: CorsConfig { - allow_origins: [ - "*", - ], + allow_origins: [], allow_methods: [ "*", ], @@ -2002,9 +1994,7 @@ fn serve_frontend_config_keeps_tcp_transport_for_non_local_only_topology() { sse_keep_alive_interval: None, }, cors: CorsConfig { - allow_origins: [ - "*", - ], + allow_origins: [], allow_methods: [ "*", ], @@ -2142,9 +2132,7 @@ fn frontend_config_uses_external_coordinator_when_coordinator_address_is_present sse_keep_alive_interval: None, }, cors: CorsConfig { - allow_origins: [ - "*", - ], + allow_origins: [], allow_methods: [ "*", ], diff --git a/rust/src/server/src/config.rs b/rust/src/server/src/config.rs index 3f95b5891c..ca91ba40cd 100644 --- a/rust/src/server/src/config.rs +++ b/rust/src/server/src/config.rs @@ -65,7 +65,7 @@ pub struct ApiServerOptions { pub sse_keep_alive_interval: Option, } -/// CORS settings mirroring Python's `CORSMiddleware`; the default is permissive. +/// CORS settings mirroring Python's `CORSMiddleware`; origins are denied by default. #[derive(Debug, Clone, PartialEq, Eq, Serialize)] pub struct CorsConfig { /// Allowed origins. `["*"]` allows any origin. @@ -81,7 +81,7 @@ pub struct CorsConfig { impl Default for CorsConfig { fn default() -> Self { Self { - allow_origins: vec!["*".to_string()], + allow_origins: Vec::new(), allow_methods: vec!["*".to_string()], allow_headers: vec!["*".to_string()], allow_credentials: false, diff --git a/rust/src/server/src/middleware/cors.rs b/rust/src/server/src/middleware/cors.rs index 89d1b6f3b7..bfc030a3a9 100644 --- a/rust/src/server/src/middleware/cors.rs +++ b/rust/src/server/src/middleware/cors.rs @@ -105,7 +105,7 @@ pub fn cors_layer(cfg: &CorsConfig) -> CorsLayer { // Emit `Vary: Origin` only when the allow-origin is dynamic (explicit // origins, or credentials); the wildcard + no-credentials case emits no - // `Vary` at all, and an empty list disables the header here. + // `Vary` at all. An empty origin list never permits an origin. let vary: Vec = if !wildcard_origins || cfg.allow_credentials { vec![header::ORIGIN] } else { diff --git a/rust/src/server/src/routes/tests.rs b/rust/src/server/src/routes/tests.rs index 118e412b45..7ba4bbf0f4 100644 --- a/rust/src/server/src/routes/tests.rs +++ b/rust/src/server/src/routes/tests.rs @@ -1784,7 +1784,7 @@ async fn api_key_auth_allows_unguarded_route_without_token() { #[tokio::test(flavor = "multi_thread", worker_threads = 2)] #[serial] -async fn cors_default_simple_request_allows_any_origin() { +async fn cors_default_simple_request_omits_allow_origin() { let (mut app, _engine_task) = test_app_with_cors(CorsConfig::default()).await; let response = app .call( @@ -1799,17 +1799,12 @@ async fn cors_default_simple_request_allows_any_origin() { .expect("call app"); assert_eq!(response.status(), StatusCode::OK); - assert_eq!( - header_value(&response, "access-control-allow-origin"), - Some("*") - ); - // Wildcard origins without credentials emit no `Vary` (Starlette parity). - assert_eq!(header_value(&response, "vary"), None); + assert_eq!(header_value(&response, "access-control-allow-origin"), None); } #[tokio::test(flavor = "multi_thread", worker_threads = 2)] #[serial] -async fn cors_default_preflight_returns_explicit_methods_and_max_age() { +async fn cors_default_preflight_omits_allow_origin() { let (mut app, _engine_task) = test_app_with_cors(CorsConfig::default()).await; let response = app .call( @@ -1825,6 +1820,32 @@ async fn cors_default_preflight_returns_explicit_methods_and_max_age() { .await .expect("call app"); + assert_eq!(response.status(), StatusCode::OK); + assert_eq!(header_value(&response, "access-control-allow-origin"), None); +} + +#[tokio::test(flavor = "multi_thread", worker_threads = 2)] +#[serial] +async fn cors_wildcard_preflight_returns_explicit_methods_and_max_age() { + let cors = CorsConfig { + allow_origins: vec!["*".to_string()], + ..CorsConfig::default() + }; + let (mut app, _engine_task) = test_app_with_cors(cors).await; + let response = app + .call( + Request::builder() + .method("OPTIONS") + .uri("/v1/chat/completions") + .header("origin", "http://example.com") + .header("access-control-request-method", "POST") + .header("access-control-request-headers", "content-type") + .body(Body::empty()) + .expect("build request"), + ) + .await + .expect("call app"); + assert_eq!(response.status(), StatusCode::OK); // `*` methods expand to the explicit method list, matching Starlette // (never the literal `*`). @@ -1917,6 +1938,7 @@ async fn cors_explicit_origin_disallowed_omits_allow_origin() { #[serial] async fn cors_wildcard_with_credentials_reflects_origin_without_panic() { let cors = CorsConfig { + allow_origins: vec!["*".to_string()], allow_credentials: true, ..CorsConfig::default() }; @@ -1971,8 +1993,12 @@ async fn cors_unauthorized_response_has_no_cors_headers() { #[tokio::test(flavor = "multi_thread", worker_threads = 2)] #[serial] async fn cors_preflight_bypasses_auth_and_returns_cors_headers() { + let cors = CorsConfig { + allow_origins: vec!["*".to_string()], + ..CorsConfig::default() + }; let (mut app, _engine_task) = - test_app_with_cors_and_keys(CorsConfig::default(), vec!["secret".to_string()]).await; + test_app_with_cors_and_keys(cors, vec!["secret".to_string()]).await; let response = app .call( Request::builder() diff --git a/tests/entrypoints/launchers/test_cli_args.py b/tests/entrypoints/launchers/test_cli_args.py index 84279e1555..0a4c97141d 100644 --- a/tests/entrypoints/launchers/test_cli_args.py +++ b/tests/entrypoints/launchers/test_cli_args.py @@ -44,6 +44,13 @@ def serve_parser(): return _build_vllm_parsers()["vllm serve"] +def test_cors_origins_require_explicit_opt_in(serve_parser): + assert serve_parser.parse_args([]).allowed_origins == [] + assert serve_parser.parse_args( + ["--allowed-origins", '["http://trusted.example"]'] + ).allowed_origins == ["http://trusted.example"] + + ### Test config parsing def test_config_arg_parsing(serve_parser, cli_config_file): args = serve_parser.parse_args([]) diff --git a/tests/entrypoints/serve/middleware/test_cors_middleware.py b/tests/entrypoints/serve/middleware/test_cors_middleware.py new file mode 100644 index 0000000000..543c101a93 --- /dev/null +++ b/tests/entrypoints/serve/middleware/test_cors_middleware.py @@ -0,0 +1,62 @@ +# SPDX-License-Identifier: Apache-2.0 +# SPDX-FileCopyrightText: Copyright contributors to the vLLM project + +from fastapi import FastAPI +from starlette.testclient import TestClient + +from vllm.entrypoints.launchers.cli_args import FrontendArgs +from vllm.entrypoints.serve.middleware.register import init_entrypoints_middleware + + +def _make_client(allowed_origins: list[str] | None = None) -> TestClient: + args = FrontendArgs() + if allowed_origins is not None: + args.allowed_origins = allowed_origins + + app = FastAPI() + + @app.get("/v1/models") + async def models(): + return {"data": [{"id": "private-model"}]} + + @app.post("/v1/chat/completions") + async def chat(): + return {"message": {"content": "ok"}} + + init_entrypoints_middleware(args, app, ()) + return TestClient(app) + + +def test_default_origins_prevent_cross_origin_reads_and_json_requests(): + client = _make_client() + response = client.get("/v1/models", headers={"Origin": "http://attacker.example"}) + assert response.status_code == 200 + assert "access-control-allow-origin" not in response.headers + + preflight = client.options( + "/v1/chat/completions", + headers={ + "Origin": "http://attacker.example", + "Access-Control-Request-Method": "POST", + "Access-Control-Request-Headers": "content-type", + }, + ) + assert preflight.status_code == 400 + assert "access-control-allow-origin" not in preflight.headers + + +def test_explicit_origin_allows_browser_requests(): + client = _make_client(["http://trusted.example"]) + response = client.get("/v1/models", headers={"Origin": "http://trusted.example"}) + assert response.headers["access-control-allow-origin"] == "http://trusted.example" + + preflight = client.options( + "/v1/chat/completions", + headers={ + "Origin": "http://trusted.example", + "Access-Control-Request-Method": "POST", + "Access-Control-Request-Headers": "content-type", + }, + ) + assert preflight.status_code == 200 + assert preflight.headers["access-control-allow-origin"] == "http://trusted.example" diff --git a/vllm/entrypoints/launchers/cli_args.py b/vllm/entrypoints/launchers/cli_args.py index 5cd58da87e..e2c2625e94 100644 --- a/vllm/entrypoints/launchers/cli_args.py +++ b/vllm/entrypoints/launchers/cli_args.py @@ -316,8 +316,8 @@ class FrontendArgs(BaseFrontendArgs): while keeping logs for other endpoints.""" allow_credentials: bool = False """Allow credentials.""" - allowed_origins: list[str] = field(default_factory=lambda: ["*"]) - """Allowed origins.""" + allowed_origins: list[str] = field(default_factory=list) + """Allowed origins. Cross-origin browser access is disabled by default.""" allowed_methods: list[str] = field(default_factory=lambda: ["*"]) """Allowed methods.""" allowed_headers: list[str] = field(default_factory=lambda: ["*"])