feat(reach): align reachability flags and coana env with Node CLI

mtorp · mtorp · commit 9c9c6a7b2e06 · 2026-06-03T11:03:29.000+02:00
Port the Node CLI reachability parity items (the unified --exclude-paths
lands separately):

- G1: --reach-disable-external-tool-checks -&gt; coana --disable-external-tool-checks
- G5: forward SOCKET_CLI_VERSION + SOCKET_CALLER_USER_AGENT to coana, and
  SOCKET_CLI_API_PROXY from an explicit value or HTTPS_PROXY/HTTP_PROXY
- G6: omit SOCKET_REPO_NAME/SOCKET_BRANCH_NAME when repo/branch are the default
  sentinels, so default-named runs don't share coana cache buckets
- G7: default reach memory-limit to 8192 and concurrency to 1
- G8: add Node-style --reach-analysis-timeout/--reach-analysis-memory-limit as
  the primary names; keep --reach-timeout/--reach-memory-limit as hidden aliases
- G9: add --reach-debug -&gt; coana --debug (global --enable-debug -&gt; -d unchanged)
- G11: retry tier1 finalize with exponential backoff (3 attempts), never raising

Also removes stray always-on WARNING logging in the reachability runner.
Adds tests for flag defaults/aliases, the coana command/env builder, and the
finalize retry.
diff --git a/pyproject.toml b/pyproject.toml
@@ -6,7 +6,7 @@ build-backend = "hatchling.build"
 
 [project]
 name = "socketsecurity"
-version = "2.4.0"
+version = "2.4.1"
 requires-python = ">= 3.11"
 license = {"file" = "LICENSE"}
 dependencies = [
diff --git a/socketsecurity/__init__.py b/socketsecurity/__init__.py
@@ -1,3 +1,3 @@
 __author__ = 'socket.dev'
-__version__ = '2.4.0'
+__version__ = '2.4.1'
 USER_AGENT = f'SocketPythonCLI/{__version__}'
diff --git a/socketsecurity/config.py b/socketsecurity/config.py
@@ -119,7 +119,7 @@ class CliConfig:
     # Reachability Flags
     reach: bool = False
     reach_version: Optional[str] = None
-    reach_analysis_memory_limit: Optional[int] = None
+    reach_analysis_memory_limit: Optional[int] = 8192
     reach_analysis_timeout: Optional[int] = None
     reach_disable_analytics: bool = False
     reach_disable_analysis_splitting: bool = False  # Deprecated, kept for backwards compatibility
@@ -131,14 +131,16 @@ class CliConfig:
     reach_skip_cache: bool = False
     reach_min_severity: Optional[str] = None
     reach_output_file: Optional[str] = None
-    reach_concurrency: Optional[int] = None
+    reach_concurrency: Optional[int] = 1
     reach_additional_params: Optional[List[str]] = None
     only_facts_file: bool = False
     reach_use_only_pregenerated_sboms: bool = False
     reach_continue_on_analysis_errors: bool = False
     reach_continue_on_install_errors: bool = False
     reach_continue_on_missing_lock_files: bool = False
     reach_continue_on_no_source_files: bool = False
+    reach_debug: bool = False
+    reach_disable_external_tool_checks: bool = False
     max_purl_batch_size: int = 5000
     enable_commit_status: bool = False
     legal: bool = False
@@ -267,6 +269,8 @@ def from_args(cls, args_list: Optional[List[str]] = None) -> 'CliConfig':
             'reach_continue_on_install_errors': args.reach_continue_on_install_errors,
             'reach_continue_on_missing_lock_files': args.reach_continue_on_missing_lock_files,
             'reach_continue_on_no_source_files': args.reach_continue_on_no_source_files,
+            'reach_debug': args.reach_debug,
+            'reach_disable_external_tool_checks': args.reach_disable_external_tool_checks,
             'max_purl_batch_size': args.max_purl_batch_size,
             'enable_commit_status': args.enable_commit_status,
             'legal': args.legal or args.legal_format == "fossa",
@@ -878,18 +882,33 @@ def create_argument_parser() -> argparse.ArgumentParser:
         help="Specific version of @coana-tech/cli to use (e.g., '1.2.3')"
     )
     reachability_group.add_argument(
-        "--reach-timeout",
+        "--reach-analysis-timeout",
         dest="reach_analysis_timeout",
         type=int,
         metavar="<seconds>",
         help="Timeout for reachability analysis in seconds"
     )
