fix: keep docker stdout off JSON output

Author: clement-tourriere

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
@@ -121,11 +122,13 @@ class TestDockerPull:
     def test_docker_pull_image_success(self):
         with patch("subprocess.run") as call:
             docker_pull_image("ggshield-non-existant", DOCKER_TIMEOUT)
-            call.assert_called_once_with(
-                ["docker", "pull", "ggshield-non-existant"],
-                check=True,
-                timeout=DOCKER_TIMEOUT,
-            )
+            call.assert_called_once()
+            args, kwargs = call.call_args
+            assert args == (["docker", "pull", "ggshield-non-existant"],)
+            assert kwargs["check"] is True
+            assert kwargs["timeout"] == DOCKER_TIMEOUT
+            assert kwargs["stdout"] is sys.stderr
+            assert kwargs["stderr"] is sys.stderr
 
     def test_docker_pull_image_non_exist(self):
         with patch(
@@ -149,11 +152,14 @@ 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(
@@ -171,18 +177,21 @@ def test_docker_save_image_success(self):
             docker_save_to_tmp(
                 "ggshield-non-existant", self.TMP_ARCHIVE, DOCKER_TIMEOUT
             )
-            call.assert_called_once_with(
+            call.assert_called_once()
+            args, kwargs = call.call_args
+            assert args == (
                 [
                     "docker",
                     "save",
                     "ggshield-non-existant:latest",
                     "-o",
                     str(self.TMP_ARCHIVE),
                 ],
-                check=True,
-                stderr=-1,
-                timeout=DOCKER_TIMEOUT,
             )
+            assert kwargs["check"] is True
+            assert kwargs["stderr"] == subprocess.PIPE
+            assert kwargs["stdout"] is sys.stderr
+            assert kwargs["timeout"] == DOCKER_TIMEOUT
 
     def test_docker_save_image_does_not_exist(self):
         with patch(