Merge pull request #1000 from GitGuardian/jgriffe/SCRT-3897/improve-scan-ci-commit-selection

feat(ci): improve scan ci commit selection

Author: gg-jonathangriffe

changelog.d/20241104_101412_jonathan.griffe_improve_scan_ci_commit_selection.md

@@ -0,0 +1,7 @@
+### Changed
+
+- `ggshield secret scan ci` will now scan all of a Pull Request's commits in the following CI environments: Jenkins, Azure, Bitbucket and Drone.
+
+### Fixed
+
+- When running `ggshield secret scan ci` in a GitLab CI, new commits from the target branch that are not on the feature branch will no longer be scanned.

ggshield/core/git_hooks/ci/commit_range.py

@@ -2,254 +2,92 @@
 from typing import List, Tuple
 
 from ggshield.core.errors import UnexpectedError
+from ggshield.core.git_hooks.ci.get_scan_ci_parameters import (
+    CI_TARGET_BRANCH_ASSOC,
+    get_remote_prefix,
+)
 from ggshield.utils.git_shell import EMPTY_SHA, get_list_commit_SHA
 
 from ... import ui
-from .previous_commit import (
-    github_pull_request_previous_commit_sha,
-    github_push_previous_commit_sha,
-    gitlab_push_previous_commit_sha,
-)
 from .supported_ci import SupportedCI
 
 
-def collect_commit_range_from_ci_env() -> Tuple[List[str], SupportedCI]:
-    supported_ci = SupportedCI.from_ci_env()
-    try:
-        fcn = COLLECT_COMMIT_RANGE_FUNCTIONS[supported_ci]
-    except KeyError:
-        raise UnexpectedError(f"Not implemented for {supported_ci.value}")
-
-    return fcn(), supported_ci
-
-
-def jenkins_range() -> List[str]:  # pragma: no cover
-    head_commit = os.getenv("GIT_COMMIT")
-    previous_commit = os.getenv("GIT_PREVIOUS_COMMIT")
-
-    ui.display_verbose(
-        f"\tGIT_COMMIT: {head_commit}" f"\nGIT_PREVIOUS_COMMIT: {previous_commit}"
-    )
-
-    if previous_commit:
-        commit_list = get_list_commit_SHA(f"{previous_commit}...{head_commit}")
-        if commit_list:
-            return commit_list
-
-    commit_list = get_list_commit_SHA(f"{head_commit}~1...")
-    if commit_list:
-        return commit_list
-
-    raise UnexpectedError(
-        "Unable to get commit range. Please submit an issue with the following info:\n"
-        "\tRepository URL: <Fill if public>\n"
-        f"\tGIT_COMMIT: {head_commit}"
-        f"\tGIT_PREVIOUS_COMMIT: {previous_commit}"
-    )
-
-
-def travis_range() -> List[str]:  # pragma: no cover
-    commit_range = os.getenv("TRAVIS_COMMIT_RANGE")
-    commit_sha = os.getenv("TRAVIS_COMMIT", "HEAD")
-
-    ui.display_verbose(
-        f"TRAVIS_COMMIT_RANGE: {commit_range}" f"\nTRAVIS_COMMIT: {commit_sha}"
-    )
-
-    if commit_range:
-        commit_list = get_list_commit_SHA(commit_range)
-        if commit_list:
-            return commit_list
-
-    commit_list = get_list_commit_SHA(f"{commit_sha}~1...")
-    if commit_list:
-        return commit_list
-
-    raise UnexpectedError(
-        "Unable to get commit range. Please submit an issue with the following info:\n"
-        "\tRepository URL: <Fill if public>\n"
-        f"\tTRAVIS_COMMIT_RANGE: {commit_range}"
-        f"\tTRAVIS_COMMIT: {commit_sha}"
-    )
-
-
-def bitbucket_pipelines_range() -> List[str]:  # pragma: no cover
-    commit_sha = os.getenv("BITBUCKET_COMMIT", "HEAD")
-    ui.display_verbose(f"BITBUCKET_COMMIT: {commit_sha}")
-
-    commit_list = get_list_commit_SHA(f"{commit_sha}~1...")
-    if commit_list:
-        return commit_list
-
-    raise UnexpectedError(
-        "Unable to get commit range. Please submit an issue with the following info:\n"
-        "  Repository URL: <Fill if public>\n"
-        f"  CI_COMMIT_SHA: {commit_sha}"
-    )
-
-
-def circle_ci_range() -> List[str]:  # pragma: no cover
-    """
-    # Extract commit range (or single commit)
-    COMMIT_RANGE=$(echo "${CIRCLE_COMPARE_URL}" | cut -d/ -f7)
-
-    # Fix single commit, unfortunately we don't always get a commit range from Circle CI
-    if [[ $COMMIT_RANGE != *"..."* ]]; then
-    COMMIT_RANGE="${COMMIT_RANGE}...${COMMIT_RANGE}"
-    fi
-    """
-    compare_range = os.getenv("CIRCLE_RANGE")
-    commit_sha = os.getenv("CIRCLE_SHA1", "HEAD")
-
-    ui.display_verbose(f"CIRCLE_RANGE: {compare_range}\nCIRCLE_SHA1: {commit_sha}")
-
-    if compare_range and not compare_range.startswith("..."):
-        commit_list = get_list_commit_SHA(compare_range)
-        if commit_list:
-            return commit_list
-
-    commit_list = get_list_commit_SHA(f"{commit_sha}~1...")
-    if commit_list:
-        return commit_list
-
-    raise UnexpectedError(
-        "Unable to get commit range. Please submit an issue with the following info:\n"
-        "\tRepository URL: <Fill if public>\n"
-        f"\tCIRCLE_RANGE: {compare_range}\n"
-        f"\tCIRCLE_SHA1: {commit_sha}"
-    )
-
-
-def gitlab_ci_range() -> List[str]:  # pragma: no cover
-    before_sha = gitlab_push_previous_commit_sha()
-    commit_sha = os.getenv("CI_COMMIT_SHA", "HEAD")
-    merge_request_target_branch = os.getenv("CI_MERGE_REQUEST_TARGET_BRANCH_NAME")
-
-    ui.display_verbose(
-        f"CI_MERGE_REQUEST_TARGET_BRANCH_NAME: {merge_request_target_branch}\n"
-        f"CI_COMMIT_BEFORE_SHA: {before_sha}\n"
-        f"CI_COMMIT_SHA: {commit_sha}"
-    )
+CI_PREVIOUS_COMMIT_VAR = {
+    SupportedCI.JENKINS: "GIT_PREVIOUS_COMMIT",
+    SupportedCI.CIRCLECI: "CIRCLE_RANGE",
+    SupportedCI.TRAVIS: "TRAVIS_COMMIT_RANGE",
+    SupportedCI.GITLAB: "CI_COMMIT_BEFORE_SHA",
+    SupportedCI.GITHUB: "GITHUB_PUSH_BASE_SHA",
+    SupportedCI.DRONE: "DRONE_COMMIT_BEFORE",
+}
 
