Improve handling of quota limit reached errors

Pipfile

@@ -15,7 +15,7 @@ flake8-isort = "*"
 flake8-quotes = "*"
 ipdb = "*"
 pre-commit = "*"
-pytest = "7.2.1" # pinning because of conflicting dependencies with exceptiongroup 
+pytest = "==7.2.1" # pinning because of conflicting dependencies with exceptiongroup
 pytest-mock = "*"
 pytest-socket = "*"
 pytest-voluptuous = "*"

changelog.d/20230705_120528_samuel.guillaume_scrt_3734_ggshield_improve_handling_of_quota_limit_reached_errors.md

@@ -0,0 +1,41 @@
+<!--
+A new scriv changelog fragment.
+
+Uncomment the section that is right (remove the HTML comment wrapper).
+-->
+
+<!--
+### 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
+
+- Improve handling of "quota limit reached" errors (#309)
+
+<!--
+### Security
+
+- A bullet item for the Security category.
+
+-->

ggshield/core/errors.py

@@ -66,6 +66,14 @@ def __init__(self, instance: str, message: str):
         self.instance = instance
 
 
+class QuotaLimitReachedError(_ExitError):
+    def __init__(self):
+        super().__init__(
+            ExitCode.UNEXPECTED_ERROR,
+            "Could not perform the requested action: no more API calls available.",
+        )
+
+
 class UnknownInstanceError(AuthError):
     """
     Raised when the requested instance does not exist

ggshield/secret/repo.py

@@ -13,7 +13,7 @@
 from ggshield.core.client import check_client_api_key
 from ggshield.core.config import Config
 from ggshield.core.constants import MAX_WORKERS
-from ggshield.core.errors import ExitCode, handle_exception
+from ggshield.core.errors import ExitCode, QuotaLimitReachedError, handle_exception
 from ggshield.core.git_shell import get_list_commit_SHA, is_git_dir
 from ggshield.core.text_utils import create_progress_bar, display_error
 from ggshield.core.types import IgnoredMatch
@@ -90,6 +90,8 @@ def scan_commits_content(
             commit_files,
             scan_threads=SCAN_THREADS,
         )
+    except QuotaLimitReachedError:
+        raise
     except Exception as exc:
         results = Results.from_exception(exc)
 
@@ -163,7 +165,6 @@ def scan_commit_range(
     max_documents = client.secret_scan_preferences.maximum_documents_per_scan
 
     with create_progress_bar(doc_type="commits") as progress:
-
         task_scan = progress.add_task(
             "[green]Scanning Commits...", total=len(commit_list)
         )
@@ -192,6 +193,11 @@ def scan_commit_range(
                         ignored_detectors,
                     )
                 )
+                # Stop now if an exception has been raised by a future
+                for future in futures:
+                    exception = future.exception()
+                    if exception is not None:
+                        raise exception
 
             for future in as_completed(futures):
                 scan_collection = future.result()

ggshield/secret/secret_scanner.py

@@ -13,7 +13,7 @@
 from ggshield.core.cache import Cache
 from ggshield.core.client import check_client_api_key
 from ggshield.core.constants import MAX_WORKERS
-from ggshield.core.errors import UnexpectedError
+from ggshield.core.errors import QuotaLimitReachedError, UnexpectedError
 from ggshield.core.filter import (
     remove_ignored_from_result,
     remove_results_from_ignore_detectors,
@@ -238,6 +238,8 @@ def handle_scan_chunk_error(detail: Detail, chunk: List[Scannable]) -> None:
         raise click.UsageError(detail.detail)
     if detail.status_code is None:
         raise UnexpectedError(f"Scanning failed: {detail.detail}")
+    if detail.status_code == 403 and detail.detail == "Quota limit reached.":
+        raise QuotaLimitReachedError()
 
     details = None
 

tests/functional/conftest.py

@@ -1,3 +1,4 @@
+import abc
 import http.server
 import shutil
 import socketserver
@@ -23,7 +24,7 @@
 requires_docker = pytest.mark.skipif(not HAS_DOCKER, reason="This test requires Docker")
 
 
-class SlowGGAPIHandler(http.server.BaseHTTPRequestHandler):
+class AbstractGGAPIHandler(http.server.BaseHTTPRequestHandler, metaclass=abc.ABCMeta):
     def do_HEAD(self):
         self.send_response(200)
 
@@ -45,6 +46,12 @@ def do_GET(self):
 
         self.wfile.write(response.content)
 
+    @abc.abstractmethod
+    def do_POST(self):
+        raise NotImplementedError()
+
+
+class SlowGGAPIHandler(AbstractGGAPIHandler):
     def do_POST(self):
         if "multiscan" in self.path:
             content = b'{"detail":"Sorry, I overslept!"}'
@@ -58,6 +65,16 @@ def do_POST(self):
             self.send_response(418)
 
 
+class NoQuotaGGAPIHandler(AbstractGGAPIHandler):
+    def do_POST(self):
+        content = b'{"detail":"Quota limit reached."}'
+        self.send_response(403)
+        self.send_header("content-type", "application/json")
+        self.send_header("Content-Length", str(len(content)))
+        self.end_headers()
+        self.wfile.write(content)
+
+
 class ReuseAddressServer(socketserver.TCPServer):
     allow_reuse_address = True
 
@@ -67,6 +84,11 @@ def _start_slow_gitguardian_api(host: str, port: int):
         httpd.serve_forever()
 
 
+def _start_no_quota_gitguardian_api(host: str, port: int):
+    with ReuseAddressServer((host, port), NoQuotaGGAPIHandler) as httpd:
+        httpd.serve_forever()
+
+
 @pytest.fixture
 @pytest.mark.allow_hosts(["localhost"])
 def slow_gitguardian_api() -> Generator[str, None, None]:
@@ -78,3 +100,16 @@ def slow_gitguardian_api() -> Generator[str, None, None]:
     finally:
         server_process.kill()
         server_process.join()
+
+
+@pytest.fixture
+@pytest.mark.allow_hosts(["localhost"])
+def no_quota_gitguardian_api() -> Generator[str, None, None]:
+    host, port = "localhost", 8124
+    server_process = Process(target=_start_no_quota_gitguardian_api, args=(host, port))
+    server_process.start()
+    try:
+        yield f"http://{host}:{port}"
+    finally:
+        server_process.kill()
+        server_process.join()

tests/functional/secret/test_scan_repo.py

@@ -1,4 +1,6 @@
+import os
 from pathlib import Path
+from unittest.mock import patch
 
 from tests.conftest import GG_VALID_TOKEN
 from tests.functional.utils import recreate_censored_content, run_ggshield_scan
@@ -24,3 +26,37 @@ def test_scan_repo(tmp_path: Path) -> None:
     proc = run_ggshield_scan("repo", str(repo.path), expected_code=1, cwd=repo.path)
 
     assert recreate_censored_content(leak_content, GG_VALID_TOKEN) in proc.stdout
+
+
+def test_scan_repo_quota_limit_reached(
+    tmp_path: Path, no_quota_gitguardian_api: str, caplog
+) -> None:
+    # GIVEN a repository
+    repo = Repository.create(tmp_path)
+
+    # AND a commit containing a leak
+    secret_file = repo.path / "secret.conf"
+    leak_content = f"password = {GG_VALID_TOKEN}"
+    secret_file.write_text(leak_content)
+    repo.add("secret.conf")
+
+    # AND some clean commits on top of it
+    for _ in range(3):
+        repo.create_commit()
+
+    # WHEN scanning the repo
+    # THEN error code is 128
+    with patch.dict(
+        os.environ, {**os.environ, "GITGUARDIAN_API_URL": no_quota_gitguardian_api}
+    ):
+        proc = run_ggshield_scan(
+            "repo", str(repo.path), "--json", expected_code=128, cwd=repo.path
+        )
+
+    # AND stderr contains an error message
+    assert (
+        "Error: Could not perform the requested action: no more API calls available."
+        in proc.stderr
+    )
+    # AND stdout is empty
+    assert proc.stdout.strip() == ""

tests/unit/secret/test_secret_scanner.py

@@ -5,7 +5,7 @@
 import pytest
 from pygitguardian.models import Detail
 
-from ggshield.core.errors import ExitCode
+from ggshield.core.errors import ExitCode, QuotaLimitReachedError
 from ggshield.core.git_shell import Filemode
 from ggshield.scan import (
     Commit,
@@ -186,3 +186,9 @@ def test_handle_scan_error(detail, status_code, chunk, capsys, snapshot):
     handle_scan_chunk_error(detail, chunk)
     captured = capsys.readouterr()
     snapshot.assert_match(captured.err)
+
+
+def test_handle_scan_quota_limit_reached():
+    detail = Detail(detail="Quota limit reached.", status_code=403)
+    with pytest.raises(QuotaLimitReachedError):
+        handle_scan_chunk_error(detail, Mock())