Merge pull request #960 from GitGuardian/salomevoltz/add-ignore-hash-cmd

Add default behavior to ggshield secret ignore

changelog.d/20240912_120216_salome.voltz_add_ignore_hash_cmd.md

@@ -0,0 +1,3 @@
+### Added
+
+- It is now possible to ignore a secret manually using `ggshield secret ignore SECRET_SHA --name NAME`.

ggshield/cmd/secret/ignore.py

@@ -7,41 +7,74 @@
 from ggshield.core.cache import Cache
 from ggshield.core.config import Config
 from ggshield.core.text_utils import pluralize
+from ggshield.core.types import IgnoredMatch
 
 
 @click.command()
+@click.argument(
+    "secret_sha",
+    nargs=1,
+    type=str,
+    required=False,
+    metavar="SECRET_SHA",
+)
 @click.option(
     "--last-found",
     is_flag=True,
     help="Ignore secrets found in the last `ggshield secret scan` run.",
 )
+@click.option(
+    "--name",
+    type=str,
+    help="Name of the secret to ignore.",
+    metavar="NAME",
+)
 @add_common_options()
 @click.pass_context
 def ignore_cmd(
     ctx: click.Context,
+    secret_sha: str,
+    name: str,
     last_found: bool,
     **kwargs: Any,
 ) -> None:
     """
     Ignore some secrets.
 
-    The `secret ignore` command instructs ggshield to ignore secrets it finds during a scan.
+    The `secret ignore` command instructs ggshield to ignore secrets.
 
-    This command needs to be used with an option to determine which secrets it should ignore.
-    For now, it only handles the `--last-found` option, which ignores all secrets found during the last scan.
+    Option `--name` allows to specify the name of the secret to ignore.
 
-    The command adds the ignored secrets to the `secrets.ignored-matches` section of your local
+    Option `--last-found` ignores all secrets found during the last scan.
+
+    The command adds the ignored secrets to the `secrets.ignored_matches` section of your local
     configuration file. If no local configuration file is found, a `.gitguardian.yaml` file is created.
     """
+
+    ctx_obj = ContextObj.get(ctx)
+    config = ctx_obj.config
+    path = config.config_path
+
     if last_found:
-        ctx_obj = ContextObj.get(ctx)
-        config = ctx_obj.config
+        if secret_sha or name:
+            raise click.UsageError(
+                "Option `--last-found` cannot be used with `SECRET_SHA` or `--name`."
+            )
         nb = ignore_last_found(config, ctx_obj.cache)
-        path = config.config_path
-        secrets_word = pluralize("secret", nb)
-        click.echo(
-            f"Added {nb} {secrets_word} to the `secret.ignored-matches` section of {path}."
-        )
+    else:
+        if not name:
+            raise click.UsageError(
+                "Option `--name` is required when ignoring a secret."
+            )
+        ignored_match = IgnoredMatch(name=name, match=secret_sha)
+        config.add_ignored_match(ignored_match)
+        nb = 1
+
+    config.save()
+    secrets_word = pluralize("secret", nb)
+    click.echo(
+        f"Added {nb} {secrets_word} to the secret.ignored_matches section of {path}."
+    )
 
 
 def ignore_last_found(config: Config, cache: Cache) -> int:
@@ -52,5 +85,4 @@ def ignore_last_found(config: Config, cache: Cache) -> int:
     """
     for secret in cache.last_found_secrets:
         config.add_ignored_match(secret)
-    config.save()
     return len(cache.last_found_secrets)

tests/unit/cmd/test_ignore.py

@@ -1,16 +1,23 @@
 import os
 import tempfile
-from unittest.mock import Mock
+from unittest.mock import Mock, patch
 
 from pygitguardian.models import Match, PolicyBreak
 
+from ggshield.__main__ import cli
 from ggshield.cmd.secret.ignore import ignore_last_found
 from ggshield.core.cache import Cache
 from ggshield.core.config import Config
+from ggshield.core.errors import ExitCode
 from ggshield.core.scan import Commit, ScanContext, ScanMode
 from ggshield.core.types import IgnoredMatch
 from ggshield.verticals.secret import SecretScanner
-from tests.unit.conftest import _MULTIPLE_SECRETS_PATCH, my_vcr
+from tests.unit.conftest import (
+    _MULTIPLE_SECRETS_PATCH,
+    assert_invoke_exited_with,
+    assert_invoke_ok,
+    my_vcr,
+)
 
 
 DOT_GITGUARDIAN_YAML = os.path.join(tempfile.gettempdir(), ".gitguardian.yml")
@@ -27,6 +34,59 @@ def compare_matches_ignore(match):
     return (match.name, match.match) if isinstance(match, IgnoredMatch) else (match,)
 
 
+def test_ignore_sha(cli_fs_runner):
+    """
+    GIVEN an empty cache and an empty config ignored_matches section
+    WHEN I ignore a secret sha
+    THEN the ignore secret is added to the config and saved
+    """
+    ignored_match = IgnoredMatch(
+        name="test_name",
+        match="41b8889e5e794b21cb1349d8eef1815960bf5257330fd40243a4895f26c2b5c8",
+    )
+    config = Config()
+
+    with patch("ggshield.cmd.utils.context_obj.ContextObj.get") as mock_get_ctx:
+        mock_get_ctx.return_value.config = config
+
+        cmd = ["secret", "ignore", ignored_match.match, "--name", ignored_match.name]
+        result = cli_fs_runner.invoke(cli, cmd, color=False, catch_exceptions=False)
+
+        assert_invoke_ok(result)
+        assert config.user_config.secret.ignored_matches == [ignored_match]
+
+
+def test_error_sha_last_found(cli_fs_runner):
+    """
+    GIVEN any config and cache
+    WHEN I run the command with invalid arguments
+    THEN an error should be raised
+    """
+
+    cmd = ["secret", "ignore", "some_secret_sha", "--last-found"]
+    result = cli_fs_runner.invoke(cli, cmd, color=False, catch_exceptions=False)
+
+    assert_invoke_exited_with(result, ExitCode.USAGE_ERROR)
+    assert (
+        "Option `--last-found` cannot be used with `SECRET_SHA` or `--name`."
+        in result.output
+    )
+
+
+def test_error_ignore_sha_no_name(cli_fs_runner):
+    """
+    GIVEN any config and cache
+    WHEN I run the command with a secret sha but no name
+    THEN an error should be raised
+    """
+
+    cmd = ["secret", "ignore", "some_secret_sha"]
+    result = cli_fs_runner.invoke(cli, cmd, color=False, catch_exceptions=False)
+
+    assert_invoke_exited_with(result, ExitCode.USAGE_ERROR)
+    assert "Option `--name` is required when ignoring a secret." in result.output
+
+
 def test_cache_catches_last_found_secrets(client, isolated_fs):
     """
     GIVEN an empty cache and an empty config ignored_matches section
@@ -66,7 +126,7 @@ def test_cache_catches_last_found_secrets(client, isolated_fs):
 
 def test_cache_catches_nothing(client, isolated_fs):
     """
-    GIVEN a cache of last found secrets same as config ignored-matches
+    GIVEN a cache of last found secrets same as config ignored_matches
     WHEN I run a scan (therefore finding no secret)
     THEN config matches is unchanged and cache is empty
     """
@@ -97,7 +157,7 @@ def test_ignore_last_found(client, isolated_fs):
     """
     GIVEN a cache of last found secrets not empty
     WHEN I run a ignore last found command
-    THEN config ignored-matches is updated accordingly
+    THEN config ignored_matches is updated accordingly
     """
     config = Config()
 
@@ -117,7 +177,7 @@ def test_ignore_last_found(client, isolated_fs):
 
 def test_ignore_last_found_with_manually_added_secrets(client, isolated_fs):
     """
-    GIVEN a cache containing part of config ignored-matches secrets
+    GIVEN a cache containing part of config ignored_matches secrets
     WHEN I run ignore command
     THEN only new discovered secrets are added to the config
     """

tests/unit/core/config/test_user_config.py

@@ -159,13 +159,13 @@ def test_load_mixed_versions(self, local_config_path, global_config_path):
     def test_load_ignored_matches_with_empty_names(self):
         config_path = "config.yaml"
         # Use write_text() here because write_yaml() cannot generate a key with a really
-        # empty value, like we need for secret.ignored-matches[0].name
+        # empty value, like we need for secret.ignored_matches[0].name
         write_text(
             config_path,
             """
             version: 2
             secret:
-              ignored-matches:
+              ignored_matches:
                 - name:
                   match: abcd
                 - name: ""
@@ -535,7 +535,7 @@ def test_user_config_unknown_keys(self, local_config_path, capsys):
                 "iac": {"ignored_paths": ["myglobalpath"], "iac_unknown": [""]},
                 "secret": {
                     "secret_invalid_key": "invalid key",
-                    "ignored-matches": [
+                    "ignored_matches": [
                         {
                             "name": "",
                             "match": "one",