+    # Backwards-compatible alias for the pre-alignment name. Kept working, hidden from help.
     reachability_group.add_argument(
-        "--reach-memory-limit",
+        "--reach-timeout",
+        dest="reach_analysis_timeout",
+        type=int,
+        help=argparse.SUPPRESS
+    )
+    reachability_group.add_argument(
+        "--reach-analysis-memory-limit",
         dest="reach_analysis_memory_limit",
         type=int,
+        default=8192,
         metavar="<mb>",
-        help="Memory limit for reachability analysis in MB"
+        help="Memory limit for reachability analysis in MB (default: 8192)"
+    )
+    # Backwards-compatible alias for the pre-alignment name. Kept working, hidden from help.
+    reachability_group.add_argument(
+        "--reach-memory-limit",
+        dest="reach_analysis_memory_limit",
+        type=int,
+        help=argparse.SUPPRESS
     )
     reachability_group.add_argument(
         "--reach-ecosystems",
@@ -956,8 +975,9 @@ def create_argument_parser() -> argparse.ArgumentParser:
         "--reach-concurrency",
         dest="reach_concurrency",
         type=int,
+        default=1,
         metavar="<number>",
-        help="Concurrency level for reachability analysis (must be >= 1)"
+        help="Concurrency level for reachability analysis (must be >= 1, default: 1)"
     )
     reachability_group.add_argument(
         "--reach-additional-params",
@@ -1002,6 +1022,20 @@ def create_argument_parser() -> argparse.ArgumentParser:
         action="store_true",
         help=argparse.SUPPRESS
     )
