Repository: gitguardian/ggshield
Issue: `scan ci` uses wrong value for "mode" HTTP header

## Description

`ggshield secret scan ci` calls are supposed to be reported as "ci/$CI_NAME" in the usage statistics visible by our users.

This was changed by mistake in ggshield 1.12.0: calls are now reported as "repo".

## TODO

Bring back the code from ggshield < 1.12.0 (see https://github.com/GitGuardian/ggshield/blob/v1.11.0/ggshield/ci.py#L296).

Changed files:
ggshield/cmd/secret/scan/range.py, ggshield/cmd/secret/scan/prereceive.py, ggshield/cmd/secret/scan/ci.py, tests/cmd/scan/test_ci.py, tests/cmd/scan/test_prepush.py, ggshield/scan/repo.py, ggshield/cmd/secret/scan/prepush.py

diff --git a/ggshield/cmd/secret/scan/ci.py b/ggshield/cmd/secret/scan/ci.py
index 67e1efa..309bb36 100644
--- a/ggshield/cmd/secret/scan/ci.py
+++ b/ggshield/cmd/secret/scan/ci.py
@@ -6,7 +6,12 @@ import click
 from ggshield.core.cache import ReadOnlyCache
 from ggshield.core.extra_headers import add_extra_header
 from ggshield.core.git_shell import check_git_dir, get_list_commit_SHA
-from ggshield.core.utils import EMPTY_SHA, SupportedCI, handle_exception
+from ggshield.core.utils import (
+    EMPTY_SHA,
+    SupportedCI,
+    SupportedScanMode,
+    handle_exception,
+)
 from ggshield.scan.repo import scan_commit_range
 
 
@@ -244,7 +249,7 @@ def azure_range(verbose: bool) -> List[str]:  # pragma: no cover
 
 @click.command()
 @click.pass_context
-def ci_cmd(ctx: click.Context) -> int:  # pragma: no cover
+def ci_cmd(ctx: click.Context) -> int:
     """
     scan in a CI environment.
     """
@@ -290,6 +295,8 @@ def ci_cmd(ctx: click.Context) -> int:  # pragma: no cover
 
         add_extra_header(ctx, "Ci-Mode", ci_mode.name)
 
+        mode_header = f"{SupportedScanMode.CI.value}/{ci_mode.value}"
+
         if config.verbose:
             click.echo(f"Commits to scan: {len(commit_list)}", err=True)
 
@@ -302,6 +309,7 @@ def ci_cmd(ctx: click.Context) -> int:  # pragma: no cover
             exclusion_regexes=ctx.obj["exclusion_regexes"],
             matches_ignore=config.secret.ignored_matches,
             scan_id=" ".join(commit_list),
+            mode_header=mode_header,
             ignored_detectors=config.secret.ignored_detectors,
         )
     except Exception as error:
diff --git a/ggshield/cmd/secret/scan/prepush.py b/ggshield/cmd/secret/scan/prepush.py
index 2b3c284..16dc6f9 100644
--- a/ggshield/cmd/secret/scan/prepush.py
+++ b/ggshield/cmd/secret/scan/prepush.py
@@ -6,7 +6,12 @@ from typing import List, Optional, Tuple
 import click
 
 from ggshield.core.git_shell import check_git_dir, get_list_commit_SHA
-from ggshield.core.utils import EMPTY_SHA, EMPTY_TREE, handle_exception
+from ggshield.core.utils import (
+    EMPTY_SHA,
+    EMPTY_TREE,
+    SupportedScanMode,
+    handle_exception,
+)
 from ggshield.scan.repo import scan_commit_range
 
 
@@ -78,6 +83,7 @@ def prepush_cmd(ctx: click.Context, prepush_args: List[str]) -> int:  # pragma:
             exclusion_regexes=ctx.obj["exclusion_regexes"],
             matches_ignore=config.secret.ignored_matches,
             scan_id=" ".join(commit_list),
+            mode_header=SupportedScanMode.PRE_PUSH.value,
             ignored_detectors=config.secret.ignored_detectors,
         )
     except Exception as error:
diff --git a/ggshield/cmd/secret/scan/prereceive.py b/ggshield/cmd/secret/scan/prereceive.py
index a2eecf1..caa3031 100644
--- a/ggshield/cmd/secret/scan/prereceive.py
+++ b/ggshield/cmd/secret/scan/prereceive.py
@@ -14,6 +14,7 @@ from ggshield.core.utils import (
     EMPTY_SHA,
     EMPTY_TREE,
     PRERECEIVE_TIMEOUT,
+    SupportedScanMode,
     handle_exception,
 )
 from ggshield.output import GitLabWebUIOutputHandler
