Merge pull request #1059 from GitGuardian/severine/fix-paths-exclusion-in-docker

fix: handle ignored_paths in docker scan

Author: agateau-gg

changelog.d/20250226_170555_severine.bonnechere_fix_paths_exclusion_in_docker.md

@@ -0,0 +1,3 @@
+### Fixed
+
+- `ggshield secret scan docker` now correctly handles ignored paths (#548).

ggshield/cmd/secret/scan/docker.py

@@ -61,6 +61,7 @@ def docker_name_cmd(
             cache=ctx_obj.cache,
             secret_config=config.user_config.secret,
             scan_context=scan_context,
+            exclusion_regexes=ctx_obj.exclusion_regexes,
         )
 
         return output_handler.process_scan(scan)

ggshield/cmd/secret/scan/dockerarchive.py

@@ -48,6 +48,7 @@ def docker_archive_cmd(
         cache=ctx_obj.cache,
         secret_config=config.user_config.secret,
         scan_context=scan_context,
+        exclusion_regexes=ctx_obj.exclusion_regexes,
     )
 
     return output_handler.process_scan(scan)

ggshield/verticals/secret/docker.py

@@ -1,21 +1,20 @@
+from __future__ import annotations
+
 import json
 import re
 import subprocess
 import tarfile
 from contextlib import contextmanager
 from dataclasses import dataclass
 from pathlib import Path
-from typing import Any, Dict, Generator, Iterable, List
+from typing import TYPE_CHECKING
 
 from click import UsageError
-from pygitguardian import GGClient
 
 from ggshield.core import ui
-from ggshield.core.cache import Cache
-from ggshield.core.config.user_config import SecretConfig
 from ggshield.core.dirs import get_cache_dir
 from ggshield.core.errors import UnexpectedError
-from ggshield.core.scan import ScanContext, Scannable, StringScannable
+from ggshield.core.scan import Scannable, StringScannable
 from ggshield.core.scan.id_cache import IDCache
 from ggshield.core.scanner_ui import create_scanner_ui
 from ggshield.utils.files import is_path_binary
@@ -24,6 +23,15 @@
 from .secret_scanner import SecretScanner
 
 
+if TYPE_CHECKING:
+    from typing import Any, Dict, Generator, Iterable, List, Set
+
+    from pygitguardian import GGClient
+
+    from ggshield.core.cache import Cache
+    from ggshield.core.config.user_config import SecretConfig
+    from ggshield.core.scan import ScanContext
+
 FILEPATH_BANLIST = [
     r"^/?usr/(?!share/nginx)",
     r"^/?lib/",
@@ -42,6 +50,7 @@
 FILEPATH_BANLIST_PATTERNS = {
     re.compile(banned_filepath) for banned_filepath in FILEPATH_BANLIST
 }
+# Note that FILEPATH_BANLIST_PATTERNS comes in addition to what's done in _exclude_callback
 
 LAYER_TO_SCAN_PATTERN = re.compile(r"\b(copy|add)\b", re.IGNORECASE)
 
@@ -225,7 +234,9 @@ def _load_layer_infos(self) -> None:
         ]
         self.layer_infos = [x for x in layer_infos if x.should_scan()]
 
-    def get_layer_scannables(self, layer_info: LayerInfo) -> Iterable[Scannable]:
+    def get_layer_scannables(
+        self, layer_info: LayerInfo, exclusion_regexes: Set[re.Pattern[str]]
+    ) -> Iterable[Scannable]:
         """
         Extracts Scannable to be scanned for given layer.
         """
@@ -244,17 +255,21 @@ def get_layer_scannables(self, layer_info: LayerInfo) -> Iterable[Scannable]:
             if file_info.size == 0:
                 continue
 
-            if not _validate_filepath(filepath=file_info.path):
+            if not _validate_filepath(
+                filepath=file_info.path, exclusion_regexes=exclusion_regexes
+            ):
                 continue
 
             yield DockerContentScannable(layer_id, layer_archive, file_info)
 
 
 def _validate_filepath(
     filepath: str,
+    exclusion_regexes: Set[re.Pattern[str]],
 ) -> bool:
     if any(
-        banned_pattern.search(filepath) for banned_pattern in FILEPATH_BANLIST_PATTERNS
+        banned_pattern.search(filepath)
+        for banned_pattern in FILEPATH_BANLIST_PATTERNS | exclusion_regexes
     ):
         return False
 
@@ -328,6 +343,7 @@ def docker_scan_archive(
     cache: Cache,
     secret_config: SecretConfig,
     scan_context: ScanContext,
+    exclusion_regexes: Set[re.Pattern[str]],
 ) -> SecretScanCollection:
     scanner = SecretScanner(
         client=client,
@@ -347,7 +363,7 @@ def docker_scan_archive(
             )
 
         for info in docker_image.layer_infos:
-            files = list(docker_image.get_layer_scannables(info))
+            files = list(docker_image.get_layer_scannables(info, exclusion_regexes))
             file_count = len(files)
             if file_count == 0:
                 continue

tests/unit/cmd/scan/test_docker.py

@@ -1,4 +1,5 @@
 import json
+import re
 import sys
 from pathlib import Path
 from unittest.mock import Mock, patch
@@ -7,7 +8,11 @@
 import pytest
 
 from ggshield.__main__ import cli
+from ggshield.cmd.secret.scan.secret_scan_common_options import (
+    IGNORED_DEFAULT_WILDCARDS,
+)
 from ggshield.core.errors import ExitCode
+from ggshield.core.filter import init_exclusion_regexes
 from ggshield.core.scan import StringScannable
 from ggshield.verticals.secret import SecretScanCollection
 from ggshield.verticals.secret.docker import DockerImage, LayerInfo, _validate_filepath
@@ -18,30 +23,37 @@
     assert_invoke_exited_with,
     assert_invoke_ok,
     my_vcr,
+    write_text,
 )
 
 
 class TestDockerUtils:
     @pytest.mark.parametrize(
-        "filepath, valid",
+        "filepath, exclusion_regexes, valid",
         (
-            ["/usr/bin/secret.py", False],
-            ["usr/bin/secret.py", False],
-            ["/my/file/secret.py", True],
-            ["/my/file/usr/bin/secret.py", True],
-            ["/usr/share/nginx/secret.py", True],
-            ["/gems/secret.py", True],
-            ["/npm-bis/secret.py", True],
-            ["/banned/extension/secret.exe", False],
-            ["/banned/extension/secret.mng", False],
-            ["/banned/extension/secret.tar", False],
-            ["/banned/extension/secret.other", True],
+            ["/usr/bin/secret.py", set(), False],
+            ["usr/bin/secret.py", set(), False],
+            ["/my/file/secret.py", set(), True],
+            ["/my/file/usr/bin/secret.py", set(), True],
+            ["/usr/share/nginx/secret.py", set(), True],
+            ["/gems/secret.py", set(), True],
+            ["/npm-bis/secret.py", set(), True],
+            ["/banned/extension/secret.exe", set(), False],
+            ["/banned/extension/secret.mng", set(), False],
+            ["/banned/extension/secret.tar", set(), False],
+            ["/banned/extension/secret.other", set(), True],
+            [
+                "/banned/extension/secret.other",
+                {re.compile(r"secret")},
+                False,
+            ],
         ),
     )
-    def test_docker_filepath_validation(self, filepath, valid):
+    def test_docker_filepath_validation(self, filepath, exclusion_regexes, valid):
         assert (
             _validate_filepath(
                 filepath=filepath,
+                exclusion_regexes=exclusion_regexes,
             )
             is valid
         )
@@ -108,12 +120,14 @@ def test_docker_scan_failed_to_save(
         "image_path", [DOCKER_EXAMPLE_PATH, DOCKER__INCOMPLETE_MANIFEST_EXAMPLE_PATH]
     )
     @pytest.mark.parametrize("json_output", (False, True))
+    @pytest.mark.parametrize("ignore_secret_file", (True, False))
     def test_docker_scan_archive(
         self,
         docker_image_open_mock: Mock,
         cli_fs_runner: click.testing.CliRunner,
         image_path: Path,
         json_output: bool,
+        ignore_secret_file: bool,
     ):
         assert image_path.exists()
 
@@ -131,7 +145,9 @@ def create_docker_image() -> Mock(spec=DockerImage):
             scannable = StringScannable(
                 content=UNCHECKED_SECRET_PATCH, url="file_secret"
             )
-            docker_image.get_layer_scannables.return_value = [scannable]
+            docker_image.get_layer_scannables.return_value = (
+                [scannable] if not ignore_secret_file else []
+            )
 
             return docker_image
 
@@ -142,6 +158,15 @@ def create_docker_image() -> Mock(spec=DockerImage):
         with my_vcr.use_cassette("test_scan_file_secret"):
             json_arg = ["--json"] if json_output else []
             cli_fs_runner.mix_stderr = False
+            if ignore_secret_file:
+                config = """
+version: 2
+secret:
+    ignored_paths:
+        - "file_secret"
+
+"""
+                write_text(Path(".gitguardian.yaml"), config)
             result = cli_fs_runner.invoke(
                 cli,
                 [
@@ -153,12 +178,31 @@ def create_docker_image() -> Mock(spec=DockerImage):
                     str(image_path),
                 ],
             )
-            assert_invoke_exited_with(result, ExitCode.SCAN_FOUND_PROBLEMS)
+            assert_invoke_exited_with(
+                result,
+                (
+                    ExitCode.SCAN_FOUND_PROBLEMS
+                    if not ignore_secret_file
+                    else ExitCode.SUCCESS
+                ),
+            )
             docker_image_open_mock.assert_called_once_with(image_path)
-            docker_image.get_layer_scannables.assert_called_once_with(layer_info)
+            docker_image.get_layer_scannables.assert_called_once_with(
+                layer_info,
+                init_exclusion_regexes(
+                    IGNORED_DEFAULT_WILDCARDS
+                    + (["file_secret"] if ignore_secret_file else [])
+                ),
+            )
 
             if json_output:
                 output = json.loads(result.output)
-                assert len(output["entities_with_incidents"]) == 1
+                if not ignore_secret_file:
+                    assert len(output["entities_with_incidents"]) == 1
+                else:
+                    assert output["total_incidents"] == 0
             else:
-                assert "file_secret: 1 secret detected" in result.output
+                if not ignore_secret_file:
+                    assert "file_secret: 1 secret detected" in result.output
+                else:
+                    assert "No secrets have been found" in result.output

tests/unit/verticals/secret/test_scan_docker.py

@@ -94,7 +94,7 @@ def test_docker_archive(self, image_path: Path):
 
             # Fill layer_ids and layer_files
             for info in image.layer_infos:
-                if files := list(image.get_layer_scannables(info)):
+                if files := list(image.get_layer_scannables(info, set())):
                     layer_ids.append(info.diff_id)
                     layer_files.append(files)