PrakshiGoyal10 commented on PR #69998: URL: https://github.com/apache/airflow/pull/69998#issuecomment-5214770323
Thanks for the detailed review. I've reworked the repair path along these lines. Rebased onto `main` and pushed; CI is green (static checks, provider tests, and the CodeQL analysis all pass now). Point by point: - **Mutation on GET → confirmation + POST.** `GET` now renders a read-only confirmation page; only a same-site `POST` repairs and clears tasks. Rather than add a bespoke CSRF token, this leans on the same mechanism the core UI mutations rely on: the API server's auth cookie is `SameSite=Lax`, so it isn't sent on a cross-site `POST` — moving the mutation off `GET` is what closes the CSRF hole. - **Trusted identifiers only.** The link now carries only Airflow identifiers — the run's launch `task_id`, and for a single-task repair the target `task_id`. `databricks_conn_id` and the Databricks run id are derived server-side from the launch task's `WorkflowRunMetadata` XCom, never from the request, so a crafted link can't point the repair at an arbitrary connection or run. Task keys are reconstructed from the serialized Dag. - **Open redirect.** The redirect target is now a same-site relative path built from validated identifiers, which resolves both CodeQL findings. - **Version gating.** Registration is gated to Airflow 3.1+. I confirmed `resolve_user_from_token` is absent on 3.0.6, and that 3.0.6's `get_user` reads only the bearer header (not the `_token` cookie), so cookie-based link navigation couldn't work there regardless — 3.1+ is the right floor. - **Error hygiene.** Databricks failures now return a generic `502` with no upstream exception text, and task clearing happens only after a successful repair call. - **Output encoding.** Confirmation-page values are HTML-escaped so identifiers can't become reflected markup. - **`related: #52280`** added to the PR body. Tests cover the GET-doesn't-mutate behaviour, confirmation-page escaping, server-side identifier resolution on the POST, and the generic 502. Appreciate the offer to prepare a patch branch — I think the current revision covers the items raised, but happy to iterate if anything still looks off. --- Drafted-by: Claude Code (Opus 4.8); reviewed by @PrakshiGoyal10 -- 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]
