Fix cmd.script bg=True deleting tempfile before child runs

changelog/50273.fixed.md

@@ -0,0 +1 @@
+Fixed ``cmd.script`` with ``bg=True`` deleting the temporary script before the background process could execute it, which caused ``No such file or directory`` on POSIX. Background runs now use a self-cleaning wrapper so the child removes the tempfile after exit. Refs #50273 #69959

changelog/69959.fixed.md

@@ -0,0 +1 @@
+Fixed ``cmd.script`` deleting the temporary script before a background (``bg=True``) process could run it. This caused PowerShell ``-File`` "does not exist" errors on Windows and "No such file or directory" on POSIX. Background runs now use a self-cleaning wrapper so the child removes the tempfile after exit. Refs #69959 #50273

salt/modules/cmdmod.py

@@ -289,6 +289,122 @@ def _prep_powershell_cmd(win_shell, cmd, encoded_cmd):
     return new_cmd
 
 
+def _ps_single_quote(value):
+    """Escape a string for use inside a PowerShell single-quoted literal."""
+    return str(value).replace("'", "''")
+
+
+def _is_powershell_shell(shell):
+    """Return True if shell names Windows PowerShell or PowerShell Core."""
+    if not shell:
+        return False
+    shell_l = str(shell).lower().strip()
+    return shell_l in ("powershell", "pwsh") or shell_l.endswith(
+        ("powershell.exe", "pwsh.exe")
+    )
+
+
+def _is_cmd_shell(shell):
+    """Return True if shell names cmd.exe."""
+    if not shell:
+        return False
+    shell_l = str(shell).lower().strip()
+    return shell_l in ("cmd", "cmd.exe") or shell_l.endswith("cmd.exe")
+
+
+def _prepare_bg_script(path, args, shell=None, win_cwd=None, cwd=None):
+    """
+    Build a self-cleaning command for ``cmd.script`` when ``bg=True``.
+
+    The parent must not delete ``path`` (or ``win_cwd``) after spawning the
+    background process. Instead we invoke a wrapper that runs the real script
+    and removes the tempfile(s) when finished. Refs #69959 #50273.
+    """
+    args = list(args) if args else []
+
+    if salt.utils.platform.is_windows() and _is_powershell_shell(shell):
+        wrapper_dir = cwd if cwd else None
+        wrapper_path = salt.utils.files.mkstemp(dir=wrapper_dir, suffix=".ps1")
+        script_q = _ps_single_quote(path)
+        win_cwd_block = ""
+        if win_cwd:
+            cwd_q = _ps_single_quote(win_cwd)
+            win_cwd_block = (
+                f"    Set-Location $env:TEMP\n"
+                f"    Remove-Item -LiteralPath '{cwd_q}' -Recurse -Force "
+                f"-ErrorAction SilentlyContinue\n"
+            )
+        content = (
+            f"$script = '{script_q}'\n"
+            "try {\n"
+            "    & $script @args\n"
+            "    exit $LASTEXITCODE\n"
+            "} finally {\n"
+            "    Remove-Item -LiteralPath $script -Force "
+            "-ErrorAction SilentlyContinue\n"
+            "    $wrapper = $PSCommandPath\n"
+            f"{win_cwd_block}"
+            "    Remove-Item -LiteralPath $wrapper -Force "
+            "-ErrorAction SilentlyContinue\n"
+            "}\n"
+        )
+        with salt.utils.files.fopen(wrapper_path, "w") as fh_:
+            fh_.write(content)
+        log.debug(
+            "cmd.script: bg=True PowerShell wrapper %s for script %s",
+            wrapper_path,
+            path,
+        )
+        return [wrapper_path, *args]
+
+    if salt.utils.platform.is_windows() and (
+        _is_cmd_shell(shell) or str(path).lower().endswith((".bat", ".cmd"))
+    ):
+        wrapper_dir = cwd if cwd else None
+        wrapper_path = salt.utils.files.mkstemp(dir=wrapper_dir, suffix=".cmd")
+        lines = [
+            "@echo off",
+            f'set "SALT_BG_SCRIPT={path}"',
+            'call "%SALT_BG_SCRIPT%" %*',
+            "set SALT_BG_EC=%ERRORLEVEL%",
+            'del /f /q "%SALT_BG_SCRIPT%" >nul 2>&1',
+        ]
+        if win_cwd:
+            lines.append(f'set "SALT_BG_CWD={win_cwd}"')
+            lines.append("cd /d %TEMP%")
+            lines.append('rd /s /q "%SALT_BG_CWD%" >nul 2>&1')
+        lines.extend(
+            [
+                'del /f /q "%~f0" >nul 2>&1',
+                "exit /b %SALT_BG_EC%",
+            ]
+        )
+        with salt.utils.files.fopen(wrapper_path, "w") as fh_:
+            fh_.write("\r\n".join(lines) + "\r\n")
+        log.debug(
+            "cmd.script: bg=True cmd wrapper %s for script %s",
+            wrapper_path,
+            path,
+        )
+        return [wrapper_path, *args]
+
+    # POSIX: wrap with /bin/sh. Do not use exec — replacing the shell would
+    # skip the EXIT trap and leave the tempfile behind.
+    # argv: sh -c BODY salt-cmd-script "$path" "$path" args...
+    # $1 is the tempfile to remove; "$@" after shift is the real script argv.
+    sh_body = 'script="$1"; shift; trap \'rm -f -- "$script"\' EXIT; "$@"'
+    log.debug("cmd.script: bg=True POSIX /bin/sh wrapper for script %s", path)
+    return [
+        "/bin/sh",
+        "-c",
+        sh_body,
+        "salt-cmd-script",
+        path,
+        path,
+        *args,
+    ]
+
+
 def _run(
     cmd,
     cwd=None,
@@ -3091,7 +3207,12 @@ def _cleanup_tempfile(path):
     if isinstance(args, str):
         args = salt.utils.args.shlex_split(args)
 
-    new_cmd = [path, *args] if args else [path]
+    if bg:
+        new_cmd = _prepare_bg_script(
+            path, args, shell=shell, win_cwd=cwd if win_cwd else None, cwd=cwd
+        )
+    else:
+        new_cmd = [path, *args] if args else [path]
 
     ret = {}
     try:
@@ -3126,10 +3247,13 @@ def _cleanup_tempfile(path):
             exc,
             exc_info_on_loglevel=logging.DEBUG,
         )
