Skip to content

Commit 5e7ef3e

Browse files
ntkatholeclaudejyejare
authored
fix: Restrict UI server CORS to trusted origins (#6986)
* fix: Restrict UI server CORS to trusted origins The Feast UI server configured CORS with allow_origins=["*"] together with allow_credentials=True, so any website a user visited could issue authenticated cross-origin requests to the server and read the responses (CVE-2024-11602 / CWE-346, Origin Validation Error). The bundled Web UI is served from the same FastAPI app as the API, so it is same-origin and needs no permissive CORS policy. Disable CORS by default and let operators opt in to cross-origin access with an explicit allowlist of trusted origins. Following MLflow's convention, this is exposed as `feast ui --cors-allowed-origins` or the FEAST_UI_CORS_ALLOWED_ORIGINS environment variable (comma-separated). The wildcard origin is never combined with credentials again. Signed-off-by: ntkathole <[email protected]> * ci: Use pinned env for unit tests to fix protobuf gencode/runtime skew The unit-test-python jobs install an exact, mutually compatible dependency set via `uv pip sync` from the pinned pyXX-ci-requirements.txt (protobuf==7.36.2 alongside grpcio-health-checking==1.84.0, whose generated code targets protobuf gencode 7.35.1). The Test Python step then runs `uv run`, which auto-syncs the environment. Because uv.lock is not committed, that sync re-resolves the uncapped pyproject dependencies (`protobuf>=4.24.0`) and leaves the environment with protobuf 6.33.6 while grpcio's health_pb2 still targets gencode 7.35.1. Every test that imports grpc_health/v1/health.proto (registry/UI/MCP/feature-server servers) then fails at import time with: VersionError: ... grpc_health/v1/health.proto: gencode 7.35.1 runtime 6.33.6. Runtime version cannot be older than the linked gencode version. Set UV_NO_SYNC=1 for the step so `uv run` uses the already-installed, pinned environment instead of re-resolving. This is unrelated to the CORS change in this PR; it is a pre-existing CI hazard that surfaces for any test importing the health proto. Co-Authored-By: Claude Opus 4.8 <[email protected]> Signed-off-by: ntkathole <[email protected]> * ci: Disable uv auto-sync for all Makefile test targets The previous commit set UV_NO_SYNC only for the unit-test workflow step. The same protobuf gencode/runtime skew also breaks every other uv-based test workflow (integration-test-python-local, and the label-gated integration/remote-rbac/dbt/master/nightly jobs), because they all run `make install-python-dependencies-ci` (pinned `uv pip sync`) followed by a `make test-python-*` target that invokes `uv run`, which auto-syncs and re-resolves the uncapped pyproject deps. Export UV_NO_SYNC=1 from the Makefile so every `uv run` recipe uses the already-installed, pinned environment instead of re-resolving. pixi-based targets (ray/duckdb/registration) are unaffected. This generalizes the unit-test fix to all uv-based test targets in one place; the unit-test workflow keeps its step-level env because it also runs a standalone `uv run` outside make. Co-Authored-By: Claude Opus 4.8 <[email protected]> Signed-off-by: ntkathole <[email protected]> * fix: Warn on wildcard CORS origin and document origin format Address review feedback on the CORS allowlist: - When `*` is configured (an explicit, opt-in operator choice, like Feast's own documented `auth_type=no_auth`), keep honoring it but log a warning: with credentials, Starlette reflects the caller's origin for any site, so the wildcard is intended for local development only. This keeps the operator in control while making the risk visible instead of silent. - Document that each origin must be a bare scheme+host[:port] with no path or trailing slash, since browsers send the Origin header without a path and a value like `https://app.example.com/ui` would silently never match. - Add a unit test asserting `*` is accepted and warns. Co-Authored-By: Claude Opus 4.8 <[email protected]> Signed-off-by: ntkathole <[email protected]> --------- Signed-off-by: ntkathole <[email protected]> Co-authored-by: Claude Opus 4.8 <[email protected]> Co-authored-by: Jitendra Yejare <[email protected]>
1 parent f118b47 commit 5e7ef3e

8 files changed

Lines changed: 249 additions & 7 deletions

File tree

‎.github/workflows/unit_tests.yml‎

Lines changed: 9 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -57,6 +57,15 @@ jobs:
5757
- name: Install dependencies
5858
run: make install-python-dependencies-ci
5959
- name: Test Python
60+
env:
61+
# The exact dependency set is installed by `uv pip sync` from the
62+
# pinned pyXX-ci-requirements.txt above (compatible protobuf +
63+
# grpcio-health-checking). uv.lock is not committed, so letting
64+
# `uv run` auto-sync here would re-resolve the uncapped pyproject
65+
# deps and downgrade protobuf below grpcio's generated-code version,
66+
# breaking the protobuf gencode/runtime check at import time. Use the
67+
# already-installed environment instead of re-resolving.
68+
UV_NO_SYNC: "1"
6069
run: |
6170
# Set up environment for Ray workers to access packages
6271
export PATH="${{ github.workspace }}/.venv/bin:$PATH"

‎Makefile‎

Lines changed: 10 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -21,6 +21,16 @@ TOOL_DIR := $(ROOT_DIR)/tools
2121
export GOBIN=$(TOOL_DIR)/bin
2222
export PATH := $(TOOL_DIR)/bin:$(PATH)
2323

24+
# CI installs the exact, mutually compatible dependency set via
25+
# `uv pip sync` from the pinned sdk/python/requirements/pyXX-ci-requirements.txt
26+
# (install-python-dependencies-ci). uv.lock is not committed, so a plain
27+
# `uv run` would auto-sync and re-resolve the uncapped pyproject dependencies,
28+
# which can install an inconsistent set -- e.g. protobuf older than the gencode
29+
# baked into grpcio's health_pb2, which then fails the protobuf gencode/runtime
30+
# check at import time. Run every `uv run` target against the already-installed
31+
# environment instead of re-resolving. (pixi-based targets are unaffected.)
32+
export UV_NO_SYNC := 1
33+
2434
MVN := mvn -f java/pom.xml ${MAVEN_EXTRA_OPTS}
2535
OS := linux
2636
ifeq ($(shell uname -s), Darwin)

‎docs/reference/alpha-web-ui.md‎

Lines changed: 34 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -30,11 +30,45 @@ Options:
3030
-h, --host TEXT Specify a host for the server [default: 0.0.0.0]
3131
-p, --port INTEGER Specify a port for the server [default: 8888]
3232
-r, --registry_ttl_sec INTEGER Number of seconds after which the registry is refreshed. Default is 5 seconds.
33+
--cors-allowed-origins TEXT Comma-separated list of trusted origins allowed to make cross-origin (CORS) requests.
3334
--help Show this message and exit.
3435
```
3536

3637
This will spin up a Web UI on localhost which automatically refreshes its view of the registry every `registry_ttl_sec`
3738

39+
#### Cross-Origin Resource Sharing (CORS)
40+
41+
By default the UI server does **not** allow cross-origin requests: the bundled
42+
Web UI is served from the same origin as the API, so it keeps working without
43+
any CORS policy. This avoids [CVE-2024-11602](https://nvd.nist.gov/vuln/detail/CVE-2024-11602),
44+
where the server previously reflected every origin (`Access-Control-Allow-Origin: *`)
45+
together with credentials, letting any site a user visited issue authenticated
46+
requests to the server.
47+
48+
If you host the UI (or another browser client) on a different origin, opt in by
49+
passing an explicit allowlist of trusted origins, either with the
50+
`--cors-allowed-origins` flag or the `FEAST_UI_CORS_ALLOWED_ORIGINS`
51+
environment variable:
52+
53+
```bash
54+
feast ui --cors-allowed-origins "https://feast.example.com,https://app.example.com"
55+
56+
# or, equivalently:
57+
export FEAST_UI_CORS_ALLOWED_ORIGINS="https://feast.example.com,https://app.example.com"
58+
feast ui
59+
```
60+
61+
Each value must be a bare origin — scheme and host with an optional port, and
62+
**no path, query, or trailing slash** (e.g. `https://app.example.com` or
63+
`http://localhost:3000`, not `https://app.example.com/ui`). Browsers send the
64+
`Origin` header without a path, so a value that includes one never matches and
65+
the request is silently rejected.
66+
67+
For production, list the exact origins you trust. Passing `*` is accepted but
68+
**allows credentialed cross-origin requests from any site** (the CVE-2024-11602
69+
pattern) and is intended for local development only; the server logs a warning
70+
when it is used.
71+
3872
#### Curl Generator Feature Server URL
3973

4074
The Curl Generator uses a default Feature Server URL when building the example curl command. You can configure

‎sdk/python/feast/cli/ui.py‎

Lines changed: 21 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -44,6 +44,22 @@
4444
show_default=False,
4545
help="path to TLS(SSL) certificate public key. You need to pass --key arg as well to start server in TLS mode",
4646
)
47+
@click.option(
48+
"--cors-allowed-origins",
49+
"cors_allowed_origins",
50+
type=click.STRING,
51+
default="",
52+
envvar="FEAST_UI_CORS_ALLOWED_ORIGINS",
53+
show_default=False,
54+
help=(
55+
"Comma-separated list of trusted origins allowed to make cross-origin "
56+
"(CORS) requests to the UI server, e.g. "
57+
"'https://feast.example.com,https://app.example.com'. May also be set "
58+
"via the FEAST_UI_CORS_ALLOWED_ORIGINS environment variable. By default "
59+
"no cross-origin requests are allowed. Avoid '*', which combined with "
60+
"credentials exposes the server to CVE-2024-11602."
61+
),
62+
)
4763
@click.pass_context
4864
def ui(
4965
ctx: click.Context,
@@ -52,6 +68,7 @@ def ui(
5268
root_path: str = "",
5369
tls_key_path: str = "",
5470
tls_cert_path: str = "",
71+
cors_allowed_origins: str = "",
5572
):
5673
"""
5774
Shows the Feast UI over the current directory
@@ -60,11 +77,15 @@ def ui(
6077
raise click.BadParameter(
6178
"Please configure --key and --cert args to start the feature server in SSL mode."
6279
)
80+
cors_allowed_origins_list = [
81+
origin.strip() for origin in cors_allowed_origins.split(",") if origin.strip()
82+
]
6383
store = create_feature_store(ctx)
6484
store.serve_ui(
6585
host=host,
6686
port=port,
6787
root_path=root_path,
6888
tls_key_path=tls_key_path,
6989
tls_cert_path=tls_cert_path,
90+
cors_origins=cors_allowed_origins_list,
7091
)

‎sdk/python/feast/feature_store.py‎

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -4794,6 +4794,7 @@ def serve_ui(
47944794
root_path: str = "",
47954795
tls_key_path: str = "",
47964796
tls_cert_path: str = "",
4797+
cors_origins: Optional[List[str]] = None,
47974798
) -> None:
47984799
"""Start the UI server locally"""
47994800
if flags_helper.is_test():
@@ -4810,6 +4811,7 @@ def serve_ui(
48104811
root_path=root_path,
48114812
tls_key_path=tls_key_path,
48124813
tls_cert_path=tls_cert_path,
4814+
cors_origins=cors_origins,
48134815
)
48144816

48154817
def serve_registry(

‎sdk/python/feast/ui_server.py‎

Lines changed: 30 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -874,16 +874,37 @@ def get_app(
874874
store: "feast.FeatureStore",
875875
project_id: str,
876876
root_path: str = "",
877+
cors_origins: Optional[List[str]] = None,
877878
):
878879
app = FastAPI()
879880

880-
app.add_middleware(
881-
CORSMiddleware,
882-
allow_origins=["*"],
883-
allow_credentials=True,
884-
allow_methods=["*"],
885-
allow_headers=["*"],
886-
)
881+
# CORS is only required for cross-origin browsers calling the API. The UI is
882+
# served from this same app (same origin), so it keeps working without any
883+
# permissive CORS policy. Allowing every origin together with credentials
884+
# (``allow_origins=["*"]`` + ``allow_credentials=True``) lets any website a
885+
# user visits issue authenticated cross-origin requests to this server
886+
# (CVE-2024-11602 / CWE-346), so it is disabled by default. Operators who
887+
# need cross-origin access must opt in with an explicit list of trusted
888+
# origins via ``feast ui --cors-allowed-origins``.
889+
if cors_origins:
890+
if "*" in cors_origins:
891+
# Honor an explicit wildcard (an informed, opt-in operator choice,
892+
# like Feast's own ``auth_type=no_auth``), but make the risk loud:
893+
# with credentials, Starlette reflects the caller's origin for any
894+
# site, which is the CVE-2024-11602 pattern. Not a safe default.
895+
logger.warning(
896+
"CORS is configured to allow all origins ('*'). This permits "
897+
"credentialed cross-origin requests from any site and is "
898+
"intended for development only. Configure an explicit list of "
899+
"trusted origins for production deployments."
900+
)
901+
app.add_middleware(
902+
CORSMiddleware,
903+
allow_origins=cors_origins,
904+
allow_credentials=True,
905+
allow_methods=["*"],
906+
allow_headers=["*"],
907+
)
887908

888909
_setup_rest_mode(app, store)
889910

@@ -1207,11 +1228,13 @@ def start_server(
12071228
root_path: str = "",
12081229
tls_key_path: str = "",
12091230
tls_cert_path: str = "",
1231+
cors_origins: Optional[List[str]] = None,
12101232
):
12111233
app = get_app(
12121234
store,
12131235
project_id,
12141236
root_path,
1237+
cors_origins=cors_origins,
12151238
)
12161239

12171240
logger.info(f"Starting Feast UI server on {host}:{port}")
Lines changed: 53 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,53 @@
1+
from unittest.mock import MagicMock
2+
3+
from click.testing import CliRunner
4+
5+
from feast.cli.ui import ui
6+
7+
8+
def _run(mocker, args, env=None):
9+
mock_store = MagicMock()
10+
mocker.patch("feast.cli.ui.create_feature_store", return_value=mock_store)
11+
result = CliRunner().invoke(ui, args, obj={}, env=env)
12+
assert result.exit_code == 0, result.output
13+
return mock_store
14+
15+
16+
def test_cors_allowed_origins_defaults_to_empty(mocker):
17+
"""Without the flag, no origins are forwarded (CORS stays disabled)."""
18+
store = _run(mocker, [])
19+
store.serve_ui.assert_called_once()
20+
assert store.serve_ui.call_args.kwargs["cors_origins"] == []
21+
22+
23+
def test_cors_allowed_origins_flag_is_split(mocker):
24+
"""The comma-separated flag is parsed into a trimmed list of origins."""
25+
store = _run(
26+
mocker,
27+
[
28+
"--cors-allowed-origins",
29+
"https://feast.example.com, https://app.example.com",
30+
],
31+
)
32+
assert store.serve_ui.call_args.kwargs["cors_origins"] == [
33+
"https://feast.example.com",
34+
"https://app.example.com",
35+
]
36+
37+
38+
def test_cors_allowed_origins_from_env_var(mocker):
39+
"""FEAST_UI_CORS_ALLOWED_ORIGINS is honored when the flag is absent."""
40+
store = _run(
41+
mocker,
42+
[],
43+
env={"FEAST_UI_CORS_ALLOWED_ORIGINS": "https://feast.example.com"},
44+
)
45+
assert store.serve_ui.call_args.kwargs["cors_origins"] == [
46+
"https://feast.example.com"
47+
]
48+
49+
50+
def test_help_advertises_cors_flag():
51+
result = CliRunner().invoke(ui, ["--help"])
52+
assert "--cors-allowed-origins" in result.output
53+
assert "FEAST_UI_CORS_ALLOWED_ORIGINS" in result.output

‎sdk/python/tests/unit/test_ui_server.py‎

Lines changed: 90 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -326,6 +326,96 @@ def test_registry_refresh_endpoint_error(mock_feature_store):
326326
assertpy.assert_that(resp.status_code).is_equal_to(500)
327327

328328

329+
# ---------- CORS tests (CVE-2024-11602) ----------
330+
331+
332+
@contextlib.contextmanager
333+
def _cors_client(mock_feature_store, cors_origins):
334+
"""Yield a TestClient for a UI app built with the given cors_origins.
335+
336+
The static-files directory is only valid while the TemporaryDirectory is
337+
open, so the client must be used inside this context.
338+
"""
339+
mock_registry = MagicMock()
340+
mock_registry.list_projects.return_value = []
341+
mock_feature_store.registry = mock_registry
342+
343+
with tempfile.TemporaryDirectory() as temp_dir:
344+
_create_mock_ui_files(temp_dir)
345+
346+
with _setup_importlib_mocks(temp_dir):
347+
app = get_app(
348+
mock_feature_store,
349+
TEST_PROJECT_NAME,
350+
cors_origins=cors_origins,
351+
)
352+
yield TestClient(app)
353+
354+
355+
def test_cors_disabled_by_default(mock_feature_store):
356+
"""Without cors_origins, no cross-origin request is granted.
357+
358+
Regression test for CVE-2024-11602: the server used to send
359+
``Access-Control-Allow-Origin: *`` with credentials, letting any website a
360+
user visits read authenticated responses from this server. The CVE's
361+
proof of concept is exactly this -- a cross-origin GET carrying an Origin
362+
header.
363+
"""
364+
with _cors_client(mock_feature_store, cors_origins=None) as client:
365+
response = client.get("/health", headers={"Origin": "https://evil.example.com"})
366+
assertpy.assert_that(response.status_code).is_equal_to(EXPECTED_SUCCESS_STATUS)
367+
assertpy.assert_that(response.headers).does_not_contain_key(
368+
"access-control-allow-origin"
369+
)
370+
371+
372+
def test_cors_allows_only_configured_origins(mock_feature_store):
373+
"""With an explicit allowlist, only trusted origins are echoed back."""
374+
trusted = "https://feast.example.com"
375+
with _cors_client(mock_feature_store, cors_origins=[trusted]) as client:
376+
allowed = client.get("/health", headers={"Origin": trusted})
377+
assertpy.assert_that(
378+
allowed.headers.get("access-control-allow-origin")
379+
).is_equal_to(trusted)
380+
381+
denied = client.get("/health", headers={"Origin": "https://evil.example.com"})
382+
assertpy.assert_that(denied.headers).does_not_contain_key(
383+
"access-control-allow-origin"
384+
)
385+
386+
387+
def test_cors_never_uses_wildcard(mock_feature_store):
388+
"""An untrusted origin is never reflected even when an allowlist is set."""
389+
with _cors_client(
390+
mock_feature_store, cors_origins=["https://feast.example.com"]
391+
) as client:
392+
response = client.get(
393+
"/health", headers={"Origin": "https://attacker.example.com"}
394+
)
395+
assertpy.assert_that(
396+
response.headers.get("access-control-allow-origin")
397+
).is_none()
398+
399+
400+
def test_wildcard_cors_origin_is_allowed_but_warns(mock_feature_store, caplog):
401+
"""An explicit '*' is honored (informed opt-in) but logs a warning.
402+
403+
We intentionally do not reject '*': it is a deliberate, documented operator
404+
choice for local development (mirroring Feast's own `auth_type=no_auth`).
405+
The warning makes the risk visible so it can't be set unknowingly.
406+
"""
407+
import logging
408+
409+
with caplog.at_level(logging.WARNING, logger="feast.ui_server"):
410+
with _cors_client(mock_feature_store, cors_origins=["*"]) as client:
411+
response = client.get(
412+
"/health", headers={"Origin": "https://any.example.com"}
413+
)
414+
415+
assertpy.assert_that(response.status_code).is_equal_to(EXPECTED_SUCCESS_STATUS)
416+
assertpy.assert_that(caplog.text.lower()).contains("allow all origins")
417+
418+
329419
@patch("feast.ui_server.uvicorn")
330420
@patch("feast.ui_server.get_app")
331421
def test_start_server_dual_stack_binds_prebuilt_socket(mock_get_app, mock_uvicorn):

0 commit comments

Comments
 (0)