From c13014b01483b82091afa74d5b2695ec4d0e859a Mon Sep 17 00:00:00 2001 From: ro-mo-do Date: Mon, 29 Jun 2026 11:29:58 -0500 Subject: [PATCH 1/3] 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. --- src/datacustomcode/token_provider.py | 2 ++ 1 file changed, 2 insertions(+) diff --git a/src/datacustomcode/token_provider.py b/src/datacustomcode/token_provider.py index 981aebf..1d6db70 100644 --- a/src/datacustomcode/token_provider.py +++ b/src/datacustomcode/token_provider.py @@ -98,6 +98,7 @@ def get_token(self) -> "AccessTokenResponse": """Get token from Salesforce SF CLI""" import json import subprocess + import sys from datacustomcode.deploy import AccessTokenResponse @@ -109,6 +110,7 @@ def _run_sf_command(args: list[str], description: str) -> dict: text=True, check=True, timeout=30, + shell=sys.platform == "win32", ) except FileNotFoundError as exc: raise RuntimeError( From 6ee5618cccaf5363b725da7aaff59329d77ccebb Mon Sep 17 00:00:00 2001 From: ro-mo-do Date: Tue, 7 Jul 2026 12:18:17 -0500 Subject: [PATCH 2/3] 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. --- src/datacustomcode/token_provider.py | 21 +++++++++++---------- 1 file changed, 11 insertions(+), 10 deletions(-) diff --git a/src/datacustomcode/token_provider.py b/src/datacustomcode/token_provider.py index 1d6db70..5327933 100644 --- a/src/datacustomcode/token_provider.py +++ b/src/datacustomcode/token_provider.py @@ -97,11 +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 - import sys 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,14 +118,7 @@ def _run_sf_command(args: list[str], description: str) -> dict: text=True, check=True, timeout=30, - shell=sys.platform == "win32", ) - 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}'" @@ -144,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", {}) @@ -161,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", From 3c9c261d4adced91aef8fc15cd21d987ccd625a3 Mon Sep 17 00:00:00 2001 From: ro-mo-do Date: Tue, 7 Jul 2026 13:25:47 -0500 Subject: [PATCH 3/3] fix(tests): mock shutil.which so existing tests pass without sf installed MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit 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. --- tests/io/reader/test_sf_cli.py | 7 +++++- tests/test_token_provider.py | 46 ++++++++++++++++++---------------- 2 files changed, 30 insertions(+), 23 deletions(-) diff --git a/tests/io/reader/test_sf_cli.py b/tests/io/reader/test_sf_cli.py index eca3a56..b7f1b8e 100644 --- a/tests/io/reader/test_sf_cli.py +++ b/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() diff --git a/tests/test_token_provider.py b/tests/test_token_provider.py index 4fa4a02..b150a73 100644 --- a/tests/test_token_provider.py +++ b/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()