Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Original file line number Diff line number Diff line change
@@ -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.

-->
11 changes: 7 additions & 4 deletions ggshield/verticals/secret/docker.py
Original file line number Diff line number Diff line change
Expand Up @@ -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

Expand Down Expand Up @@ -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:
Expand Down Expand Up @@ -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
Expand All @@ -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,
)
Expand Down
21 changes: 15 additions & 6 deletions tests/unit/verticals/secret/test_scan_docker.py
Original file line number Diff line number Diff line change
@@ -1,5 +1,6 @@
import re
import subprocess
import sys
from pathlib import Path
from typing import Dict, List
from unittest.mock import patch
Expand Down Expand Up @@ -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,
)

Expand All @@ -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,
)

Expand All @@ -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,
)

Expand Down