From 40c7e6de1375e57a8bbcf35759068c31990795ba Mon Sep 17 00:00:00 2001 From: Justin Bradfield Date: Mon, 13 Jul 2026 14:37:01 -0500 Subject: [PATCH] environmentd: allow widening CORS allowlist via system variable Add the http_additional_cors_allowed_origins system variable (superuser settable). Origins listed there (comma-separated, same syntax as --cors-allowed-origin) are allowed for CORS in addition to the origins provided at startup. The variable can only widen the allowlist: startup origins can never be removed through it. The variable value reaches the HTTP servers the same way connection limits do: the coordinator registers a system variable callback that writes the parsed origins into a shared handle the CORS predicate reads on every origin check, so ALTER SYSTEM SET takes effect without a restart. A domain constraint rejects origins that are not valid header values at SET time. The MCP server-side Origin check (DNS rebinding defense) honors the same union. Co-Authored-By: Claude Fable 5 --- src/adapter/src/coord.rs | 15 ++++++ src/environmentd/src/environmentd/main.rs | 4 +- src/environmentd/src/http.rs | 54 +++++++++++++++++---- src/environmentd/src/http/mcp.rs | 43 +++++++++++------ src/environmentd/src/lib.rs | 28 +++++++---- src/environmentd/src/test_util.rs | 2 - src/http-util/src/lib.rs | 28 +++++++++++ src/sql/src/session/vars.rs | 9 ++++ src/sql/src/session/vars/constraints.rs | 31 ++++++++++++ src/sql/src/session/vars/definitions.rs | 57 ++++++++++++++++++++++- src/sqllogictest/src/runner.rs | 2 - test/testdrive/session.td | 1 + 12 files changed, 234 insertions(+), 40 deletions(-) diff --git a/src/adapter/src/coord.rs b/src/adapter/src/coord.rs index 35ae628c21069..7e34c2a9ab815 100644 --- a/src/adapter/src/coord.rs +++ b/src/adapter/src/coord.rs @@ -1251,6 +1251,9 @@ pub struct Config { pub aws_privatelink_availability_zones: Option>, pub connection_context: ConnectionContext, pub connection_limit_callback: Box () + Send + Sync + 'static>, + /// Called with the raw value of the `http_additional_cors_allowed_origins` + /// system variable whenever it changes, and at least once at startup. + pub additional_cors_origins_callback: Box () + Send + Sync + 'static>, pub webhook_concurrency_limit: WebhookConcurrencyLimiter, pub http_host_name: Option, pub tracing_handle: TracingHandle, @@ -4654,6 +4657,7 @@ pub fn serve( aws_privatelink_availability_zones, connection_context, connection_limit_callback, + additional_cors_origins_callback, remote_system_parameters, webhook_concurrency_limit, http_host_name, @@ -4957,6 +4961,17 @@ pub fn serve( connection_limit_callback, ); + // Same pattern for the additional CORS origins, which the HTTP + // servers consult on every origin check. + catalog.system_config_mut().register_callback( + &mz_sql::session::vars::HTTP_ADDITIONAL_CORS_ALLOWED_ORIGINS, + Arc::new(move |system_vars: &SystemVars| { + (additional_cors_origins_callback)( + system_vars.http_additional_cors_allowed_origins(), + ); + }), + ); + let (group_commit_tx, group_commit_rx) = appends::notifier(); let parent_span = tracing::Span::current(); diff --git a/src/environmentd/src/environmentd/main.rs b/src/environmentd/src/environmentd/main.rs index 60a744a57d0d2..fa5eb7664e278 100644 --- a/src/environmentd/src/environmentd/main.rs +++ b/src/environmentd/src/environmentd/main.rs @@ -799,8 +799,7 @@ fn run(mut args: Args) -> Result<(), anyhow::Error> { } allowed_origins }; - let cors_allowed_origin = mz_http_util::build_cors_allowed_origin(&allowed_origins); - let cors_allowed_origin_list = allowed_origins.clone(); + let cors_allowed_origin_list = allowed_origins; // Configure controller. let entered = info_span!("environmentd::configure_controller").entered(); @@ -1107,7 +1106,6 @@ fn run(mut args: Args) -> Result<(), anyhow::Error> { external_login_password_mz_system: args.external_login_password_mz_system, frontegg, frontegg_oauth_issuer_url, - cors_allowed_origin, cors_allowed_origin_list, egress_addresses: args.announce_egress_address, http_host_name: args.http_host_name, diff --git a/src/environmentd/src/http.rs b/src/environmentd/src/http.rs index 8322f9c693ba9..31e06274c4bd2 100644 --- a/src/environmentd/src/http.rs +++ b/src/environmentd/src/http.rs @@ -58,7 +58,7 @@ use std::collections::BTreeMap; use std::fmt::Debug; use std::net::{IpAddr, SocketAddr}; use std::pin::Pin; -use std::sync::Arc; +use std::sync::{Arc, RwLock}; use std::time::{Duration, SystemTime}; use anyhow::Context; @@ -158,10 +158,14 @@ pub struct HttpConfig { pub frontegg: Option, pub oidc_rx: Delayed, pub adapter_client_rx: Shared>, - pub allowed_origin: AllowOrigin, - /// Raw list of allowed CORS origins, used by the MCP endpoints for - /// server-side Origin validation to defend against DNS rebinding. + /// Base allowlist of CORS origins, fixed at startup. The + /// `http_additional_cors_allowed_origins` system variable can widen the + /// allowlist at runtime, but entries in this list can never be removed + /// through it. pub allowed_origin_list: Vec, + /// Origins from the `http_additional_cors_allowed_origins` system + /// variable. Updated by the coordinator whenever the variable changes. + pub additional_allowed_origins: Arc>>, pub active_connection_counter: ConnectionCounter, pub helm_chart_version: Option, /// Externally-visible host name for this environment (without scheme). @@ -214,6 +218,34 @@ pub struct WebhookState { dyncfgs: Arc, } +/// The CORS origin allowlist: a base list fixed at startup, plus any origins +/// from the `http_additional_cors_allowed_origins` system variable, which is +/// read dynamically on every check. The system variable only widens the +/// allowlist, base entries can never be removed through it. +#[derive(Clone, Debug)] +pub struct AllowedOrigins { + base: Arc>, + additional: Arc>>, +} + +impl AllowedOrigins { + fn is_allowed(&self, origin: &HeaderValue) -> bool { + if mz_http_util::origin_is_allowed(origin, &self.base) { + return true; + } + let additional = self + .additional + .read() + .expect("additional CORS origins lock poisoned"); + mz_http_util::origin_is_allowed(origin, &additional) + } + + fn to_allow_origin(&self) -> AllowOrigin { + let this = self.clone(); + AllowOrigin::predicate(move |origin, _| this.is_allowed(origin)) + } +} + #[derive(Clone, Debug)] struct HelmChartVersion(Option); @@ -232,8 +264,8 @@ impl HttpServer { frontegg, oidc_rx, adapter_client_rx, - allowed_origin, allowed_origin_list, + additional_allowed_origins, active_connection_counter, helm_chart_version, http_host_name, @@ -252,6 +284,11 @@ impl HttpServer { let tls_enabled = tls.is_some(); let webhook_cache = WebhookAppenderCache::new(); + let allowed_origins = AllowedOrigins { + base: Arc::new(allowed_origin_list), + additional: additional_allowed_origins, + }; + // Compute OAuth discovery once per listener so the Bearer challenge // and the discovery handler always agree, and the middleware doesn't // re-derive (and re-allocate the Frontegg issuer) on each request. @@ -612,7 +649,6 @@ impl HttpServer { // DNS rebinding attack the browser considers the request // same-origin, so no preflight fires and CORS enforcement is // bypassed. - let mcp_allowed_origins = Arc::new(allowed_origin_list.clone()); mcp_router = mcp_router .layer(auth_middleware.clone()) .layer(Extension(oauth_metadata::McpOAuthConfig { @@ -622,12 +658,12 @@ impl HttpServer { .layer(Extension(adapter_client_rx.clone())) .layer(Extension(active_connection_counter.clone())) .layer(Extension(HelmChartVersion(helm_chart_version.clone()))) - .layer(Extension(mcp_allowed_origins)) + .layer(Extension(allowed_origins.clone())) .layer(Extension(mcp_metrics)) .layer( CorsLayer::new() .allow_methods(Method::POST) - .allow_origin(allowed_origin.clone()) + .allow_origin(allowed_origins.to_allow_origin()) .allow_headers([AUTHORIZATION, CONTENT_TYPE]), ); router = router.merge(mcp_router); @@ -647,7 +683,7 @@ impl HttpServer { HeaderName::from_static("x-materialize-version"), ]) .allow_methods(Any) - .allow_origin(allowed_origin) + .allow_origin(allowed_origins.to_allow_origin()) .expose_headers(Any) .max_age(Duration::from_secs(60) * 60), ); diff --git a/src/environmentd/src/http/mcp.rs b/src/environmentd/src/http/mcp.rs index 1d60733792e94..d287783c0c98e 100644 --- a/src/environmentd/src/http/mcp.rs +++ b/src/environmentd/src/http/mcp.rs @@ -23,13 +23,11 @@ //! //! Data products are discovered via `mz_internal.mz_mcp_data_products` system view. -use std::sync::Arc; - use anyhow::anyhow; use axum::Extension; use axum::Json; use axum::response::IntoResponse; -use http::{HeaderMap, HeaderValue, StatusCode}; +use http::{HeaderMap, StatusCode}; use mz_adapter_types::dyncfgs::{ ENABLE_MCP_AGENT, ENABLE_MCP_AGENT_QUERY_TOOL, ENABLE_MCP_AGENT_READ_DATA_PRODUCT_TOOL, ENABLE_MCP_DEVELOPER, ENABLE_MCP_DEVELOPER_QUERY_TOOL, MCP_MAX_RESPONSE_SIZE, @@ -48,9 +46,9 @@ use serde_json::json; use thiserror::Error; use tracing::{debug, warn}; -use crate::http::AuthedClient; use crate::http::mcp_metrics::{McpMetrics, ToolCallGuard}; use crate::http::sql::{SqlRequest, SqlResponse, SqlResult, execute_request}; +use crate::http::{AllowedOrigins, AuthedClient}; // To add a new tool: add entry to tools/list, add handler function, add dispatch case. @@ -413,7 +411,7 @@ pub async fn handle_mcp_method_not_allowed() -> impl IntoResponse { /// Agent endpoint: exposes user data products. pub async fn handle_mcp_agent( headers: HeaderMap, - Extension(allowed_origins): Extension>>, + Extension(allowed_origins): Extension, Extension(metrics): Extension, client: AuthedClient, Json(body): Json, @@ -429,7 +427,7 @@ pub async fn handle_mcp_agent( /// Developer endpoint: exposes system catalog (mz_*) only. pub async fn handle_mcp_developer( headers: HeaderMap, - Extension(allowed_origins): Extension>>, + Extension(allowed_origins): Extension, Extension(metrics): Extension, client: AuthedClient, Json(body): Json, @@ -452,10 +450,10 @@ pub async fn handle_mcp_developer( /// attacker arranges same-origin DNS rebinding (no preflight fires). fn validate_origin( headers: &HeaderMap, - allowed: &[HeaderValue], + allowed: &AllowedOrigins, ) -> Option { let origin = headers.get(http::header::ORIGIN)?; - if mz_http_util::origin_is_allowed(origin, allowed) { + if allowed.is_allowed(origin) { return None; } warn!( @@ -1539,6 +1537,10 @@ fn validate_system_catalog_query(sql: &str) -> Result<(), McpRequestError> { #[cfg(test)] mod tests { + use std::sync::Arc; + + use http::HeaderValue; + use super::*; use crate::http::sql::{Description, SqlError}; @@ -1617,28 +1619,43 @@ mod tests { /// The DNS-rebinding defense: a disallowed `Origin` is rejected with 403, /// an allowed one passes, and a missing one passes (non-browser clients). + /// Origins added via the `http_additional_cors_allowed_origins` system + /// variable are picked up without rebuilding the allowlist. #[mz_ore::test] fn test_validate_origin() { - let allowed = [HeaderValue::from_static("https://good.example")]; + use std::sync::RwLock; + + let additional: Arc>> = Default::default(); + let good = HeaderValue::from_static("https://good.example"); + let allowed = AllowedOrigins { + base: Arc::new(vec![good.clone()]), + additional: Arc::clone(&additional), + }; assert!(validate_origin(&HeaderMap::new(), &allowed).is_none()); let mut ok = HeaderMap::new(); - ok.insert(http::header::ORIGIN, allowed[0].clone()); + ok.insert(http::header::ORIGIN, good); assert!(validate_origin(&ok, &allowed).is_none()); - let mut bad = HeaderMap::new(); - bad.insert( + let mut extra = HeaderMap::new(); + extra.insert( http::header::ORIGIN, HeaderValue::from_static("https://evil.example"), ); - let rejected = validate_origin(&bad, &allowed); + let rejected = validate_origin(&extra, &allowed); assert_eq!( rejected .expect("disallowed origin must be rejected") .status(), StatusCode::FORBIDDEN, ); + + // Adding the origin via the shared handle (as the coordinator does + // when the system variable changes) allows it dynamically. + *additional.write().expect("lock poisoned") = + vec![HeaderValue::from_static("https://evil.example")]; + assert!(validate_origin(&extra, &allowed).is_none()); } /// The two constructors set the JSON-RPC version and put the payload in diff --git a/src/environmentd/src/lib.rs b/src/environmentd/src/lib.rs index cdf664becc385..95187c49eed37 100644 --- a/src/environmentd/src/lib.rs +++ b/src/environmentd/src/lib.rs @@ -69,7 +69,6 @@ use mz_server_core::{ use mz_sql::catalog::EnvironmentId; use mz_sql::session::vars::{Value, VarInput}; use tokio::sync::oneshot; -use tower_http::cors::AllowOrigin; use tracing::{Instrument, info, info_span}; use crate::deployment::preflight::{PreflightInput, PreflightOutput}; @@ -111,13 +110,11 @@ pub struct Config { pub frontegg: Option, /// Frontegg workspace URL advertised in MCP OAuth discovery. pub frontegg_oauth_issuer_url: Option, - /// Origins for which cross-origin resource sharing (CORS) for HTTP requests - /// is permitted. - pub cors_allowed_origin: AllowOrigin, - /// Raw list of allowed CORS origins. Retained alongside `cors_allowed_origin` - /// (which is the computed predicate) so that endpoints like MCP can perform - /// server-side Origin validation to defend against DNS rebinding attacks - /// (where same-origin requests bypass CORS enforcement). + /// Base allowlist of origins for which cross-origin resource sharing + /// (CORS) for HTTP requests is permitted. The + /// `http_additional_cors_allowed_origins` system variable can widen the + /// allowlist at runtime, but entries in this list can never be removed + /// through it. pub cors_allowed_origin_list: Vec, /// Public IP addresses which the cloud environment has configured for /// egress. @@ -396,6 +393,12 @@ impl Listeners { let mcp_metrics = http::mcp_metrics::McpMetrics::register_into(&metrics_registry); let oauth_metadata_metrics = http::oauth_metadata::OauthMetadataMetrics::register_into(&metrics_registry); + // Shared handle for origins from the + // `http_additional_cors_allowed_origins` system variable. The HTTP + // servers read it on every origin check, the coordinator updates it + // via `additional_cors_origins_callback` whenever the variable + // changes. + let additional_cors_origins: Arc>> = Default::default(); let mut http_listener_handles = BTreeMap::new(); for (name, listener) in self.http { let authenticator_kind = listener.config.authenticator_kind(); @@ -416,8 +419,8 @@ impl Listeners { authenticator_kind, frontegg: config.frontegg.clone(), oidc_rx: authenticator_oidc_rx.clone(), - allowed_origin: config.cors_allowed_origin.clone(), allowed_origin_list: config.cors_allowed_origin_list.clone(), + additional_allowed_origins: Arc::clone(&additional_cors_origins), concurrent_webhook_req: webhook_concurrency_limit.semaphore(), dyncfgs: Arc::clone(&config.system_dyncfgs), metrics: metrics.clone(), @@ -749,10 +752,17 @@ impl Listeners { connection_limiter.update_limit(limit); connection_limiter.update_superuser_reserved(superuser_reserved); }); + let additional_cors_origins_callback = Box::new(move |origins: String| { + let parsed = mz_http_util::parse_origin_list(&origins); + *additional_cors_origins + .write() + .expect("additional CORS origins lock poisoned") = parsed; + }); let (adapter_handle, adapter_client) = mz_adapter::serve(mz_adapter::Config { connection_context: config.controller.connection_context.clone(), connection_limit_callback, + additional_cors_origins_callback, controller_config: config.controller, controller_envd_epoch: envd_epoch, storage: adapter_storage, diff --git a/src/environmentd/src/test_util.rs b/src/environmentd/src/test_util.rs index b29bd530d32c3..b4f1af0718725 100644 --- a/src/environmentd/src/test_util.rs +++ b/src/environmentd/src/test_util.rs @@ -83,7 +83,6 @@ use tokio::runtime::Runtime; use tokio_postgres::config::{Host, SslMode}; use tokio_postgres::{AsyncMessage, Client}; use tokio_stream::wrappers::TcpListenerStream; -use tower_http::cors::AllowOrigin; use tracing::Level; use tracing_capture::SharedStorage; use tracing_subscriber::EnvFilter; @@ -879,7 +878,6 @@ impl Listeners { metrics_registry: metrics_registry.clone(), now: config.now, environment_id: config.environment_id, - cors_allowed_origin: AllowOrigin::list([]), cors_allowed_origin_list: Vec::new(), cluster_replica_sizes: ClusterReplicaSizeMap::for_tests(), bootstrap_default_cluster_replica_size: config.default_cluster_replica_size, diff --git a/src/http-util/src/lib.rs b/src/http-util/src/lib.rs index eeacb5d3c4593..417ec136cd53e 100644 --- a/src/http-util/src/lib.rs +++ b/src/http-util/src/lib.rs @@ -194,6 +194,18 @@ pub async fn handle_tracing() -> impl IntoResponse { ) } +/// Parses a comma-separated list of origins into header values, for use with +/// [`origin_is_allowed`]. Entries are trimmed. Empty entries and entries that +/// are not valid header values are skipped. +pub fn parse_origin_list(origins: &str) -> Vec { + origins + .split(',') + .map(|entry| entry.trim()) + .filter(|entry| !entry.is_empty()) + .filter_map(|entry| HeaderValue::from_str(entry).ok()) + .collect() +} + /// Returns true if `origin` matches any entry in `allowed`. Supports bare `*` /// (any origin), exact match, and wildcard subdomains (`*.example.com`). pub fn origin_is_allowed(origin: &HeaderValue, allowed: &[HeaderValue]) -> bool { @@ -272,6 +284,22 @@ mod tests { use tower::{Service, ServiceBuilder, ServiceExt}; use tower_http::cors::CorsLayer; + #[mz_ore::test] + fn test_parse_origin_list() { + let parsed = super::parse_origin_list( + " https://a.example ,, *.example.com , https://bad\u{7f}value , * ", + ); + assert_eq!( + parsed, + vec![ + HeaderValue::from_static("https://a.example"), + HeaderValue::from_static("*.example.com"), + HeaderValue::from_static("*"), + ] + ); + assert!(super::parse_origin_list("").is_empty()); + } + #[mz_ore::test(tokio::test)] async fn test_cors() { async fn test_request(cors: &CorsLayer, origin: &HeaderValue) -> Option { diff --git a/src/sql/src/session/vars.rs b/src/sql/src/session/vars.rs index a34499861cc15..e98e76a36acfe 100644 --- a/src/sql/src/session/vars.rs +++ b/src/sql/src/session/vars.rs @@ -1249,6 +1249,7 @@ impl SystemVars { &ENABLE_LAUNCHDARKLY, &MAX_CONNECTIONS, &NETWORK_POLICY, + &HTTP_ADDITIONAL_CORS_ALLOWED_ORIGINS, &SUPERUSER_RESERVED_CONNECTIONS, &KEEP_N_SOURCE_STATUS_HISTORY_ENTRIES, &KEEP_N_SINK_STATUS_HISTORY_ENTRIES, @@ -1439,6 +1440,7 @@ impl SystemVars { SESSION_SYSTEM_VARS.contains_key(UncasedStr::new(name)) || name == ENABLE_RBAC_CHECKS.name() || name == NETWORK_POLICY.name() + || name == HTTP_ADDITIONAL_CORS_ALLOWED_ORIGINS.name() } /// Returns a [`Var`] representing the configuration parameter with the @@ -2073,6 +2075,13 @@ impl SystemVars { self.expect_value::(&NETWORK_POLICY).clone() } + /// Returns the `http_additional_cors_allowed_origins` configuration + /// parameter. + pub fn http_additional_cors_allowed_origins(&self) -> String { + self.expect_value::(&HTTP_ADDITIONAL_CORS_ALLOWED_ORIGINS) + .clone() + } + /// Returns the `superuser_reserved_connections` configuration parameter. pub fn superuser_reserved_connections(&self) -> u32 { *self.expect_value(&SUPERUSER_RESERVED_CONNECTIONS) diff --git a/src/sql/src/session/vars/constraints.rs b/src/sql/src/session/vars/constraints.rs index f373fc559c430..0f766996b5a09 100644 --- a/src/sql/src/session/vars/constraints.rs +++ b/src/sql/src/session/vars/constraints.rs @@ -28,6 +28,8 @@ pub static NUMERIC_BOUNDED_0_1_INCLUSIVE: NumericInRange> = pub static BYTESIZE_AT_LEAST_1MB: ByteSizeInRange> = ByteSizeInRange(ByteSize::mb(1)..); +pub static CORS_ORIGIN_LIST: CorsOriginList = CorsOriginList; + #[derive(Debug)] pub enum ValueConstraint { /// Variable is read-only and cannot be updated. @@ -136,6 +138,35 @@ impl DomainConstraint for NonZeroDuration { } } +/// Validates a comma-separated list of CORS origins: every non-empty entry +/// must be a valid HTTP header value. Empty entries are permitted so that "" +/// (no additional origins) and trailing commas are accepted. +#[derive(Debug, Clone, Eq, PartialEq)] +pub struct CorsOriginList; + +impl DomainConstraint for CorsOriginList { + type Value = String; + + fn check(&self, var: &dyn Var, origins: &String) -> Result<(), VarError> { + let invalid_values: Vec = origins + .split(',') + .map(|entry| entry.trim()) + .filter(|entry| !entry.is_empty()) + .filter(|entry| http::HeaderValue::from_str(entry).is_err()) + .map(|entry| entry.to_string()) + .collect(); + if invalid_values.is_empty() { + Ok(()) + } else { + Err(VarError::InvalidParameterValue { + name: var.name(), + invalid_values, + reason: "origins must be valid HTTP header values".to_string(), + }) + } + } +} + #[derive(Debug, Clone, Eq, PartialEq)] pub struct NumericInRange(pub R); diff --git a/src/sql/src/session/vars/definitions.rs b/src/sql/src/session/vars/definitions.rs index 106939a6b315c..0822a7cb73ead 100644 --- a/src/sql/src/session/vars/definitions.rs +++ b/src/sql/src/session/vars/definitions.rs @@ -40,8 +40,8 @@ use uncased::UncasedStr; use crate::session::user::{SUPPORT_USER, SYSTEM_USER, User}; use crate::session::vars::constraints::{ - BYTESIZE_AT_LEAST_1MB, DomainConstraint, NON_ZERO_DURATION, NUMERIC_BOUNDED_0_1_INCLUSIVE, - NUMERIC_NON_NEGATIVE, ValueConstraint, + BYTESIZE_AT_LEAST_1MB, CORS_ORIGIN_LIST, DomainConstraint, NON_ZERO_DURATION, + NUMERIC_BOUNDED_0_1_INCLUSIVE, NUMERIC_NON_NEGATIVE, ValueConstraint, }; use crate::session::vars::errors::VarError; use crate::session::vars::polyfill::{LazyValueFn, lazy_value, value}; @@ -1524,6 +1524,21 @@ pub static NETWORK_POLICY: VarDefinition = VarDefinition::new_lazy( true, ); +/// Additional origins for which cross-origin resource sharing (CORS) is +/// allowed on the HTTP API, as a comma-separated list. These origins are +/// added to the allowlist provided at startup via `--cors-allowed-origin`. +/// Startup origins can never be removed through this variable, it only +/// widens the allowlist. Entries use the same syntax as the CLI flag: an +/// exact origin (`https://console.example.com`), a wildcard subdomain +/// (`*.example.com`), or a bare `*` allowing all origins. +pub static HTTP_ADDITIONAL_CORS_ALLOWED_ORIGINS: VarDefinition = VarDefinition::new_lazy( + "http_additional_cors_allowed_origins", + lazy_value!(String; || "".to_string()), + "Comma-separated list of additional origins for which cross-origin resource sharing (CORS) is allowed on the HTTP API, in addition to the origins provided at startup.", + true, +) +.with_constraint(&CORS_ORIGIN_LIST); + pub static FORCE_SOURCE_TABLE_SYNTAX: VarDefinition = VarDefinition::new( "force_source_table_syntax", value!(bool; false), @@ -2436,4 +2451,42 @@ mod tests { let features_for_item_parsing = OptimizerFeatures::from(&vars); assert_eq!(features_for_item_parsing, false_features); } + + /// `http_additional_cors_allowed_origins` is modifiable and visible to + /// external superusers, while dyncfg-backed variables stay internal-only. + #[mz_ore::test] + fn cors_origins_var_user_settable() { + use mz_repr::user::ExternalUserMetadata; + + use crate::session::user::User; + + let vars = SystemVars::new(); + + let name = HTTP_ADDITIONAL_CORS_ALLOWED_ORIGINS.name(); + assert!(vars.user_modifiable(name)); + assert!(!vars.user_modifiable("webhook_max_request_size_bytes")); + + let external_user = User { + name: "user".into(), + external_metadata: Some(ExternalUserMetadata { + user_id: uuid::Uuid::nil(), + admin: true, + }), + internal_metadata: None, + authenticator_kind: None, + groups: None, + }; + assert!( + vars.get(name) + .expect("var exists") + .visible(&external_user, &vars) + .is_ok() + ); + assert!( + vars.get("webhook_max_request_size_bytes") + .expect("var exists") + .visible(&external_user, &vars) + .is_err() + ); + } } diff --git a/src/sqllogictest/src/runner.rs b/src/sqllogictest/src/runner.rs index a86b2caf09e29..063e95535c252 100644 --- a/src/sqllogictest/src/runner.rs +++ b/src/sqllogictest/src/runner.rs @@ -104,7 +104,6 @@ use tokio::sync::oneshot; use tokio_postgres::types::{FromSql, Kind as PgKind, Type as PgType}; use tokio_postgres::{NoTls, Row, SimpleQueryMessage}; use tokio_stream::wrappers::TcpListenerStream; -use tower_http::cors::AllowOrigin; use tracing::{error, info}; use uuid::Uuid; use uuid::fmt::Simple; @@ -1227,7 +1226,6 @@ impl<'a> RunnerInner<'a> { tls: None, frontegg: None, frontegg_oauth_issuer_url: None, - cors_allowed_origin: AllowOrigin::list([]), cors_allowed_origin_list: Vec::new(), unsafe_mode: true, all_features: false, diff --git a/test/testdrive/session.td b/test/testdrive/session.td index 1572e4011965a..b746783dd89ab 100644 --- a/test/testdrive/session.td +++ b/test/testdrive/session.td @@ -35,6 +35,7 @@ enable_rbac_checks on "User facing gl enable_session_rbac_checks off "User facing session boolean flag indicating whether to apply RBAC checks before executing statements (Materialize)." extra_float_digits 3 "Adjusts the number of digits displayed for floating-point values (PostgreSQL)." failpoints "Allows failpoints to be dynamically activated." +http_additional_cors_allowed_origins "" "Comma-separated list of additional origins for which cross-origin resource sharing (CORS) is allowed on the HTTP API, in addition to the origins provided at startup." idle_in_transaction_session_timeout "2 min" "Sets the maximum allowed duration that a session can sit idle in a transaction before being terminated. If this value is specified without units, it is taken as milliseconds. A value of zero disables the timeout (PostgreSQL)." integer_datetimes on "Reports whether the server uses 64-bit-integer dates and times (PostgreSQL)." IntervalStyle postgres "Sets the display format for interval values (PostgreSQL)."