Skip to content

Commit 80f1ab4

Browse files
Tighten deserialization allowlist regex to require full-string match (#66499)
* Tighten deserialization allowlist regex to use full-string match The ``allowed_deserialization_classes_regexp`` allowlist used ``re.match()``, which only anchors at the start of the string. A pattern like ``airflow\.models\.Variable`` therefore also admitted classnames such as ``airflow.models.Variable_Malicious``. Switch to ``re.fullmatch()`` so the admin's pattern matches the entire classname; document the semantics in the config description so operators know to use ``.*`` for prefix-style allowances. * Add newsfragment for #66499 --------- Co-authored-by: Rahul Vats <43964496+vatsrahul1001@users.noreply.github.com>
1 parent c5408b4 commit 80f1ab4

4 files changed

Lines changed: 40 additions & 3 deletions

File tree

Lines changed: 14 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,14 @@
1+
Tighten ``[core] allowed_deserialization_classes_regexp`` to require full-string matches
2+
3+
Patterns in ``[core] allowed_deserialization_classes_regexp`` are now matched
4+
against the entire classname using ``re.fullmatch()`` instead of ``re.match()``.
5+
Previously a pattern such as ``airflow\.models\.Variable`` admitted not only
6+
the intended class but also names that started with it
7+
(e.g. ``airflow.models.Variable_Malicious``), because ``re.match`` only anchors
8+
at the start of the string.
9+
10+
The default value of this option is empty, so out-of-the-box deployments are
11+
unaffected. Deployments that configured this option with patterns relying on
12+
prefix-match semantics — for example ``airflow\.models\.`` to mean "any class
13+
under ``airflow.models``" — must add ``.*`` to the pattern
14+
(``airflow\.models\..*``) to retain the previous behaviour.

airflow-core/src/airflow/config_templates/config.yml

Lines changed: 4 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -261,7 +261,10 @@ core:
261261
allowed_deserialization_classes_regexp:
262262
description: |
263263
Space-separated list of classes that may be imported during deserialization. Items are processed
264-
as regex expressions. Python built-in classes (like dict) are always allowed.
264+
as regex expressions and matched against the full classname (``re.fullmatch`` semantics), so a
265+
pattern such as ``airflow\.models\.Variable`` does not also admit ``airflow.models.VariableXYZ``.
266+
Use ``.*`` (e.g. ``airflow\.models\..*``) to allow a prefix and any suffix. Python built-in
267+
classes (like dict) are always allowed.
265268
This is a secondary option to ``[core] allowed_deserialization_classes``.
266269
version_added: 2.8.2
267270
type: string

task-sdk/src/airflow/sdk/serde/__init__.py

Lines changed: 3 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -332,8 +332,10 @@ def _match_glob(classname: str):
332332
@functools.cache
333333
def _match_regexp(classname: str):
334334
"""Check if the given classname matches a pattern from allowed_deserialization_classes_regexp using regexp."""
335+
# fullmatch (not match) so a pattern like ``airflow\.models\.Variable`` cannot also admit
336+
# ``airflow.models.Variable_Malicious`` — re.match only anchors at the start of the string.
335337
patterns = _get_regexp_patterns()
336-
return any(p.match(classname) is not None for p in patterns)
338+
return any(p.fullmatch(classname) is not None for p in patterns)
337339

338340

339341
def _stringify(classname: str, version: int, value: T | None) -> str:

task-sdk/tests/task_sdk/serde/test_serde.py

Lines changed: 19 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -367,7 +367,7 @@ def test_allow_list_match_class(self):
367367
@conf_vars(
368368
{
369369
("core", "allowed_deserialization_classes"): "",
370-
("core", "allowed_deserialization_classes_regexp"): r"unit\.airflow\..",
370+
("core", "allowed_deserialization_classes_regexp"): r"unit\.airflow\..*",
371371
}
372372
)
373373
@pytest.mark.usefixtures("recalculate_patterns")
@@ -394,6 +394,24 @@ def test_allow_list_match_class_regexp(self):
394394
assert _match("unit.airflow.deep")
395395
assert _match("unit.airflow.FALSE") is False
396396

397+
@conf_vars(
398+
{
399+
("core", "allowed_deserialization_classes"): "",
400+
("core", "allowed_deserialization_classes_regexp"): r"unit\.airflow\.Variable",
401+
}
402+
)
403+
@pytest.mark.usefixtures("recalculate_patterns")
404+
def test_allow_list_regexp_does_not_prefix_match(self):
405+
"""
406+
A pattern without an explicit end anchor must not admit classes that share
407+
the pattern as a prefix. ``re.match`` would let ``unit.airflow.Variable_Malicious``
408+
through because it only anchors at the start of the string; ``re.fullmatch``
409+
rejects it. Patterns with ``.*`` at the end retain prefix-style behaviour.
410+
"""
411+
assert _match("unit.airflow.Variable")
412+
assert _match("unit.airflow.Variable_Malicious") is False
413+
assert _match("unit.airflow.VariableSubclass") is False
414+
397415
def test_incompatible_version(self):
398416
data = dict(
399417
{

0 commit comments

Comments
 (0)