-    _cleanup_tempfile(path)
-    # If a temp working directory was created (Windows), let's remove that
-    if win_cwd:
-        _cleanup_tempfile(cwd)
+    # Background runs own tempfile cleanup via _prepare_bg_script wrappers.
+    # Deleting here races the child and causes missing-file errors (#69959).
+    if not bg:
+        _cleanup_tempfile(path)
+        # If a temp working directory was created (Windows), let's remove that
+        if win_cwd:
+            _cleanup_tempfile(cwd)
 
     if hide_output:
         ret["stdout"] = ret["stderr"] = ""

tests/pytests/functional/modules/cmd/test_script.py

@@ -1,5 +1,7 @@
+import os
 import shlex
 import stat
+import time
 from textwrap import dedent
 
 import pytest
@@ -323,3 +325,64 @@ def test_script_pipe_spaces_runas(modules, pipe_script_with_space_runas, account
         password=account.password,
     )
     assert result["stdout"] == "1"
+
+
+@pytest.fixture
+def bg_marker_script(state_tree, tmp_path):
+    """
+    Script that writes a marker file (and its own path) for bg=True tests.
+    """
+    marker = tmp_path / "bg_marker.txt"
+    if salt.utils.platform.is_windows():
+        file_name = "bg_marker.bat"
+        # %~f0 is the full path to this bat file
+        contents = dedent(
+            f"""\
+            @echo off
+            echo bg-ok^|%~f0>"{marker}"
+            """
+        )
+    else:
+        file_name = "bg_marker.sh"
+        contents = dedent(
+            f"""\
+            #!/bin/sh
+            printf 'bg-ok|%s\\n' "$0" > "{marker}"
+            """
+        )
+    with pytest.helpers.temp_file(file_name, contents, state_tree) as script_path:
+        if not salt.utils.platform.is_windows():
+            script_path.chmod(0o755)
+        yield file_name, marker
+
+
+def _wait_for_marker(marker_path, timeout=30):
+    deadline = time.time() + timeout
+    while time.time() < deadline:
+        if marker_path.is_file() and marker_path.stat().st_size > 0:
+            return marker_path.read_text(encoding="utf-8").strip()
+        time.sleep(0.1)
+    raise AssertionError(f"Marker file not written within {timeout}s: {marker_path}")
+
+
+def _wait_until_gone(path, timeout=30):
+    deadline = time.time() + timeout
+    while time.time() < deadline:
+        if not os.path.exists(path):
+            return
+        time.sleep(0.1)
+    raise AssertionError(f"Temp path still present after {timeout}s: {path}")
+
+
+def test_script_bg_writes_marker_and_cleans_temp(modules, bg_marker_script):
+    """
+    Regression for #69959 / #50273: cmd.script with bg=True must leave the
+    tempfile in place until the child runs, then clean it up.
+    """
+    file_name, marker = bg_marker_script
+    ret = modules.cmd.script(f"salt://{file_name}", bg=True)
+    assert isinstance(ret["pid"], int)
+    contents = _wait_for_marker(marker)
+    payload, script_path = contents.split("|", 1)
+    assert payload == "bg-ok"
+    _wait_until_gone(script_path)

