Skip to content

Commit

Permalink
chore: rename break_type to detector in Secret model
Browse files Browse the repository at this point in the history
Also fixes some tests
  • Loading branch information
gg-mmill committed Dec 20, 2024
1 parent fb04647 commit 521885c
Show file tree
Hide file tree
Showing 13 changed files with 48 additions and 38 deletions.
2 changes: 1 addition & 1 deletion ggshield/verticals/secret/output/schemas.py
Original file line number Diff line number Diff line change
Expand Up @@ -12,7 +12,7 @@ class IgnoreReasonSchema(BaseSchema):
class FlattenedPolicyBreak(BaseSchema):
policy = fields.String(required=True)
occurrences = fields.List(fields.Nested(ExtendedMatchSchema), required=True)
break_type = fields.String(data_key="type", required=True)
detector = fields.String(data_key="type", required=True)
validity = fields.String(required=False, allow_none=True)
ignore_sha = fields.String(required=True)
total_occurrences = fields.Integer(required=True)
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -17,7 +17,7 @@ def format_secret(secret: Secret) -> str:
f'{x.match_type}: "{censor_match(x)}"' for x in secret.matches
)
validity = translate_validity(secret.validity)
return f"{secret.break_type} (Validity: {validity}, {match_str})"
return f"{secret.detector} (Validity: {validity}, {match_str})"


