This is an automated email from the ASF dual-hosted git repository.
pierrejeambrun 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 02dbd2f6170 Stop backfill endpoints disclosing ids across Dags you
cannot see (#71113)
02dbd2f6170 is described below
commit 02dbd2f6170448bcd8bf83e83617fe84c1bddb21
Author: rjgoyln <[email protected]>
AuthorDate: Fri Aug 28 00:33:54 2026 +0800
Stop backfill endpoints disclosing ids across Dags you cannot see (#71113)
* Stop backfill endpoints disclosing which backfill ids exist across Dags
The routes that name a backfill in their path resolved the authorization
subject from the request when the id matched no row, so a caller could tell
"no such backfill" apart from "a backfill you may not see" and enumerate ids
belonging to Dags they have no access to.
The path names the backfill, so nothing the caller supplies alongside it can
be the subject, and an id they may not see has to answer exactly as a
missing
one does. A caller who may read the Dag keeps the Forbidden answer: they can
already list that Dag's backfills, so hiding it would only cost them the
reason their request was refused.
closes: #71080
* Add newsfragment for the backfill authorization change
* Give every backfill route one answer for an id that is not there
For an id named in the path the access dependency now answers before the
handler
does, so the three different messages the handlers used to describe that
single
condition are no longer what a caller sees. A client matching on the detail
string
would otherwise have to know which route it hit, and which of the two places
answered.
---
airflow-core/newsfragments/71113.significant.rst | 23 +++++++
.../core_api/routes/public/backfills.py | 16 +++--
.../src/airflow/api_fastapi/core_api/security.py | 69 +++++++++++++-------
.../core_api/routes/public/test_backfills.py | 64 +++++++++++++++++-
.../unit/api_fastapi/core_api/test_security.py | 75 +++++++++++++---------
5 files changed, 188 insertions(+), 59 deletions(-)
diff --git a/airflow-core/newsfragments/71113.significant.rst
b/airflow-core/newsfragments/71113.significant.rst
new file mode 100644
index 00000000000..1770df4a8a2
--- /dev/null
+++ b/airflow-core/newsfragments/71113.significant.rst
@@ -0,0 +1,23 @@
+Backfill endpoints no longer disclose which backfill ids exist across Dags
+
+The five routes that name a backfill in their path -- ``GET
/backfills/{backfill_id}``,
+``GET /backfills/{backfill_id}/dag_runs`` and the ``pause``, ``unpause`` and
``cancel``
+routes -- resolved the Dag they authorize against from the ``dag_id`` supplied
on the
+request whenever the path's id matched no row. An unknown id and a backfill on
a Dag the
+caller cannot see therefore answered differently, which enumerates backfill
ids across Dags.
+
+The backfill named in the path is now the only thing those routes authorize
against.
+
+**Behaviour changes:**
+
+- A backfill whose Dag the caller may not read returns ``404`` with detail
+ ``Backfill not found``, the same answer an unknown id gets, where it
previously returned
+ ``403``. A caller who may read the Dag still gets ``403`` for a write they
are not allowed.
+- A ``backfill_id`` in the path is never authorized against a ``dag_id`` in
the request body
+ or query string. ``GET /backfills``, ``POST /backfills`` and ``POST
/backfills/dry_run``
+ name no backfill in their path and keep authorizing off the request.
+- All five routes now answer an unknown id with the same detail, ``Backfill
not found``.
+ ``GET /backfills/{backfill_id}/dag_runs`` previously answered
+ ``Backfill with id {backfill_id} not found`` and the ``pause``, ``unpause``
and ``cancel``
+ routes ``Could not find backfill with id {backfill_id}``. Clients matching
on ``detail``
+ must be updated.
diff --git
a/airflow-core/src/airflow/api_fastapi/core_api/routes/public/backfills.py
b/airflow-core/src/airflow/api_fastapi/core_api/routes/public/backfills.py
index a814da2d943..01a5928eb82 100644
--- a/airflow-core/src/airflow/api_fastapi/core_api/routes/public/backfills.py
+++ b/airflow-core/src/airflow/api_fastapi/core_api/routes/public/backfills.py
@@ -44,7 +44,11 @@ from airflow.api_fastapi.core_api.datamodels.backfills
import (
from airflow.api_fastapi.core_api.openapi.exceptions import (
create_openapi_http_exception_doc,
)
-from airflow.api_fastapi.core_api.security import GetUserDep,
requires_access_backfill
+from airflow.api_fastapi.core_api.security import (
+ BACKFILL_NOT_FOUND,
+ GetUserDep,
+ requires_access_backfill,
+)
from airflow.api_fastapi.logging.decorators import action_logging
from airflow.exceptions import DagNotFound, DagRunTypeNotAllowed
from airflow.models import DagRun
@@ -126,7 +130,7 @@ def get_backfill(
).one_or_none()
if backfill:
return backfill
- raise HTTPException(status.HTTP_404_NOT_FOUND, "Backfill not found")
+ raise HTTPException(status.HTTP_404_NOT_FOUND, BACKFILL_NOT_FOUND)
@backfills_router.get(
@@ -149,7 +153,7 @@ def list_backfill_dag_runs(
"""List Dag runs associated with a backfill, including skipped slots."""
backfill = session.get(Backfill, backfill_id)
if not backfill:
- raise HTTPException(status.HTTP_404_NOT_FOUND, f"Backfill with id
{backfill_id} not found")
+ raise HTTPException(status.HTTP_404_NOT_FOUND, BACKFILL_NOT_FOUND)
select_stmt, total_entries = paginated_select(
statement=select(BackfillDagRun)
@@ -185,7 +189,7 @@ def pause_backfill(backfill_id: NonNegativeInt, session:
SessionDep) -> Backfill
select(Backfill).where(Backfill.id ==
backfill_id).options(joinedload(Backfill.dag_model))
).one_or_none()
if not b:
- raise HTTPException(status.HTTP_404_NOT_FOUND, f"Could not find
backfill with id {backfill_id}")
+ raise HTTPException(status.HTTP_404_NOT_FOUND, BACKFILL_NOT_FOUND)
if b.completed_at:
raise HTTPException(status.HTTP_409_CONFLICT, "Backfill is already
completed.")
if b.is_paused is False:
@@ -211,7 +215,7 @@ def unpause_backfill(backfill_id: NonNegativeInt, session:
SessionDep) -> Backfi
select(Backfill).where(Backfill.id ==
backfill_id).options(joinedload(Backfill.dag_model))
).one_or_none()
if not b:
- raise HTTPException(status.HTTP_404_NOT_FOUND, f"Could not find
backfill with id {backfill_id}")
+ raise HTTPException(status.HTTP_404_NOT_FOUND, BACKFILL_NOT_FOUND)
if b.completed_at:
raise HTTPException(status.HTTP_409_CONFLICT, "Backfill is already
completed.")
if b.is_paused:
@@ -237,7 +241,7 @@ def cancel_backfill(backfill_id: NonNegativeInt, session:
SessionDep) -> Backfil
select(Backfill).where(Backfill.id ==
backfill_id).options(joinedload(Backfill.dag_model))
).one_or_none()
if not b:
- raise HTTPException(status.HTTP_404_NOT_FOUND, f"Could not find
backfill with id {backfill_id}")
+ raise HTTPException(status.HTTP_404_NOT_FOUND, BACKFILL_NOT_FOUND)
if b.completed_at is not None:
raise HTTPException(status.HTTP_409_CONFLICT, "Backfill is already
completed.")
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 9aa1ddc783f..04472a8d303 100644
--- a/airflow-core/src/airflow/api_fastapi/core_api/security.py
+++ b/airflow-core/src/airflow/api_fastapi/core_api/security.py
@@ -417,6 +417,32 @@ ReadableBackfillsFilterDep = Annotated[
# does; see the comment there for why any divergence is a cross-Dag
authorization bypass.
_BACKFILL_ID_ADAPTER: TypeAdapter[NonNegativeInt] = TypeAdapter(NonNegativeInt)
+# Shared with the backfill routes: for an id named in the path this dependency
answers before
+# the handler does, so the two must not describe the same condition
differently.
+BACKFILL_NOT_FOUND = "Backfill not found"
+
+
+def _authorize_backfill_in_path(method: ResourceMethod, dag_id: str | None,
user: BaseUser) -> None:
+ """Authorize a backfill named by the request path against that backfill's
Dag alone."""
+ # ``dag_id`` is None when the id matched no row. Answering 404 there while
a backfill on a Dag
+ # the caller may not read answers 403 would tell them which backfill ids
exist across Dags.
+ if dag_id is None:
+ raise HTTPException(status.HTTP_404_NOT_FOUND, BACKFILL_NOT_FOUND)
+
+ details = DagDetails(id=dag_id, team_name=DagModel.get_team_name(dag_id))
+ auth_manager = get_auth_manager()
+ if auth_manager.is_authorized_dag(
+ method=method, access_entity=DagAccessEntity.RUN, details=details,
user=user
+ ):
+ return
+ # A caller who may read the Dag can already list its backfills, so the id
is no secret from
+ # them: hiding it would only cost them the reason their request was
refused.
+ if method != "GET" and auth_manager.is_authorized_dag(
+ method="GET", access_entity=DagAccessEntity.RUN, details=details,
user=user
+ ):
+ raise HTTPException(status.HTTP_403_FORBIDDEN, "Forbidden")
+ raise HTTPException(status.HTTP_404_NOT_FOUND, BACKFILL_NOT_FOUND)
+
def requires_access_backfill(
method: ResourceMethod,
@@ -428,9 +454,6 @@ def requires_access_backfill(
user: GetUserDep,
session: SessionDep,
) -> None:
- dag_id = None
-
- # Try to retrieve the dag_id from the backfill_id path param
backfill_id_raw = request.path_params.get("backfill_id")
try:
# Must parse exactly as the handler does (e.g. pydantic's lax mode
coerces "1.0" to 1
@@ -444,26 +467,26 @@ def requires_access_backfill(
backfill_id = None
if backfill_id is not None:
- backfill = session.scalars(select(Backfill).where(Backfill.id ==
backfill_id)).one_or_none()
- dag_id = backfill.dag_id if backfill else None
-
- # Try to retrieve the dag_id from the request body (POST backfill)
- # TODO: a backfill_id that parses but matches no row also lands here,
so an unknown
- # backfill is authorized against the body's dag_id and answers 404
where an unauthorized
- # one answers 403 - disclosing which ids exist. Not exploitable for a
cross-Dag action;
- # tracked at https://github.com/apache/airflow/issues/71080
- if dag_id is None:
- # Not a json body, ignore
- with suppress(JSONDecodeError):
- 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",
- )
+ # The path names the backfill, so its row is the only
authorization subject: what the
+ # caller supplies alongside must not decide a decision the path
already scoped.
+ dag_id = session.scalar(select(Backfill.dag_id).where(Backfill.id
== backfill_id))
+ _authorize_backfill_in_path(method, dag_id, user)
+ return
+
+ # Left: the routes naming their Dag in the body (create, dry run) or
in the query string
+ # (list, read by ``requires_access_dag``), and ids the handler's own
parser will reject.
+ dag_id = None
+ # Not a json body, ignore
+ with suppress(JSONDecodeError):
+ 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,
diff --git
a/airflow-core/tests/unit/api_fastapi/core_api/routes/public/test_backfills.py
b/airflow-core/tests/unit/api_fastapi/core_api/routes/public/test_backfills.py
index fd9d0d5c9c3..eeebcd6a220 100644
---
a/airflow-core/tests/unit/api_fastapi/core_api/routes/public/test_backfills.py
+++
b/airflow-core/tests/unit/api_fastapi/core_api/routes/public/test_backfills.py
@@ -23,10 +23,12 @@ from unittest import mock
import pendulum
import pytest
+from fastapi.testclient import TestClient
from sqlalchemy import and_, func, select
from sqlalchemy.exc import OperationalError, ProgrammingError
from airflow._shared.timezones import timezone
+from airflow.api_fastapi.auth.managers.simple.user import SimpleAuthManagerUser
from airflow.dag_processing.dagbag import DagBag
from airflow.models import DagModel, DagRun, TaskInstance
from airflow.models.backfill import (
@@ -80,6 +82,21 @@ def clean_db():
_clean_db()
[email protected]
+def dag_reader_test_client(test_client):
+ """A caller who may read the Dags but not write them: viewer is below the
role edits require."""
+ auth_manager = test_client.app.state.auth_manager
+ token = auth_manager._get_token_signer().generate(
+ auth_manager.serialize_user(SimpleAuthManagerUser(username="reader",
role="viewer"))
+ )
+ with mock.patch("airflow.models.revoked_token.RevokedToken.is_revoked",
return_value=False):
+ yield TestClient(
+ test_client.app,
+ headers={"Authorization": f"Bearer {token}"},
+ base_url=str(test_client.base_url),
+ )
+
+
def make_dags():
with DAG(
DAG_ID,
@@ -204,6 +221,21 @@ class TestGetBackfill(TestBackfillEndpoint):
assert response.status_code == 404
assert response.json().get("detail") == "Backfill not found"
+ def test_unknown_backfill_is_indistinguishable_from_an_unreadable_one(
+ self, session, unauthorized_test_client
+ ):
+ """Telling the two apart discloses which backfill ids exist across
Dags."""
+ (dag,) = self._create_dag_models()
+ backfill = Backfill(dag_id=dag.dag_id, from_date=timezone.utcnow(),
to_date=timezone.utcnow())
+ session.add(backfill)
+ session.commit()
+
+ existing = unauthorized_test_client.get(f"/backfills/{backfill.id}")
+ unknown = unauthorized_test_client.get(f"/backfills/{231984098}")
+
+ assert existing.status_code == 404
+ assert (existing.status_code, existing.json()) ==
(unknown.status_code, unknown.json())
+
def test_invalid_id(self, test_client):
response = test_client.get("/backfills/invalid_id")
assert response.status_code == 422
@@ -300,6 +332,7 @@ class TestListBackfillDagRuns(TestBackfillEndpoint):
"""Non-existent backfill returns 404."""
response = test_client.get("/backfills/999999/dag_runs")
assert response.status_code == 404
+ assert response.json().get("detail") == "Backfill not found"
def test_list_backfill_dag_runs_pagination(self, test_client, session):
"""Limit and offset work correctly."""
@@ -1504,6 +1537,11 @@ class TestCancelBackfill(TestBackfillEndpoint):
states = [x.state for x in dag_runs]
assert states == ["running", "failed", "failed", "failed", "failed"]
+ def test_cancel_backfill_not_found(self, test_client):
+ response = test_client.put("/backfills/999999/cancel")
+ assert response.status_code == 404
+ assert response.json().get("detail") == "Backfill not found"
+
def test_invalid_id(self, test_client):
response = test_client.put("/backfills/invalid_id/cancel")
assert response.status_code == 422
@@ -1551,7 +1589,7 @@ class TestPauseBackfill(TestBackfillEndpoint):
response =
unauthenticated_test_client.put(f"/backfills/{backfill.id}/pause")
assert response.status_code == 401
- def test_pause_backfill_403(self, session, unauthorized_test_client):
+ def test_pause_backfill_404_when_the_dag_is_unreadable(self, session,
unauthorized_test_client):
(dag,) = self._create_dag_models()
from_date = timezone.utcnow()
to_date = timezone.utcnow()
@@ -1559,8 +1597,27 @@ class TestPauseBackfill(TestBackfillEndpoint):
session.add(backfill)
session.commit()
response =
unauthorized_test_client.put(f"/backfills/{backfill.id}/pause")
+ assert response.status_code == 404
+
+ def test_pause_backfill_403(self, session, dag_reader_test_client):
+ (dag,) = self._create_dag_models()
+ from_date = timezone.utcnow()
+ to_date = timezone.utcnow()
+ backfill = Backfill(dag_id=dag.dag_id, from_date=from_date,
to_date=to_date)
+ session.add(backfill)
+ session.commit()
+ response =
dag_reader_test_client.put(f"/backfills/{backfill.id}/pause")
assert response.status_code == 403
+ def test_pause_backfill_unknown_id_is_not_authorized_by_a_body_dag_id(
+ self, session, dag_reader_test_client
+ ):
+ (dag,) = self._create_dag_models()
+ session.commit()
+ response = dag_reader_test_client.put(f"/backfills/{231984098}/pause",
json={"dag_id": dag.dag_id})
+ assert response.status_code == 404
+ assert response.json().get("detail") == "Backfill not found"
+
def test_invalid_id(self, test_client):
response = test_client.put("/backfills/invalid_id/pause")
assert response.status_code == 422
@@ -1600,6 +1657,11 @@ class TestUnpauseBackfill(TestBackfillEndpoint):
}
check_last_log(session, dag_id=None, event="unpause_backfill",
logical_date=None)
+ def test_unpause_backfill_not_found(self, test_client):
+ response = test_client.put("/backfills/999999/unpause")
+ assert response.status_code == 404
+ assert response.json().get("detail") == "Backfill not found"
+
def test_invalid_id(self, test_client):
response = test_client.put("/backfills/invalid_id/unpause")
assert response.status_code == 422
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 3709d50d358..168a8edafdd 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
@@ -57,7 +57,6 @@ from airflow.api_fastapi.core_api.security import (
resolve_user_from_token,
)
from airflow.models import Connection, Pool, Variable
-from airflow.models.backfill import Backfill
from airflow.models.dag import DagModel
from airflow.models.dagbundle import DagBundleModel
from airflow.models.team import Team
@@ -342,10 +341,8 @@ class TestFastApiSecurity:
mock_get_auth_manager.return_value = auth_manager
mock_get_team_name.return_value = "team1"
- backfill = Mock()
- backfill.dag_id = "backfill_dag_id"
session = Mock()
- session.scalars.return_value.one_or_none.return_value = backfill
+ session.scalar.return_value = "backfill_dag_id"
request = Mock()
request.path_params = {"backfill_id": "42"}
@@ -377,7 +374,6 @@ class TestFastApiSecurity:
mock_get_team_name.return_value = "team1"
session = Mock()
- session.scalars.return_value.one_or_none.return_value = None
request = Mock()
request.path_params = {}
@@ -400,25 +396,25 @@ class TestFastApiSecurity:
@patch.object(DagModel, "get_team_name")
@patch("airflow.api_fastapi.core_api.security.get_auth_manager")
async def test_requires_access_backfill_unauthorized(self,
mock_get_auth_manager, mock_get_team_name):
- """When is_authorized_dag returns False, Forbidden is raised."""
+ """A caller who may not read the backfill's Dag is told it does not
exist."""
auth_manager = Mock()
auth_manager.is_authorized_dag.return_value = False
mock_get_auth_manager.return_value = auth_manager
mock_get_team_name.return_value = None
- backfill = Mock()
- backfill.dag_id = "unauthorized_dag"
session = Mock()
- session.scalars.return_value.one_or_none.return_value = backfill
+ session.scalar.return_value = "unauthorized_dag"
request = Mock()
request.path_params = {"backfill_id": "1"}
user = Mock()
inner = requires_access_backfill("GET")
- with pytest.raises(HTTPException, match="Forbidden"):
+ with pytest.raises(HTTPException) as exc_info:
await inner(request, user, session)
+ assert exc_info.value.status_code == 404
+ assert exc_info.value.detail == "Backfill not found"
auth_manager.is_authorized_dag.assert_called_once_with(
method="GET",
access_entity=DagAccessEntity.RUN,
@@ -430,33 +426,56 @@ class TestFastApiSecurity:
@pytest.mark.asyncio
@patch.object(DagModel, "get_team_name")
@patch("airflow.api_fastapi.core_api.security.get_auth_manager")
- async def
test_requires_access_backfill_backfill_not_found_falls_back_to_body(
+ async def test_requires_access_backfill_forbidden_when_the_dag_is_readable(
self, mock_get_auth_manager, mock_get_team_name
):
- """When backfill_id is int but Backfill not found, dag_id from body is
used."""
- auth_manager = Mock()
+ """A caller who may read the Dag but not write it keeps the Forbidden
answer."""
+ auth_manager = Mock(spec=BaseAuthManager)
+ auth_manager.is_authorized_dag.side_effect = lambda *, method,
**kwargs: method == "GET"
+ mock_get_auth_manager.return_value = auth_manager
+ mock_get_team_name.return_value = None
+
+ session = Mock(spec=Session)
+ session.scalar.return_value = "readable_dag"
+
+ request = Mock(spec=Request)
+ request.path_params = {"backfill_id": "1"}
+ user = Mock(spec=BaseUser)
+
+ with pytest.raises(HTTPException) as exc_info:
+ await requires_access_backfill("PUT")(request, user, session)
+
+ assert exc_info.value.status_code == 403
+
+ @pytest.mark.db_test
+ @pytest.mark.asyncio
+ @patch.object(DagModel, "get_team_name")
+ @patch("airflow.api_fastapi.core_api.security.get_auth_manager")
+ async def
test_requires_access_backfill_unknown_id_ignores_the_request_dag_id(
+ self, mock_get_auth_manager, mock_get_team_name
+ ):
+ """An unknown backfill is not authorized against a Dag the caller
names."""
+ auth_manager = Mock(spec=BaseAuthManager)
auth_manager.is_authorized_dag.return_value = True
mock_get_auth_manager.return_value = auth_manager
mock_get_team_name.return_value = "team1"
- session = Mock()
- session.scalars.return_value.one_or_none.return_value = None
+ session = Mock(spec=Session)
+ session.scalar.return_value = None
- request = Mock()
+ request = Mock(spec=Request)
request.path_params = {"backfill_id": "999"}
- request.json = AsyncMock(return_value={"dag_id": "fallback_dag_id"})
+ request.query_params = {"dag_id": "caller_dag_id"}
+ request.json = AsyncMock(return_value={"dag_id": "caller_dag_id"})
- user = Mock()
+ user = Mock(spec=BaseUser)
- inner = requires_access_backfill("POST")
- await inner(request, user, session)
+ with pytest.raises(HTTPException) as exc_info:
+ await requires_access_backfill("PUT")(request, user, session)
- auth_manager.is_authorized_dag.assert_called_once_with(
- method="POST",
- access_entity=DagAccessEntity.RUN,
- details=DagDetails(id="fallback_dag_id", team_name="team1"),
- user=user,
- )
+ assert exc_info.value.status_code == 404
+ assert exc_info.value.detail == "Backfill not found"
+ auth_manager.is_authorized_dag.assert_not_called()
@pytest.mark.db_test
@pytest.mark.asyncio
@@ -478,10 +497,8 @@ class TestFastApiSecurity:
mock_get_auth_manager.return_value = auth_manager
mock_get_team_name.return_value = "team1"
- backfill = Mock(spec=Backfill)
- backfill.dag_id = "backfill_dag"
session = Mock(spec=Session)
- session.scalars.return_value.one_or_none.return_value = backfill
+ session.scalar.return_value = "backfill_dag"
request = Mock(spec=Request)
request.path_params = {"backfill_id": backfill_id}