+    reachability_group.add_argument(
+        "--reach-debug",
+        dest="reach_debug",
+        action="store_true",
+        help="Enable debug output for the reachability analysis (passes --debug to the coana CLI). "
+             "Independent of the global --enable-debug flag."
+    )
+    reachability_group.add_argument(
+        "--reach-disable-external-tool-checks",
+        dest="reach_disable_external_tool_checks",
+        action="store_true",
+        help="Disable coana's external tool availability checks during reachability analysis "
+             "(passes --disable-external-tool-checks to the coana CLI)."
+    )
 
     parser.add_argument(
         '--version',
diff --git a/socketsecurity/core/__init__.py b/socketsecurity/core/__init__.py
@@ -71,6 +71,11 @@
 # Stream the facts file in 1 MiB chunks so large files aren't held fully in memory.
 SOCKET_FACTS_BROTLI_CHUNK_SIZE = 1024 * 1024
 
+# Tier 1 reachability finalize retry policy. The finalize call links the tier1 scan to the
+# full scan and can fail transiently (network/API blips); a few backoff retries make it robust.
+TIER1_FINALIZE_MAX_ATTEMPTS = 3
+TIER1_FINALIZE_BACKOFF_SECONDS = 1.0
+
 
 def _humanize_alert_type(alert_type: str) -> str:
     """Convert a camelCase/PascalCase alert type into a Title-Cased label.
@@ -549,20 +554,43 @@ def finalize_tier1_scan(self, full_scan_id: str, facts_file_path: str) -> bool:
             log.debug(f"Failed to read tier1ReachabilityScanId from {facts_file_path}: {e}")
             return False
 
-        # Call the SDK to finalize the tier 1 scan
-        try:
-            success = self.sdk.fullscans.finalize_tier1(
-                full_scan_id=full_scan_id,
-                tier1_reachability_scan_id=tier1_scan_id,
-            )
+        # Call the SDK to finalize the tier 1 scan, retrying transient failures with backoff.
+        last_error: Optional[Exception] = None
+        for attempt in range(1, TIER1_FINALIZE_MAX_ATTEMPTS + 1):
+            try:
+                success = self.sdk.fullscans.finalize_tier1(
+                    full_scan_id=full_scan_id,
+                    tier1_reachability_scan_id=tier1_scan_id,
+                )
 
-            if success:
-                log.debug(f"Successfully finalized tier 1 scan {tier1_scan_id} for full scan {full_scan_id}")
-            return success
+                if success:
+                    log.debug(f"Successfully finalized tier 1 scan {tier1_scan_id} for full scan {full_scan_id}")
+                    return True
 
-        except Exception as e:
-            log.debug(f"Unable to finalize tier 1 scan: {e}")
-            return False
+                log.debug(
+                    f"finalize_tier1 returned a falsy result for scan {tier1_scan_id} "
+                    f"(attempt {attempt}/{TIER1_FINALIZE_MAX_ATTEMPTS})"
+                )
+            except Exception as e:
+                last_error = e
+                log.debug(
+                    f"Unable to finalize tier 1 scan (attempt {attempt}/{TIER1_FINALIZE_MAX_ATTEMPTS}): {e}"
+                )
+
+            if attempt < TIER1_FINALIZE_MAX_ATTEMPTS:
+                time.sleep(TIER1_FINALIZE_BACKOFF_SECONDS * (2 ** (attempt - 1)))
+
+        if last_error is not None:
+            log.debug(
+                f"Giving up finalizing tier 1 scan {tier1_scan_id} after "
+                f"{TIER1_FINALIZE_MAX_ATTEMPTS} attempts: {last_error}"
+            )
+        else:
+            log.debug(
+                f"Giving up finalizing tier 1 scan {tier1_scan_id} after "
+                f"{TIER1_FINALIZE_MAX_ATTEMPTS} attempts"
+            )
+        return False
 
     @staticmethod
     def _compress_facts_file(source_path: str) -> str:
diff --git a/socketsecurity/core/tools/reachability.py b/socketsecurity/core/tools/reachability.py
@@ -1,15 +1,31 @@
 from socketdev import socketdev
 from typing import List, Optional, Dict, Any
 import os
+import platform
 import subprocess
 import json
 import pathlib
 import logging
 import sys
 
+from socketsecurity import __version__
+
 log = logging.getLogger(__name__)
 
 
+def _build_caller_user_agent() -> str:
+    """Build the SOCKET_CALLER_USER_AGENT string forwarded to the coana CLI.
+
+    Mirrors the Node CLI's ``<product>/<version> <runtime>/<version> <platform>/<arch>``
+    shape so the backend can attribute reachability calls to the Python CLI.
+    """
+    return (
+        f"socket/{__version__} "
+        f"python/{platform.python_version()} "
+        f"{platform.system().lower()}/{platform.machine().lower()}"
+    )
+
+
 class ReachabilityAnalyzer:
     def __init__(self, sdk: socketdev, api_token: str):
         self.sdk = sdk
@@ -108,6 +124,9 @@ def run_reachability_analysis(
         continue_on_install_errors: bool = False,
         continue_on_missing_lock_files: bool = False,
         continue_on_no_source_files: bool = False,
+        reach_debug: bool = False,
+        disable_external_tool_checks: bool = False,
+        proxy: Optional[str] = None,
     ) -> Dict[str, Any]:
         """
         Run reachability analysis.
@@ -147,8 +166,7 @@ def run_reachability_analysis(
         
         # Add required arguments
         output_dir = str(pathlib.Path(output_path).parent)
-        log.warning(f"output_dir: {output_dir}")
-        log.warning(f"output_path: {output_path}")
+        log.debug(f"output_dir: {output_dir}, output_path: {output_path}")
         cmd.extend([
             "--output-dir", output_dir,
             "--socket-mode", output_path,
@@ -197,6 +215,12 @@ def run_reachability_analysis(
         if enable_debug:
             cmd.append("-d")
 
+        if reach_debug:
+            cmd.append("--debug")
+
+        if disable_external_tool_checks:
+            cmd.append("--disable-external-tool-checks")
+
         if use_only_pregenerated_sboms:
             cmd.append("--use-only-pregenerated-sboms")
 
@@ -222,14 +246,26 @@ def run_reachability_analysis(
         # Required environment variables for Coana CLI
         env["SOCKET_ORG_SLUG"] = org_slug
         env["SOCKET_CLI_API_TOKEN"] = self.api_token
-        
-        # Optional environment variables
+
+        # Identify the calling CLI to the coana tool / backend (parity with the Node CLI).
+        env["SOCKET_CLI_VERSION"] = __version__
+        env["SOCKET_CALLER_USER_AGENT"] = _build_caller_user_agent()
+
+        # Forward an HTTP(S) proxy to coana if one is configured. Prefer an explicit value,
+        # otherwise fall back to the standard proxy environment variables.
+        proxy_url = proxy or os.environ.get("HTTPS_PROXY") or os.environ.get("HTTP_PROXY")
+        if proxy_url:
+            env["SOCKET_CLI_API_PROXY"] = proxy_url
+
+        # Optional environment variables.
+        # NOTE: repo/branch are intentionally omitted by the caller (passed as None) when they
+        # are the default sentinels, to avoid polluting coana's per-repo/branch cache buckets.
         if repo_name:
             env["SOCKET_REPO_NAME"] = repo_name
-        
+
         if branch_name:
             env["SOCKET_BRANCH_NAME"] = branch_name
-        
+
         # Set NODE_TLS_REJECT_UNAUTHORIZED=0 if allow_unverified is True
         if allow_unverified:
             env["NODE_TLS_REJECT_UNAUTHORIZED"] = "0"
diff --git a/socketsecurity/socketcli.py b/socketsecurity/socketcli.py
@@ -95,6 +95,12 @@ def _write_attribution_file(config, payload: dict) -> None:
 
 DEFAULT_API_TIMEOUT = 1200
 
+# Sentinel repo/branch names used when none can be detected from git or supplied via flags.
+# When the repo/branch are these defaults we skip forwarding SOCKET_REPO_NAME/SOCKET_BRANCH_NAME
+# to the coana CLI so unrelated default-named runs don't share reachability cache buckets.
+DEFAULT_REPO_NAME = "socket-default-repo"
+DEFAULT_BRANCH_NAME = "socket-default-branch"
+
 
 def get_api_request_timeout(config: CliConfig) -> int:
     return config.timeout if config.timeout is not None else DEFAULT_API_TIMEOUT
@@ -288,16 +294,21 @@ def main_code():
     except NoSuchPathError:
         raise Exception(f"Unable to find path {config.target_path}")
 
+    # Track whether repo/branch fell back to the default sentinels so reachability can skip
+    # forwarding them as coana cache-bucket keys (computed before any workspace suffixing).
+    repo_defaulted = not config.repo
+    branch_defaulted = not config.branch
+
     if not config.repo:
-        base_repo_name = "socket-default-repo"
+        base_repo_name = DEFAULT_REPO_NAME
         if config.workspace_name:
             config.repo = f"{base_repo_name}-{config.workspace_name}"
         else:
             config.repo = base_repo_name
         log.debug(f"Using default repository name: {config.repo}")
-        
+
     if not config.branch:
-        config.branch = "socket-default-branch"
+        config.branch = DEFAULT_BRANCH_NAME
         log.debug(f"Using default branch name: {config.branch}")
 
     # Calculate the scan paths - combine target_path with sub_paths if provided
@@ -384,8 +395,8 @@ def main_code():
                     enable_analysis_splitting=config.reach_enable_analysis_splitting or False,
                     detailed_analysis_log_file=config.reach_detailed_analysis_log_file or False,
                     lazy_mode=config.reach_lazy_mode or False,
-                    repo_name=config.repo,
-                    branch_name=config.branch,
+                    repo_name=None if repo_defaulted else config.repo,
+                    branch_name=None if branch_defaulted else config.branch,
                     version=config.reach_version,
                     concurrency=config.reach_concurrency,
                     additional_params=config.reach_additional_params,
@@ -396,6 +407,8 @@ def main_code():
                     continue_on_install_errors=config.reach_continue_on_install_errors,
                     continue_on_missing_lock_files=config.reach_continue_on_missing_lock_files,
                     continue_on_no_source_files=config.reach_continue_on_no_source_files,
+                    reach_debug=config.reach_debug,
+                    disable_external_tool_checks=config.reach_disable_external_tool_checks,
                 )
                 
                 log.info("Reachability analysis completed successfully")
diff --git a/tests/unit/test_config.py b/tests/unit/test_config.py
@@ -166,6 +166,55 @@ def test_config_file_json_sets_defaults(self, tmp_path):
         assert config.sarif_reachability == "reachable"
 
 
+class TestReachAlignmentFlags:
+    """Tests for the reachability flag/default alignment with the Node CLI."""
+
+    BASE_ARGS = ["--api-token", "test-token", "--repo", "test-repo"]
+
+    def test_reach_defaults_memory_and_concurrency(self):
+        """G7: memory-limit defaults to 8192 and concurrency to 1."""
+        config = CliConfig.from_args(self.BASE_ARGS + ["--reach"])
+        assert config.reach_analysis_memory_limit == 8192
+        assert config.reach_concurrency == 1
+        # Timeout intentionally left unset (omitted -> coana default).
+        assert config.reach_analysis_timeout is None
+
+    def test_reach_node_style_name_aliases(self):
+        """G8: Node-style primary names map to the same dests."""
+        config = CliConfig.from_args(
+            self.BASE_ARGS
+            + ["--reach", "--reach-analysis-timeout", "300", "--reach-analysis-memory-limit", "2048"]
+        )
+        assert config.reach_analysis_timeout == 300
+        assert config.reach_analysis_memory_limit == 2048
+
+    def test_reach_legacy_name_aliases_still_work(self):
+        """G8: pre-alignment names keep working (hidden aliases)."""
+        config = CliConfig.from_args(
+            self.BASE_ARGS + ["--reach", "--reach-timeout", "111", "--reach-memory-limit", "512"]
+        )
+        assert config.reach_analysis_timeout == 111
+        assert config.reach_analysis_memory_limit == 512
+
+    def test_reach_debug_flag(self):
+        """G9: dedicated --reach-debug flag, independent of --enable-debug."""
+        config = CliConfig.from_args(self.BASE_ARGS + ["--reach", "--reach-debug"])
+        assert config.reach_debug is True
+        assert config.enable_debug is False
+
+    def test_reach_disable_external_tool_checks_flag(self):
+        """G1: --reach-disable-external-tool-checks parses to its dest."""
+        config = CliConfig.from_args(
+            self.BASE_ARGS + ["--reach", "--reach-disable-external-tool-checks"]
+        )
+        assert config.reach_disable_external_tool_checks is True
+
+    def test_reach_new_flags_default_false(self):
+        config = CliConfig.from_args(self.BASE_ARGS + ["--reach"])
+        assert config.reach_debug is False
+        assert config.reach_disable_external_tool_checks is False
+
+
 def test_pyproject_requires_python_matches_tomllib_usage():
     pyproject = tomllib.loads(Path("pyproject.toml").read_text(encoding="utf-8"))
     requires_python = pyproject["project"]["requires-python"]
diff --git a/tests/unit/test_reachability.py b/tests/unit/test_reachability.py
diff --git a/tests/unit/test_tier1_finalize.py b/tests/unit/test_tier1_finalize.py
diff --git a/uv.lock b/uv.lock
