Handle the case where exclude-signatures is a list of strings (#372)

* Update tomlkit to 0.11.1

* Handle the case where exclude-signatures is a list of strings

Add error handling when tartufo config is not found

Only open the file once when writing the config

Add a couple more tests

* Add changelog fragment

* Pass str to click.echo in util.process_issues

Update test

* Update changelog

* Update tomlkit to 0.11.4

CHANGELOG.md

@@ -1,3 +1,11 @@
+Unreleased
+----------------------
+
+Bug fixes:
+* [#372](https://github.com/godaddy/tartufo/pull/372)
+  - [#371](https://github.com/godaddy/tartufo/issues/371) Handle the case where exclude-signatures is a list of strings
+  - [#373](https://github.com/godaddy/tartufo/issues/373) Pass a string to click.echo rather than bytes
+
 v3.2.1 - 20 July 2022
 ----------------------
 

pyproject.toml

@@ -50,7 +50,7 @@ sphinx-autodoc-typehints = {version = "^1.12.0", optional = true}
 sphinx-click = {version = "^3.0.2", optional = true}
 sphinx-rtd-theme = {version = "^1.0.0", optional = true}
 sphinxcontrib-spelling = {version = "^7.2.1", optional = true}
-tomlkit = "^0.7.2"
+tomlkit = "^0.11.4"
 cached-property = "^1.5.2"
 
 [tool.poetry.dev-dependencies]

tartufo/commands/update_signatures.py

@@ -10,6 +10,7 @@
     Optional,
     Sequence,
     Tuple,
+    Union,
 )
 
 import click
@@ -22,6 +23,18 @@
 DeprecationSetT = MutableSet[Sequence[str]]
 
 
+def unwrap_signature(data: Union[str, MutableMapping[str, str]]) -> str:
+    """Handles the case where a signature can be a string or a dict
+
+    :param data: The string, or dictionary to pull the signature from
+    :returns: The unwrapped signature
+    """
+    if isinstance(data, str):
+        return data
+
+    return data["signature"]
+
+
 def scan_local_repo(
     options: types.GlobalOptions,
     repo_path: str,
@@ -101,13 +114,20 @@ def replace_deprecated_signatures(
     updated = 0
 
     for old_sig, new_sig in deprecations:
-        targets = functools.partial(lambda o, s: o == s["signature"], old_sig)
+        targets = functools.partial(lambda o, s: o == unwrap_signature(s), old_sig)
         # Iterate all the deprecations and update them everywhere
         # they are found in the exclude-signatures section of config
-        for target_signature in filter(targets, config_data["exclude_signatures"]):
+        for i, target_signature in enumerate(config_data["exclude_signatures"]):
+            if not targets(target_signature):
+                continue
+
             updated += 1
             click.echo(f"{updated}) {old_sig!r} -> {new_sig!r}")
-            target_signature["signature"] = new_sig
+
+            if isinstance(target_signature, str):
+                config_data["exclude_signatures"][i] = new_sig
+            else:
+                target_signature["signature"] = new_sig
 
     return updated
 
@@ -120,16 +140,18 @@ def write_updated_signatures(
     :param config_path: The path to the tartufo config file
     :param config_data: The updated config data
     """
-    with open(str(config_path), "r") as file:
-        result = tomlkit.loads(file.read())
+    with open(str(config_path), "r+") as file:
+        file_content = file.read()
+        result = tomlkit.loads(file_content)
+        file.seek(0)
 
-    # Assign the new signatures and write it to the config
-    result["tool"]["tartufo"]["exclude-signatures"] = config_data[  # type: ignore
-        "exclude_signatures"
-    ]
+        # Assign the new signatures and write it to the config
+        result["tool"]["tartufo"]["exclude-signatures"] = config_data[  # type: ignore
+            "exclude_signatures"
+        ]
 
-    with open(str(config_path), "w") as file:
         file.write(tomlkit.dumps(result))
+        file.truncate()
 
 
 def remove_duplicated_entries(config_data: MutableMapping[str, Any]) -> int:
@@ -138,17 +160,19 @@ def remove_duplicated_entries(config_data: MutableMapping[str, Any]) -> int:
 
     :param config_data: The config data to check for duplicates
     """
-    seen = set()
+    seen: MutableSet[str] = set()
     count = 0
 
-    for i, exclude in enumerate(config_data["exclude_signatures"].copy()):
-        if exclude["signature"] in seen:
+    for i, exclude in enumerate(config_data["exclude_signatures"]):
+        signature = unwrap_signature(exclude)
+
+        if signature in seen:
             # Remove this duplicated signature
-            del config_data["exclude_signatures"][i]
+            config_data["exclude_signatures"].pop(i)
             count += 1
         else:
             # Mark this signature as seen
-            seen.add(exclude["signature"])
+            seen.add(signature)
 
     return count
 
@@ -202,7 +226,15 @@ def main(
     remove_duplicates: bool,
 ) -> GitRepoScanner:
     """Update deprecated signatures for a local repository."""
-    config_path, config_data = load_config_from_path(pathlib.Path(repo_path))
+    try:
+        config_path, config_data = load_config_from_path(pathlib.Path(repo_path))
+    except FileNotFoundError:
+        util.fail(
+            util.style_warning("No tartufo config found, exiting..."),
+            ctx,
+            code=0,
+        )
+
     if not config_data.get("exclude_signatures"):
         util.fail(
             util.style_warning("No signatures found in configuration, exiting..."),

tartufo/util.py

@@ -104,7 +104,7 @@ def echo_result(
             )
     else:
         for issue in scanner.scan():
-            click.echo(bytes(issue))
+            click.echo(str(issue))
         if scanner.issue_count == 0:
             if not options.quiet:
                 click.echo(f"Time: {now}\nAll clear. No secrets detected.")

tests/test_update_signatures.py

@@ -28,6 +28,17 @@ def test_with_no_signatures_in_config(
             result.output, "No signatures found in configuration, exiting...\n"
         )
 
+    @mock.patch("tartufo.commands.update_signatures.load_config_from_path")
+    def test_with_no_config(self, mock_load_config: mock.MagicMock) -> None:
+        mock_load_config.side_effect = FileNotFoundError()
+
+        runner = CliRunner()
+        with runner.isolated_filesystem():
+            result = runner.invoke(cli.main, ["update-signatures", "."])
+
+        mock_load_config.assert_called_once()
+        self.assertEqual(result.output, "No tartufo config found, exiting...\n")
+
     @mock.patch("tartufo.commands.update_signatures.GitRepoScanner")
     @mock.patch("tartufo.commands.update_signatures.load_config_from_path")
     def test_with_no_deprecated_signatures(
@@ -280,6 +291,19 @@ def test_found_output_with_no_signatures(
         mock_scan_local.assert_called_once()
         self.assertEqual(result.output, "Found 0 deprecated signatures.\n")
 
+    def test_replace_deprecated_with_list_of_strings(self) -> None:
+        deprecations: Set[Sequence[str]] = set()
+        deprecations.update((("123", "abc"), ("456", "def")))
+        config_data = {"exclude_signatures": ["123", "456"]}
+        expected_result = {"exclude_signatures": ["abc", "def"]}
+
+        count = update_signatures.replace_deprecated_signatures(
+            deprecations, config_data
+        )
+
+        self.assertEqual(count, 2)
+        self.assertEqual(config_data, expected_result)
+
     def test_remove_duplicated_entries(self) -> None:
         initial_data = {
             "exclude_signatures": [

tests/test_util.py

@@ -115,13 +115,13 @@ def test_echo_result_echos_all_when_not_json(self, mock_click, mock_scanner):
         mock_scanner.exclude_signatures = []
         mock_scanner.scan.return_value = (1, 2, 3, 4)
         util.echo_result(options, mock_scanner, "", "")
-        # Ensure that the issues are output as a byte stream
+
         mock_click.echo.assert_has_calls(
             [
-                mock.call(bytes(1)),
-                mock.call(bytes(2)),
-                mock.call(bytes(3)),
-                mock.call(bytes(4)),
+                mock.call(str(1)),
+                mock.call(str(2)),
+                mock.call(str(3)),
+                mock.call(str(4)),
             ]
         )
         self.assertEqual(mock_click.echo.call_count, 4)