diff --git a/cycode/cli/apps/scan/scan_result.py b/cycode/cli/apps/scan/scan_result.py index 16c97a90..ea598a12 100644 --- a/cycode/cli/apps/scan/scan_result.py +++ b/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( diff --git a/cycode/cli/consts.py b/cycode/cli/consts.py index c2153d07..278c9596 100644 --- a/cycode/cli/consts.py +++ b/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' diff --git a/cycode/cli/utils/path_utils.py b/cycode/cli/utils/path_utils.py index ea6d2f38..c4ae78fe 100644 --- a/cycode/cli/utils/path_utils.py +++ b/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 ``/``, 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 '' diff --git a/tests/cli/utils/test_path_utils.py b/tests/cli/utils/test_path_utils.py new file mode 100644 index 00000000..52a50684 --- /dev/null +++ b/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 diff --git a/tests/test_zip_file.py b/tests/test_zip_file.py deleted file mode 100644 index 15c53c17..00000000 --- a/tests/test_zip_file.py +++ /dev/null @@ -1,22 +0,0 @@ -import os - -from cycode.cli.utils.path_utils import concat_unique_id - - -def test_concat_unique_id_to_file_with_leading_slash() -> None: - filename = os.path.join('path', 'to', 'file') # we should care about slash characters in tests - unique_id = 'unique_id' - - expected_path = os.path.join(unique_id, filename) - - filename = os.sep + filename - assert concat_unique_id(filename, unique_id) == expected_path - - -def test_concat_unique_id_to_file_without_leading_slash() -> None: - filename = os.path.join('path', 'to', 'file') # we should care about slash characters in tests - unique_id = 'unique_id' - - expected_path = os.path.join(unique_id, *filename.split('/')) - - assert concat_unique_id(filename, unique_id) == expected_path