Fix Windows FileNotFoundError when invoking sf CLI (#121)

* Fix Windows FileNotFoundError when invoking sf CLI

On Windows, subprocess.run(["sf", ...]) raises FileNotFoundError because
sf is a .cmd wrapper, not a plain executable. Passing shell=True (only on
win32) lets cmd.exe resolve the .cmd extension. No-op on macOS/Linux.

* fix: use shutil.which to resolve sf path, remove shell=True

Addresses reviewer concern about command injection risk with shell=True.
shutil.which resolves the sf executable (falling back to sf.cmd on
Windows), keeping shell=False everywhere while still working on Windows.

* fix(tests): mock shutil.which so existing tests pass without sf installed

The previous commit replaced shell=True + "sf" string with shutil.which()
to resolve the sf executable path before calling subprocess.run. This
moved the "sf not found" check earlier — before _run_sf_command is even
defined — meaning tests that only mocked subprocess.run were now hitting
the shutil.which() call for real. On CI (Linux, no SF CLI), which()
returns None and raises immediately, causing all 11 SFCLITokenProvider
and SFCLIDataCloudReader tests to fail with "sf command was not found"
rather than exercising the error condition they were testing.

Fix: mock shutil.which alongside subprocess.run in each affected test,
using the same unittest.mock.patch pattern already used throughout the
test suite. return_value="sf" keeps the resolved path consistent with the
hardcoded "sf" string the command-list assertions in test_sf_cli.py
expect. The "sf not found" tests are updated to patch shutil.which
returning None instead of patching subprocess.run with FileNotFoundError,
since that FileNotFoundError path was removed from the production code.

No new test files, no new imports, no new patterns — all changes are
confined to existing test classes and follow the same nesting style used
in TestCredentialsTokenProvider and the rest of the suite.

Author: ro-mo-do

src/datacustomcode/token_provider.py

@@ -97,10 +97,19 @@ def __init__(self, sf_cli_org: str):
     def get_token(self) -> "AccessTokenResponse":
         """Get token from Salesforce SF CLI"""
         import json
+        import shutil
         import subprocess
 
         from datacustomcode.deploy import AccessTokenResponse
 
+        sf_path = shutil.which("sf") or shutil.which("sf.cmd")
+        if sf_path is None:
+            raise RuntimeError(
+                "The 'sf' command was not found. "
+                "Install Salesforce CLI: "
+                "https://developer.salesforce.com/tools/salesforcecli"
+            )
+
         def _run_sf_command(args: list[str], description: str) -> dict:
             try:
                 result = subprocess.run(
@@ -110,12 +119,6 @@ def _run_sf_command(args: list[str], description: str) -> dict:
                     check=True,
                     timeout=120,
                 )
-            except FileNotFoundError as exc:
-                raise RuntimeError(
-                    "The 'sf' command was not found. "
-                    "Install Salesforce CLI: "
-                    "https://developer.salesforce.com/tools/salesforcecli"
-                ) from exc
             except subprocess.TimeoutExpired as exc:
                 raise RuntimeError(
                     f"'{description}' timed out for org '{self.sf_cli_org}'"
@@ -142,7 +145,7 @@ def _run_sf_command(args: list[str], description: str) -> dict:
 
         # Get org info from sf org display
         display_data = _run_sf_command(
-            ["sf", "org", "display", "--target-org", self.sf_cli_org, "--json"],
+            [sf_path, "org", "display", "--target-org", self.sf_cli_org, "--json"],
             "sf org display",
         )
         result_data = display_data.get("result", {})
@@ -159,7 +162,7 @@ def _run_sf_command(args: list[str], description: str) -> dict:
         try:
             token_data = _run_sf_command(
                 [
-                    "sf",
+                    sf_path,
                     "org",
                     "auth",
                     "show-access-token",

tests/io/reader/test_sf_cli.py

@@ -73,6 +73,11 @@ class TestGetToken:
     def reader(self):
         return _make_reader()
 
+    @pytest.fixture(autouse=True)
+    def mock_sf_which(self):
+        with patch("shutil.which", return_value="sf"):
+            yield
+
     def _run_result(self, stdout: str) -> MagicMock:
         result = MagicMock()
         result.stdout = stdout
@@ -118,7 +123,7 @@ def test_returns_token_and_instance_url(self, reader):
         )
 
     def test_file_not_found_raises_runtime_error(self, reader):
-        with patch("subprocess.run", side_effect=FileNotFoundError):
+        with patch("shutil.which", return_value=None):
             with pytest.raises(RuntimeError, match="'sf' command was not found"):
                 reader._get_token()
 

tests/test_token_provider.py

@@ -178,24 +178,25 @@ def test_successful_token_retrieval(self):
             }
         )
 
-        with patch("subprocess.run") as mock_run:
-            mock_run.side_effect = [
-                MagicMock(stdout=display_output),
-                MagicMock(stdout=token_output),
-            ]
+        with patch("shutil.which", return_value="sf"):
+            with patch("subprocess.run") as mock_run:
+                mock_run.side_effect = [
+                    MagicMock(stdout=display_output),
+                    MagicMock(stdout=token_output),
+                ]
 
-            result = provider.get_token()
+                result = provider.get_token()
 
-            assert isinstance(result, AccessTokenResponse)
-            assert result.access_token == "cli_access_token"
-            assert result.instance_url == "https://cli.salesforce.com"
+                assert isinstance(result, AccessTokenResponse)
+                assert result.access_token == "cli_access_token"
+                assert result.instance_url == "https://cli.salesforce.com"
 
-            # Verify both commands were called
-            assert mock_run.call_count == 2
-            display_call = mock_run.call_args_list[0]
-            token_call = mock_run.call_args_list[1]
-            assert "org" in display_call[0][0] and "display" in display_call[0][0]
-            assert "show-access-token" in token_call[0][0]
+                # Verify both commands were called
+                assert mock_run.call_count == 2
+                display_call = mock_run.call_args_list[0]
+                token_call = mock_run.call_args_list[1]
+                assert "org" in display_call[0][0] and "display" in display_call[0][0]
+                assert "show-access-token" in token_call[0][0]
 
     def test_fallback_to_display_token_on_older_cli(self):
         """Test fallback to sf org display token when show-access-token unavailable."""
@@ -224,17 +225,18 @@ def side_effect(*args, **kwargs):
             mock.stdout = display_output
             return mock
 
-        with patch("subprocess.run", side_effect=side_effect):
-            result = provider.get_token()
+        with patch("shutil.which", return_value="sf"):
+            with patch("subprocess.run", side_effect=side_effect):
+                result = provider.get_token()
 
-            assert isinstance(result, AccessTokenResponse)
-            assert result.access_token == "display_token"
-            assert result.instance_url == "https://cli.salesforce.com"
+                assert isinstance(result, AccessTokenResponse)
+                assert result.access_token == "display_token"
+                assert result.instance_url == "https://cli.salesforce.com"
 
     def test_sf_command_not_found(self):
-        """Test that FileNotFoundError is wrapped in RuntimeError."""
+        """Test that a missing sf binary raises RuntimeError."""
         provider = SFCLITokenProvider("test_org")
 
-        with patch("subprocess.run", side_effect=FileNotFoundError()):
+        with patch("shutil.which", return_value=None):
             with pytest.raises(RuntimeError, match="'sf' command was not found"):
                 provider.get_token()