From 58bb31f7686a49c23e686da756e215f0534452cf Mon Sep 17 00:00:00 2001 From: deacon Date: Sun, 15 Mar 2026 20:10:49 -0400 Subject: [PATCH 1/3] 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. --- app/atomic_svc.py | 8 +++++--- tests/test_atomic_svc.py | 11 +++++++++++ 2 files changed, 16 insertions(+), 3 deletions(-) diff --git a/app/atomic_svc.py b/app/atomic_svc.py index 32d3804..17bfafd 100644 --- a/app/atomic_svc.py +++ b/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): diff --git a/tests/test_atomic_svc.py b/tests/test_atomic_svc.py index 1329d3d..b5a66df 100644 --- a/tests/test_atomic_svc.py +++ b/tests/test_atomic_svc.py @@ -178,6 +178,17 @@ 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 ' separates it from the ability command. + 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 in result, f"Unexpected stray semicolon 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' From 735ca57f42991ca9f1adbe505306b532d7fae546 Mon Sep 17 00:00:00 2001 From: deacon Date: Mon, 16 Mar 2026 00:30:00 -0400 Subject: [PATCH 2/3] 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. --- tests/test_atomic_svc.py | 7 ++++--- 1 file changed, 4 insertions(+), 3 deletions(-) diff --git a/tests/test_atomic_svc.py b/tests/test_atomic_svc.py index b5a66df..53420ee 100644 --- a/tests/test_atomic_svc.py +++ b/tests/test_atomic_svc.py @@ -183,10 +183,11 @@ def test_handle_multiline_command_no_extra_semicolon_after_fi(self): (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 ' separates it from the ability command. - commands = 'if [ -x "$(command -v ip)" ]; then : ; else apt-get install iproute2 -y; fi;\n \n ip neighbour show' + # 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 in result, f"Unexpected stray semicolon in: {result!r}" + assert '; ;' not in result, f"Unexpected stray semicolon (double-space) in: {result!r}" assert 'ip neighbour show' in result def test_use_default_inputs(self, atomic_svc, atomic_test): From a303c71c81a342c60c19f7e36f3c2fb98c834861 Mon Sep 17 00:00:00 2001 From: deacon Date: Wed, 18 Mar 2026 09:06:07 -0400 Subject: [PATCH 3/3] 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. --- tests/test_atomic_svc.py | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/tests/test_atomic_svc.py b/tests/test_atomic_svc.py index 53420ee..b8d4a53 100644 --- a/tests/test_atomic_svc.py +++ b/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 @@ -187,7 +188,8 @@ def test_handle_multiline_command_no_extra_semicolon_after_fi(self): # 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 in result, f"Unexpected stray semicolon (double-space) in: {result!r}" + 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):