tests/pytests/functional/modules/cmd/test_script_powershell.py

@@ -1,3 +1,5 @@
+import os
+import time
 from textwrap import dedent
 
 import pytest
@@ -51,6 +53,26 @@ def echo_script(state_tree):
         yield exit_code
 
 
+@pytest.fixture(scope="module")
+def marker_script(state_tree):
+    """
+    Write a marker file so bg=True tests can observe that the real script ran.
+    Also records $PSCommandPath so we can assert tempfile cleanup.
+    """
+    script_contents = dedent(
+        """\
+        param (
+            [Parameter(Mandatory=$true)]
+            [string]$OutFile,
+            [string]$Payload = "ok"
+        )
+        Set-Content -LiteralPath $OutFile -Value "$Payload|$PSCommandPath"
+        """
+    )
+    with pytest.helpers.temp_file("marker.ps1", script_contents, state_tree):
+        yield
+
+
 @pytest.fixture(params=["powershell", "pwsh"])
 def shell(request):
     """
@@ -115,3 +137,45 @@ def test_echo_runas(cmd, shell, account, echo_script, args, expected):
     assert ret["retcode"] == 0
     assert ret["stderr"] == ""
     assert ret["stdout"] == expected
+
+
+def _wait_for_marker(marker_path, timeout=30):
+    """Poll until the background script writes the marker file."""
+    deadline = time.time() + timeout
+    while time.time() < deadline:
+        if marker_path.is_file() and marker_path.stat().st_size > 0:
+            return marker_path.read_text(encoding="utf-8").strip()
+        time.sleep(0.1)
+    raise AssertionError(f"Marker file not written within {timeout}s: {marker_path}")
+
+
+def _wait_until_gone(path, timeout=30):
+    """Poll until path is removed by the bg self-cleanup wrapper."""
+    deadline = time.time() + timeout
+    while time.time() < deadline:
+        if not os.path.exists(path):
+            return
+        time.sleep(0.1)
+    raise AssertionError(f"Temp path still present after {timeout}s: {path}")
+
+
+def test_script_bg_writes_marker_and_cleans_temp(cmd, shell, marker_script, tmp_path):
+    """
+    Regression for #69959 / #50273: cmd.script bg=True must not delete the
+    tempfile before PowerShell can open it, and must still clean up afterward.
+    """
+    marker = tmp_path / "marker.txt"
+    ret = cmd.script(
+        "salt://marker.ps1",
+        args=["-OutFile", str(marker), "-Payload", "bg-ok"],
+        shell=shell,
+        saltenv="base",
+        bg=True,
+    )
+    assert isinstance(ret["pid"], int)
+    # Background runs do not wait for the process; retcode is not meaningful.
+    contents = _wait_for_marker(marker)
+    payload, script_path = contents.split("|", 1)
+    assert payload == "bg-ok"
+    assert script_path.lower().endswith(".ps1")
+    _wait_until_gone(script_path)

tests/pytests/unit/modules/test_cmdmod.py

@@ -1497,3 +1497,71 @@ def test_prep_powershell_json(text, expected):
     """
     result = cmdmod._prep_powershell_json(text)
     assert result == expected
