Keep the commit id in archive names for Windows drive paths

cycode/cli/apps/scan/scan_result.py

@@ -7,7 +7,7 @@
 from cycode.cli.apps.scan.aggregation_report import try_get_aggregation_report_url_if_needed
 from cycode.cli.apps.scan.detection_excluder import exclude_irrelevant_document_detections
 from cycode.cli.models import Document, DocumentDetections, LocalScanResult
-from cycode.cli.utils.path_utils import concat_unique_id, get_path_by_os, normalize_file_path
+from cycode.cli.utils.path_utils import concat_unique_id, normalize_file_path
 from cycode.cyclient.models import (
     Detection,
     DetectionSchema,
@@ -49,7 +49,7 @@ def _get_document_detections(
 
     document_detections = []
     for detections_per_file in scan_result.detections_per_file:
-        file_name = get_path_by_os(detections_per_file.file_name)
+        file_name = detections_per_file.file_name
         commit_id = detections_per_file.commit_id
 
         logger.debug(

cycode/cli/consts.py

@@ -1,7 +1,27 @@
+import sys
+from enum import Enum
+
 PROGRAM_NAME = 'cycode'
 APP_NAME = 'CycodeCLI'
 CLI_CONTEXT_SETTINGS = {'terminal_width': 10**9, 'max_content_width': 10**9, 'help_option_names': ['-h', '--help']}
 
+
+class OperatingSystem(str, Enum):
+    WINDOWS = 'windows'
+    MACOS = 'macos'
+    LINUX = 'linux'
+
+
+def _detect_operating_system() -> OperatingSystem:
+    if sys.platform == 'win32':
+        return OperatingSystem.WINDOWS
+    if sys.platform == 'darwin':
+        return OperatingSystem.MACOS
+    return OperatingSystem.LINUX
+
+
+OPERATING_SYSTEM = _detect_operating_system()
+
 PRE_COMMIT_COMMAND_SCAN_TYPE = 'pre-commit'
 PRE_COMMIT_COMMAND_SCAN_TYPE_OLD = 'pre_commit'
 PRE_RECEIVE_COMMAND_SCAN_TYPE = 'pre-receive'

cycode/cli/utils/path_utils.py

@@ -2,10 +2,12 @@
 import os
 import tempfile
 from functools import cache
+from pathlib import PurePath, PurePosixPath, PureWindowsPath
 from typing import TYPE_CHECKING, AnyStr, Optional, Union
 
 import typer
 
+from cycode.cli.consts import OPERATING_SYSTEM, OperatingSystem
 from cycode.cli.logger import logger
 from cycode.cli.utils.binary_utils import is_binary_string
 
@@ -125,12 +127,29 @@ def change_filename_extension(filename: str, extension: str) -> str:
     return f'{base_name}.{extension}'
 
 
+def _to_relative_posix_parts(path: str, operating_system: OperatingSystem = OPERATING_SYSTEM) -> tuple[str, ...]:
+    """Split a path into OS-agnostic components: drop the root, keep the UNC server and share.
+
+    The archive entry name is read back by the server as ``<unique id>/<path>``, so it must use forward slashes
+    and must never start with a root. The flavor comes from the running OS rather than from the path itself, so
+    a colon in a POSIX file name is not mistaken for a drive letter.
+    """
+    pure: PurePath = PureWindowsPath(path) if operating_system is OperatingSystem.WINDOWS else PurePosixPath(path)
+    if not pure.anchor:
+        return pure.parts
+
+    # the anchor is 'C:\\', '/' or '\\\\server\\share\\': keep the names it carries, drop the drive and separators
+    anchor_parts = tuple(p for p in pure.anchor.replace('\\', '/').split('/') if p and not p.endswith(':'))
+    return anchor_parts + pure.parts[1:]
+
+
 def concat_unique_id(filename: str, unique_id: str) -> str:
-    if filename.startswith(os.sep):
-        # remove leading slash to join the path correctly
-        filename = filename[len(os.sep) :]
+    """Prefix the file name with the unique id (e.g. a commit SHA) to build the name of the archive entry.
 
-    return os.path.join(unique_id, filename)
+    The server reads the unique id back from the first path component, so the file name must become relative
+    before joining; otherwise the prefix is lost and a Windows drive letter is reported as the commit id.
+    """
+    return str(PurePosixPath(unique_id, *_to_relative_posix_parts(filename)))
 
 
 def get_path_from_context(ctx: typer.Context) -> Optional[str]:
@@ -141,8 +160,6 @@ def get_path_from_context(ctx: typer.Context) -> Optional[str]:
 
 
 def normalize_file_path(path: str) -> str:
-    if path.startswith('/'):
-        return path[1:]
-    if path.startswith('./'):
-        return path[2:]
-    return path
+    """Make a path comparable with the file name the server reports back: no drive, no root, forward slashes."""
+    parts = _to_relative_posix_parts(path)
+    return str(PurePosixPath(*parts)) if parts else ''

tests/cli/utils/test_path_utils.py

@@ -0,0 +1,65 @@
+from pathlib import PurePosixPath
+
+import pytest
+
+from cycode.cli.consts import OperatingSystem
+from cycode.cli.utils.path_utils import _to_relative_posix_parts, concat_unique_id, normalize_file_path
+
+
+def test_concat_unique_id_to_file_with_leading_slash() -> None:
+    assert concat_unique_id('/path/to/file', 'unique_id') == 'unique_id/path/to/file'
+
+
+def test_concat_unique_id_to_file_without_leading_slash() -> None:
+    assert concat_unique_id('path/to/file', 'unique_id') == 'unique_id/path/to/file'
+
+
+def test_concat_unique_id_keeps_unique_id_for_windows_drive_path() -> None:
+    unique_id = 'a' * 40
+
+    # the entry name is always posix: the drive and the root go away, backslashes become forward slashes
+    for path in ('C:\\repo\\creds.txt', 'C:/repo/creds.txt'):
+        parts = _to_relative_posix_parts(path, OperatingSystem.WINDOWS)
+        assert str(PurePosixPath(unique_id, *parts)) == f'{unique_id}/repo/creds.txt'
+
+
+def test_concat_unique_id_keeps_server_and_share_for_unc_path() -> None:
+    unique_id = 'a' * 40
+
+    # the server and the share are part of the path: two shares must not collapse onto the same entry
+    parts = _to_relative_posix_parts('\\\\server\\share\\creds.txt', OperatingSystem.WINDOWS)
+    assert str(PurePosixPath(unique_id, *parts)) == f'{unique_id}/server/share/creds.txt'
+
+
+@pytest.mark.parametrize(
+    ('path', 'expected'),
+    [
+        ('/repo/creds.txt', 'repo/creds.txt'),
+        ('./repo/creds.txt', 'repo/creds.txt'),
+        ('repo/creds.txt', 'repo/creds.txt'),
+        ('', ''),
+    ],
+)
+def test_normalize_file_path(path: str, expected: str) -> None:
+    assert normalize_file_path(path) == expected
+
+
+def test_normalize_file_path_keeps_colon_in_posix_name() -> None:
+    # a colon is a legal character in a posix file name and must not be read as a drive
+    assert str(PurePosixPath(*_to_relative_posix_parts('x:y/creds.txt', OperatingSystem.LINUX))) == 'x:y/creds.txt'
+
+
+@pytest.mark.parametrize(
+    ('path', 'expected'),
+    [
+        ('C:\\repo\\creds.txt', 'repo/creds.txt'),
+        ('C:/repo/creds.txt', 'repo/creds.txt'),
+        ('.\\repo\\creds.txt', 'repo/creds.txt'),
+        ('/repo/creds.txt', 'repo/creds.txt'),
+        # the server and the share are kept, so two shares do not normalize onto the same path
+        ('\\\\server\\share\\creds.txt', 'server/share/creds.txt'),
+        ('\\\\server\\other\\creds.txt', 'server/other/creds.txt'),
+    ],
+)
+def test_normalize_file_path_on_windows(path: str, expected: str) -> None:
+    assert str(PurePosixPath(*_to_relative_posix_parts(path, OperatingSystem.WINDOWS))) == expected