From 7498c937b0d9177bb6bef5541eb4d8e98048843b Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Salom=C3=A9=20Voltz?= Date: Thu, 12 Sep 2024 12:06:16 +0200 Subject: [PATCH 1/2] feat(secret_ignore): Add default behavior to ignore secrets using hash --- ...120216_salome.voltz_add_ignore_hash_cmd.md | 3 ++ ggshield/cmd/secret/ignore.py | 49 ++++++++++++++----- 2 files changed, 41 insertions(+), 11 deletions(-) create mode 100644 changelog.d/20240912_120216_salome.voltz_add_ignore_hash_cmd.md diff --git a/changelog.d/20240912_120216_salome.voltz_add_ignore_hash_cmd.md b/changelog.d/20240912_120216_salome.voltz_add_ignore_hash_cmd.md new file mode 100644 index 0000000000..adae3a4d76 --- /dev/null +++ b/changelog.d/20240912_120216_salome.voltz_add_ignore_hash_cmd.md @@ -0,0 +1,3 @@ +### Added + +- Default behavior for `ggshield secret ignore` command to allow ignoring secrets using their hash with optional `--name` argument. diff --git a/ggshield/cmd/secret/ignore.py b/ggshield/cmd/secret/ignore.py index 3cdd1eeec9..8902983f6b 100644 --- a/ggshield/cmd/secret/ignore.py +++ b/ggshield/cmd/secret/ignore.py @@ -7,41 +7,69 @@ 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( + "hash", + nargs=1, + type=str, + required=False, + metavar="HASH", +) @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.", +) @add_common_options() @click.pass_context def ignore_cmd( ctx: click.Context, + hash: 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. + + 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 hash or name: + raise click.UsageError( + "Option `--last-found` cannot be used with `HASH` 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: + match = IgnoredMatch(name=name if name else "", match=hash) + config.add_ignored_match(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 +80,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) From ef6bb9dc10fd0f52b0e6d4482350cacbc72f8d1d Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Salom=C3=A9=20Voltz?= Date: Thu, 12 Sep 2024 13:48:05 +0200 Subject: [PATCH 2/2] fix(secret_ignore): Add tests --- ...120216_salome.voltz_add_ignore_hash_cmd.md | 2 +- ggshield/cmd/secret/ignore.py | 23 +++--- tests/unit/cmd/test_ignore.py | 70 +++++++++++++++++-- tests/unit/core/config/test_user_config.py | 6 +- 4 files changed, 83 insertions(+), 18 deletions(-) diff --git a/changelog.d/20240912_120216_salome.voltz_add_ignore_hash_cmd.md b/changelog.d/20240912_120216_salome.voltz_add_ignore_hash_cmd.md index adae3a4d76..e28e518f6d 100644 --- a/changelog.d/20240912_120216_salome.voltz_add_ignore_hash_cmd.md +++ b/changelog.d/20240912_120216_salome.voltz_add_ignore_hash_cmd.md @@ -1,3 +1,3 @@ ### Added -- Default behavior for `ggshield secret ignore` command to allow ignoring secrets using their hash with optional `--name` argument. +- It is now possible to ignore a secret manually using `ggshield secret ignore SECRET_SHA --name NAME`. diff --git a/ggshield/cmd/secret/ignore.py b/ggshield/cmd/secret/ignore.py index 8902983f6b..38318c5970 100644 --- a/ggshield/cmd/secret/ignore.py +++ b/ggshield/cmd/secret/ignore.py @@ -12,11 +12,11 @@ @click.command() @click.argument( - "hash", + "secret_sha", nargs=1, type=str, required=False, - metavar="HASH", + metavar="SECRET_SHA", ) @click.option( "--last-found", @@ -27,12 +27,13 @@ "--name", type=str, help="Name of the secret to ignore.", + metavar="NAME", ) @add_common_options() @click.pass_context def ignore_cmd( ctx: click.Context, - hash: str, + secret_sha: str, name: str, last_found: bool, **kwargs: Any, @@ -46,7 +47,7 @@ def ignore_cmd( 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 + 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. """ @@ -55,20 +56,24 @@ def ignore_cmd( path = config.config_path if last_found: - if hash or name: + if secret_sha or name: raise click.UsageError( - "Option `--last-found` cannot be used with `HASH` or `--name`." + "Option `--last-found` cannot be used with `SECRET_SHA` or `--name`." ) nb = ignore_last_found(config, ctx_obj.cache) else: - match = IgnoredMatch(name=name if name else "", match=hash) - config.add_ignored_match(match) + 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}." + f"Added {nb} {secrets_word} to the secret.ignored_matches section of {path}." ) diff --git a/tests/unit/cmd/test_ignore.py b/tests/unit/cmd/test_ignore.py index 57bd0242a8..ccadb4482a 100644 --- a/tests/unit/cmd/test_ignore.py +++ b/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 """ diff --git a/tests/unit/core/config/test_user_config.py b/tests/unit/core/config/test_user_config.py index 78b8db3834..6baa7bd592 100644 --- a/tests/unit/core/config/test_user_config.py +++ b/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",