Restrict exception-node deserialization to known classes without importing the stored name

[potiuk]

When deserializing AIRFLOW_EXC_SER / BASE_EXC_SER nodes, the exception class was resolved by import_string() on a name taken from the serialized blob. This resolves it against in-memory classes instead, so deserializing a stored DAG never imports a class named in the blob:

• AIRFLOW_EXC_SER — looked up in a map of loaded AirflowException subclasses, built once from the in-memory subclass tree and never rebuilt; a name that isn't a registered AirflowException subclass is rejected.
• BASE_EXC_SER — resolved against the fixed {KeyError, AttributeError} set, which is all the encoder ever emits for this node type.

Unknown or disallowed names raise DeserializationError instead of being imported. The trigger-node branch is handled separately in #67926.

Tests

• AIRFLOW_EXC_SER name that isn't a registered AirflowException subclass is rejected (not imported)
• genuine AirflowException subclass round-trips via the registry
• BASE_EXC_SER name outside {KeyError, AttributeError} (e.g. eval, ValueError) rejected
• KeyError / AttributeError still round-trip

Was generative AI tooling used to co-author this PR?

• Yes — Claude Opus 4.8

Generated-by: Claude Opus 4.8 following the guidelines at
https://github.com/apache/airflow/blob/main/contributing-docs/05_pull_requests.rst#gen-ai-assisted-contributions

[potiuk]

@ashb - would that be a good enough check ?

[vatsrahul1001]

Moving to 3.3.1 as we are still awaiting reviews and I am about to get rc1 out

[potiuk]

Gentle nudge on this one — it's a small, security-flavoured change to exception-node deserialization (stops serialized_objects from import_string-ing a class name taken straight from the stored blob) and it's milestoned for 3.3.1. @ashb / @bolkedebruin are the serialization codeowners here; @uranusjr and @kaxil are the most active hands in serialized_objects.py lately — a quick look from any of you would help it land. The trigger-node branch is handled separately in #67926.

---

Drafted-by: Claude Code (Opus 4.8); reviewed by @potiuk before posting

[uranusjr]

I’m not exactly sure but this can potentially block exception classes that the user creates but not eagerly imported? Like lazy imports in a plugin? I did not think it entirely though to be honest, but building an allow list on launch smells wrong in a dynamic environment like Python.

[potiuk]

I’m not exactly sure but this can potentially block exception classes that the user creates but not eagerly imported? Like lazy imports in a plugin? I did not think it entirely though to be honest, but building an allow list on launch smells wrong in a dynamic environment like Python.

IMHO it prevents some classes of issues - especially that we ware taking about deserialisation - so potentially passing an exception that has not been imported by Scheduler yet. It is a "defense-in-depth" really, nothing exploitable.

[amoghrajesh]

@potiuk I have a qn here.

[potiuk]

Reworked the mechanism off the back of this round of review — the approach changed, so a summary rather than a diff description.

The prebuilt allow-list is gone. Resolution now splits the stored name, looks the module up in sys.modules, and reads the class out of that module's namespace, requiring the result to be an AirflowException subclass. No import, no cache, no map.

That drops three separate problems the map had:

• It could not be complete. Built once from the subclass tree, it permanently rejected any subclass registered later — a provider or plugin loaded lazily. Not "blocked at launch", blocked for the life of the process.
• It broke old blobs. Keying on cls.__module__ cannot follow a re-export. These exceptions moved to airflow.sdk.exceptions in 3.2.0, so the map only held airflow.sdk.exceptions.AirflowException while every blob written by 3.0/3.1 stores airflow.exceptions.AirflowException. import_string followed the re-export; the map could not. Upgrading would have stranded stored blobs carrying an exception node.
• It needed invalidation. With no map there is nothing to go stale.

The property the change exists for is unchanged: nothing in the stored blob can cause an import. A module that is not already loaded does not resolve, and the class is read from vars(module) rather than with getattr, so a module-level __getattr__ — which Airflow uses for deprecation shims and lazy provider re-exports, some of which import on access — stays out of the path. subprocess.check_output, os.system and builtins.eval are all still refused with their modules loaded.

This also answers the concern raised earlier in this thread about lazily-imported plugin exceptions: an allow-list built at launch was the wrong shape for that, and there no longer is one.

---

Drafted-by: Claude Code (Opus 5); reviewed by @potiuk before posting

[potiuk]

All review threads are addressed and resolved. The mechanism changed rather than being patched: the prebuilt allow-list is gone in favour of resolving the stored name against sys.modules, which fixes the pre-3.2.0 airflow.exceptions.* spelling that a __module__-keyed map silently dropped — that would have stranded every blob written by 3.0/3.1 on upgrade. It also removes the staleness problem for lazily-imported plugin subclasses, and the BASE_EXC_SER comment and message no longer claim a guarantee the encode branch does not give.

This one has never had an approval; a look from any of you would let it land for 3.3.1.

@kaxil @amoghrajesh @uranusjr

---

Drafted-by: Claude Code (Opus 5); reviewed by @potiuk before posting

[amoghrajesh]

Some nits, otherwise LGTM

[ashb]

I thought there was another pr where we removed this entirely?

[potiuk]

Not in main at least. In a number of places this serialization is left for migration purposes I think. And yes I already raised a question while working on a number of those cases that having a more general solution where we get rid of similar serialization issues would be great. But I do not think we are there yet - cc: @kaxil @amoghrajesh - I think we can discuss it separately

[potiuk]

Yes — #68662. It removes the encode side entirely and makes decode legacy-only, returning str(BaseException(*args)) without resolving or calling anything.

That is a stronger position than this PR, which still resolves a payload-supplied name — constrained to loaded AirflowException subclasses — and then calls it. The cost is that a legacy node deserializes to a string rather than an exception object.

So the real question is whether anything still needs a real exception object back. If not, #68662 is the better fix and this should close in its favour. If something does, this keeps round-trip fidelity and #68662 breaks it.

No attachment to this one either way — flagging it so the two do not both land.

---

Drafted-by: Claude Code (Opus 5); reviewed by @potiuk before posting