MannXo opened a new pull request, #70863: URL: https://github.com/apache/airflow/pull/70863
`SSHRemoteJobOperator(remote_base_dir=...)` ran the job to completion and then failed the task on the last step, while removing the job directory: ```text Invalid job directory: '/tmp-data/airflow-ssh-jobs/af_ ....'. Expected Path under '/tmp/airflow-ssh-jobs' for safety. ``` The cleanup guard in `_validate_job_dir` compared the job directory against the platform default base dir, but the base dir is configurable through `remote_base_dir`. The operator's own `_validate_base_dir` accepts any non-traversal path, so a custom value passed validation at submit time and only blew up at cleanup, after an hour-long job had already finished. The job directory was also left behind. Validating against the *configured* base dir keeps the guard doing its job. Cleanup runs `rm -rf` / `Remove-Item -Recurse -Force`, so it must only ever target a directory the operator created, and it still can. `base_dir` is threaded as an optional argument through `build_posix_cleanup_command` / `build_windows_cleanup_command` to `_validate_job_dir`, defaulting to the platform default so existing callers and behaviour are unchanged. The operator passes `self.remote_base_dir` rather than `self._paths.base_dir`, because cleanup happens in `execute_complete` after deferral, on a fresh operator instance where `_paths` is `None`. A configured base dir with a trailing separator is also tolerated, since `RemoteJobPaths` joins with a single separator and the prefix would otherwise never match. closes: #69813 ### Tests 10 new tests, all reverted-and-checked: 9 of them fail without the source change. - `build_posix_cleanup_command` / `build_windows_cleanup_command` accept a custom base dir, including one with a trailing separator and one unrelated to the platform default. - The guard is narrowed to the configured base dir, not lifted: a sibling directory, and the platform default itself, are both still rejected once a custom base dir is set. - A sibling that shares the base dir's name prefix (`/opt/jobs-evil` against `/opt/jobs`) is rejected, since the check appends the separator before comparing. - At the operator level, `_cleanup_remote_job` with a custom `remote_base_dir` now issues the cleanup command, and still refuses a job dir outside that base dir without contacting the host. The tenth test, `test_cleanup_still_rejects_job_dir_outside_remote_base_dir`, passes before and after by design. It is not a regression test for this bug but a guard against fixing it by removing the check, which is the obvious wrong fix here. `providers/ssh/tests/unit/ssh/utils/test_remote_job.py` and `.../operators/test_ssh_remote_job.py` in full: 57 passed, 2 skipped. The 2 skips are the process-group teardown tests, which need a POSIX host with `setsid`; they skip on macOS regardless of this change. `ruff format --check` and `ruff check` are clean. I did not add a changelog entry, since provider changelogs are regenerated from `git log` at release time. Happy to add one to `providers/ssh/docs/changelog.rst` if you would prefer it explicit. --- ##### Was generative AI tooling used to co-author this PR? - [X] Yes — Claude Code (Opus 5) Generated-by: Claude Code (Opus 5) following [the guidelines](https://github.com/apache/airflow/blob/main/contributing-docs/05_pull_requests.rst#gen-ai-assisted-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: [email protected] For queries about this service, please contact Infrastructure at: [email protected]
