Skip to content

Commit a481344

Browse files
committed
Reject empty subprocess coordinator artifact roots
An explicitly empty root is likely a configuration or templating error. Treating it as omitted can silently scan the task's Dag bundle and run unintended artifacts.
1 parent 0aec44e commit a481344

10 files changed

Lines changed: 67 additions & 22 deletions

File tree

‎airflow-core/docs/authoring-and-scheduling/language-sdks/go.rst‎

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -510,7 +510,8 @@ All ``kwargs`` in the ``coordinators`` config entry are passed to the
510510
- *(optional)*
511511
- One or more directories scanned recursively for executable bundles. Accepts a string,
512512
a path, or a list of strings/paths. When omitted, bundles are located through a Dag
513-
bundle instead (see the note below).
513+
bundle instead (see the note below). Explicitly setting this option to ``null`` or
514+
an empty list is invalid.
514515
* - ``dag_bundle_name``
515516
- *(auto: task's own bundle)*
516517
- Name of a configured Dag bundle to load executable bundles from. Mutually exclusive

‎airflow-core/docs/authoring-and-scheduling/language-sdks/java.rst‎

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1025,7 +1025,8 @@ All ``kwargs`` in the ``coordinators`` config entry are passed to the
10251025
- *(optional)*
10261026
- One or more directories scanned recursively for ``.jar`` files. Accepts a string,
10271027
a path, or a list of strings/paths. When omitted, JARs are located through a Dag
1028-
bundle instead (see the note below).
1028+
bundle instead (see the note below). Explicitly setting this option to ``null`` or
1029+
an empty list is invalid.
10291030
* - ``dag_bundle_name``
10301031
- *(auto: task's own bundle)*
10311032
- Name of a configured Dag bundle to load JARs from. Mutually exclusive with

‎airflow-core/docs/authoring-and-scheduling/language-sdks/typescript.rst‎

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -449,7 +449,8 @@ All ``kwargs`` in the ``coordinators`` config entry are passed to the
449449
- *(optional)*
450450
- One or more directories searched recursively, in order, for an integrity-verified ``*.min.mjs``
451451
bundle that declares the requested Dag. Accepts a string, a path, or a list of strings/paths. When
452-
omitted, the bundle is located through a Dag bundle instead (see the note below).
452+
omitted, the bundle is located through a Dag bundle instead (see the note below). Explicitly setting
453+
this option to ``null`` or an empty list is invalid.
453454
* - ``dag_bundle_name``
454455
- *(auto: task's own bundle)*
455456
- Name of a configured Dag bundle to load the ``*.min.mjs`` bundle from. Mutually exclusive with

‎task-sdk/src/airflow/sdk/coordinators/_bundle_metadata.py‎

Lines changed: 27 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -22,7 +22,7 @@
2222
import os
2323
import pathlib
2424
import stat
25-
from typing import TYPE_CHECKING, Any
25+
from typing import TYPE_CHECKING, Any, Final
2626

2727
import attrs
2828
import structlog
@@ -38,6 +38,13 @@
3838
log: FilteringBoundLogger = structlog.get_logger(logger_name="coordinators")
3939

4040

41+
class _UnsetArtifactRoots:
42+
"""Sentinel distinguishing an omitted artifact-roots option from an empty value."""
43+
44+
45+
ARTIFACT_ROOTS_NOT_CONFIGURED: Final = _UnsetArtifactRoots()
46+
47+
4148
def convert_roots(
4249
value: None | os.PathLike[str] | pathlib.Path | list[os.PathLike[str] | pathlib.Path],
4350
) -> list[pathlib.Path]:
@@ -96,6 +103,25 @@ def _sorted_children(directory: pathlib.Path) -> list[pathlib.Path]:
96103
return []
97104

98105

106+
def convert_configured_roots(
107+
value: _UnsetArtifactRoots
108+
| None
109+
| os.PathLike[str]
110+
| pathlib.Path
111+
| list[os.PathLike[str] | pathlib.Path],
112+
) -> list[pathlib.Path]:
113+
"""Normalize configured roots while rejecting explicitly empty values."""
114+
if isinstance(value, _UnsetArtifactRoots):
115+
return []
116+
roots = convert_roots(value)
117+
if not roots:
118+
raise ValueError(
119+
"Artifact roots must contain at least one path when provided; "
120+
"omit the option to use the task's Dag bundle."
121+
)
122+
return roots
123+
124+
99125
def validate_schema_version(instance, _, value) -> str:
100126
"""Attrs validator resolving a bundle's supervisor schema version to a known one."""
101127
return get_schema_version_migrator().resolve_version(str(value))

‎task-sdk/src/airflow/sdk/coordinators/executable/coordinator.py‎

Lines changed: 4 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -31,8 +31,9 @@
3131
import structlog
3232

3333
from airflow.sdk.coordinators._bundle_metadata import (
34+
ARTIFACT_ROOTS_NOT_CONFIGURED,
3435
ResolvedBundle,
35-
convert_roots,
36+
convert_configured_roots,
3637
extract_supervisor_schema_version,
3738
parse_metadata_mapping,
3839
)
@@ -346,8 +347,8 @@ class ExecutableCoordinator(SubprocessCoordinator):
346347
"""
347348

348349
executables_root: list[pathlib.Path] = attrs.field(
349-
converter=convert_roots,
350-
factory=list,
350+
default=ARTIFACT_ROOTS_NOT_CONFIGURED,
351+
converter=convert_configured_roots,
351352
)
352353

353354
@property

‎task-sdk/src/airflow/sdk/coordinators/java/coordinator.py‎

Lines changed: 7 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -29,7 +29,11 @@
2929
import attrs
3030
import structlog
3131

32-
from airflow.sdk.coordinators._bundle_metadata import convert_roots, validate_schema_version
32+
from airflow.sdk.coordinators._bundle_metadata import (
33+
ARTIFACT_ROOTS_NOT_CONFIGURED,
34+
convert_configured_roots,
35+
validate_schema_version,
36+
)
3337
from airflow.sdk.coordinators._subprocess import SubprocessCoordinator
3438

3539
if TYPE_CHECKING:
@@ -200,8 +204,8 @@ class JavaCoordinator(SubprocessCoordinator):
200204
java_executable: str = "java"
201205
jvm_args: list[str] = attrs.field(factory=list)
202206
jars_root: list[pathlib.Path] = attrs.field(
203-
converter=convert_roots,
204-
factory=list,
207+
default=ARTIFACT_ROOTS_NOT_CONFIGURED,
208+
converter=convert_configured_roots,
205209
)
206210
main_class: str = ""
207211

‎task-sdk/src/airflow/sdk/coordinators/node/coordinator.py‎

Lines changed: 8 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -26,7 +26,12 @@
2626
import attrs
2727
import structlog
2828

29-
from airflow.sdk.coordinators._bundle_metadata import ResolvedBundle, convert_roots, walk_files
29+
from airflow.sdk.coordinators._bundle_metadata import (
30+
ARTIFACT_ROOTS_NOT_CONFIGURED,
31+
ResolvedBundle,
32+
convert_configured_roots,
33+
walk_files,
34+
)
3035
from airflow.sdk.coordinators._subprocess import SubprocessCoordinator
3136
from airflow.sdk.coordinators.node._bundle_reader import read_bundle
3237

@@ -119,8 +124,8 @@ class NodeCoordinator(SubprocessCoordinator):
119124

120125
node_executable: str = "node"
121126
bundles_root: list[pathlib.Path] = attrs.field(
122-
converter=convert_roots,
123-
factory=list,
127+
default=ARTIFACT_ROOTS_NOT_CONFIGURED,
128+
converter=convert_configured_roots,
124129
)
125130

126131
@property

‎task-sdk/tests/task_sdk/coordinators/executable/test_coordinator.py‎

Lines changed: 4 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -375,9 +375,10 @@ def test_executables_root_optional_defaults_to_empty(self):
375375
assert coordinator.executables_root == []
376376
assert coordinator.dag_bundle_name is None
377377

378-
def test_none_executables_root_normalized_to_empty(self):
379-
coordinator = ExecutableCoordinator(executables_root=None)
380-
assert coordinator.executables_root == []
378+
@pytest.mark.parametrize("executables_root", [None, []], ids=["none", "empty-list"])
379+
def test_explicit_empty_executables_root_raises(self, executables_root):
380+
with pytest.raises(ValueError, match="must contain at least one path when provided"):
381+
ExecutableCoordinator(executables_root=executables_root)
381382

382383
def test_root_and_dag_bundle_name_are_mutually_exclusive(self, tmp_path):
383384
with pytest.raises(ValueError, match="at most one of 'executables_root' or 'dag_bundle_name'"):

‎task-sdk/tests/task_sdk/coordinators/java/test_coordinator.py‎

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -244,6 +244,11 @@ def test_jars_root_optional_defaults_to_empty(self):
244244
assert coordinator.dag_bundle_name is None
245245
assert coordinator.main_class == ""
246246

247+
@pytest.mark.parametrize("jars_root", [None, []], ids=["none", "empty-list"])
248+
def test_explicit_empty_jars_root_raises(self, jars_root):
249+
with pytest.raises(ValueError, match="must contain at least one path when provided"):
250+
JavaCoordinator(jars_root=jars_root)
251+
247252
def test_explicit_root_does_not_require_main_class(self, tmp_path):
248253
coordinator = JavaCoordinator(jars_root=tmp_path)
249254
assert coordinator.main_class == ""

‎task-sdk/tests/task_sdk/coordinators/node/test_coordinator.py‎

Lines changed: 6 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -85,9 +85,10 @@ def test_bundles_root_optional_defaults_to_empty(self):
8585
assert coordinator.bundles_root == []
8686
assert coordinator.dag_bundle_name is None
8787

88-
def test_none_bundles_root_normalized_to_empty(self):
89-
coordinator = NodeCoordinator(bundles_root=None)
90-
assert coordinator.bundles_root == []
88+
@pytest.mark.parametrize("bundles_root", [None, []], ids=["none", "empty-list"])
89+
def test_explicit_empty_bundles_root_raises(self, bundles_root):
90+
with pytest.raises(ValueError, match="must contain at least one path when provided"):
91+
NodeCoordinator(bundles_root=bundles_root)
9192

9293
def test_root_and_dag_bundle_name_are_mutually_exclusive(self):
9394
with pytest.raises(ValueError, match="at most one of 'bundles_root' or 'dag_bundle_name'"):
@@ -116,9 +117,8 @@ def test_selects_bundle_by_dag_id(self, tmp_path):
116117
bundles_root=tmp_path,
117118
)
118119

119-
command, schema_version = coordinator._build_execute_task_command(
120-
what=_make_ti(dag_id="sales"), roots=[tmp_path]
121-
)
120+
with coordinator._set_scan_roots([tmp_path]):
121+
command, schema_version = coordinator._build_execute_task_command(what=_make_ti(dag_id="sales"))
122122

123123
assert command == ["/opt/node/bin/node", str(selected)]
124124
assert schema_version == SCHEMA_VERSION

0 commit comments

Comments
 (0)