Merge pull request #1172 from GitGuardian/fix/docker-stdout-json

fix: keep docker stdout off JSON output

Author: clement-tourriere

changelog.d/20260209_112852_clement.tourriere_docker_stdout_json.md

@@ -0,0 +1,42 @@
+<!--
+A new scriv changelog fragment.
+
+Uncomment the section that is right (remove the HTML comment wrapper).
+For top level release notes, leave all the headers commented out.
+-->
+
+<!--
+### Removed
+
+- A bullet item for the Removed category.
+
+-->
+<!--
+### Added
+
+- A bullet item for the Added category.
+
+-->
+<!--
+### Changed
+
+- A bullet item for the Changed category.
+
+-->
+<!--
+### Deprecated
+
+- A bullet item for the Deprecated category.
+
+-->
+
+### Fixed
+
+- Prevent docker scan stdout from leaking into JSON output.
+<!--
+
+### Security
+
+- A bullet item for the Security category.
+
+-->

ggshield/verticals/secret/docker.py

@@ -3,11 +3,12 @@
 import json
 import re
 import subprocess
+import sys
 import tarfile
 from contextlib import contextmanager
 from dataclasses import dataclass
 from pathlib import Path
-from typing import TYPE_CHECKING
+from typing import TYPE_CHECKING, BinaryIO, cast
 
 from click import UsageError
 
@@ -101,10 +102,9 @@ def is_longer_than(self, max_utf8_encoded_size: int) -> bool:
                 self._content,
                 self._utf8_encoded_size,
             ) = Scannable._is_file_longer_than(
-                fp, max_utf8_encoded_size  # type:ignore
+                cast(BinaryIO, fp),
+                max_utf8_encoded_size,
             )
-            # mypy complains that fp is IO[bytes] but _is_file_longer_than() expects
-            # BinaryIO. They are compatible, ignore the error.
         return result
 
     def _read_content(self) -> None:
@@ -323,6 +323,8 @@ def _run_docker_command(command: List[str], timeout: int) -> bool:
         subprocess.run(
             command,
             check=True,
+            stdout=sys.stderr,
+            stderr=sys.stderr,
             timeout=timeout,
         )
         return True
@@ -348,6 +350,7 @@ def docker_save_to_tmp(image_name: str, destination_path: Path, timeout: int) ->
         subprocess.run(
             command,
             check=True,
+            stdout=sys.stderr,
             stderr=subprocess.PIPE,
             timeout=timeout,
         )

tests/unit/verticals/secret/test_scan_docker.py

@@ -1,5 +1,6 @@
 import re
 import subprocess
+import sys
 from pathlib import Path
 from typing import Dict, List
 from unittest.mock import patch
@@ -124,6 +125,8 @@ def test_docker_pull_image_success(self):
             call.assert_called_once_with(
                 ["docker", "pull", "ggshield-non-existant"],
                 check=True,
+                stdout=sys.stderr,
+                stderr=sys.stderr,
                 timeout=DOCKER_TIMEOUT,
             )
 
@@ -149,16 +152,21 @@ def test_docker_pull_image_timeout(self):
                 docker_pull_image("ggshield-non-existant", DOCKER_TIMEOUT)
 
     def test_docker_pull_image_platform_fallback(self):
-        with patch(
-            "subprocess.run", side_effect=subprocess.CalledProcessError(1, cmd=[])
-        ) as call, pytest.raises(
-            click.UsageError,
-            match='Image "ggshield-non-existant" not found',
+        with (
+            patch(
+                "subprocess.run", side_effect=subprocess.CalledProcessError(1, cmd=[])
+            ) as call,
+            pytest.raises(
+                click.UsageError,
+                match='Image "ggshield-non-existant" not found',
+            ),
         ):
             docker_pull_image("ggshield-non-existant", DOCKER_TIMEOUT)
             call.assert_called_once_with(
                 ["docker", "pull", "ggshield-non-existant", "--platform=linux/amd64"],
                 check=True,
+                stdout=sys.stderr,
+                stderr=sys.stderr,
                 timeout=DOCKER_TIMEOUT,
             )
 
@@ -180,7 +188,8 @@ def test_docker_save_image_success(self):
                     str(self.TMP_ARCHIVE),
                 ],
                 check=True,
-                stderr=-1,
+                stderr=subprocess.PIPE,
+                stdout=sys.stderr,
                 timeout=DOCKER_TIMEOUT,
             )