fix: resolve 14 bugs with TDD regression tests (#273)

* fix: resolve 14 verified production bugs with TDD regression tests

Bugs fixed (identified via semi-formal code reasoning analysis with
41 agents across 3 rounds + devil's advocate review):

Critical:
- C2: --output-formats CLI flag now propagates through orchestrator
  to execution engine to report phase (was silently ignored)
- C5: hasattr-on-dict bugs in sarif_utils.py and
  scanner_statistics_calculator.py replaced with proper dict key
  checks, restoring issue_severity extraction from Bandit

High:
- H2: FlatJSON reporter no longer injects phantom test finding on
  clean scans (test scaffolding removed from production)
- H3: Bedrock severity grouping uses "level" key (was "severity")
- H4: SecurityHub model_post_init__ renamed to model_post_init
- H5: Trivy scanner uses --severity with inclusion list mapping
  (was --severity-threshold which does not exist in Trivy CLI)
- H8: report_id strftime uses %m month (was %M minute)
- H-NEW: GitLabSASTReporter and UnusedSuppressionsReporter added
  to ASH_REPORTERS discovery list

Medium:
- M1: cancel_scan logs warning when process_id is None
- M3: EnginePhase uses AshEventType enum lookup (was string)
- M6: find_executable checks all command variants (was early return)
- M7: glob uses recursive=True for nested .gitignore discovery
- M8: Syft scanner preserves exclude paths after args rebuild
- M9: OCSF suppression counter uses StatusId.integer_3 (was 4)

Cleanup:
- Removed debug print statements from mcp_server.py
- Fixed split["."] syntax error in inspect_findings_app.py
- Deleted 409-line dead code file models/uv_tool_installation.py
- Deleted empty placeholder scan_tracking_modified.py

46 new regression tests added across 9 test files.
1016 tests pass, 0 failures, 0 regressions.

* fix: Windows CI compatibility for glob and find_executable tests

- Glob recursive tests now check both forward and backslash path separators
- find_executable test allows multiple which() calls (now iterates all variants)

* fix: replace hardcoded /tmp/ paths with tempfile.gettempdir() in tests

Resolves Bandit B108 findings flagged by ASH scan on test files.

* refactor(tests): organize regression tests by domain, not severity

Redistribute tests from test_low_fixes.py and test_medium_fixes.py
into domain-appropriate locations:
- M1 (cancel_scan) -> core/resource_management/test_cancel_scan_warning.py
- M3 (engine_phase) -> core/test_engine_phase_events.py
- M6 (find_executable) -> utils/test_find_executable_fix.py
- M7 (glob recursive) -> utils/test_glob_recursive_fix.py
- L1 (mcp debug) -> cli/test_mcp_debug_prints_fix.py
- L2 (inspect split) -> cli/test_inspect_split_fix.py
- Dead code -> test_dead_code_removal.py

No behavior change. Same 1016 tests pass.

* fix(syft): create target results directory before subprocess execution

The target_results_dir (e.g., /out/scanners/syft/source/) was not
created before calling syft, causing [Errno 2] when syft tried to
write its output file. Moved the mkdir call before _run_subprocess
and removed the redundant late mkdir that ran after the file read.

* fix(syft): use --exclude flag instead of --skip-path

Syft uses --exclude for path exclusion, not --skip-path (which is
silently ignored, causing syft to produce no output). Same class of
bug as H5 (Trivy wrong flag name). Previously masked by M8 bug
(exclude paths were discarded before reaching the command line).

* fix: proper exit code handling across all scanners

Two fixes for scanner exit code handling:

1. executionSuccessful in SARIF Invocation now reflects actual exit
   codes instead of being hardcoded True. Exit 0 and 1 (findings
   found) are treated as successful for most tools. Grype uses
   inverted semantics (exit 1=error, 2=findings) and is handled
   separately. 8 scanners fixed, 35 new tests.

2. Multi-subprocess scanners (cfn_nag, npm_audit) now retain the
   worst exit code across loop iterations instead of only the last.
   Uses max(self.exit_code, new_code) in plugin_base.py. 5 new tests.

Research verified exit code semantics from official documentation for
all 13 external tools ASH wraps: Bandit, Checkov, Grype, Syft,
Semgrep, OpenGrep, npm/yarn/pnpm audit, cfn_nag, CDK Nag, Trivy,
Snyk, detect-secrets, and Ferret.

* fix(syft): stop passing global_ignore_paths as --exclude to syft

global_ignore_paths (patterns like **/.venv/**) are designed for
SARIF-level finding suppression, not for syft's --exclude flag.
Passing them caused syft to produce empty output in Docker containers,
failing the container scan tests. Syft generates SBOMs (inventory),
not security findings, so SARIF ignore paths are irrelevant.

Scanner-specific exclude paths from config still work via
_process_config_options.

* chore: bump version to 3.3.0

Author: awsmadi

README.md

@@ -91,18 +91,18 @@ ASH v3 integrates multiple open-source security tools as scanners. Tools like Ba
 curl -sSfL https://astral.sh/uv/install.sh | sh
 
 # Create an alias for ASH
-alias ash="uvx git+https://github.com/awslabs/automated-security-helper.git@v3.2.7"
+alias ash="uvx git+https://github.com/awslabs/automated-security-helper.git@v3.3.0"
 ```
 
 ```powershell
 # Install uv on Windows with PowerShell if it isn't installed already
 irm https://astral.sh/uv/install.ps1 | iex
 
 # Create a function for ASH
-function ash { uvx git+https://github.com/awslabs/automated-security-helper.git@v3.2.7 $args }
+function ash { uvx git+https://github.com/awslabs/automated-security-helper.git@v3.3.0 $args }
 ```
 
-> **Floating tag `v3`**: We also maintain a `v3` floating tag that always points to the latest stable v3.x release. You can use `@v3` instead of `@v3.2.7` to stay up to date automatically. Pin a specific version (e.g., `@v3.2.7`) when you need reproducible builds.
+> **Floating tag `v3`**: We also maintain a `v3` floating tag that always points to the latest stable v3.x release. You can use `@v3` instead of `@v3.3.0` to stay up to date automatically. Pin a specific version (e.g., `@v3.3.0`) when you need reproducible builds.
 
 ### Other Installation Methods
 
@@ -122,13 +122,13 @@ ash --help
 #### Using `pip`
 
 ```bash
-pip install git+https://github.com/awslabs/automated-security-helper.git@v3.2.7
+pip install git+https://github.com/awslabs/automated-security-helper.git@v3.3.0
 ```
 
 #### Clone the Repository
 
 ```bash
-git clone https://github.com/awslabs/automated-security-helper.git --branch v3.2.7
+git clone https://github.com/awslabs/automated-security-helper.git --branch v3.3.0
 cd automated-security-helper
 pip install .
 ```

automated_security_helper/base/engine_phase.py

@@ -10,6 +10,7 @@
 from automated_security_helper.core.enums import ExecutionPhase
 from automated_security_helper.models.asharp_model import AshAggregatedResults
 from automated_security_helper.plugins import ash_plugin_manager
+from automated_security_helper.plugins.events import AshEventType
 from automated_security_helper.utils.log import ASH_LOGGER
 
 
@@ -87,7 +88,14 @@ def execute(
         self.initialize_progress()
 
         # Notify phase start
-        start_event = f"{self.phase_name.upper()}_PHASE_START"
+        start_event_name = f"{self.phase_name.upper()}_PHASE_START"
+        try:
+            start_event = AshEventType[start_event_name]
+        except KeyError:
+            ASH_LOGGER.warning(
+                f"No AshEventType member '{start_event_name}'; falling back to string"
+            )
+            start_event = start_event_name
         ASH_LOGGER.debug(f"EnginePhase.execute: Notifying {start_event} event")
         self.notify_event(start_event, **kwargs)
 
@@ -126,7 +134,14 @@ def execute(
             self._update_summary_stats()
 
         # Notify phase complete
-        complete_event = f"{self.phase_name.upper()}_PHASE_COMPLETE"
+        complete_event_name = f"{self.phase_name.upper()}_PHASE_COMPLETE"
+        try:
+            complete_event = AshEventType[complete_event_name]
+        except KeyError:
+            ASH_LOGGER.warning(
+                f"No AshEventType member '{complete_event_name}'; falling back to string"
+            )
+            complete_event = complete_event_name
         ASH_LOGGER.debug(f"EnginePhase.execute: Notifying {complete_event} event")
         self.notify_event(complete_event, results=results, **kwargs)
 
@@ -213,7 +228,11 @@ def update_progress(self, completed: int, description: str = None) -> None:
             )
 
             # Notify progress event
-            progress_event = f"{self.phase_name.upper()}_PHASE_PROGRESS"
+            progress_event_name = f"{self.phase_name.upper()}_PHASE_PROGRESS"
+            try:
+                progress_event = AshEventType[progress_event_name]
+            except KeyError:
+                progress_event = progress_event_name
             self.notify_event(
                 progress_event, completed=completed, description=description
             )

automated_security_helper/base/plugin_base.py

@@ -288,8 +288,9 @@ def _run_subprocess(
             if "stderr" in response and response["stderr"]:
                 self.errors.extend(response["stderr"].splitlines())
 
-            # Set exit code (default to 1 if not available)
-            self.exit_code = response.get("returncode", 1)
+            # Accumulate worst exit code across multiple subprocess calls
+            new_code = response.get("returncode", 1)
+            self.exit_code = max(self.exit_code, new_code)
 
             return response
 
@@ -385,8 +386,8 @@ def _try_uv_tool_execution(
             if response["stderr"]:
                 self.errors.extend(response["stderr"].splitlines())
 
-            # Set exit code
-            self.exit_code = response["returncode"]
+            # Accumulate worst exit code across multiple subprocess calls
+            self.exit_code = max(self.exit_code, response["returncode"])
 
             self._plugin_log(
                 f"UV tool execution completed for {self.command} with exit code {response['returncode']}",

automated_security_helper/cli/inspect/inspect_findings_app.py

@@ -111,7 +111,7 @@ def compose(self) -> ComposeResult:
                 if self.finding.get("code_snippet"):
                     md_text += f"""\n\n## Code
 
-```{Path(self.finding["file"]).name.split["."][-1]}
+```{Path(self.finding["file"]).name.split(".")[-1]}
 {self.finding.get("code_snippet")}
 ```
 """

automated_security_helper/cli/mcp_server.py

@@ -985,12 +985,6 @@ async def get_scan_summary(
     Args:
         output_dir: Path to the scan output directory (absolute path recommended)
     """
-    import sys
-
-    print("=" * 80, file=sys.stderr)
-    print("DEBUG: get_scan_summary FUNCTION CALLED!", file=sys.stderr)
-    print("=" * 80, file=sys.stderr)
-
     try:
         # Ensure output_dir is an absolute path
         if not Path(output_dir).is_absolute():

automated_security_helper/core/execution_engine.py

@@ -63,6 +63,7 @@ def __init__(
         python_based_plugins_only: bool = False,
         simple_mode: bool = False,
         ash_plugin_modules: List[str] = [],
+        output_formats: List[str] = [],
     ):
         """Initialize the execution engine.
 
@@ -130,6 +131,7 @@ def __init__(
             python_based_plugins_only  # Store the python_based_plugins_only flag
         )
         self._simple_mode = simple_mode  # Store the simple_mode flag
+        self._output_formats = output_formats  # CLI --output-formats
 
         # Get debug and verbose flags from environment or config
         debug_flag = debug or os.environ.get("ASH_DEBUG", "").lower() in [
@@ -510,11 +512,7 @@ def execute_phases(
                     # )
                     self._results = report_phase.execute(
                         report_dir=self._context.output_dir.joinpath("reports"),
-                        cli_output_formats=(
-                            self._context.config.output_formats
-                            if hasattr(self._context.config, "output_formats")
-                            else None
-                        ),
+                        cli_output_formats=self._output_formats or None,
                         aggregated_results=self._results,
                         python_based_plugins_only=self._python_only,
                     )

automated_security_helper/core/orchestrator.py

@@ -234,6 +234,7 @@ def model_post_init(self, context):
             debug=self.debug,
             python_based_plugins_only=self.python_based_plugins_only,
             ash_plugin_modules=self.ash_plugin_modules,  # Pass the ash_plugin_modules to the execution engine
+            output_formats=self.output_formats,
             **exec_engine_params,
         )
 

automated_security_helper/core/resource_management/scan_registry.py

@@ -383,7 +383,12 @@ def cancel_scan(self, scan_id: str) -> bool:
                 return False
 
             # Try to terminate the process if we have a process ID
-            if entry.process_id:
+            if entry.process_id is None:
+                self._logger.warning(
+                    f"Scan {scan_id} has no process ID (thread-based scan); "
+                    "cannot send termination signal"
+                )
+            elif entry.process_id:
                 try:
                     os.kill(entry.process_id, signal.SIGTERM)
                     self._logger.info(

automated_security_helper/core/resource_management/scan_tracking_modified.py

@@ -32,6 +32,7 @@
 from automated_security_helper.config.default_config import get_default_config
 from automated_security_helper.core.constants import ASH_DEFAULT_SEVERITY_LEVEL
 from automated_security_helper.models.asharp_model import AshAggregatedResults
+from automated_security_helper.schemas.sarif_schema_model import PropertyBag
 from automated_security_helper.utils.log import ASH_LOGGER
 
 
@@ -372,9 +373,15 @@ def extract_sarif_counts_for_scanner(
                         if not is_suppressed:
                             # Check if scanner provides issue_severity in properties (e.g., Bandit)
                             properties = result.properties or {}
+                            if isinstance(properties, PropertyBag):
+                                properties = properties.model_dump(
+                                    mode="json",
+                                    exclude_unset=True,
+                                    exclude_none=True,
+                                )
                             issue_severity = (
                                 properties.get("issue_severity", "").upper()
-                                if hasattr(properties, "get")
+                                if isinstance(properties, dict)
                                 else ""
                             )