CM-68642: stop SCA scan on restore command failure with --stop-on-error

execute_commands swallowed a non-zero restore exit (shell() returns None)
into an empty-string success, so restore() never returned None and the
--stop-on-error escalation in _try_restore_dependencies was unreachable.
Treat a None from shell() as a failed command and propagate None.

Add tests exercising the real execute_commands with only shell() mocked,
covering the failure sentinel the previous mocked tests skipped.

Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>

Author: omer-roth

cycode/cli/files_collector/sca/base_restore_dependencies.py

@@ -40,6 +40,9 @@ def execute_commands(
 
         for command in commands:
             command_output = shell(command=command, timeout=timeout, working_directory=working_directory)
+            if command_output is None:  # shell returns None when the command exited non-zero
+                logger.debug('Restore command failed, %s', {'command': command})
+                return None
             if command_output:
                 outputs.append(command_output)
 

tests/cli/files_collector/sca/test_base_restore_dependencies.py

@@ -11,7 +11,7 @@
 import pytest
 import typer
 
-from cycode.cli.files_collector.sca.base_restore_dependencies import BaseRestoreDependencies
+from cycode.cli.files_collector.sca.base_restore_dependencies import BaseRestoreDependencies, execute_commands
 from cycode.cli.models import Document
 
 _LOCK_FILE_NAME = 'generated.lock'
@@ -68,6 +68,39 @@ def side_effect(
     return side_effect
 
 
+class TestExecuteCommands:
+    """Directly test the shell-failure sentinel handling in execute_commands."""
+
+    def test_returns_none_when_a_command_fails(self) -> None:
+        """shell() returns None on non-zero exit; execute_commands must propagate None, not ''."""
+        with patch(f'{_BASE_MODULE}.shell', return_value=None):
+            result = execute_commands([['poetry', 'lock']], timeout=30)
+
+        assert result is None
+
+    def test_stops_at_first_failing_command(self) -> None:
+        """A failure in an earlier command short-circuits; later commands do not run."""
+        mock_shell = MagicMock(side_effect=[None, 'should-not-run'])
+        with patch(f'{_BASE_MODULE}.shell', mock_shell):
+            result = execute_commands([['a'], ['b']], timeout=30)
+
+        assert result is None
+        assert mock_shell.call_count == 1
+
+    def test_empty_output_success_is_not_treated_as_failure(self) -> None:
+        """A successful command with empty stdout ('') must NOT be treated as a failure."""
+        with patch(f'{_BASE_MODULE}.shell', return_value=''):
+            result = execute_commands([['poetry', 'lock']], timeout=30)
+
+        assert result == ''
+
+    def test_joins_successful_outputs(self) -> None:
+        with patch(f'{_BASE_MODULE}.shell', side_effect=['out1', 'out2']):
+            result = execute_commands([['a'], ['b']], timeout=30)
+
+        assert result == 'out1\nout2'
+
+
 class TestCleanupGeneratedFile:
     def test_generated_lockfile_is_deleted_after_restore(self, handler: _MinimalRestoreHandler, tmp_path: Path) -> None:
         doc = _make_doc(tmp_path)
@@ -132,6 +165,24 @@ def test_failed_command_returns_none_and_no_file_created(
         assert result is None
         assert not lock_path.exists()
 
+    def test_shell_failure_propagates_to_none_and_no_lockfile(
+        self, handler: _MinimalRestoreHandler, tmp_path: Path
+    ) -> None:
+        """End-to-end failure path: shell() returns None (non-zero exit) -> restore returns None.
+
+        Regression for the stop-on-error bug: execute_commands must NOT swallow a failed
+        command into an empty-string success. This exercises the real execute_commands with
+        only shell() mocked (the layer where a non-zero exit is signalled as None).
+        """
+        doc = _make_doc(tmp_path)
+        lock_path = tmp_path / _LOCK_FILE_NAME
+
+        with patch(f'{_BASE_MODULE}.shell', return_value=None):
+            result = handler.try_restore_dependencies(doc)
+
+        assert result is None, 'A failed restore command must return None so stop-on-error can fire'
+        assert not lock_path.exists()
+
     def test_generated_file_content_available_in_document_after_deletion(
         self, handler: _MinimalRestoreHandler, tmp_path: Path
     ) -> None: