You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@airflow.apache.org by "naruto-lgtm (via GitHub)" <gi...@apache.org> on 2026/06/16 09:52:37 UTC
[PR] quote remaining identifiers as single path segments in sdk client [airflow]
naruto-lgtm opened a new pull request, #68617:
URL: https://github.com/apache/airflow/pull/68617
Repro: pass an identifier containing `/` or `..` to one of the remaining lookups in the task SDK client, e.g. `connections.get("x/../../variables/secret_key")`. httpx collapses the dot-segments and the request lands on an unrelated API path.
Cause: #68129 quoted `dag_id` in `DagsOperations.get`, but the sibling single-segment lookups still interpolate the identifier into the URL unescaped: `ConnectionOperations.get`, `TaskInstanceOperations.get_previous`, and the four `DagRunOperations` methods.
Fix: wrap each identifier in `quote(..., safe='')` so it stays one path segment, same as the dags lookup.
---
##### Was generative AI tooling used to co-author this PR?
- [ ] Yes (please specify the tool below)
--
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] quote remaining identifiers as single path segments in sdk client [airflow]
Posted by "boring-cyborg[bot] (via GitHub)" <gi...@apache.org>.
boring-cyborg[bot] commented on PR #68617:
URL: https://github.com/apache/airflow/pull/68617#issuecomment-4717199574
Congratulations on your first Pull Request and welcome to the Apache Airflow community! If you have any issues or are unsure about any anything please check our [Contributors' Guide](https://github.com/apache/airflow/blob/main/contributing-docs/README.rst)
Here are some useful points:
- Pay attention to the quality of your code (ruff, mypy and type annotations). Our [prek-hooks]( https://github.com/apache/airflow/blob/main/contributing-docs/08_static_code_checks.rst#prerequisites-for-prek-hooks) will help you with that.
- In case of a new feature add useful documentation (in docstrings or in `docs/` directory). Adding a new operator? Check this short [guide](https://github.com/apache/airflow/blob/main/airflow-core/docs/howto/custom-operator.rst) Consider adding an example Dag that shows how users should use it.
- Consider using [Breeze environment](https://github.com/apache/airflow/blob/main/dev/breeze/doc/README.rst) for testing locally, it's a heavy docker but it ships with a working Airflow and a lot of integrations.
- Be patient and persistent. It might take some time to get a review or get the final approval from Committers.
- Please follow [ASF Code of Conduct](https://www.apache.org/foundation/policies/conduct) for all communication including (but not limited to) comments on Pull Requests, Mailing list and Slack.
- Be sure to read the [Airflow Coding style]( https://github.com/apache/airflow/blob/main/contributing-docs/05_pull_requests.rst#coding-style-and-best-practices).
- Always keep your Pull Requests rebased, otherwise your build might fail due to changes not related to your commits.
Apache Airflow is a community-driven project and together we are making it better 🚀.
In case of doubts contact the developers at:
Mailing List: dev@airflow.apache.org
Slack: https://s.apache.org/airflow-slack
--
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] Stop Task SDK client identifiers from reaching other API endpoints [airflow]
Posted by "boring-cyborg[bot] (via GitHub) via commits" <co...@airflow.apache.org>.
boring-cyborg[bot] commented on PR #68617:
URL: https://github.com/apache/airflow/pull/68617#issuecomment-6705344044
Awesome work, congrats on your first merged pull request! You are invited to check our [Issue Tracker](https://github.com/apache/airflow/issues) for additional contributions.
--
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] Correctly quote remaining identifiers in single path segments in TaskSDK client [airflow]
Posted by "naruto-lgtm (via GitHub)" <gi...@apache.org>.
naruto-lgtm commented on PR #68617:
URL: https://github.com/apache/airflow/pull/68617#issuecomment-5031039208
Rebased on main and resolved the conflicts. The only substantive one: main dropped the all_map_indices param from TaskStateStoreOperations.clear since I branched, so the rebased version keeps main's signature and just routes the path through _path (the now-obsolete test for that param went away with it). Full test_client.py passes locally and the PR is marked ready for review 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] Correctly quote remaining identifiers in single path segments in TaskSDK client [airflow]
Posted by "ashb (via GitHub)" <gi...@apache.org>.
ashb commented on PR #68617:
URL: https://github.com/apache/airflow/pull/68617#issuecomment-6171097434
@naruto-lgtm If you update this one last time, and ping me I can see about landing this.
--
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] quote remaining identifiers as single path segments in sdk client [airflow]
Posted by "ashb (via GitHub)" <gi...@apache.org>.
ashb commented on PR #68617:
URL: https://github.com/apache/airflow/pull/68617#issuecomment-4742169370
> One nuance worth flagging: variable and xcom keys are matched with a :path converter server-side, so they can legitimately contain slashes. Quoting them sends a real slash as %2F, which the ASGI layer decodes back to / before routing, so multi-segment keys still resolve while a leading .. can no longer collapse onto another endpoint.
I think that's the correct behaviour. There was also problem reported elsewhere when people were using an external secrets backend for variables and needed the variable to start with a slash, i.e. `Variable.get("/foo/bar")` which was previously having problems with. I think this change should fix that? (or at least get part way towards 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] Correctly quote remaining identifiers in single path segments in TaskSDK client [airflow]
Posted by "naruto-lgtm (via GitHub)" <gi...@apache.org>.
naruto-lgtm commented on PR #68617:
URL: https://github.com/apache/airflow/pull/68617#issuecomment-4756837175
Yeah, this fixes that one. Under the old f-string `Variable.get("/foo/bar")` went out as `variables//foo/bar`, and httpx's own URL normalization collapses that empty segment down to `variables/foo/bar` before the request even leaves the client, so the leading slash was silently dropped. With the quoting it goes as `variables/%2Ffoo%2Fbar`, which nothing collapses, and the server decodes `%2F` back to `/` after the route match, so the key arrives as `/foo/bar` intact.
So the leading-slash case is genuinely fixed on the transport side. Anything still off for those keys would be downstream in the secrets-backend lookup rather than the URL construction.
--
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] quote remaining identifiers as single path segments in sdk client [airflow]
Posted by "naruto-lgtm (via GitHub)" <gi...@apache.org>.
naruto-lgtm commented on PR #68617:
URL: https://github.com/apache/airflow/pull/68617#issuecomment-4720414106
Done. Rewrote the subject in imperative mood and added a body covering the why (it finishes the path-segment quoting started in #68129).
--
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] quote remaining identifiers as single path segments in sdk client [airflow]
Posted by "ashb (via GitHub)" <gi...@apache.org>.
ashb commented on PR #68617:
URL: https://github.com/apache/airflow/pull/68617#issuecomment-4720478179
This is almost approaching the point where it might be worth having a helper method such as:
```python
def _url(self, safe_prefix, *parts):
return urljoin(safe_prefix + "/", ....)
```
Up to you if you want to do it in this PR or not
--
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] Correctly quote remaining identifiers in single path segments in TaskSDK client [airflow]
Posted by "github-actions[bot] (via GitHub)" <gi...@apache.org>.
github-actions[bot] commented on PR #68617:
URL: https://github.com/apache/airflow/pull/68617#issuecomment-5548038968
This pull request has been automatically marked as stale because it has not had recent activity. It will be closed in 5 days if no further activity occurs. Thank you for your contributions.
--
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] quote remaining identifiers as single path segments in sdk client [airflow]
Posted by "naruto-lgtm (via GitHub)" <gi...@apache.org>.
naruto-lgtm commented on PR #68617:
URL: https://github.com/apache/airflow/pull/68617#issuecomment-4742013252
Done. Routed variables, xcoms, the task store, HITL and connection-tests through `_path` too.
One nuance worth flagging: variable and xcom keys are matched with a `:path` converter server-side, so they can legitimately contain slashes. Quoting them sends a real slash as `%2F`, which the ASGI layer decodes back to `/` before routing, so multi-segment keys still resolve while a leading `..` can no longer collapse onto another endpoint. The store key is a single segment on the server, so that one's straightforward.
AssetStateStore already passes its key/name/uri as query params rather than path segments, so it wasn't affected.
I left `task-instances/{id}` and `task-reschedules/{id}` as-is, since those ids are server-issued UUIDs (strongly typed, and the task's own id rather than a lookup), so there's no traversal surface. Happy to run them through the helper too if you'd rather it be uniform everywhere.
Tests cover the segments whose behaviour actually changes (variable key, the xcom dag_id/run_id/task_id, the store key); the UUID-only endpoints are a no-op through the helper so I didn't add tests for those.
--
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] Correctly quote remaining identifiers in single path segments in TaskSDK client [airflow]
Posted by "potiuk (via GitHub)" <gi...@apache.org>.
potiuk commented on PR #68617:
URL: https://github.com/apache/airflow/pull/68617#issuecomment-5021932630
@naruto-lgtm This PR has been converted to **draft** because it does not yet meet our [Pull Request quality criteria](https://github.com/apache/airflow/blob/main/contributing-docs/05_pull_requests.rst#pull-request-quality-criteria).
**Issues found:**
- :x: **Merge conflicts**: This PR has merge conflicts with `main` and cannot be merged as-is. Rebase locally and resolve them: `git fetch upstream main && git rebase upstream/main`, resolve the conflicts, then force-push.
> **Note:** Your branch is **721 commits behind `main`**. Some check failures may be caused by changes in the base branch rather than by your PR. Please rebase your branch and push again to get up-to-date CI results.
**What to do next:**
- Fix each issue listed above.
- Make sure static checks pass locally (`prek run --from-ref main --stage pre-commit`).
- Mark the PR as "Ready for review" when you're done.
Converting a PR to draft is **not** a rejection — it is an invitation to bring the PR up to the project's standards so that maintainer review time is spent productively. There is no rush — take your time and work at your own pace. We appreciate your contribution and are happy to wait for updates. If you have questions, feel free to ask on the [Airflow Slack](https://s.apache.org/airflow-slack).
---
_Note: This comment was drafted by an AI-assisted triage tool and may contain mistakes. Once you have addressed the points above, an Apache Airflow maintainer — a real person — will take the next look at your PR. We use this [two-stage triage process](https://github.com/apache/airflow/blob/main/contributing-docs/25_maintainer_pr_triage.md#why-the-first-pass-is-automated) so that our maintainers' limited time is spent where it matters most: the conversation with you._
--
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] quote remaining identifiers as single path segments in sdk client [airflow]
Posted by "naruto-lgtm (via GitHub)" <gi...@apache.org>.
naruto-lgtm commented on PR #68617:
URL: https://github.com/apache/airflow/pull/68617#issuecomment-4727124812
Did it in this PR. Added a small `_path(prefix, *identifiers)` helper and routed all the identifier endpoints (including the original `dags` one) through it, so the quoting is the default rather than something each call site has to remember. The `/clear` and `/state` suffixes just get appended to the helper output. Output is byte-for-byte the same as before, so the existing path assertions still hold.
--
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] Correctly quote remaining identifiers in single path segments in TaskSDK client [airflow]
Posted by "naruto-lgtm (via GitHub)" <gi...@apache.org>.
naruto-lgtm commented on PR #68617:
URL: https://github.com/apache/airflow/pull/68617#issuecomment-4911937924
any update?
--
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] Stop Task SDK client identifiers from reaching other API endpoints [airflow]
Posted by "shahar1 (via GitHub) via commits" <co...@airflow.apache.org>.
shahar1 merged PR #68617:
URL: https://github.com/apache/airflow/pull/68617
--
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