Skip to content

Commit 653105f

Browse files
committed
Restrict exception-node deserialization to BaseException subclasses (validate before import)
Stacked on apache#68528, which removed the dead AIP-44 BASE_TRIGGER path; the remaining hardening here is for the exception nodes. When a serialized DAG carries an exception node (AIRFLOW_EXC_SER / BASE_EXC_SER), deserialization called import_string on the stored class path and instantiated whatever it resolved to, without validating the path or the resulting type. Add _safe_import_for_deserialize, which validates the class path against a trusted-namespace prefix list (airflow., plus builtins. for the BASE_EXC_SER builtin case) before importing, and then confirms the resolved object is a BaseException subclass. A path outside the trusted namespaces is rejected without being imported.
1 parent 01b7563 commit 653105f

2 files changed

Lines changed: 67 additions & 2 deletions

File tree

airflow-core/src/airflow/serialization/serialized_objects.py

Lines changed: 31 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -235,6 +235,33 @@ def _decode_priority_weight_strategy(var: str) -> PriorityWeightStrategy:
235235
return priority_weight_strategy_class()
236236

237237

238+
# Module prefixes whose code is trusted to import while deserializing a stored DAG.
239+
# The class path from the serialized blob is validated against these *before*
240+
# import_string runs, so a path outside the trusted namespaces is rejected without
241+
# importing it. Mirrors decode_timetable's prefix gate / the priority-weight registry.
242+
_TRUSTED_DESERIALIZE_PREFIXES = ("airflow.",)
243+
244+
245+
def _safe_import_for_deserialize(cls_name: str, base: type, *, allow_builtins: bool = False) -> type:
246+
"""
247+
Resolve ``cls_name`` to a subclass of ``base``, validating the name before importing.
248+
249+
The check runs on the string before any import, so a class path outside the
250+
trusted namespaces is never imported. A post-import ``issubclass`` check is
251+
kept as a second gate.
252+
"""
253+
module_path = cls_name.rpartition(".")[0]
254+
trusted = cls_name.startswith(_TRUSTED_DESERIALIZE_PREFIXES) or (
255+
allow_builtins and module_path == "builtins"
256+
)
257+
if not trusted:
258+
raise ValueError(f"Refusing to deserialize disallowed class path {cls_name!r}")
259+
cls = import_string(cls_name)
260+
if not (isinstance(cls, type) and issubclass(cls, base)):
261+
raise ValueError(f"{cls_name!r} is not a {base.__name__} subclass")
262+
return cls
263+
264+
238265
def _encode_start_trigger_args(var: StartTriggerArgs) -> dict[str, Any]:
239266
"""Encode a StartTriggerArgs."""
240267

@@ -659,9 +686,11 @@ def deserialize(cls, encoded_var: Any) -> Any:
659686
kwargs = deser["kwargs"]
660687
del deser
661688
if type_ == DAT.AIRFLOW_EXC_SER:
662-
exc_cls = import_string(exc_cls_name)
689+
exc_cls = _safe_import_for_deserialize(exc_cls_name, BaseException)
663690
else:
664-
exc_cls = import_string(f"builtins.{exc_cls_name}")
691+
exc_cls = _safe_import_for_deserialize(
692+
f"builtins.{exc_cls_name}", BaseException, allow_builtins=True
693+
)
665694
return exc_cls(*args, **kwargs)
666695
elif type_ == DAT.SET:
667696
return {cls.deserialize(v) for v in var}

airflow-core/tests/unit/serialization/test_dag_serialization.py

Lines changed: 36 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -2786,6 +2786,42 @@ def test_create_dagrun_accepts_partition_key_for_partition_at_runtime_dag(self,
27862786
dr = dag_maker.create_dagrun(partition_key="runtime-key")
27872787
assert dr.partition_key == "runtime-key"
27882788

2789+
def test_airflow_exc_deserialization_rejects_disallowed_class_path(self):
2790+
"""An AIRFLOW_EXC_SER class path outside the trusted namespaces is rejected before import."""
2791+
from airflow.serialization.enums import DagAttributeTypes
2792+
2793+
encoded = BaseSerialization._encode(
2794+
BaseSerialization.serialize(
2795+
{"exc_cls_name": "subprocess.check_output", "args": [], "kwargs": {}}
2796+
),
2797+
type_=DagAttributeTypes.AIRFLOW_EXC_SER,
2798+
)
2799+
with pytest.raises(ValueError, match="Refusing to deserialize disallowed class path"):
2800+
BaseSerialization.deserialize(encoded)
2801+
2802+
def test_base_exc_deserialization_rejects_non_exception(self):
2803+
"""A builtins name that is not a BaseException subclass is rejected."""
2804+
from airflow.serialization.enums import DagAttributeTypes
2805+
2806+
encoded = BaseSerialization._encode(
2807+
BaseSerialization.serialize({"exc_cls_name": "eval", "args": ["1"], "kwargs": {}}),
2808+
type_=DagAttributeTypes.BASE_EXC_SER,
2809+
)
2810+
with pytest.raises(ValueError, match="not a BaseException subclass"):
2811+
BaseSerialization.deserialize(encoded)
2812+
2813+
def test_base_exc_deserialization_roundtrips_builtin_exception(self):
2814+
"""A genuine builtin exception still deserializes."""
2815+
from airflow.serialization.enums import DagAttributeTypes
2816+
2817+
encoded = BaseSerialization._encode(
2818+
BaseSerialization.serialize({"exc_cls_name": "ValueError", "args": ["boom"], "kwargs": {}}),
2819+
type_=DagAttributeTypes.BASE_EXC_SER,
2820+
)
2821+
result = BaseSerialization.deserialize(encoded)
2822+
assert isinstance(result, ValueError)
2823+
assert result.args == ("boom",)
2824+
27892825

27902826
def test_kubernetes_optional():
27912827
"""Test that serialization module loads without kubernetes, but deserialization of PODs requires it"""

0 commit comments

Comments
 (0)