Skip to content

Fix operators declaring template_fields/template_ext as bare strings - #70916

Merged
potiuk merged 1 commit into
apache:mainfrom
Eason09053360:fix-string-template-fields-providers
Aug 1, 2026
Merged

Fix operators declaring template_fields/template_ext as bare strings#70916
potiuk merged 1 commit into
apache:mainfrom
Eason09053360:fix-string-template-fields-providers

Conversation

@Eason09053360

Copy link
Copy Markdown
Contributor

A bare string is a sequence of characters, so "chat_id" declares the fields "c", "h", "a"... rather than "chat_id".

Airflow papers over this for template_fields by wrapping a string in a list, at the cost of a UserWarning on every task build. template_ext gets no such fallback: ".bat" expands to (".", "b", "a", "t"), so any templated value ending in one of those characters is mistaken for a script path and its contents silently loaded in its place.


Was generative AI tooling used to co-author this PR?
    • Yes — Claude Code (Opus 5)

Generated-by: Claude Code (Opus 5) following the guidelines


  • Read the Pull Request Guidelines for more information. Note: commit author/co-author name and email in commits become permanently public when merged.
  • For fundamental code changes, an Airflow Improvement Proposal (AIP) is needed.
  • When adding dependency, check compliance with the ASF 3rd Party License Policy.
  • For significant user-facing changes create newsfragment: {pr_number}.significant.rst, in airflow-core/newsfragments. You can add this file in a follow-up commit after the PR is created so you know the PR number.

A bare string is a sequence of characters, so "chat_id" declares the
fields "c", "h", "a"... rather than "chat_id".

Airflow papers over this for template_fields by wrapping a string in a
list, at the cost of a UserWarning on every task build. template_ext gets
no such fallback: ".bat" expands to (".", "b", "a", "t"), so any templated
value ending in one of those characters is mistaken for a script path and
its contents silently loaded in its place.

@potiuk potiuk left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Good sweep, and the template_ext case is worth calling out separately because it is meaningfully worse than the template_fields one.

template_ext = ".bat" iterates as (".", "b", "a", "t"), and resolve_template_files does:

if isinstance(content, str) and content.endswith(tuple(self.template_ext)):
    setattr(self, field, env.loader.get_source(env, content)[0])
except Exception:
    log.exception("Failed to resolve template field %r", field)

So for CmdOperator any command ending in ., b, a or t is treated as a script path — ls -lt, pip install requests, echo a all qualify. Usually the loader fails and it is merely logged, but if the string happens to resolve inside the Dag folder or template_searchpath, the command is silently replaced by that file's contents and executed. Unlike template_fields, there is no wrapping fallback and no warning, so nothing tells you it happened.

Two of the three are example dags, so they mislead by being copied rather than by running; TelegramFileOperator is the one that ships, where the effect is a UserWarning on every task build and a wrong class attribute.

Adding TestTelegramFileOperator with both the corrected tuple and an actual render is the right coverage. Not testing the example dags is fine — that is not the convention for them.


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

@potiuk
potiuk merged commit b528a94 into apache:main Aug 1, 2026
79 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

area:providers provider:edge Edge Executor / Worker (AIP-69) / edge3 provider:telegram

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants