ephraimbuddy commented on code in PR #72297:
URL: https://github.com/apache/airflow/pull/72297#discussion_r4397944652
##########
airflow-core/src/airflow/api_fastapi/execution_api/routes/xcoms.py:
##########
@@ -860,7 +863,12 @@ def _find_writer_id(
dag_bag: DBDagBag,
) -> UUID | None:
"""Resolve the task instance a write targets with the rule reads use,
working from the caller."""
- caller = session.get(TaskInstance, token.id,
execution_options={"include_all_attempts": True})
+ # Dag-parse tokens identify a parsing attempt, not a task instance;
resolve by coordinates only.
+ caller = (
+ session.get(TaskInstance, token.id,
execution_options={"include_all_attempts": True})
+ if isinstance(token, TIToken)
+ else None
Review Comment:
**Callback attempt identity:** `SetXCom` carries logical coordinates but
loses the callback’s task-instance UUID. If the original attempt has been
archived by a retry or clear, `_find_writer_id()` can resolve those coordinates
to its successor and write the old callback’s XCom onto the newer attempt.
Please preserve and validate the originating attempt identity, or reject stale
writes, with a regression test confirming that the successor’s XCom remains
unchanged.
---
Drafted-by: Codex (GPT-6); reviewed by @ephraimbuddy before posting
##########
airflow-core/src/airflow/api_fastapi/execution_api/routes/xcoms.py:
##########
@@ -679,6 +679,7 @@ def get_xcom(
@router.post(
"/{dag_id}/{run_id}/{task_id}/{key:path}",
status_code=status.HTTP_201_CREATED,
+ dependencies=[ExecutionOrDagParseToken, DagInGrantedBundle],
Review Comment:
**Security scope:** This also enables XCom writes over HTTP for `dag_parse`
tokens, so the security implications extend beyond the in-process client.
`DagInGrantedBundle` appears to preserve the bundle boundary, but the
permission covers eligible tasks/runs throughout that bundle, rather than being
tied to a specific callback. Please make that scope explicit and add HTTP tests
proving same-bundle writes succeed and cross-bundle writes are rejected. The
in-process tests bypass those authorization checks.
---
Drafted-by: Codex (GPT-6); reviewed by @ephraimbuddy before posting
--
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]
