potiuk commented on PR #67667: URL: https://github.com/apache/airflow/pull/67667#issuecomment-4725465993
Thanks @shahar1 — all addressed in 446f362: 1. **Containment gap (relative base):** confirmed and fixed. The check now applies to relative bases too, not just absolute ones — only `.`/`""` (the SFTP login dir) skip it. Added test cases for `incoming` + `../.ssh/authorized_keys` and `uploads/in` + `../../etc/passwd`, which fail without the fix. 2. **Broad `AirflowException`:** switched to `ValueError` to match the sibling check in `gcs.py` and the AGENTS.md guidance; reverted the `known_airflow_exceptions.txt` bump. 3. **`test_gcs.py` / pre-existing logic:** you're right — the `gcs.py` containment check shipped in #67509, so this PR's only change there was a comment. Dropped both the comment and the test, so the PR no longer touches `gcs.py`. 4. **Over-commenting:** trimmed the `gcs_to_sftp.py` comment block down to the essential "why". The `normpath`-return shape change is intentional (needed for the check; it only collapses `//` and `./` on valid paths). `test_gcs_to_sftp.py` passes locally (41 passed), and the four reject cases each fail against the pre-fix code. --- Drafted-by: Claude Code (Opus 4.8); reviewed by @potiuk 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]
