Enabled ruff S ruleset for security checks

Strategically disabled on various lines in cmd2 code where we are using subprocess.Popen() in a way it doesn't like as well as when we are using asserts intentionally like in transcript testing.

Author: tleonhardt

cmd2/argparse_completer.py

@@ -292,14 +292,14 @@ def update_mutex_groups(arg_action: argparse.Action) -> None:
 
             # If we're in a flag REMAINDER arg, force all future tokens to go to that until a double dash is hit
             if flag_arg_state is not None and flag_arg_state.is_remainder:
-                if token == '--':
+                if token == '--':  # noqa: S105
                     flag_arg_state = None
                 else:
                     consume_argument(flag_arg_state)
                 continue
 
             # Handle '--' which tells argparse all remaining arguments are non-flags
-            if token == '--' and not skip_remaining_flags:
+            if token == '--' and not skip_remaining_flags:  # noqa: S105
                 # Check if there is an unfinished flag
                 if (
                     flag_arg_state is not None

cmd2/argparse_custom.py

@@ -1046,9 +1046,6 @@ def _format_usage(
                 req_parts = re.findall(part_regexp, req_usage)
                 opt_parts = re.findall(part_regexp, opt_usage)
                 pos_parts = re.findall(part_regexp, pos_usage)
-                assert ' '.join(req_parts) == req_usage
-                assert ' '.join(opt_parts) == opt_usage
-                assert ' '.join(pos_parts) == pos_usage
 
                 # End cmd2 customization
 

cmd2/cmd2.py

@@ -1327,7 +1327,7 @@ def ppaged(self, msg: Any, *, end: str = '\n', chop: bool = False) -> None:
                 with self.sigint_protection:
                     import subprocess
 
-                    pipe_proc = subprocess.Popen(pager, shell=True, stdin=subprocess.PIPE, stdout=self.stdout)
+                    pipe_proc = subprocess.Popen(pager, shell=True, stdin=subprocess.PIPE, stdout=self.stdout)  # noqa: S602
                     pipe_proc.communicate(final_msg.encode('utf-8', 'replace'))
             except BrokenPipeError:
                 # This occurs if a command's output is being piped to another process and that process closes before the
@@ -2589,7 +2589,7 @@ def _run_cmdfinalization_hooks(self, stop: bool, statement: Optional[Statement])
                 # caused by certain binary characters having been printed to it.
                 import subprocess
 
-                proc = subprocess.Popen(['stty', 'sane'])
+                proc = subprocess.Popen(['stty', 'sane'])  # noqa: S603, S607
                 proc.communicate()
 
         data = plugin.CommandFinalizationData(stop, statement)
@@ -2862,7 +2862,7 @@ def _redirect_output(self, statement: Statement) -> utils.RedirectionSavedState:
                     kwargs['executable'] = shell
 
             # For any stream that is a StdSim, we will use a pipe so we can capture its output
-            proc = subprocess.Popen(  # type: ignore[call-overload]
+            proc = subprocess.Popen(  # type: ignore[call-overload]  # noqa: S602
                 statement.pipe_to,
                 stdin=subproc_stdin,
                 stdout=subprocess.PIPE if isinstance(self.stdout, utils.StdSim) else self.stdout,  # type: ignore[unreachable]
@@ -4212,7 +4212,7 @@ def do_shell(self, args: argparse.Namespace) -> None:
         # still receive the SIGINT since it is in the same process group as us.
         with self.sigint_protection:
             # For any stream that is a StdSim, we will use a pipe so we can capture its output
-            proc = subprocess.Popen(  # type: ignore[call-overload]
+            proc = subprocess.Popen(  # type: ignore[call-overload]  # noqa: S602
                 expanded_command,
                 stdout=subprocess.PIPE if isinstance(self.stdout, utils.StdSim) else self.stdout,  # type: ignore[unreachable]
                 stderr=subprocess.PIPE if isinstance(sys.stderr, utils.StdSim) else sys.stderr,  # type: ignore[unreachable]
@@ -4425,7 +4425,7 @@ def py_quit() -> None:
             if py_code_to_run:
                 try:
                     interp.runcode(py_code_to_run)  # type: ignore[arg-type]
-                except BaseException:  # noqa: BLE001
+                except BaseException:  # noqa: BLE001, S110
                     # We don't care about any exception that happened in the Python code
                     pass
 
@@ -4448,7 +4448,7 @@ def py_quit() -> None:
                     # Since quit() or exit() raise an EmbeddedConsoleExit, interact() exits before printing
                     # the exitmsg. Therefore, we will not provide it one and print it manually later.
                     interp.interact(banner=banner, exitmsg='')
-                except BaseException:  # noqa: BLE001
+                except BaseException:  # noqa: BLE001, S110
                     # We don't care about any exception that happened in the interactive console
                     pass
                 finally:

cmd2/transcript.py

@@ -108,9 +108,9 @@ def _test_transcript(self, fname: str, transcript: Iterator[str]) -> None:
             # Read the expected result from transcript
             if ansi.strip_style(line).startswith(self.cmdapp.visible_prompt):
                 message = f'\nFile {fname}, line {line_num}\nCommand was:\n{command}\nExpected: (nothing)\nGot:\n{result}\n'
-                assert not result.strip(), message
+                assert not result.strip(), message  # noqa: S101
                 # If the command signaled the application to quit there should be no more commands
-                assert not stop, stop_msg
+                assert not stop, stop_msg  # noqa: S101
                 continue
             expected_parts = []
             while not ansi.strip_style(line).startswith(self.cmdapp.visible_prompt):
@@ -124,13 +124,13 @@ def _test_transcript(self, fname: str, transcript: Iterator[str]) -> None:
 
             if stop:
                 # This should only be hit if the command that set stop to True had output text
-                assert finished, stop_msg
+                assert finished, stop_msg  # noqa: S101
 
             # transform the expected text into a valid regular expression
             expected = ''.join(expected_parts)
             expected = self._transform_transcript_expected(expected)
             message = f'\nFile {fname}, line {line_num}\nCommand was:\n{command}\nExpected:\n{expected}\nGot:\n{result}\n'
-            assert re.match(expected, result, re.MULTILINE | re.DOTALL), message
+            assert re.match(expected, result, re.MULTILINE | re.DOTALL), message  # noqa: S101
 
     def _transform_transcript_expected(self, s: str) -> str:
         r"""Parse the string with slashed regexes into a valid regex.

cmd2/utils.py

@@ -627,7 +627,8 @@ def _reader_thread_func(self, read_stdout: bool) -> None:
             write_stream = self._stderr
 
         # The thread should have been started only if this stream was a pipe
-        assert read_stream is not None
+        if read_stream is None:
+            raise ValueError("read_stream is None")
 
         # Run until process completes
         while self._proc.poll() is None:

pyproject.toml

@@ -206,7 +206,7 @@ select = [
     "RET", # flake8-return (various warnings related to implicit vs explicit return statements)
     "RSE", # flake8-raise (warn about unnecessary parentheses on raised exceptions)
     # "RUF", # Ruff-specific rules (miscellaneous grab bag of lint checks specific to Ruff)
-    # "S", # flake8-bandit (security oriented checks, but extremely pedantic - do not attempt to apply to unit test files)
+    "S", # flake8-bandit (security oriented checks, but extremely pedantic - do not attempt to apply to unit test files)
     # "SIM", # flake8-simplify (rules to attempt to simplify code)
     # "SLF",  # flake8-self (warn when protected members are accessed outside of a class or file)
     "SLOT", # flake8-slots (warn about subclasses that should define __slots__)