-    if before_sha and before_sha != EMPTY_SHA:
-        commit_list = get_list_commit_SHA(f"{before_sha}~1...")
-        if commit_list:
-            return commit_list
+CI_COMMIT_VAR = {
+    SupportedCI.JENKINS: "GIT_COMMIT",
+    SupportedCI.CIRCLECI: "CIRCLE_SHA1",
+    SupportedCI.TRAVIS: "TRAVIS_COMMIT",
+    SupportedCI.GITLAB: "CI_COMMIT_SHA",
+    SupportedCI.GITHUB: "GITHUB_SHA",
+    SupportedCI.AZURE: "BUILD_SOURCEVERSION",
+    SupportedCI.BITBUCKET: "BITBUCKET_COMMIT",
+}
 
-    if merge_request_target_branch and merge_request_target_branch != EMPTY_SHA:
-        commit_list = get_list_commit_SHA(f"origin/{merge_request_target_branch}...")
-        if commit_list:
-            return commit_list
 
-    commit_list = get_list_commit_SHA(f"{commit_sha}~1...")
+def collect_commit_range_from_ci_env() -> Tuple[List[str], SupportedCI]:
+    ci_mode = SupportedCI.from_ci_env()
+
+    base_commit_var = CI_COMMIT_VAR.get(ci_mode)
+    base_commit = os.getenv(base_commit_var, "HEAD") if base_commit_var else "HEAD"
+
+    ui.display_verbose(f"\tIdentified base commit as {base_commit}")
+
+    target_branch_var = CI_TARGET_BRANCH_ASSOC.get(ci_mode)
+    target_branch = None
+    if target_branch_var:
+        target_branch = os.getenv(target_branch_var)
+        if target_branch and target_branch != EMPTY_SHA:
+            ui.display_verbose(f"\tIdentified target branch as {target_branch}")
+            commit_list = get_list_commit_SHA(
+                f"{get_remote_prefix()}{target_branch}..{base_commit}"
+            )
+            if commit_list:
+                return commit_list, ci_mode
+
+    previous_commit = None
+    previous_commit_var = CI_PREVIOUS_COMMIT_VAR.get(ci_mode)
+    if previous_commit_var:
+        previous_commit = os.getenv(previous_commit_var)
+        if (
+            previous_commit is None or previous_commit == EMPTY_SHA
+        ) and ci_mode == SupportedCI.GITHUB:
+            previous_commit = os.getenv("GITHUB_DEFAULT_BRANCH")
+        if (
+            previous_commit is not None
+            and previous_commit != EMPTY_SHA
+            and not previous_commit.startswith("...")
+        ):
+            ui.display_verbose(
+                f"\tIdentified previous commit or commit range as {previous_commit}"
+            )
+            if ci_mode in [SupportedCI.CIRCLECI, SupportedCI.TRAVIS]:
+                # for these ci envs, previous_commit is a range of commits
+                commit_range = previous_commit
+            elif ci_mode == SupportedCI.GITLAB:
+                commit_range = f"{previous_commit}~1..{base_commit}"
+            else:
+                commit_range = f"{previous_commit}..{base_commit}"
+            commit_list = get_list_commit_SHA(commit_range)
+            if commit_list:
+                return commit_list, ci_mode
+
+    commit_list = get_list_commit_SHA(f"{base_commit}~1...")
     if commit_list:
-        return commit_list
-
-    raise UnexpectedError(
-        "Unable to get commit range. Please submit an issue with the following info:\n"
-        "  Repository URL: <Fill if public>\n"
-        f"  CI_MERGE_REQUEST_TARGET_BRANCH_NAME: {merge_request_target_branch}\n"
-        f"  CI_COMMIT_BEFORE_SHA: {before_sha}\n"
-        f"  CI_COMMIT_SHA: {commit_sha}"
-    )
-
-
-def github_actions_range() -> List[str]:  # pragma: no cover
-    push_before_sha = github_push_previous_commit_sha()
-    push_base_sha = os.getenv("GITHUB_PUSH_BASE_SHA")
-    pull_req_base_sha = github_pull_request_previous_commit_sha()
-    default_branch = os.getenv("GITHUB_DEFAULT_BRANCH")
-    head_sha = os.getenv("GITHUB_SHA", "HEAD")
-
-    ui.display_verbose(
-        f"github_push_before_sha: {push_before_sha}\n"
-        f"github_push_base_sha: {push_base_sha}\n"
-        f"github_pull_base_sha: {pull_req_base_sha}\n"
-        f"github_default_branch: {default_branch}\n"
-        f"github_head_sha: {head_sha}"
-    )
-
-    # The PR base sha has to be checked before the push_before_sha
-    # because the first one is only populated in case of PR
-    # whereas push_before_sha can be populated in PR in case of
-    # push force event in a PR
-    if pull_req_base_sha and pull_req_base_sha != EMPTY_SHA:
-        commit_list = get_list_commit_SHA(f"{pull_req_base_sha}..")
-        if commit_list:
-            return commit_list
-
-    if push_before_sha and push_before_sha != EMPTY_SHA:
-        commit_list = get_list_commit_SHA(f"{push_before_sha}...")
-        if commit_list:
-            return commit_list
-
-    if push_base_sha and push_base_sha != "null":
-        commit_list = get_list_commit_SHA(f"{push_base_sha}...")
-        if commit_list:
-            return commit_list
-
-    if default_branch:
-        commit_list = get_list_commit_SHA(f"{default_branch}...")
-        if commit_list:
-            return commit_list
-
-    if head_sha:
-        commit_list = get_list_commit_SHA(f"{head_sha}~1...")
-        if commit_list:
-            return commit_list
-
-    raise UnexpectedError(
-        "Unable to get commit range. Please submit an issue with the following info:\n"
-        "  Repository URL: <Fill if public>\n"
-        f"github_push_before_sha: {push_before_sha}\n"
-        f"github_push_base_sha: {push_base_sha}\n"
-        f"github_pull_base_sha: {pull_req_base_sha}\n"
-        f"github_default_branch: {default_branch}\n"
-        f"github_head_sha: {head_sha}"
-    )
-
-
-def drone_range() -> List[str]:  # pragma: no cover
-    before_sha = os.getenv("DRONE_COMMIT_BEFORE")
-
-    ui.display_verbose(f"DRONE_COMMIT_BEFORE: {before_sha}\n")
-
-    if before_sha and before_sha != EMPTY_SHA:
-        commit_list = get_list_commit_SHA(f"{before_sha}..")
-        if commit_list:
-            return commit_list
-
-    raise UnexpectedError(
-        "Unable to get commit range. Please submit an issue with the following info:\n"
-        "  Repository URL: <Fill if public>\n"
-        f"  DRONE_COMMIT_BEFORE: {before_sha}"
-    )
-
-
-def azure_range() -> List[str]:  # pragma: no cover
-    head_commit = os.getenv("BUILD_SOURCEVERSION")
-
-    ui.display_verbose(f"BUILD_SOURCEVERSION: {head_commit}\n")
-
-    if head_commit:
-        commit_list = get_list_commit_SHA(f"{head_commit}~1...")
-        if commit_list:
-            return commit_list
+        return commit_list, ci_mode
 
     raise UnexpectedError(
         "Unable to get commit range. Please submit an issue with the following info:\n"
         "  Repository URL: <Fill if public>\n"
-        f"  BUILD_SOURCEVERSION: {head_commit}"
+        f"  CI_TYPE: {ci_mode.value}\n"
+        f"  TARGET_BRANCH: {target_branch}\n"
+        f"  PREVIOUS_COMMIT: {previous_commit}\n"
+        f"  BASE_COMMIT: {base_commit}"
     )