+
+
+def test_ps_single_quote():
+    assert cmdmod._ps_single_quote(r"C:\temp\file.ps1") == r"C:\temp\file.ps1"
+    assert cmdmod._ps_single_quote("O'Brien") == "O''Brien"
+
+
+@pytest.mark.parametrize(
+    "shell, expected",
+    [
+        ("powershell", True),
+        ("pwsh", True),
+        (r"C:\Windows\System32\WindowsPowerShell\v1.0\powershell.exe", True),
+        ("cmd", False),
+        (None, False),
+    ],
+)
+def test_is_powershell_shell(shell, expected):
+    assert cmdmod._is_powershell_shell(shell) is expected
+
+
+def test_prepare_bg_script_posix():
+    path = "/tmp/__salt.tmp.abc123.sh"
+    ret = cmdmod._prepare_bg_script(path, ["arg1", "arg two"], shell="/bin/sh")
+    assert ret[0] == "/bin/sh"
+    assert ret[1] == "-c"
+    assert "trap" in ret[2]
+    # exec would replace /bin/sh and skip the EXIT trap (tempfile leak).
+    assert "exec" not in ret[2]
+    assert ret[3] == "salt-cmd-script"
+    assert ret[4] == path
+    assert ret[5] == path
+    assert ret[6:] == ["arg1", "arg two"]
+
+
+@pytest.mark.skip_unless_on_windows
+def test_prepare_bg_script_powershell(tmp_path):
+    script = tmp_path / "__salt.tmp.real.ps1"
+    script.write_text("Write-Output hi\n", encoding="utf-8")
+    ret = cmdmod._prepare_bg_script(
+        str(script), ["-OutFile", "x"], shell="powershell", cwd=str(tmp_path)
+    )
+    assert len(ret) == 3
+    wrapper = ret[0]
+    assert wrapper.endswith(".ps1")
+    assert ret[1:] == ["-OutFile", "x"]
+    with salt.utils.files.fopen(wrapper) as fh_:
+        content = fh_.read()
+    assert str(script) in content
+    assert "& $script @args" in content
+    assert "Remove-Item -LiteralPath $script" in content
+    os.remove(wrapper)
+
+
+@pytest.mark.skip_unless_on_windows
+def test_prepare_bg_script_cmd(tmp_path):
+    script = tmp_path / "__salt.tmp.real.bat"
+    script.write_text("@echo off\necho hi\n", encoding="utf-8")
+    ret = cmdmod._prepare_bg_script(
+        str(script), ["a", "b"], shell="cmd", cwd=str(tmp_path)
+    )
+    assert ret[0].endswith(".cmd")
+    assert ret[1:] == ["a", "b"]
+    with salt.utils.files.fopen(ret[0]) as fh_:
+        content = fh_.read()
+    assert str(script) in content
+    assert "SALT_BG_SCRIPT" in content
+    os.remove(ret[0])