@@ -163,6 +164,7 @@ def prereceive_cmd(ctx: click.Context, web: bool, prereceive_args: List[str]) ->
                 exclusion_regexes=ctx.obj["exclusion_regexes"],
                 matches_ignore=config.secret.ignored_matches,
                 scan_id=" ".join(commit_list),
+                mode_header=SupportedScanMode.PRE_RECEIVE.value,
                 ignored_detectors=config.secret.ignored_detectors,
             )
             if return_code:
diff --git a/ggshield/cmd/secret/scan/range.py b/ggshield/cmd/secret/scan/range.py
index 0a38006..cb3a6d5 100644
--- a/ggshield/cmd/secret/scan/range.py
+++ b/ggshield/cmd/secret/scan/range.py
@@ -1,7 +1,7 @@
 import click
 
 from ggshield.core.git_shell import get_list_commit_SHA
-from ggshield.core.utils import handle_exception
+from ggshield.core.utils import SupportedScanMode, handle_exception
 from ggshield.scan.repo import scan_commit_range
 
 
@@ -32,6 +32,7 @@ def range_cmd(ctx: click.Context, commit_range: str) -> int:  # pragma: no cover
             exclusion_regexes=ctx.obj["exclusion_regexes"],
             matches_ignore=config.secret.ignored_matches,
             scan_id=commit_range,
+            mode_header=SupportedScanMode.COMMIT_RANGE.value,
             ignored_detectors=config.secret.ignored_detectors,
         )
     except Exception as error:
diff --git a/ggshield/scan/repo.py b/ggshield/scan/repo.py
index ac2c3e1..b1ff7ad 100644
--- a/ggshield/scan/repo.py
+++ b/ggshield/scan/repo.py
@@ -51,6 +51,7 @@ def scan_repo_path(
                 exclusion_regexes=set(),
                 matches_ignore=config.secret.ignored_matches,
                 scan_id=scan_id,
+                mode_header=SupportedScanMode.PATH.value,
                 ignored_detectors=config.secret.ignored_detectors,
             )
     except Exception as error:
@@ -63,6 +64,7 @@ def scan_commit(
     cache: Cache,
     verbose: bool,
     matches_ignore: Iterable[IgnoredMatch],
+    mode_header: str,
     ignored_detectors: Optional[Set[str]] = None,
 ) -> ScanCollection:  # pragma: no cover
     try:
@@ -70,7 +72,7 @@ def scan_commit(
             client=client,
             cache=cache,
             matches_ignore=matches_ignore,
-            mode_header=SupportedScanMode.REPO.value,
+            mode_header=mode_header,
             ignored_detectors=ignored_detectors,
         )
     except Exception as exc:
