This is an automated email from the ASF dual-hosted git repository.
potiuk pushed a commit to branch main
in repository https://gitbox.apache.org/repos/asf/airflow.git
The following commit(s) were added to refs/heads/main by this push:
new e1d881b5c4f Fix `SSHRemoteJobOperator` mangling environment values
with spaces or shell characters (#74810)
e1d881b5c4f is described below
commit e1d881b5c4f104d64817f6d7f19fe6e6023a81fd
Author: Kaxil Naik <[email protected]>
AuthorDate: Fri Oct 16 00:13:50 2026 +0100
Fix `SSHRemoteJobOperator` mangling environment values with spaces or shell
characters (#74810)
* Fix SSHRemoteJobOperator mangling environment values with spaces or shell
characters
On POSIX hosts each environment value was escaped for one level of single
quotes, but the export lines are spliced into the single-quoted job_script
literal, so the value's opening quote closed that literal. A value with a
space broke the wrapper, and $(...), backticks or ; in a value were run by
the wrapper shell instead of reaching the job.
Quote each value with shlex.quote, then escape the export block for the
job_script literal the same way the command already is.
* Document that SSHRemoteJobOperator passes environment values literally
Values used to be expanded by the remote shell as a side effect of the
quoting bug, so $VAR in a value no longer works. Say so in the docstring and
the operator docs, escape the exports and command in one step, and cover
backslashes and braced variables in the test.
---
providers/ssh/docs/operators/ssh_remote_job.rst | 4 +++
.../providers/ssh/operators/ssh_remote_job.py | 4 ++-
.../src/airflow/providers/ssh/utils/remote_job.py | 9 ++---
.../ssh/tests/unit/ssh/utils/test_remote_job.py | 42 ++++++++++++++++++++--
4 files changed, 52 insertions(+), 7 deletions(-)
diff --git a/providers/ssh/docs/operators/ssh_remote_job.rst
b/providers/ssh/docs/operators/ssh_remote_job.rst
index 7a8783b9861..f675bf6288e 100644
--- a/providers/ssh/docs/operators/ssh_remote_job.rst
+++ b/providers/ssh/docs/operators/ssh_remote_job.rst
@@ -114,6 +114,10 @@ With Environment Variables
},
)
+The job receives each value as a literal string: the remote shell does not
expand ``$VAR``,
+``~`` or ``$(...)`` inside it. To build a value from the remote environment,
such as extending
+``PATH``, do it in ``command``, for example
``command="PATH=$PATH:/opt/tools/bin python process_data.py"``.
+
Windows Remote Host
^^^^^^^^^^^^^^^^^^^
diff --git
a/providers/ssh/src/airflow/providers/ssh/operators/ssh_remote_job.py
b/providers/ssh/src/airflow/providers/ssh/operators/ssh_remote_job.py
index f0d9e5160eb..eab60940600 100644
--- a/providers/ssh/src/airflow/providers/ssh/operators/ssh_remote_job.py
+++ b/providers/ssh/src/airflow/providers/ssh/operators/ssh_remote_job.py
@@ -63,7 +63,9 @@ class SSHRemoteJobOperator(BaseOperator):
:param ssh_conn_id: SSH connection ID from Airflow Connections
:param command: Command to execute on the remote host (templated)
:param remote_host: Override the host from the connection (templated)
- :param environment: Environment variables to set for the command
(templated)
+ :param environment: Environment variables to set for the command
(templated).
+ Values are passed as literal strings; the remote shell does not expand
``$VAR``
+ or ``$(...)`` in them.
:param remote_base_dir: Base directory for job artifacts (templated).
Defaults to /tmp/airflow-ssh-jobs on POSIX,
C:\\Windows\\Temp\\airflow-ssh-jobs on Windows
:param poll_interval: Seconds between status polls (default: 5)
diff --git a/providers/ssh/src/airflow/providers/ssh/utils/remote_job.py
b/providers/ssh/src/airflow/providers/ssh/utils/remote_job.py
index 7cbfd5a529f..557bfef679e 100644
--- a/providers/ssh/src/airflow/providers/ssh/utils/remote_job.py
+++ b/providers/ssh/src/airflow/providers/ssh/utils/remote_job.py
@@ -183,10 +183,11 @@ def build_posix_wrapper_command(
if environment:
for key, value in environment.items():
_validate_env_var_name(key)
- escaped_value = value.replace("'", "'\"'\"'")
- env_exports += f"export {key}='{escaped_value}'\n"
+ env_exports += f"export {key}={shlex.quote(value)}\n"
- escaped_command = command.replace("'", "'\"'\"'")
+ # The exports and the command are spliced into the single-quoted
``job_script`` literal
+ # below, so escape them for it once more on top of the per-value quoting.
+ escaped_job_body = (env_exports + command).replace("'", "'\"'\"'")
# Launch detached under ``setsid`` so the job is its own
session/process-group
# leader, letting cancellation signal the whole job tree instead of
orphaning the
@@ -221,7 +222,7 @@ set +e
echo -n "$$" > "'"$pid_file"'"
export LOG_FILE="'"$log_file"'"
export STATUS_FILE="'"$status_file"'"
-{env_exports}{escaped_command} >>"'"$log_file"'" 2>&1
+{escaped_job_body} >>"'"$log_file"'" 2>&1
ec=$?
echo -n "$ec" > "'"$exit_code_tmp"'"
mv "'"$exit_code_tmp"'" "'"$exit_code_file"'"
diff --git a/providers/ssh/tests/unit/ssh/utils/test_remote_job.py
b/providers/ssh/tests/unit/ssh/utils/test_remote_job.py
index 2960de56584..e743c9fdd93 100644
--- a/providers/ssh/tests/unit/ssh/utils/test_remote_job.py
+++ b/providers/ssh/tests/unit/ssh/utils/test_remote_job.py
@@ -126,8 +126,8 @@ class TestBuildPosixWrapperCommand:
environment={"MY_VAR": "my_value", "OTHER": "test"},
)
- assert "export MY_VAR='my_value'" in wrapper
- assert "export OTHER='test'" in wrapper
+ assert "export MY_VAR=my_value" in wrapper
+ assert "export OTHER=test" in wrapper
def test_escapes_quotes(self):
"""Test that single quotes in command are escaped."""
@@ -504,3 +504,41 @@ class TestPosixPathQuoting:
subprocess.run(["sh", "-c", cmd], capture_output=True, check=False)
assert not marker.exists()
+
+
[email protected](shutil.which("bash") is None, reason="needs bash to run
the wrapper")
+class TestPosixEnvironmentQuoting:
+ """An environment value must reach the job verbatim and never run as shell
code."""
+
+ @pytest.mark.parametrize(
+ "value",
+ [
+ pytest.param("two words", id="space"),
+ pytest.param("x$(touch {marker})", id="command-substitution"),
+ pytest.param("x`touch {marker}`", id="backticks"),
+ pytest.param("x; touch {marker}", id="semicolon"),
+ pytest.param("it's'; touch {marker}; '", id="single-quotes"),
+ pytest.param('say "hi" to $HOME', id="double-quotes-and-variable"),
+ pytest.param("${PATH}:~/bin", id="braced-variable-and-tilde"),
+ pytest.param("C:\\data\\\\share", id="backslashes"),
+ pytest.param("line one\nline two", id="newline"),
+ pytest.param("", id="empty"),
+ ],
+ )
+ def test_value_reaches_job_verbatim(self, value, tmp_path):
+ marker = tmp_path / "injected"
+ value = value.replace("{marker}", str(marker))
+ paths = RemoteJobPaths(job_id="env_job", remote_os="posix",
base_dir=str(tmp_path / "jobs"))
+ wrapper = build_posix_wrapper_command('printf %s "$MY_VAR"', paths,
environment={"MY_VAR": value})
+
+ subprocess.run(["bash", "-c", wrapper], check=True,
capture_output=True)
+ # The job runs detached, so wait for it to record its exit code.
+ exit_code_file = Path(paths.exit_code_file)
+ deadline = time.monotonic() + 5
+ while time.monotonic() < deadline and not exit_code_file.exists():
+ time.sleep(0.02)
+
+ assert not marker.exists()
+ assert exit_code_file.exists(), "job never recorded its exit code"
+ assert exit_code_file.read_text() == "0"
+ assert Path(paths.log_file).read_text() == value