-
-
-COLLECT_COMMIT_RANGE_FUNCTIONS = {
-    SupportedCI.AZURE: azure_range,
-    SupportedCI.DRONE: drone_range,
-    SupportedCI.GITHUB: github_actions_range,
-    SupportedCI.GITLAB: gitlab_ci_range,
-    SupportedCI.CIRCLECI: circle_ci_range,
-    SupportedCI.BITBUCKET: bitbucket_pipelines_range,
-    SupportedCI.TRAVIS: travis_range,
-    SupportedCI.JENKINS: jenkins_range,
-}

ggshield/core/git_hooks/ci/get_scan_ci_parameters.py

@@ -32,6 +32,17 @@ def travis_scan_ci_args() -> Tuple[str, str]:
 }
 
 
+def get_remote_prefix(wd: Optional[Union[str, Path]] = None) -> str:
+    remotes = get_remotes(wd=wd)
+    if len(remotes) == 0:
+        # note: this should not happen in practice, esp. in a CI job
+        ui.display_verbose("\tNo remote found.")
+        return ""
+    else:
+        ui.display_verbose(f"\tUsing first remote {remotes[0]}.")
+        return f"{remotes[0]}/"
+
+
 def get_scan_ci_parameters(
     current_ci: SupportedCI, wd: Optional[Union[str, Path]] = None
 ) -> Union[Tuple[str, str], None]:
@@ -50,15 +61,7 @@ def get_scan_ci_parameters(
     if current_ci == SupportedCI.TRAVIS:
         return travis_scan_ci_args()
 
-    remotes = get_remotes(wd=wd)
-    if len(remotes) == 0:
-        # note: this should not happen in practice, esp. in a CI job
-        ui.display_verbose("\tNo remote found.")
-        remote_prefix = ""
-    else:
-        ui.display_verbose(f"\tUsing first remote {remotes[0]}.")
-        remote_prefix = f"{remotes[0]}/"
-
+    remote_prefix = get_remote_prefix(wd=wd)
     target_branch_var = CI_TARGET_BRANCH_ASSOC.get(current_ci)
     if not target_branch_var:
         raise UnexpectedError(f"Using scan ci is not supported for {current_ci.value}.")