fix: skip whitespace-only lines in _concatenate_shell_commands to prevent stray semicolons (caldera#3097) (#48)

* fix: skip whitespace-only lines in _concatenate_shell_commands to prevent stray semicolons

When the prereq block (ending with 'fi;') is combined with the ability command via
'dep_construct \n command', a whitespace-only line between them caused
_concatenate_shell_commands to append '; ' after it, producing commands like
'fi;  ;  ip neighbour show' (issue #3097 on mitre/caldera).

Filtering out whitespace-only lines before concatenation eliminates the stray separator.
A regression test is included.

* fix: update regression test to use double-space pattern that triggers the actual bug

The original regression test used a single-space whitespace line (' \n ')
between the prereq block and the command, but the real bug (issue #3097)
produced 'fi;  ;  ip neighbour show' with double spaces. Update the test
input to '  \n  ' and the assertion to check for ';  ;' so the test
accurately reproduces the triggering condition.

* fix: use regex to assert against general whitespace-between-semicolons pattern

Replace the exact string check for ';  ;' (double-space only) with a
regex assertion re.search(r';\s+;', result) so the regression test
catches any amount of whitespace between consecutive semicolons, not
just the double-space case from the original bug report.

Author: deacon-mp

app/atomic_svc.py

@@ -189,10 +189,12 @@ def _handle_multiline_commands(cmd, executor):
     @staticmethod
     def _concatenate_shell_commands(command_lines):
         """Concatenate multiple shell command lines. The ; character won't be added at the end of each command if the
-        command line ends in "then" or "do" or already ends with a ; character."""
+        command line ends in "then" or "do" or already ends with a ; character.
+        Whitespace-only lines are skipped to avoid producing stray ';' separators."""
         to_concat = []
-        num_lines = len(command_lines)
-        for index, cmd in enumerate(command_lines):
+        non_empty_lines = [cmd for cmd in command_lines if cmd.strip()]
+        num_lines = len(non_empty_lines)
+        for index, cmd in enumerate(non_empty_lines):
             to_concat.append(cmd)
             if re.search(r'do\s*$', cmd) or re.search(r'then\s*$', cmd) or re.search(r';\s*$', cmd):
                 if not re.search(r'\s+$', cmd):

tests/test_atomic_svc.py

@@ -1,5 +1,6 @@
 import hashlib
 import os
+import re
 import pytest
 
 from plugins.atomic.app.atomic_svc import AtomicService
@@ -178,6 +179,19 @@ def test_handle_multiline_command_shell_ifthen(self):
         want = 'if condition; then innercommand; innercommand2; fi'
         assert AtomicService._handle_multiline_commands(commands, 'sh') == want
 
+    def test_handle_multiline_command_no_extra_semicolon_after_fi(self):
+        """Regression test for issue #3097: whitespace-only lines between prereq block
+        (ending with 'fi;') and the ability command must not produce a stray '; ' separator,
+        which resulted in commands like 'fi;  ;  ip neighbour show'."""
+        # Simulate the precmd built by _prepare_executor:
+        # dep_construct ends with 'fi;', then '  \n  ' (two spaces) separates it from the
+        # ability command — matching the double-space pattern of the actual bug report.
+        commands = 'if [ -x "$(command -v ip)" ]; then : ; else apt-get install iproute2 -y; fi;\n  \n  ip neighbour show'
+        result = AtomicService._handle_multiline_commands(commands, 'sh')
+        assert not re.search(r';\s+;', result), \
+            f"Unexpected consecutive semicolons with only whitespace between them in: {result!r}"
+        assert 'ip neighbour show' in result
+
     def test_use_default_inputs(self, atomic_svc, atomic_test):
         platform = 'windows'
         string_to_analyze = '#{recon_commands} -a'