class SecretGitLabWebUIOutputHandler(SecretOutputHandler):
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -118,7 +118,7 @@ def serialized_secret(
"occurrences": [],
"ignore_sha": ignore_sha,
"policy": secrets[0].policy,
"break_type": secrets[0].break_type,
"detector": secrets[0].detector,
"total_occurrences": len(secrets),
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -76,12 +76,12 @@ def _create_sarif_result_dict(
f"- [{m.match_type}]({id})" for id, m in enumerate(secret.matches)
)
extended_matches = cast(List[ExtendedMatch], secret.matches)
message = f"Secret detected: {secret.break_type}.\nMatches: {matches_str}"
markdown_message = f"Secret detected: {secret.break_type}\nMatches:\n{matches_li}"
message = f"Secret detected: {secret.detector}.\nMatches: {matches_str}"
markdown_message = f"Secret detected: {secret.detector}\nMatches:\n{matches_li}"

# Create dict
dct = {
"ruleId": secret.break_type,
"ruleId": secret.detector,
"level": "error",
"message": {
"text": message,
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -297,7 +297,7 @@ def secret_header(
)

start_line = format_text(">>", STYLE["detector_line_start"])
secret_type = format_text(secret.break_type, STYLE["secret_type"])
secret_type = format_text(secret.detector, STYLE["secret_type"])
number_occurrences = format_text(str(len(secrets)), STYLE["occurrence_count"])
ignore_sha = format_text(ignore_sha, STYLE["ignore_sha"])

Expand Down
4 changes: 2 additions & 2 deletions ggshield/verticals/secret/secret_scan_collection.py
Original file line number Diff line number Diff line change
Expand Up @@ -83,7 +83,7 @@ class Secret:
Named Secret since we are dropping other kind of policy breaks.
"""

break_type: str
detector: str
validity: str
known_secret: bool
incident_url: Optional[str]
Expand Down Expand Up @@ -186,7 +186,7 @@ def from_scan_result(
validity=policy_break.validity,
known_secret=policy_break.known_secret,
incident_url=policy_break.incident_url,
break_type=policy_break.break_type,
detector=policy_break.break_type,
matches=[
ExtendedMatch.from_match(match, lines, result.is_on_patch)
for match in policy_break.matches
Expand Down
2 changes: 1 addition & 1 deletion ggshield/verticals/secret/secret_scanner.py
Original file line number Diff line number Diff line change
Expand Up @@ -216,7 +216,7 @@ def _collect_results(
result = Result.from_scan_result(file, scan_result, self.secret_config)
for secret in result.secrets:
self.cache.add_found_policy_break(
secret.break_type,
secret.detector,
secret.get_ignore_sha(),
file.filename,
)
Expand Down
14 changes: 14 additions & 0 deletions tests/factories.py
Original file line number Diff line number Diff line change
Expand Up @@ -6,6 +6,7 @@

from ggshield.core.scan.scannable import StringScannable
from ggshield.utils.git_shell import Filemode
from ggshield.verticals.secret.secret_scan_collection import Secret
from tests.factory_constants import DETECTOR_NAMES, MATCH_NAMES


Expand Down Expand Up @@ -87,3 +88,16 @@ class Meta:
policy_breaks = []
policies = ["Secrets detection"]
is_diff = False


class SecretFactory(factory.Factory):
class Meta:
model = Secret

detector = factory.lazy_attribute(lambda obj: random.choice(DETECTOR_NAMES))
validity = "valid"
known_secret = True
incident_url = None
matches = []
ignore_reason = None
diff_kind = None
16 changes: 7 additions & 9 deletions tests/unit/verticals/secret/output/test_gitlab_webui_output.py
Original file line number Diff line number Diff line change
@@ -1,28 +1,26 @@
from pygitguardian.models import Match, PolicyBreak
from pygitguardian.models import Match

from ggshield.core.config.user_config import SecretConfig
from ggshield.verticals.secret import Results, SecretScanCollection
from ggshield.verticals.secret.output.secret_gitlab_webui_output_handler import (
SecretGitLabWebUIOutputHandler,
format_secret,
)
from tests.factories import SecretFactory


def test_format_secret():
policy = PolicyBreak(
"PayPal",
"Secrets detection",
"valid",
[
secret = SecretFactory(
matches=[
Match("AZERTYUIOP", "client_id", line_start=123),
Match("abcdefghijk", "client_secret", line_start=456),
],
)
out = format_secret(policy)
out = format_secret(secret)

assert policy.break_type in out
assert secret.detector in out
assert "Validity: Valid" in out
for match in policy.matches:
for match in secret.matches:
assert match.match_type in out
# match value itself must be obfuscated
assert match.match not in out
Expand Down
10 changes: 5 additions & 5 deletions tests/unit/verticals/secret/output/test_json_output.py
Original file line number Diff line number Diff line change
Expand Up @@ -99,7 +99,7 @@
"incidents": validators.All(
[
{
"break_type": str,
"detector": str,
"policy": str,
"total_occurrences": validators.All(int, min=1),
Required("incident_url"): validators.Match(
Expand Down Expand Up @@ -509,14 +509,14 @@ def test_ignore_known_secrets(verbose, ignore_known_secrets, secrets_types):
incident_for_secret_type = {incident["type"]: incident for incident in incidents}

for secret in known_secrets:
assert incident_for_secret_type[secret.break_type]["known_secret"]
assert incident_for_secret_type[secret.break_type]["incident_url"].startswith(
assert incident_for_secret_type[secret.detector]["known_secret"]
assert incident_for_secret_type[secret.detector]["incident_url"].startswith(
"https://dashboard.gitguardian.com/workspace/1/incidents/"
)

for secret in new_secrets:
assert not incident_for_secret_type[secret.break_type]["known_secret"]
assert not incident_for_secret_type[secret.break_type]["incident_url"]
assert not incident_for_secret_type[secret.detector]["known_secret"]
assert not incident_for_secret_type[secret.detector]["incident_url"]


@pytest.mark.parametrize("with_incident_details", [True, False])
Expand Down
14 changes: 7 additions & 7 deletions tests/unit/verticals/secret/output/test_sarif_output.py
Original file line number Diff line number Diff line change
Expand Up @@ -5,7 +5,7 @@

import pytest
from pygitguardian import GGClient
from pygitguardian.models import PolicyBreak, ScanResult, SecretIncident
from pygitguardian.models import ScanResult, SecretIncident
from pytest_voluptuous import S
from voluptuous import Optional as VOptional
from voluptuous import validators
Expand All @@ -15,6 +15,7 @@
from ggshield.verticals.secret import Result, Results, SecretScanCollection
from ggshield.verticals.secret.output import SecretSARIFOutputHandler
from ggshield.verticals.secret.output.secret_sarif_output_handler import SCHEMA_URL
from ggshield.verticals.secret.secret_scan_collection import Secret
from tests.unit.conftest import (
_MULTI_SECRET_ONE_LINE_FULL_PATCH,
_MULTI_SECRET_ONE_LINE_PATCH_SCAN_RESULT,
Expand Down Expand Up @@ -206,11 +207,11 @@ def test_sarif_output_for_flat_scan_with_secrets(
sarif_results = json_dict["runs"][0]["results"]

# Check each found secret is correctly represented
for sarif_result, policy_break in zip(sarif_results, scan_result.policy_breaks):
for sarif_result, secret in zip(sarif_results, result.secrets):
check_sarif_result(
sarif_result,
scannable.content,
policy_break,
secret,
with_incident_details and known_incidents,
)

Expand Down Expand Up @@ -278,20 +279,19 @@ def test_sarif_output_for_nested_scan(init_secrets_engine_version):
def check_sarif_result(
sarif_result: Dict[str, Any],
content: str,
policy_break: PolicyBreak,
secret: Secret,
contains_incident_details: bool = False,
):
"""Check sarif_result contains a representation of policy_break, applied to content"""

# Check the secret name
secret_name = sarif_result["ruleId"]
assert secret_name == policy_break.break_type
assert secret_name == secret.detector

# Check the matches point to the right part of the content. `expected_matches`
# and `actual matches` are dicts of match_name => matched_text.
expected_matches = {
m.match_type: content[m.index_start : m.index_end + 1]
for m in policy_break.matches
m.match_type: content[m.index_start : m.index_end + 1] for m in secret.matches
}

actual_matches = {}
Expand Down
4 changes: 2 additions & 2 deletions tests/unit/verticals/secret/output/test_text_output.py
Original file line number Diff line number Diff line change
Expand Up @@ -161,7 +161,7 @@ def assert_policies_displayed(output, verbose, ignore_known_secrets, secrets):
for secret in secrets:
if not ignore_known_secrets or verbose:
# All secrets are displayed no matter if they're known or not
assert f"Secret detected: {secret.break_type}" in output
assert f"Secret detected: {secret.detector}" in output
if secret.known_secret:
assert "Known by GitGuardian dashboard: YES" in output
assert (
Expand All @@ -172,7 +172,7 @@ def assert_policies_displayed(output, verbose, ignore_known_secrets, secrets):
assert "Incident URL: N/A" in output
else:
if secret.known_secret:
assert f"Secret detected: {secret.break_type}" not in output
assert f"Secret detected: {secret.detector}" not in output

if ignore_known_secrets:
secrets_number = sum(1 for x in secrets if not x.known_secret)
Expand Down
8 changes: 3 additions & 5 deletions tests/unit/verticals/secret/test_secret_scanner.py
Original file line number Diff line number Diff line change
Expand Up @@ -399,11 +399,9 @@ def test_scan_ignore_known_secrets(scan_mock: Mock, client, ignore_known_secrets
results = scanner.scan([scannable], scanner_ui=Mock())

if ignore_known_secrets:
assert [pbreak.break_type for pbreak in results.results[0].secrets] == [
"unknown"
]
assert [pbreak.detector for pbreak in results.results[0].secrets] == ["unknown"]
else:
assert [pbreak.break_type for pbreak in results.results[0].secrets] == [
assert [pbreak.detector for pbreak in results.results[0].secrets] == [
"known",
"unknown",
]
Expand Down Expand Up @@ -485,6 +483,6 @@ def test_all_secrets_is_used(scan_mock: Mock, client):
secret_config=SecretConfig(),
)
results = scanner.scan([scannable], scanner_ui=Mock())
assert [pbreak.break_type for pbreak in results.results[0].secrets] == [
assert [pbreak.detector for pbreak in results.results[0].secrets] == [
"not-excluded"
]

0 comments on commit 521885c

Please sign in to comment.