You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@airflow.apache.org by "haseebmalik18 (via GitHub)" <gi...@apache.org> on 2026/08/04 05:00:26 UTC
[PR] Reject serde-reserved keys in HITL params_input #71036 [airflow]
haseebmalik18 opened a new pull request, #71054:
URL: https://github.com/apache/airflow/pull/71054
A HITL response whose `params_input` contains a serde-reserved key (`__classname__` or `__id__`) cannot be serialized when the task resumes, so the response is discarded and the user cannot resubmit a corrected one.
This rejects those keys with a 400 on the public HITL route before anything is recorded, so the task stays in `AWAITING_INPUT` and the user can resubmit.
closes: #71036
--
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: commits-unsubscribe@airflow.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] Reject serde-reserved keys in HITL params_input [airflow]
Posted by "haseebmalik18 (via GitHub)" <gi...@apache.org>.
haseebmalik18 commented on PR #71054:
URL: https://github.com/apache/airflow/pull/71054#issuecomment-5579343717
Addressed the comments
--
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: commits-unsubscribe@airflow.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] Reject serde-reserved keys in HITL params_input [airflow]
Posted by "github-actions[bot] (via GitHub)" <gi...@apache.org>.
github-actions[bot] commented on PR #71054:
URL: https://github.com/apache/airflow/pull/71054#issuecomment-5785509263
### Backport failed to create: v3-3-test. View the failure log <a href='https://github.com/apache/airflow/actions/runs/35794093467'> Run details </a>
Note: As of [Merging PRs targeted for Airflow 3.X](https://github.com/apache/airflow/blob/main/dev/README_AIRFLOW3_DEV.md#merging-prs-targeted-for-airflow-3x)
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](https://apache-airflow.slack.com/archives/C03G9H97MM2) Slack channel.
<table>
<tr>
<th>Status</th>
<th>Branch</th>
<th>Result</th>
</tr>
<tr>
<td>❌</td>
<td>v3-3-test</td>
<td><a href="https://github.com/apache/airflow/commit/1e1fcd224061f5ed88ad3cc533e1243f3fcadeac"><img src='https://img.shields.io/badge/Commit-1e1fcd2-red' alt='Commit Link'></a></td>
</tr>
</table>
You can attempt to backport this manually by running:
```bash
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:
```bash
cherry_picker --continue
```
If you don't have cherry-picker installed, see the [installation guide](https://github.com/apache/airflow/blob/main/dev/README.md#how-to-backport-pr-with-cherry-picker-cli).
--
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: commits-unsubscribe@airflow.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] Reject serde-reserved keys in HITL params_input [airflow]
Posted by "jason810496 (via GitHub)" <gi...@apache.org>.
jason810496 commented on code in PR #71054:
URL: https://github.com/apache/airflow/pull/71054#discussion_r3726472209
##########
airflow-core/src/airflow/api_fastapi/core_api/routes/public/hitl.py:
##########
@@ -80,6 +82,27 @@
log = structlog.get_logger(__name__)
+# Keys that ``serde.serialize`` refuses at any depth. A ``params_input`` carrying one could not be
Review Comment:
IMO, we should follow the convetion in XCom and validate at the data model layer:
https://github.com/apache/airflow/blob/9a85faa9c55b907bd105ee41567bb7d55dab9f31/airflow-core/src/airflow/api_fastapi/core_api/datamodels/xcom.py#L87-L119
https://github.com/apache/airflow/blob/9a85faa9c55b907bd105ee41567bb7d55dab9f31/shared/serialization/src/airflow_shared/serialization/__init__.py#L20-L44
--
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: commits-unsubscribe@airflow.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] Reject serde-reserved keys in HITL params_input [airflow]
Posted by "haseebmalik18 (via GitHub)" <gi...@apache.org>.
haseebmalik18 commented on code in PR #71054:
URL: https://github.com/apache/airflow/pull/71054#discussion_r3739561078
##########
airflow-core/src/airflow/api_fastapi/core_api/routes/public/hitl.py:
##########
@@ -80,6 +82,27 @@
log = structlog.get_logger(__name__)
+# Keys that ``serde.serialize`` refuses at any depth. A ``params_input`` carrying one could not be
Review Comment:
Good point, thanks! Only thing: XCom's validator raises `ValueError` so it comes back as 422, but the issue asks for 400 here? This route already does its option checks with `HTTPException(400)`, so I matched that. Would you still want it at the datamodel layer even if it becomes 422?
I also kept the keys to just `__classname__`/`__id__` (what serialize actually rejects) instead of the full `FORBIDDEN_XCOM_KEYS`, since the wider set would reject valid things like `{"__version__": 1}`. What do you think?
--
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: commits-unsubscribe@airflow.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] Reject serde-reserved keys in HITL params_input [airflow]
Posted by "vatsrahul1001 (via GitHub)" <gi...@apache.org>.
vatsrahul1001 commented on PR #71054:
URL: https://github.com/apache/airflow/pull/71054#issuecomment-5568607109
LGTM! @Lee-W can you review this again?
--
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: commits-unsubscribe@airflow.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] Reject serde-reserved keys in HITL params_input [airflow]
Posted by "vatsrahul1001 (via GitHub)" <gi...@apache.org>.
vatsrahul1001 commented on PR #71054:
URL: https://github.com/apache/airflow/pull/71054#issuecomment-5176539541
Moved to 3.3.2
--
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: commits-unsubscribe@airflow.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] Reject serde-reserved keys in HITL params_input [airflow]
Posted by "haseebmalik18 (via GitHub)" <gi...@apache.org>.
haseebmalik18 commented on PR #71054:
URL: https://github.com/apache/airflow/pull/71054#issuecomment-5643430440
Addressed review comments
--
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: commits-unsubscribe@airflow.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] Reject serde-reserved keys in HITL params_input [airflow]
Posted by "haseebmalik18 (via GitHub)" <gi...@apache.org>.
haseebmalik18 closed pull request #71054: Reject serde-reserved keys in HITL params_input
URL: https://github.com/apache/airflow/pull/71054
--
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: commits-unsubscribe@airflow.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] Reject serde-reserved keys in HITL params_input [airflow]
Posted by "haseebmalik18 (via GitHub)" <gi...@apache.org>.
haseebmalik18 commented on PR #71054:
URL: https://github.com/apache/airflow/pull/71054#issuecomment-5195628693
@Lee-W Addressed comments
--
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: commits-unsubscribe@airflow.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] Reject serde-reserved keys in HITL params_input [airflow]
Posted by "haseebmalik18 (via GitHub)" <gi...@apache.org>.
haseebmalik18 commented on code in PR #71054:
URL: https://github.com/apache/airflow/pull/71054#discussion_r3739561078
##########
airflow-core/src/airflow/api_fastapi/core_api/routes/public/hitl.py:
##########
@@ -80,6 +82,27 @@
log = structlog.get_logger(__name__)
+# Keys that ``serde.serialize`` refuses at any depth. A ``params_input`` carrying one could not be
Review Comment:
Good point, thanks! Only thing: XCom's validator raises `ValueError` so it comes back as 422, but the issue asks for 400 here? This route already does its option checks with `HTTPException(400)`, so I matched that. Would you still want it at the datamodel layer even if it becomes 422?
I also kept the keys to just `__classname__`/`__id__` (what serialize actually rejects) instead of the full `FORBIDDEN_XCOM_KEYS`, since the wider set would reject valid things like `{"__version__": 1}`. Sound ok?
--
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: commits-unsubscribe@airflow.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] Reject serde-reserved keys in HITL params_input [airflow]
Posted by "haseebmalik18 (via GitHub)" <gi...@apache.org>.
haseebmalik18 commented on code in PR #71054:
URL: https://github.com/apache/airflow/pull/71054#discussion_r3807387061
##########
airflow-core/src/airflow/api_fastapi/core_api/routes/public/hitl.py:
##########
@@ -80,6 +82,27 @@
log = structlog.get_logger(__name__)
+# Keys that ``serde.serialize`` refuses at any depth. A ``params_input`` carrying one could not be
Review Comment:
@jason810496 Following up
--
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: commits-unsubscribe@airflow.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] Reject serde-reserved keys in HITL params_input [airflow]
Posted by "jason810496 (via GitHub)" <gi...@apache.org>.
jason810496 commented on code in PR #71054:
URL: https://github.com/apache/airflow/pull/71054#discussion_r3974863182
##########
airflow-core/src/airflow/api_fastapi/core_api/datamodels/common.py:
##########
@@ -32,6 +34,56 @@
log = logging.getLogger(__name__)
+
+def find_reserved_keys(
+ value: Any,
+ reserved_keys: Collection[str],
Review Comment:
Let's use `FORBIDDEN_XCOM_KEYS` for this function and no need to introduce the `SERDE_RESERVED_DICT_KEYS`. Since `FORBIDDEN_XCOM_KEYS` is wider than the `SERDE_RESERVED_DICT_KEYS`.
```suggestion
```
##########
airflow-core/docs/tutorial/hitl.rst:
##########
@@ -221,6 +221,12 @@ calls involved (``~`` works as a wildcard for ``dag_id`` and ``dag_run_id``):
PATCH /api/v2/dags/{dag_id}/dagRuns/{dag_run_id}/taskInstances/{task_id}/{map_index}/hitlDetails
{"chosen_options": ["Approve"], "params_input": {}}
+.. note::
+
+ Keys in ``params_input`` may not be Airflow's reserved serialization keys (``__classname__`` or
Review Comment:
```suggestion
Keys in ``params_input`` must not be Airflow's reserved serialization keys (``__classname__`` or
```
##########
shared/serialization/src/airflow_shared/serialization/__init__.py:
##########
@@ -42,6 +42,11 @@
}
)
+# serde.serialize refuses to serialize a dict that contains either of these keys. This is narrower
+# than FORBIDDEN_XCOM_KEYS above, which also blocks keys that only matter when XCom reads a value
+# back and decodes it into an object.
+SERDE_RESERVED_DICT_KEYS = frozenset({CLASSNAME, SCHEMA_ID})
+
Review Comment:
```suggestion
```
##########
airflow-core/src/airflow/api_fastapi/core_api/datamodels/hitl.py:
##########
@@ -33,6 +35,21 @@ class UpdateHITLDetailPayload(BaseModel):
chosen_options: list[str] = Field(min_length=1)
params_input: Mapping = Field(default_factory=dict)
+ @field_validator("params_input")
+ @classmethod
+ def _check_serde_reserved_keys(cls, params_input: Mapping) -> Mapping:
+ # serde.serialize refuses these keys, and it only runs once the task resumes, long after the
+ # response was stored and the request returned. Reject it here instead, while the user can
+ # still correct the input and resubmit.
+ found = find_reserved_keys(params_input, SERDE_RESERVED_DICT_KEYS, root="params_input")
Review Comment:
Then please remember to update this downstream function.
##########
airflow-core/src/airflow/api_fastapi/core_api/datamodels/common.py:
##########
@@ -32,6 +34,56 @@
log = logging.getLogger(__name__)
+
+def find_reserved_keys(
+ value: Any,
+ reserved_keys: Collection[str],
+ *,
+ root: str = "value",
+ decode_json_strings: bool = False,
+) -> tuple[str, list[str]] | None:
+ """
+ Find the first mapping under ``value`` that holds one of ``reserved_keys``.
+
+ Returns the dotted path to that mapping and the reserved keys it holds, or ``None`` if there
+ are none.
+
+ Write paths that take arbitrary user data use this to turn away bad input while there is still
+ a request to fail. Storing it instead pushes the failure into whatever reads the value back,
+ where the error surfaces far from the person who submitted it.
+
+ Pass ``decode_json_strings`` when the read path parses stored strings back into containers, the
+ way XCom's does. Without it, a payload sent as ``json.dumps({...})`` slips through and comes
+ back out as a dict holding the reserved key. Leave it off where a string stays a string, as
+ with ``serde.serialize``.
+ """
+ reserved = frozenset(reserved_keys)
+
+ def walk(obj: Any, path: str) -> tuple[str, list[str]] | None:
+ if isinstance(obj, str):
+ if not decode_json_strings:
+ return None
+ try:
+ decoded = json.loads(obj)
+ except (ValueError, TypeError):
+ return None
+ return walk(decoded, path) if isinstance(decoded, (dict, list)) else None
+ if isinstance(obj, Mapping):
+ found = reserved & obj.keys()
Review Comment:
```suggestion
def walk(obj: Any, path: str) -> tuple[str, list[str]] | None:
if isinstance(obj, str):
if not decode_json_strings:
return None
try:
decoded = json.loads(obj)
except (ValueError, TypeError):
return None
return walk(decoded, path) if isinstance(decoded, (dict, list)) else None
if isinstance(obj, Mapping):
found = FORBIDDEN_XCOM_KEYS & obj.keys()
```
--
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: commits-unsubscribe@airflow.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] Reject serde-reserved keys in HITL params_input [airflow]
Posted by "amoghrajesh (via GitHub)" <gi...@apache.org>.
amoghrajesh commented on PR #71054:
URL: https://github.com/apache/airflow/pull/71054#issuecomment-5176275116
This doesn't have to be in 3.3.1, but if we are comfortable (cc @Lee-W) we can consider it.
--
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: commits-unsubscribe@airflow.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] Reject serde-reserved keys in HITL params_input [airflow]
Posted by "Lee-W (via GitHub)" <gi...@apache.org>.
Lee-W commented on PR #71054:
URL: https://github.com/apache/airflow/pull/71054#issuecomment-5176443270
A few nitpicks, but overall this looks good. I also don't think it's worth blocking the 3.3.1 release.
--
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: commits-unsubscribe@airflow.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] Reject serde-reserved keys in HITL params_input [airflow]
Posted by "jason810496 (via GitHub)" <gi...@apache.org>.
jason810496 commented on code in PR #71054:
URL: https://github.com/apache/airflow/pull/71054#discussion_r3930245384
##########
airflow-core/src/airflow/api_fastapi/core_api/routes/public/hitl.py:
##########
@@ -80,6 +82,27 @@
log = structlog.get_logger(__name__)
+# Keys that ``serde.serialize`` refuses at any depth. A ``params_input`` carrying one could not be
Review Comment:
Sorry for the late reply. Yes, it's fine to make it 422 status code as both 400 and 422 are valid the this use case.
--
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: commits-unsubscribe@airflow.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] Reject serde-reserved keys in HITL params_input [airflow]
Posted by "Lee-W (via GitHub)" <gi...@apache.org>.
Lee-W commented on code in PR #71054:
URL: https://github.com/apache/airflow/pull/71054#discussion_r3710647214
##########
airflow-core/tests/unit/api_fastapi/core_api/routes/public/test_hitl.py:
##########
@@ -408,6 +409,75 @@ def test_should_respond_400_for_invalid_option(
assert response.status_code == 400
assert "Invalid options" in response.json()["detail"]
+ @pytest.mark.usefixtures("sample_hitl_detail")
+ @pytest.mark.parametrize("reserved_key", [CLASSNAME, SCHEMA_ID])
Review Comment:
it's easy to miss if we add another reserved_key. let's make it a `_variable` in `airflow-core/src/airflow/api_fastapi/core_api/routes/public/hitl.py` so that we can just import it here. also worth mention this change in the doc
--
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: commits-unsubscribe@airflow.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] Reject serde-reserved keys in HITL params_input [airflow]
Posted by "haseebmalik18 (via GitHub)" <gi...@apache.org>.
haseebmalik18 commented on PR #71054:
URL: https://github.com/apache/airflow/pull/71054#issuecomment-5647520817
Fixed stale reference
--
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: commits-unsubscribe@airflow.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] Reject serde-reserved keys in HITL params_input [airflow]
Posted by "henry3260 (via GitHub)" <gi...@apache.org>.
henry3260 commented on PR #71054:
URL: https://github.com/apache/airflow/pull/71054#issuecomment-5644911430
nit: This docstring also cites the old method (`_walk`)
https://github.com/apache/airflow/blob/0bac61574a5b6f49ae930304298650e3c94681e9/airflow-core/tests/unit/api_fastapi/core_api/routes/public/test_xcom.py#L867-L870
--
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: commits-unsubscribe@airflow.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] Reject serde-reserved keys in HITL params_input [airflow]
Posted by "potiuk (via GitHub)" <gi...@apache.org>.
potiuk merged PR #71054:
URL: https://github.com/apache/airflow/pull/71054
--
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: commits-unsubscribe@airflow.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org