build: wrap nested conditionals in braces if they contain logical operators

ssg/build_remediations.py

@@ -218,6 +218,37 @@ class BashRemediation(Remediation):
     def __init__(self, file_path):
         super(BashRemediation, self).__init__(file_path, "bash")
 
+    @staticmethod
+    def wrap_conditionals_with_operators(conditionals):
+        """
+        Wrap bash conditionals that contain operators to ensure proper short-circuit evaluation.
+
+        When multiple platform conditionals are joined with OR (||), each conditional that
+        contains operators (&& or ||) must be wrapped in parentheses to ensure proper
+        bash short-circuit evaluation.
+
+        Without proper wrapping:
+            grep ... && { version_check } || grep ... && { version_check }
+        causes all version checks to execute due to bash operator precedence.
+
+        With proper wrapping:
+            ( grep ... && { version_check } ) || ( grep ... && { version_check } )
+        ensures only the matching platform's version check executes.
+
+        Args:
+            conditionals: List of bash conditional expressions
+
+        Returns:
+            List of conditionals with those containing operators wrapped in parentheses
+        """
+        wrapped_conditionals = []
+        for cond in conditionals:
+            if " && " in cond or " || " in cond:
+                wrapped_conditionals.append("( " + cond + " )")
+            else:
+                wrapped_conditionals.append(cond)
+        return wrapped_conditionals
+
     def parse_from_file(self, env_yaml, cpe_platforms):
         self.local_env_yaml.update(env_yaml)
         result = super(BashRemediation, self).parse_from_file(
@@ -241,10 +272,14 @@ def parse_from_file(self, env_yaml, cpe_platforms):
             if inherited_conditionals:
                 all_conditions += " && ".join(inherited_conditionals)
             if rule_specific_conditionals:
+                # Wrap rule-specific conditionals that contain operators to ensure
+                # proper bash short-circuit evaluation
+                wrapped_conditionals = BashRemediation.wrap_conditionals_with_operators(
+                    rule_specific_conditionals)
                 if all_conditions:
-                    all_conditions += " && { " + " || ".join(rule_specific_conditionals) + "; }"
+                    all_conditions += " && { " + " || ".join(wrapped_conditionals) + "; }"
                 else:
-                    all_conditions = " || ".join(rule_specific_conditionals)
+                    all_conditions = " || ".join(wrapped_conditionals)
             wrapped_fix_text.append("if {0}; then".format(all_conditions))
             wrapped_fix_text.append("")
             # It is possible to indent the original body of the remediation with textwrap.indent(),

tests/unit/ssg-module/data/platform_ol_test.yml

@@ -0,0 +1,7 @@
+name: platform_ol_test
+original_expression: os_linux[ol]>=8.7
+xml_content: <ns0:platform xmlns:ns0="http://cpe.mitre.org/language/2.0" id="platform_ol_test"><ns0:logical-test operator="AND" negate="false"><ns0:fact-ref name="cpe:/a:platform_ol_test" /></ns0:logical-test></ns0:platform>
+bash_conditional: 'grep -qP "^ID=[\"'']?ol[\"'']?$" "/etc/os-release" && { real="$(grep -P "^VERSION_ID=[\"'']?[\w.]+[\"'']?$" /etc/os-release | sed "s/^VERSION_ID=[\"'']\?\([^\"'']\+\)[\"'']\?$/\1/")"; expected="8.7"; printf "%s\n%s" "$expected" "$real" | sort -VC; }'
+ansible_conditional: 'ansible_distribution == "OracleLinux" and ansible_distribution_version >= "8.7"'
+title: 'Test Oracle Linux Platform >=8.7'
+documentation_complete: true

tests/unit/ssg-module/data/platform_rhel_test.yml

@@ -0,0 +1,7 @@
+name: platform_rhel_test
+original_expression: os_linux[rhel]>=8.7
+xml_content: <ns0:platform xmlns:ns0="http://cpe.mitre.org/language/2.0" id="platform_rhel_test"><ns0:logical-test operator="AND" negate="false"><ns0:fact-ref name="cpe:/a:platform_rhel_test" /></ns0:logical-test></ns0:platform>
+bash_conditional: 'grep -qP "^ID=[\"'']?rhel[\"'']?$" "/etc/os-release" && { real="$(grep -P "^VERSION_ID=[\"'']?[\w.]+[\"'']?$" /etc/os-release | sed "s/^VERSION_ID=[\"'']\?\([^\"'']\+\)[\"'']\?$/\1/")"; expected="8.7"; printf "%s\n%s" "$expected" "$real" | sort -VC; }'
+ansible_conditional: 'ansible_distribution == "RedHat" and ansible_distribution_version >= "8.7"'
+title: 'Test RHEL Platform >=8.7'
+documentation_complete: true

tests/unit/ssg-module/data/platform_sles_test.yml

@@ -0,0 +1,7 @@
+name: platform_sles_test
+original_expression: os_linux[sles]>=15
+xml_content: <ns0:platform xmlns:ns0="http://cpe.mitre.org/language/2.0" id="platform_sles_test"><ns0:logical-test operator="AND" negate="false"><ns0:fact-ref name="cpe:/a:platform_sles_test" /></ns0:logical-test></ns0:platform>
+bash_conditional: 'grep -qP "^ID=[\"'']?sles[\"'']?$" "/etc/os-release" && { real="$(grep -P "^VERSION_ID=[\"'']?[\w.]+[\"'']?$" /etc/os-release | sed "s/^VERSION_ID=[\"'']\?\([^\"'']\+\)[\"'']\?$/\1/")"; expected="15"; printf "%s\n%s" "$expected" "$real" | sort -VC; }'
+ansible_conditional: 'ansible_distribution == "SLES" and ansible_distribution_version >= "15"'
+title: 'Test SLES Platform >=15'
+documentation_complete: true

tests/unit/ssg-module/test_build_remediations.py

@@ -130,3 +130,85 @@ def test_parse_remediation_if_platform_has_version_comparison(cpe_platforms_with
     remediation_obj = remediation_cls(rhel_bash)
     conditionals = remediation_obj.get_stripped_conditionals("bash", ["package_ntp_eq_1_0"], cpe_platforms_with_version_comparison)
     assert conditionals == ["rpm --quiet -q ntp && { real=$(rpm -q --queryformat '%{VERSION}-%{RELEASE}' ntp); ver=1.0;[[ \"$real\" == \"$ver\" ]]; }"]
+
+
+@pytest.fixture
+def cpe_platforms_with_multiple_os(env_yaml):
+    """
+    Fixture providing multiple OS platforms with version comparisons.
+    Used to test proper parenthesization when combining platform conditionals with OR operators.
+    """
+    platforms = dict()
+
+    # Load RHEL platform
+    rhel_platform_path = os.path.join(DATADIR, "platform_rhel_test.yml")
+    rhel_platform = ssg.build_yaml.Platform.from_yaml(rhel_platform_path, env_yaml)
+    platforms[rhel_platform.name] = rhel_platform
+
+    # Load Oracle Linux platform
+    ol_platform_path = os.path.join(DATADIR, "platform_ol_test.yml")
+    ol_platform = ssg.build_yaml.Platform.from_yaml(ol_platform_path, env_yaml)
+    platforms[ol_platform.name] = ol_platform
+
+    # Load SLES platform
+    sles_platform_path = os.path.join(DATADIR, "platform_sles_test.yml")
+    sles_platform = ssg.build_yaml.Platform.from_yaml(sles_platform_path, env_yaml)
+    platforms[sles_platform.name] = sles_platform
+
+    return platforms
+
+
+def test_bash_remediation_wraps_platform_conditionals_with_operators(cpe_platforms_with_multiple_os):
+    """
+    Regression test for proper wrapping of platform conditionals containing operators.
+
+    When multiple platform conditionals are joined with OR (||), each conditional that
+    contains operators (&& or ||) must be wrapped in parentheses to ensure proper
+    bash short-circuit evaluation.
+
+    Without proper wrapping:
+        grep ... && { version_check } || grep ... && { version_check }
+    causes all version checks to execute due to bash operator precedence.
+
+    With proper wrapping:
+        ( grep ... && { version_check } ) || ( grep ... && { version_check } )
+    ensures only the matching platform's version check executes.
+    """
+    remediation_cls = sbr.REMEDIATION_TO_CLASS["bash"]
+    remediation_obj = remediation_cls(rhel_bash)
+
+    # Get conditionals for all three platforms
+    platform_names = ["platform_rhel_test", "platform_ol_test", "platform_sles_test"]
+    conditionals = remediation_obj.get_stripped_conditionals(
+        "bash", platform_names, cpe_platforms_with_multiple_os
+    )
+
+    # Verify we got all three conditionals
+    assert len(conditionals) == 3, f"Expected 3 conditionals, got {len(conditionals)}"
+
+    # Each conditional should contain && operator (ID check && version check)
+    for cond in conditionals:
+        assert " && " in cond, f"Conditional should contain &&: {cond}"
+
+    # Call the production wrapping method
+    wrapped = sbr.BashRemediation.wrap_conditionals_with_operators(conditionals)
+
+    # Verify all three conditionals are wrapped
+    assert len(wrapped) == 3, f"Expected 3 wrapped conditionals, got {len(wrapped)}"
+
+    # Verify each wrapped conditional is properly wrapped with parentheses
+    for i, wrapped_cond in enumerate(wrapped):
+        assert wrapped_cond.startswith("( "), \
+            f"Conditional {i} should start with '( ': {wrapped_cond}"
+        assert wrapped_cond.endswith(" )"), \
+            f"Conditional {i} should end with ' )': {wrapped_cond}"
+        # Verify the original conditional is preserved inside
+        assert conditionals[i] in wrapped_cond, \
+            f"Wrapped conditional {i} should contain original: {wrapped_cond}"
+
+    # Verify conditionals without operators are not wrapped
+    simple_conditionals = ["test -f /etc/os-release", "[ $UID -eq 0 ]"]
+    wrapped_simple = sbr.BashRemediation.wrap_conditionals_with_operators(simple_conditionals)
+    for i, wrapped_cond in enumerate(wrapped_simple):
+        assert wrapped_cond == simple_conditionals[i], \
+            f"Simple conditional {i} should not be wrapped: {wrapped_cond}"