fix(secret): skip non-seekable files during scanning

Author: salome-voltz

changelog.d/20251020_174722_salome.voltz_scrt_5971_ggshield_runs_into_oserror_errno_22_invalid_argument_when.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
+
+- Skip non-seekable files instead of crashing.
+
+<!--
+### Security
+
+- A bullet item for the Security category.
+
+-->

ggshield/core/scan/__init__.py

@@ -2,14 +2,15 @@
 from .file import File, create_files_from_paths
 from .scan_context import ScanContext
 from .scan_mode import ScanMode
-from .scannable import DecodeError, Scannable, StringScannable
+from .scannable import DecodeError, NonSeekableFileError, Scannable, StringScannable
 
 
 __all__ = [
     "create_files_from_paths",
     "Commit",
     "DecodeError",
     "File",
+    "NonSeekableFileError",
     "ScanContext",
     "ScanMode",
     "Scannable",

ggshield/core/scan/file.py

@@ -3,7 +3,7 @@
 
 from ggshield.utils.files import ListFilesMode, is_path_binary, list_files, url_for_path
 
-from .scannable import Scannable
+from .scannable import NonSeekableFileError, Scannable
 
 
 class File(Scannable):
@@ -30,13 +30,16 @@ def is_longer_than(self, max_utf8_encoded_size: int) -> bool:
             # We already have the encoded size, easy
             return self._utf8_encoded_size > max_utf8_encoded_size
 
-        with self.path.open("rb") as fp:
-            (
-                result,
-                self._content,
-                self._utf8_encoded_size,
-            ) = Scannable._is_file_longer_than(fp, max_utf8_encoded_size)
-        return result
+        try:
+            with self.path.open("rb") as fp:
+                (
+                    result,
+                    self._content,
+                    self._utf8_encoded_size,
+                ) = Scannable._is_file_longer_than(fp, max_utf8_encoded_size)
+            return result
+        except NonSeekableFileError:
+            raise
 
     def _read_content(self) -> None:
         if self._content is None:

ggshield/core/scan/scannable.py

@@ -31,6 +31,12 @@ class DecodeError(Exception):
     pass
 
 
+class NonSeekableFileError(Exception):
+    """Raised when a file cannot be seeked"""
+
+    pass
+
+
 class Scannable(ABC):
     """Base class for content that can be scanned by GGShield"""
 
@@ -144,7 +150,10 @@ def _is_file_longer_than(
         """
         # Get the byte size
         assert fp.seekable()
-        byte_size = fp.seek(0, SEEK_END)
+        try:
+            byte_size = fp.seek(0, SEEK_END)
+        except OSError:
+            raise NonSeekableFileError("File does not support seeking operations")
 
         if byte_size > max_utf8_encoded_size * UTF8_TO_WORSE_OTHER_ENCODING_RATIO:
             # Even if the file used the worst encoding (UTF-32), encoding the content of
@@ -153,7 +162,10 @@ def _is_file_longer_than(
             return True, None, None
 
         # Determine the encoding
-        fp.seek(0, SEEK_SET)
+        try:
+            fp.seek(0, SEEK_SET)
+        except OSError:
+            raise NonSeekableFileError("File does not support seeking operations")
         charset_matches = charset_normalizer.from_fp(fp)
         charset_match = charset_matches.best()
         if charset_match is None:

ggshield/verticals/secret/secret_scanner.py

@@ -16,6 +16,7 @@
 from ggshield.core.constants import MAX_WORKERS
 from ggshield.core.errors import handle_api_error
 from ggshield.core.scan import DecodeError, ScanContext, Scannable
+from ggshield.core.scan.scannable import NonSeekableFileError
 from ggshield.core.scanner_ui.scanner_ui import ScannerUI
 from ggshield.core.text_utils import pluralize
 
@@ -157,6 +158,9 @@ def _start_scans(
             except DecodeError:
                 scanner_ui.on_skipped(scannable, "can't detect encoding")
                 continue
+            except NonSeekableFileError:
+                scanner_ui.on_skipped(scannable, "file cannot be seeked")
+                continue
 
             if content:
                 if (

tests/unit/core/scan/test_scannable.py

@@ -1,8 +1,10 @@
 from pathlib import Path
+from unittest.mock import patch
 
 import pytest
 
-from ggshield.core.scan import StringScannable
+from ggshield.core.scan import File, StringScannable
+from ggshield.core.scan.scannable import NonSeekableFileError
 
 
 def test_string_scannable_path():
@@ -32,3 +34,25 @@ def test_string_scannable_is_longer_than(content, is_longer):
     """
     scannable = StringScannable(content=content, url="u")
     assert scannable.is_longer_than(50) == is_longer
+
+
+@patch("pathlib.Path.open")
+def test_file_non_seekable(mock_open, tmp_path):
+    """
+    GIVEN a File instance
+    AND the file reports as seekable but seeking operations fail
+    WHEN is_longer_than() is called on it
+    THEN it raises NonSeekableFileError
+    """
+    mock_file = mock_open.return_value.__enter__.return_value
+    mock_file.seekable.return_value = True
+    mock_file.seek.side_effect = OSError(22, "Invalid argument")
+
+    test_file = tmp_path / "test.txt"
+    test_file.write_text("test content")
+    file_obj = File(test_file)
+
+    with pytest.raises(
+        NonSeekableFileError, match="File does not support seeking operations"
+    ):
+        file_obj.is_longer_than(1000)