Skip to content

Reject serde-reserved keys in HITL params_input - #71054

Merged
potiuk merged 15 commits into
apache:mainfrom
haseebmalik18:hitl-reject-serde-reserved-param-keys
Sep 22, 2026
Merged

potiuk merged 15 commits into
apache:mainfrom
haseebmalik18:hitl-reject-serde-reserved-param-keys

Conversation

@haseebmalik18

@haseebmalik18 haseebmalik18 commented Aug 4, 2026 •

Copy link
Copy Markdown
Contributor

A HITL response whose params_input contains a serde-reserved key (__classname__ or __id__) cannot be serialized when the task resumes. Today the response is silently discarded and the user has no way to resubmit a corrected one.

This validates params_input when the response is submitted and returns a 400 if it carries one of those keys, before anything is written. The task stays in AWAITING_INPUT, so the user can fix the input and resubmit.

Only the public HITL route needs this. The execution API route builds its params_input from trusted operator code rather than user input, so it is left as is.

closes: #71036

@haseebmalik18 haseebmalik18 changed the title Reject serde-reserved keys in HITL params_input #71036 Reject serde-reserved keys in HITL params_input Aug 4, 2026
@eladkal eladkal added this to the Airflow 3.3.1 milestone Aug 4, 2026
@eladkal
eladkal requested a review from potiuk August 4, 2026 07:22
@vatsrahul1001
vatsrahul1001 requested a review from Lee-W August 4, 2026 07:52
@amoghrajesh

Copy link
Copy Markdown
Contributor

This doesn't have to be in 3.3.1, but if we are comfortable (cc @Lee-W) we can consider it.

@Lee-W

Lee-W commented Aug 4, 2026

Copy link
Copy Markdown
Member

A few nitpicks, but overall this looks good. I also don't think it's worth blocking the 3.3.1 release.

@vatsrahul1001

Copy link
Copy Markdown
Contributor

Moved to 3.3.2

@haseebmalik18

haseebmalik18 commented Aug 5, 2026 •

Copy link
Copy Markdown
Contributor Author

@Lee-W Applied changes left from review

@jason810496 jason810496 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.

Thanks for fixing this.

@jason810496 jason810496 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.

Additionally, we could introduce a common function to handle at both side.

@vatsrahul1001

Copy link
Copy Markdown
Contributor

LGTM! @Lee-W can you review this again?

@jason810496 jason810496 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.

I prefer to move this to 3.3.3 milestone since the #71054 (comment) comment hasn't be addressed.

@haseebmalik18

Copy link
Copy Markdown
Contributor Author

Addressed the comments

@jason810496 jason810496 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.

LGTM once these comments be addressed, thanks.

Comment thread airflow-core/docs/tutorial/hitl.rst Resolved
@haseebmalik18

Copy link
Copy Markdown
Contributor Author

Addressed review comments

@jason810496 jason810496 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.

LGTM now, thanks for the update.

@henry3260 henry3260 left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

looks good!!

@henry3260

Copy link
Copy Markdown
Contributor

nit: This docstring also cites the old method (‎_walk)

def test_create_xcom_entry_blocks_forbidden_keys_in_json_string(self, test_client, value):
"""A forbidden payload submitted as a JSON string literal is blocked too.
``_check_forbidden_xcom_keys._walk`` previously descended dict/list/tuple but not

@haseebmalik18

Copy link
Copy Markdown
Contributor Author

Fixed stale reference

@potiuk
potiuk merged commit 1e1fcd2 into apache:main Sep 22, 2026
79 checks passed
@github-actions

Copy link
Copy Markdown
Contributor

Backport failed to create: v3-3-test. View the failure log Run details

Note: As of Merging PRs targeted for Airflow 3.X
the committer who merges the PR is responsible for backporting the PRs that are bug fixes (generally speaking) to the maintenance branches.

In matter of doubt please ask in #release-management Slack channel.

Status Branch Result
❌ v3-3-test Commit Link

You can attempt to backport this manually by running:

cherry_picker 1e1fcd2 v3-3-test

This should apply the commit to the v3-3-test branch and leave the commit in conflict state marking
the files that need manual conflict resolution.

After you have resolved the conflicts, you can continue the backport process by running:

cherry_picker --continue

If you don't have cherry-picker installed, see the installation guide.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

area:API Airflow's REST/HTTP API type:bug-fix Changelog: Bug Fixes

Projects

None yet

Development

Successfully merging this pull request may close these issues.

HITL responses with serde-reserved keys in params_input cannot be resumed or resubmitted

8 participants