potiuk opened a new pull request, #72651:
URL: https://github.com/apache/airflow/pull/72651
`decode_deadline_alert` passed the Dag-author controlled `interval` and
`callback` to `airflow.sdk.serde.deserialize`, which imports the class named
in
the payload and instantiates it with the encoded arguments.
These decoders run in the **scheduler and the API server** whenever a
serialized
Dag is loaded, so any class under the `airflow.*` allow list could be
constructed
there. The security model says a Dag author reaches those processes only
through
registered plugins and providers, and the codebase enforces that at decode
time
for timetables, priority-weight strategies and operator extra links. Deadline
fields had no equivalent gate.
### The change
Both fields are rebuilt from their encoded form directly, so neither reaches
`serde.deserialize` and the class a Dag author names is never imported:
* **interval** — a number, a `timedelta` payload, or a variable-interval
payload
carrying a `key`, each reconstructed from primitives.
* **callback** — `AsyncCallback` or `SyncCallback`, selected from a fixed map
rather than imported by name; `path` a string, `queue`/`executor` optional
strings, unexpected fields refused.
A filter in front of `deserialize` was tried first and was **not**
sufficient:
serde normalises the legacy `{"__type", "__var"}` shape into `__classname__`
*inside* `deserialize`, so a payload inspected beforehand carries no class
name
to reject. Payloads are normalised before inspection here, and that case is
tested.
### Known residual — deliberately not closed here
Callback `kwargs` are still passed through generic deserialization, so a
legitimate callback can carry an arbitrary allow-listed class under its
kwargs.
Deferring that decode to the process that runs the callback would close it,
but
the kwargs are consumed through two paths using two different encodings —
`BaseSerialization` in the triggerer, serde in the executor path — and
getting
either wrong silently hands user code an encoded dict in place of its
argument.
The residual is not specific to deadlines: it is the general property of
deserializing Dag-author data, shared with every other serde call site. A
test
asserts the *current* behaviour so the gap stays visible and any change to
it has
to be deliberate.
### Tests
The tests assert the class is **never constructed**, not merely that an
error is
raised — `deserialize()` instantiates before returning, so checking the
returned
object is already too late for a class with side effects in `__init__`.
Against unpatched sources the callback case reports `DID NOT RAISE` (the
class
was built and accepted) and the interval case reports
`Invalid interval type: _DeadlineGadget`, naming the instance that had
already
been created.
597 tests pass in `airflow-core/tests/unit/serialization/`. One pre-existing
failure there (`TestStringifiedDAGs::test_serialization`) reproduces
identically
on unmodified `main`.
---
##### Was generative AI tooling used to co-author this PR?
- [X] Yes (please specify the tool below)
Generated-by: Claude Code following [the
guidelines](https://github.com/apache/airflow/blob/main/contributing-docs/05_pull_requests.rst#gen-ai-assisted-contributions)
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