@@ -94,6 +96,7 @@ def scan_commit_range(
     exclusion_regexes: Set[re.Pattern],
     matches_ignore: Iterable[IgnoredMatch],
     scan_id: str,
+    mode_header: str,
     ignored_detectors: Optional[Set[str]] = None,
 ) -> int:  # pragma: no cover
     """
@@ -115,6 +118,7 @@ def scan_commit_range(
                 cache,
                 verbose,
                 matches_ignore,
+                mode_header,
                 ignored_detectors,
             )
             for sha in commit_list
diff --git a/tests/cmd/scan/test_ci.py b/tests/cmd/scan/test_ci.py
index d770e2b..fd332fd 100644
--- a/tests/cmd/scan/test_ci.py
+++ b/tests/cmd/scan/test_ci.py
@@ -7,6 +7,15 @@ import pytest
 
 from ggshield.cmd.main import cli
 from ggshield.cmd.secret.scan.ci import EMPTY_SHA
+from tests.conftest import assert_invoke_exited_with, assert_invoke_ok
+
+
+@pytest.fixture(autouse=True)
+def clear_current_ci_envs(monkeypatch):
+    # Make sure the tests are not affected by the fact they themselves are running inside
+    # the CI
+    monkeypatch.delenv("CI", raising=False)
+    monkeypatch.delenv("GITHUB_ACTIONS", raising=False)
 
 
 @patch("ggshield.cmd.secret.scan.ci.get_list_commit_SHA")
@@ -70,12 +79,97 @@ def test_gitlab_ci_range(
         cli,
         [
             "-v",
+            "secret",
             "scan",
             *json_arg,
             "ci",
         ],
     )
-    assert result.exit_code == 0, result.stderr
+    assert_invoke_ok(result)
     if json_output:
         json.loads(result.output)
     get_list_mock.assert_called_once_with(expected_parameter)
+
+
+@patch("ggshield.cmd.secret.scan.ci.scan_commit_range")
+@patch("ggshield.cmd.secret.scan.ci.get_list_commit_SHA")
+@patch("ggshield.cmd.secret.scan.ci.check_git_dir")
+@pytest.mark.parametrize(
+    ("env", "expected_mode"),
+    [
+        ({"CI": "true", "GITLAB_CI": "true"}, "ci/GITLAB"),
+        ({"CI": "true", "GITHUB_ACTIONS": "true"}, "ci/GITHUB ACTIONS"),
+        ({"CI": "true", "TRAVIS": "true"}, "ci/TRAVIS"),
+        ({"JENKINS_HOME": "/var/jenkins"}, "ci/JENKINS HOME"),
+        (
+            {"CI": "true", "JENKINS_URL": "https://ci.example.com"},
+            "ci/JENKINS HOME",
+        ),  # Can we really have JENKINS_URL but not JENKINS_HOME?
+        ({"CI": "true", "CIRCLECI": "true"}, "ci/CIRCLECI"),
+        ({"CI": "true", "BITBUCKET_COMMIT": "12345abcd"}, "ci/BITBUCKET PIPELINES"),
+        ({"CI": "true", "DRONE": "1", "DRONE_COMMIT_BEFORE": "c0ff331a"}, "ci/DRONE"),
+        (
+            {"BUILD_BUILDID": "1234", "BUILD_SOURCEVERSION": "b4dd3caf"},
+            "ci/AZURE PIPELINES",
+        ),
+    ],
+)
+def test_ci_cmd_uses_right_mode_header(
+    _: Mock,
+    get_list_mock: Mock,
+    scan_commit_range_mock: Mock,
+    cli_fs_runner: click.testing.CliRunner,
+    monkeypatch,
+    env: Dict[str, str],
+    expected_mode: str,
+):
+    # GIVEN a CI env
+    for key, value in env.items():
+        monkeypatch.setenv(key, value)
+
+    get_list_mock.return_value = ["a"] * 51
+    scan_commit_range_mock.return_value = 0
+
+    # WHEN `secret scan ci` is called
+    result = cli_fs_runner.invoke(cli, ["secret", "scan", "ci"])
+
+    # THEN scan succeeds
+    assert_invoke_ok(result)
+
+    # AND scan_commit_range() is called with the right mode
+    scan_commit_range_mock.assert_called_once()
+    args = scan_commit_range_mock.call_args
+
+    # TODO: When Python 3.7 is dropped, we can use the `args.kwargs` syntax
+    # assert args.kwargs["mode_header"] == expected_mode
+    assert args[1]["mode_header"] == expected_mode
+
+
+@patch("ggshield.cmd.secret.scan.ci.check_git_dir")
+def test_ci_cmd_does_not_work_outside_ci(_, cli_fs_runner: click.testing.CliRunner):
+    # GIVEN no CI env
+    # WHEN `secret scan ci` is called
+    result = cli_fs_runner.invoke(cli, ["secret", "scan", "ci"])
+
+    # THEN it fails
+    assert_invoke_exited_with(result, 1)
+
+    # And the error message explains why
+    assert "only be used in a CI environment" in result.stdout
+
+
+@patch("ggshield.cmd.secret.scan.ci.check_git_dir")
+def test_ci_cmd_does_not_work_if_ci_env_is_odd(
+    _, monkeypatch, cli_fs_runner: click.testing.CliRunner
+):
+    # GIVEN an incomplete CI env
+    monkeypatch.setenv("CI", "true")
+
+    # WHEN `secret scan ci` is called
+    result = cli_fs_runner.invoke(cli, ["secret", "scan", "ci"])
+
+    # THEN it fails
+    assert_invoke_exited_with(result, 1)
+
+    # And the error message explains why
+    assert "Current CI is not detected" in result.stdout
diff --git a/tests/cmd/scan/test_prepush.py b/tests/cmd/scan/test_prepush.py
index 282440d..845b555 100644
--- a/tests/cmd/scan/test_prepush.py
+++ b/tests/cmd/scan/test_prepush.py
@@ -114,6 +114,7 @@ class TestPrepush:
             exclusion_regexes=ANY,
             matches_ignore=ANY,
             scan_id=ANY,
+            mode_header="pre_push",
             ignored_detectors=set(),
         )
         assert_invoke_ok(result)