This is an automated email from the ASF dual-hosted git repository.
vatsrahul1001 pushed a commit to branch main
in repository https://gitbox.apache.org/repos/asf/airflow.git
The following commit(s) were added to refs/heads/main by this push:
new 448f8462919 Harden _collect_teams_to_check / requires_access_backfill
against malformed bodies (#66504)
448f8462919 is described below
commit 448f84629192d640147cb5825b9f5e5d874dd2e4
Author: Jarek Potiuk <[email protected]>
AuthorDate: Tue May 19 15:38:49 2026 +0200
Harden _collect_teams_to_check / requires_access_backfill against malformed
bodies (#66504)
* Fail closed in _collect_teams_to_check on body parse failure
For POST/PUT in multi-team mode, the helper used `with
suppress(JSONDecodeError)`
around `await request.json()`. If the body was unparseable, the suppress
swallowed the exception, `teams.add(raw)` never ran, and the calling
`requires_access_*` dependency iterated over an empty set — silently
skipping the authorization callback entirely.
Today this is unreachable because every POST/PUT route in core_api uses a
Pydantic body model, so FastAPI returns 422 before the auth dependency
runs. But the pattern would silently bypass team-scoped authz if a future
route used a raw `Request` instead. Replace the bare suppress with an
explicit try/except that adds `None` to `teams` on parse failure, so the
auth callback always runs at least once.
* Reject malformed bodies in core_api authz helpers with 400
Builds on the previous fail-closed change in _collect_teams_to_check.
Two follow-ups from review:
* On JSONDecodeError, raise HTTP 400 directly instead of falling through
to a team=None auth call — clearer failure mode and removes any
ambiguity about whether authz ran.
* Reject non-string `team_name` (in _collect_teams_to_check) and
non-string `dag_id` (in requires_access_backfill) from the raw body
with HTTP 400 before any authz decision or DB lookup. Without this,
a list / dict / int / bool would flow into Team.get_name_if_exists,
requires_access_dag, or the existence lookup with undefined behaviour
or type-confused authz decisions.
Both helpers still read the raw body before Pydantic body validation
runs on the endpoint handler, so this is defense-in-depth: every current
POST/PUT route uses a Pydantic body model and FastAPI returns 422 before
the auth dependency runs on a malformed body.
Tests: existing parse-failure test renamed and updated to assert 400;
new parametrised tests cover integer / list / dict / bool inputs for
both team_name and dag_id.
---
.../src/airflow/api_fastapi/core_api/security.py | 39 ++++++++---
.../unit/api_fastapi/core_api/test_security.py | 79 ++++++++++++++++++++++
2 files changed, 109 insertions(+), 9 deletions(-)
diff --git a/airflow-core/src/airflow/api_fastapi/core_api/security.py
b/airflow-core/src/airflow/api_fastapi/core_api/security.py
index cf4018b01ae..5ea8821ccea 100644
--- a/airflow-core/src/airflow/api_fastapi/core_api/security.py
+++ b/airflow-core/src/airflow/api_fastapi/core_api/security.py
@@ -338,7 +338,15 @@ def requires_access_backfill(
if dag_id is None:
# Not a json body, ignore
with suppress(JSONDecodeError):
- dag_id = (await request.json()).get("dag_id")
+ body = await request.json()
+ if isinstance(body, dict):
+ dag_id = body.get("dag_id")
+ if dag_id is not None and not isinstance(dag_id, str):
+ # Fail closed: reject non-string dag_id before authz decision.
+ raise HTTPException(
+ status_code=status.HTTP_400_BAD_REQUEST,
+ detail="'dag_id' must be a string",
+ )
requires_access_dag(method, DagAccessEntity.RUN, dag_id)(
request,
@@ -786,14 +794,27 @@ async def _collect_teams_to_check(
if method != "POST":
teams.add(get_existing_team(resource_id) if resource_id else None)
if method in ("POST", "PUT"):
- with suppress(JSONDecodeError):
- raw = (await request.json()).get("team_name")
- if raw and not Team.get_name_if_exists(raw):
- raise HTTPException(
- status_code=status.HTTP_400_BAD_REQUEST,
- detail=f"Team {raw!r} does not exist",
- )
- teams.add(raw)
+ try:
+ body = await request.json()
+ except JSONDecodeError:
+ # Fail closed: reject unparsable bodies before any authz decision.
+ raise HTTPException(
+ status_code=status.HTTP_400_BAD_REQUEST,
+ detail="Request body is not valid JSON",
+ )
+ raw = body.get("team_name") if isinstance(body, dict) else None
+ if raw is not None and not isinstance(raw, str):
+ # Fail closed: reject non-string team_name before authz / DB
lookup.
+ raise HTTPException(
+ status_code=status.HTTP_400_BAD_REQUEST,
+ detail="'team_name' must be a string",
+ )
+ if raw and not Team.get_name_if_exists(raw):
+ raise HTTPException(
+ status_code=status.HTTP_400_BAD_REQUEST,
+ detail=f"Team {raw!r} does not exist",
+ )
+ teams.add(raw)
return teams
diff --git a/airflow-core/tests/unit/api_fastapi/core_api/test_security.py
b/airflow-core/tests/unit/api_fastapi/core_api/test_security.py
index 7fff7260854..b19f774f28a 100644
--- a/airflow-core/tests/unit/api_fastapi/core_api/test_security.py
+++ b/airflow-core/tests/unit/api_fastapi/core_api/test_security.py
@@ -16,6 +16,7 @@
# under the License.
from __future__ import annotations
+from json import JSONDecodeError
from unittest.mock import AsyncMock, Mock, patch
import pytest
@@ -676,6 +677,84 @@ class TestFastApiSecurity:
assert exc_info.value.status_code == 400
assert "nonexistent" in exc_info.value.detail
+ @pytest.mark.db_test
+ @pytest.mark.parametrize("method", ["POST", "PUT"])
+ @patch.object(Team, "get_name_if_exists")
+ @patch.object(Connection, "get_team_name")
+ @patch("airflow.api_fastapi.core_api.security.get_auth_manager")
+ async def test_requires_access_connection_body_parse_failure_returns_400(
+ self, mock_get_auth_manager, mock_get_team_name,
mock_get_name_if_exists, method
+ ):
+ """If the request body cannot be parsed, fail closed with 400 before
any authz check."""
+ auth_manager = Mock()
+ auth_manager.is_authorized_connection.return_value = False
+ mock_get_auth_manager.return_value = auth_manager
+ mock_get_team_name.return_value = None
+ fastapi_request = Mock()
+ fastapi_request.path_params = {"connection_id": "conn_id"}
+ fastapi_request.json =
AsyncMock(side_effect=JSONDecodeError("expecting value", "", 0))
+ user = Mock()
+
+ with conf_vars({("core", "multi_team"): "True"}):
+ with pytest.raises(HTTPException) as exc_info:
+ await requires_access_connection(method)(fastapi_request, user)
+
+ assert exc_info.value.status_code == 400
+ auth_manager.is_authorized_connection.assert_not_called()
+
+ @pytest.mark.db_test
+ @pytest.mark.parametrize("method", ["POST", "PUT"])
+ @pytest.mark.parametrize("bad_team_name", [123, ["x"], {"name": "x"},
True])
+ @patch.object(Team, "get_name_if_exists")
+ @patch.object(Connection, "get_team_name")
+ @patch("airflow.api_fastapi.core_api.security.get_auth_manager")
+ async def test_requires_access_connection_non_string_team_name_returns_400(
+ self,
+ mock_get_auth_manager,
+ mock_get_team_name,
+ mock_get_name_if_exists,
+ bad_team_name,
+ method,
+ ):
+ """Non-string team_name in the body must be rejected with 400 before
authz / DB lookup."""
+ auth_manager = Mock()
+ auth_manager.is_authorized_connection.return_value = False
+ mock_get_auth_manager.return_value = auth_manager
+ mock_get_team_name.return_value = None
+ fastapi_request = Mock()
+ fastapi_request.path_params = {"connection_id": "conn_id"}
+ fastapi_request.json = AsyncMock(return_value={"team_name":
bad_team_name})
+ user = Mock()
+
+ with conf_vars({("core", "multi_team"): "True"}):
+ with pytest.raises(HTTPException) as exc_info:
+ await requires_access_connection(method)(fastapi_request, user)
+
+ assert exc_info.value.status_code == 400
+ assert "team_name" in exc_info.value.detail
+ auth_manager.is_authorized_connection.assert_not_called()
+ mock_get_name_if_exists.assert_not_called()
+
+ @pytest.mark.db_test
+ @pytest.mark.parametrize("bad_dag_id", [123, ["x"], {"name": "x"}, True])
+ @patch("airflow.api_fastapi.core_api.security.requires_access_dag")
+ async def test_requires_access_backfill_non_string_dag_id_returns_400(
+ self, mock_requires_access_dag, bad_dag_id
+ ):
+ """Non-string dag_id in the body must be rejected with 400 before
authz."""
+ fastapi_request = Mock()
+ fastapi_request.path_params = {}
+ fastapi_request.json = AsyncMock(return_value={"dag_id": bad_dag_id})
+ user = Mock()
+ session = Mock()
+
+ with pytest.raises(HTTPException) as exc_info:
+ await requires_access_backfill("POST")(fastapi_request, user,
session)
+
+ assert exc_info.value.status_code == 400
+ assert "dag_id" in exc_info.value.detail
+ mock_requires_access_dag.assert_not_called()
+
@patch.object(Connection, "get_conn_id_to_team_name_mapping")
@patch("airflow.api_fastapi.core_api.security.get_auth_manager")
def test_requires_access_connection_bulk(
