Skip to content

[v3-2-test] Return raw import-error stacktrace when file has no registered Dag (#67465) #67478

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
vatsrahul1001 merged 1 commit into from
May 25, 2026
Merged

[v3-2-test] Return raw import-error stacktrace when file has no registered Dag (#67465) #67478

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
26 changes: 7 additions & 19 deletions airflow-core/src/airflow/api_fastapi/core_api/routes/public/import_error.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -55,11 +55,6 @@
from airflow.models.errors import ParseImportError

REDACTED_STACKTRACE = "REDACTED - you do not have read permission on all Dags in the file"
REDACTED_STACKTRACE_NO_DAG = (
"REDACTED - no Dag has been registered for this file yet "
"(parse may have failed before any Dag was defined; "
"if you have Dag read access, check the dag-processor logs for the raw error)"
)
import_error_router = AirflowRouter(tags=["Import Error"], prefix="/importErrors")


Expand Down Expand Up @@ -103,13 +98,10 @@ def get_import_error(

# No Dags matched for this file -- either the file genuinely contains
# no Dags (parse failed before any Dag was defined), or the name keys
# did not resolve. Redact the stacktrace rather than returning the raw
# error, so the response stays on the deny-by-default side of the
# authorization check. The message distinguishes this case from the
# per-Dag scope mismatch below so callers (especially admins) don't
# mistake "file has no Dag yet" for "you lack permission".
# did not resolve. Return the raw error in this case; a proper
# admin-only path for unregistered files is tracked in a follow-up
# issue (see https://github.com/apache/airflow/issues/67461).
if not file_dag_ids:
error.stacktrace = REDACTED_STACKTRACE_NO_DAG
return error

# Can the user read any Dags in the file?
Expand Down Expand Up @@ -235,15 +227,11 @@ def get_import_errors(

# No Dags matched for this file -- either the file genuinely has
# no Dags yet (parse failed before any Dag was defined), or the
# name keys did not resolve. Redact the stacktrace before
# appending so the response stays on the deny-by-default side of
# the authorization check. The message distinguishes this case
# from the per-Dag scope mismatch below so callers (especially
# admins) don't mistake "file has no Dag yet" for "you lack
# permission".
# name keys did not resolve. Append the raw error in this case;
# a proper admin-only path for unregistered files is tracked in
# a follow-up issue
# (see https://github.com/apache/airflow/issues/67461).
if not dag_ids:
session.expunge(import_error)
import_error.stacktrace = REDACTED_STACKTRACE_NO_DAG
import_errors.append(import_error)
continue

Expand Down
72 changes: 39 additions & 33 deletions airflow-core/tests/unit/api_fastapi/core_api/routes/public/test_import_error.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -23,10 +23,7 @@
import pytest

from airflow.api_fastapi.auth.managers.models.resource_details import DagDetails
from airflow.api_fastapi.core_api.routes.public.import_error import (
REDACTED_STACKTRACE,
REDACTED_STACKTRACE_NO_DAG,
)
from airflow.api_fastapi.core_api.routes.public.import_error import REDACTED_STACKTRACE
from airflow.models import DagModel
from airflow.models.dagbundle import DagBundleModel
from airflow.models.errors import ParseImportError
Expand Down Expand Up @@ -280,14 +277,12 @@ def test_get_import_error__user_dont_have_read_permission_to_read_all_dags_in_fi

@mock.patch("airflow.api_fastapi.core_api.routes.public.import_error.get_auth_manager")
def test_get_import_error__no_dag_in_dagmodel(self, mock_get_auth_manager, test_client, import_errors):
"""Import error is returned with the no-Dag redaction message when no Dag
exists in ``DagModel`` for the file.
"""Import error is returned with the raw stacktrace when no Dag exists
in ``DagModel`` for the file.

When the file-to-Dag set resolves empty there is no Dag anchor to
authorize against, so the stacktrace is redacted. The message must be
the no-Dag variant, not the per-Dag-scope variant, so callers with
full Dag read access (admins) understand the redaction is about the
file's parse state and not their permissions.
Proper handling of unregistered files (separate admin-only permission,
multi-team isolation) is tracked as follow-up work; for now the endpoint
returns the raw error rather than redacting it.
"""
import_error_id = import_errors[0].id
set_mock_auth_manager__get_authorized_dag_ids(mock_get_auth_manager, set())
Expand All @@ -299,7 +294,7 @@ def test_get_import_error__no_dag_in_dagmodel(self, mock_get_auth_manager, test_
"import_error_id": import_error_id,
"timestamp": from_datetime_to_zulu_without_ms(TIMESTAMP1),
"filename": FILENAME1,
"stack_trace": REDACTED_STACKTRACE_NO_DAG,
"stack_trace": STACKTRACE1,
"bundle_name": BUNDLE_NAME,
}

Expand Down Expand Up @@ -525,21 +520,27 @@ def test_bundle_name_join_condition_for_import_errors(

@mock.patch("airflow.api_fastapi.core_api.routes.public.import_error.get_auth_manager")
def test_get_import_errors__no_dag_in_dagmodel(self, mock_get_auth_manager, test_client, import_errors):
"""Import errors are returned with the no-Dag redaction message when no Dag
exists in ``DagModel`` for the file."""
"""Import errors are returned with their raw stacktraces when no Dag
exists in ``DagModel`` for the file.

Proper handling of unregistered files is deferred to a follow-up issue
that introduces a dedicated permission and respects multi-team isolation.
"""
set_mock_auth_manager__get_authorized_dag_ids(mock_get_auth_manager, set())

response = test_client.get("/importErrors")

assert response.status_code == 200
response_json = response.json()
assert response_json["total_entries"] == 3
filenames = [error["filename"] for error in response_json["import_errors"]]
assert FILENAME1 in filenames
assert FILENAME2 in filenames
assert FILENAME3 in filenames
for entry in response_json["import_errors"]:
assert entry["stack_trace"] == REDACTED_STACKTRACE_NO_DAG
stacktrace_by_filename = {
error["filename"]: error["stack_trace"] for error in response_json["import_errors"]
}
assert stacktrace_by_filename == {
FILENAME1: STACKTRACE1,
FILENAME2: STACKTRACE2,
FILENAME3: STACKTRACE3,
}


class TestImportErrorFileAuthorization:
Expand Down Expand Up @@ -666,23 +667,24 @@ def test_single_endpoint_matches_file_via_relative_fileloc_not_fileloc(
assert response.status_code == 403

@mock.patch("airflow.api_fastapi.core_api.routes.public.import_error.get_auth_manager")
def test_single_endpoint_redacts_when_file_has_no_known_dags(
def test_single_endpoint_returns_raw_stacktrace_when_file_has_no_known_dags(
self,
mock_get_auth_manager,
test_client,
import_errors,
):
"""Single endpoint must redact the stacktrace with the no-Dag
message when the ``ParseImportError`` refers to a file with no
matching ``DagModel`` rows at all -- for example a file that
failed to parse before any Dag was defined.
"""Single endpoint returns the raw stacktrace when the
``ParseImportError`` refers to a file with no matching ``DagModel``
rows at all -- for example a file that failed to parse before any
Dag was defined. Proper handling of this case (dedicated permission,
multi-team isolation) is tracked as follow-up work.
"""
set_mock_auth_manager__get_authorized_dag_ids(mock_get_auth_manager, set())
response = test_client.get(f"/importErrors/{import_errors[0].id}")
assert response.status_code == 200
body = response.json()
assert body["filename"] == FILENAME1
assert body["stack_trace"] == REDACTED_STACKTRACE_NO_DAG
assert body["stack_trace"] == STACKTRACE1

@mock.patch("airflow.api_fastapi.core_api.routes.public.import_error.get_auth_manager")
def test_list_endpoint_redacts_mixed_file_with_colocated_dag_outside_callers_scope(
Expand Down Expand Up @@ -721,21 +723,25 @@ def permit_only_readable(requests, user):
assert mixed_entries[0]["stack_trace"] == REDACTED_STACKTRACE

@mock.patch("airflow.api_fastapi.core_api.routes.public.import_error.get_auth_manager")
def test_list_endpoint_redacts_when_file_has_no_known_dags(
def test_list_endpoint_returns_raw_stacktrace_when_file_has_no_known_dags(
self,
mock_get_auth_manager,
test_client,
import_errors,
):
"""List endpoint must redact the stacktrace with the no-Dag
message for import errors whose file has no matching ``DagModel``
rows -- closing the ``if not dag_ids: import_errors.append(import_error)``
fall-through that previously returned the raw error.
"""List endpoint returns the raw stacktrace for import errors whose
file has no matching ``DagModel`` rows. Proper handling of this case
(dedicated permission, multi-team isolation) is tracked as follow-up
work.
"""
set_mock_auth_manager__get_authorized_dag_ids(mock_get_auth_manager, set())
response = test_client.get("/importErrors")
assert response.status_code == 200
body = response.json()
assert body["total_entries"] == 3
for entry in body["import_errors"]:
assert entry["stack_trace"] == REDACTED_STACKTRACE_NO_DAG
stacktrace_by_filename = {entry["filename"]: entry["stack_trace"] for entry in body["import_errors"]}
assert stacktrace_by_filename == {
FILENAME1: STACKTRACE1,
FILENAME2: STACKTRACE2,
FILENAME3: STACKTRACE3,
}
Loading