Merge pull request #56 from GitGuardian/salomevoltz/scrt-4932-multimatch-management

salome-voltz · web-flow · commit 47fc1928316f · 2024-11-14T09:28:32.000+01:00
Improve multimatch management
diff --git a/src/lib/api-types.ts b/src/lib/api-types.ts
@@ -1,12 +1,9 @@
+/* eslint-disable @typescript-eslint/naming-convention */
 export interface Occurrence {
   type: string;
-  // eslint-disable-next-line @typescript-eslint/naming-convention
   line_start: number;
-  // eslint-disable-next-line @typescript-eslint/naming-convention
   index_start: number;
-  // eslint-disable-next-line @typescript-eslint/naming-convention
   line_end: number;
-  // eslint-disable-next-line @typescript-eslint/naming-convention
   index_end: number;
 }
 
@@ -23,13 +20,9 @@ export interface Incident {
   type: string;
   occurrences: Occurrence[];
   validity: Validity;
-  // eslint-disable-next-line @typescript-eslint/naming-convention
   ignore_sha: string;
-  // eslint-disable-next-line @typescript-eslint/naming-convention
   known_secret: boolean;
-  // eslint-disable-next-line @typescript-eslint/naming-convention
   incident_url: string;
-  // eslint-disable-next-line @typescript-eslint/naming-convention
   total_occurrences: number;
 }
 
@@ -41,6 +34,5 @@ export interface EntityWithIncidents {
  * JSON response from ggshield scan command
  */
 export interface GGShieldScanResults {
-  // eslint-disable-next-line @typescript-eslint/naming-convention
   entities_with_incidents: EntityWithIncidents[];
 }
diff --git a/src/lib/ggshield-api.ts b/src/lib/ggshield-api.ts
@@ -172,46 +172,39 @@ export async function scanFile(
     filePath,
   ]);
 
-  // Ignore errors concerning usage
-  // This occurs when the path of the file is invalid (i.e.VSCode sending an event for files not on the file system)
-  // or when the file is ignored in the .gitguardian.yaml
-  if (
-    proc.stderr.includes(
-      "Error: An ignored file or directory cannot be scanned"
-    )
-  ) {
-    updateStatusBarItem(StatusBarStatus.ignoredFile);
-    return;
-  }
-  if (proc.stderr.includes("Usage: ggshield secret scan path")) {
-    return undefined;
-  }
-  let errorMessage = "";
-  proc.stderr.split("\n").forEach((stderrLine) => {
+  if (proc.status === 128 || proc.status === 3) {
+    const errorMessage = proc.stderr
+      .split("\n")
+      .filter(
+        (stderrLine) =>
+          stderrLine.length > 0 && !stderrLine.includes("Scanning Path...") // ggshield outputs this info message on stderr, ignore it
+      )
+      .join("\n");
+    if (errorMessage.length > 0) {
+      window.showErrorMessage(`ggshield: ${errorMessage}`);
+      return undefined;
+    }
+  } else if (proc.status === 2) {
+    // Ignore errors concerning usage
+    // This occurs when the path of the file is invalid (i.e.VSCode sending an event for files not on the file system)
+    // or when the file is ignored in the .gitguardian.yaml
     if (
-      stderrLine.length > 0 &&
-      !stderrLine.includes("Scanning Path...") // ggshield outputs this info message on stderr, ignore it
+      proc.stderr.includes(
+        "Error: An ignored file or directory cannot be scanned"
+      )
     ) {
-      errorMessage += stderrLine + "\n";
+      updateStatusBarItem(StatusBarStatus.ignoredFile);
+      return;
     }
-  });
-  if (errorMessage.length > 0) {
-    window.showErrorMessage(`ggshield: ${errorMessage}`);
     return undefined;
-  }
-
-  const results = JSON.parse(proc.stdout);
-  if (!results) {
-    updateStatusBarItem(StatusBarStatus.ready);
+  } else if (proc.status === 0) {
+    updateStatusBarItem(StatusBarStatus.noSecretFound);
     return;
-  }
-  let incidentsDiagnostics: Diagnostic[] = parseGGShieldResults(results);
-  if (incidentsDiagnostics.length !== 0) {
-    updateStatusBarItem(StatusBarStatus.secretFound);
   } else {
-    updateStatusBarItem(StatusBarStatus.noSecretFound);
+    updateStatusBarItem(StatusBarStatus.secretFound);
   }
-
+  const results = JSON.parse(proc.stdout);
+  let incidentsDiagnostics: Diagnostic[] = parseGGShieldResults(results);
   diagnosticCollection.set(fileUri, incidentsDiagnostics);
 }
 
diff --git a/src/lib/ggshield-results-parser.ts b/src/lib/ggshield-results-parser.ts
@@ -1,3 +1,4 @@
+/* eslint-disable @typescript-eslint/naming-convention */
 import {
   Diagnostic,
   Range,
@@ -15,18 +16,29 @@ import {
 
 const validityDisplayName: Record<Validity, string> = {
   unknown: "Unknown",
-  // eslint-disable-next-line @typescript-eslint/naming-convention
   cannot_check: "Cannot Check",
-  // eslint-disable-next-line @typescript-eslint/naming-convention
   no_checker: "No Checker",
-  // eslint-disable-next-line @typescript-eslint/naming-convention
   failed_to_check: "Failed to Check",
-  // eslint-disable-next-line @typescript-eslint/naming-convention
   not_checked: "Not Checked",
   invalid: "Invalid",
   valid: "Valid",
 };
 
+/**
+ * Given a list of occurrences, this function searches for the matches of type "connection_uri"
+ * and returns it if found. If no "connection_uri" match is found, the original list is returned.
+ * This ensures that only the full URI match is kept, avoiding multiple matches for its components (e.g. scheme, username, password, host).
+ *
+ * @param occurrences - An array of `Occurrence` objects to be filtered.
+ * @returns An array containing the "connection_uri" occurrence, or the original list if no such match exists.
+ */
+function filterUriOccurrences(occurrences: Occurrence[]): Occurrence[] {
+  const uriOccurrence = occurrences.find(
+    ({ type }) => type === "connection_uri"
+  );
+  return uriOccurrence ? [uriOccurrence] : occurrences;
+}
+
 /**
  * Parse ggshield results and return diagnostics of found incidents
  *
@@ -45,27 +57,29 @@ export function parseGGShieldResults(
     results.entities_with_incidents.forEach(
       (entityWithIncidents: EntityWithIncidents) => {
         entityWithIncidents.incidents.forEach((incident: Incident) => {
-          incident.occurrences.forEach((occurrence: Occurrence) => {
-            let range = new Range(
-              new Position(occurrence.line_start - 1, occurrence.index_start),
-              new Position(occurrence.line_end - 1, occurrence.index_end)
-            );
-            let diagnostic = new Diagnostic(
-              range,
-              `ggshield: ${occurrence.type}
+          filterUriOccurrences(incident.occurrences).forEach(
+            (occurrence: Occurrence) => {
+              let range = new Range(
+                new Position(occurrence.line_start - 1, occurrence.index_start),
+                new Position(occurrence.line_end - 1, occurrence.index_end)
+              );
+              let diagnostic = new Diagnostic(
+                range,
+                `ggshield: ${occurrence.type}
 
 Secret detected: ${incident.type}
 Validity: ${validityDisplayName[incident.validity]}
 Known by GitGuardian dashboard: ${incident.known_secret ? "YES" : "NO"}
 Total occurrences: ${incident.total_occurrences}
 Incident URL: ${incident.incident_url || "N/A"}
 Secret SHA: ${incident.ignore_sha}`,
-              DiagnosticSeverity.Warning
-            );
+                DiagnosticSeverity.Warning
+              );
 
-            diagnostic.source = "gitguardian";
-            diagnostics.push(diagnostic);
-          });
+              diagnostic.source = "gitguardian";
+              diagnostics.push(diagnostic);
+            }
+          );
         });
       }
     );
diff --git a/src/test/constants.ts b/src/test/constants.ts
@@ -39,3 +39,48 @@ export const scanResultsWithIncident = `{
     "total_occurrences":1,
     "secrets_engine_version":"2.96.0"
     }`;
+
+export const scanResultsWithUriIncident = `{
+  "id": "test.py",
+  "type": "path_scan",
+  "entities_with_incidents": [
+    {
+      "mode": "FILE",
+      "filename": "test.py",
+      "incidents": [
+        {
+          "policy": "Secrets detection",
+          "occurrences": [
+            {
+              "match": "postgres:************************************4/thegift",
+              "type": "connection_uri",
+              "line_start": 1,
+              "line_end": 1,
+              "index_start": 16,
+              "index_end": 70
+            },
+            {
+              "match": "po****es",
+              "type": "scheme",
+              "line_start": 1,
+              "line_end": 1,
+              "index_start": 16,
+              "index_end": 24
+            }
+          ],
+          "type": "PostgreSQL Credentials",
+          "validity": "failed_to_check",
+          "ignore_sha": "981de92451ea2b27f7a163d39c92b566de7cdcc52280fdd9cf7b331d6d53ad86",
+          "total_occurrences": 1,
+          "incident_url": "",
+          "known_secret": false
+        }
+      ],
+      "total_incidents": 1,
+      "total_occurrences": 1
+    }
+  ],
+  "total_incidents": 1,
+  "total_occurrences": 1,
+  "secrets_engine_version": "2.126.0"
+}`;
diff --git a/src/test/suite/lib/ggshield-api.test.ts b/src/test/suite/lib/ggshield-api.test.ts
@@ -46,7 +46,7 @@ suite("scanFile", () => {
 
   test("successfully scans a file with incidents", async () => {
     runGGShieldCommandMock.returnWith({
-      status: 0,
+      status: 1,
       stdout: scanResultsWithIncident,
       stderr: "",
     });
@@ -79,18 +79,25 @@ suite("scanFile", () => {
     );
   });
 
-  test("displays an error message if the scan command fails", async () => {
-    runGGShieldCommandMock.returnWith({
-      status: 1,
-      stdout: "",
-      stderr: "Error",
+  const errorCodes = [128, 3];
+  errorCodes.forEach((code) => {
+    test(`displays an error message if the scan command fails with error code ${code}`, async () => {
+      runGGShieldCommandMock.returnWith({
+        status: code,
+        stdout: "",
+        stderr: "Error",
+      });
+
+      await scanFile(
+        "test.py",
+        Uri.file("test.py"),
+        {} as GGShieldConfiguration
+      );
+
+      // The error message is displayed
+      assert.strictEqual(errorMessageMock.callCount, 1);
+      assert.strictEqual(errorMessageMock.lastCall.args[0], "ggshield: Error");
     });
-
-    await scanFile("test.py", Uri.file("test.py"), {} as GGShieldConfiguration);
-
-    // The error message is displayed
-    assert.strictEqual(errorMessageMock.callCount, 1);
-    assert.strictEqual(errorMessageMock.lastCall.args[0], "ggshield: Error\n");
   });
 
   test("ignores the 'ignored file cannot be scanned' error", async () => {
diff --git a/src/test/suite/results-parser.test.ts b/src/test/suite/results-parser.test.ts
@@ -1,50 +1,18 @@
 import * as assert from "assert";
 
 import { parseGGShieldResults } from "../../lib/ggshield-results-parser";
-import { window, DiagnosticSeverity } from "vscode";
-
-const results = `{
-"id":"/Users/paulindenaurois/Development/gitguardian/ggshield-vscode-extension/sample_files/test.py",
-"type":"path_scan",
-"entities_with_incidents":[
-   {
-      "mode":"FILE",
-      "filename":"/Users/paulindenaurois/Development/gitguardian/ggshield-vscode-extension/sample_files/test.py",
-      "incidents":[
-         {
-            "policy":"Secrets detection",
-            "occurrences":[
-               {
-                  "match":"DDACC73DdB04********************************************057c78317C39",
-                  "type":"apikey",
-                  "line_start":4,
-                  "line_end":4,
-                  "index_start":11,
-                  "index_end":79,
-                  "pre_line_start":4,
-                  "pre_line_end":4
-               }
-            ],
-            "type":"Generic High Entropy Secret",
-            "validity":"no_checker",
-            "ignore_sha":"38353eb1a2aac5b24f39ed67912234d4b4a2e23976d504a88b28137ed2b9185e",
-            "total_occurrences":1,
-            "incident_url":"",
-            "known_secret":false
-         }
-      ],
-      "total_incidents":1,
-      "total_occurrences":1
-   }
-],
-"total_incidents":1,
-"total_occurrences":1,
-"secrets_engine_version":"2.96.0"
-}`;
+import { DiagnosticSeverity } from "vscode";
+import {
+  scanResultsNoIncident,
+  scanResultsWithIncident,
+  scanResultsWithUriIncident,
+} from "../constants";
 
 suite("parseGGShieldResults", () => {
   test("Should parse ggshield scan output", () => {
-    const diagnostics = parseGGShieldResults(JSON.parse(results));
+    const diagnostics = parseGGShieldResults(
+      JSON.parse(scanResultsWithIncident)
+    );
     assert.strictEqual(diagnostics.length, 1);
     const diagnostic = diagnostics[0];
     assert.ok(diagnostic.message.includes("apikey"));
@@ -56,9 +24,21 @@ suite("parseGGShieldResults", () => {
     assert.strictEqual(diagnostic.severity, DiagnosticSeverity.Warning);
   });
 
+  test("Should return an empty array if there are no incidents", () => {
+    const diagnostics = parseGGShieldResults(JSON.parse(scanResultsNoIncident));
+    assert.strictEqual(diagnostics.length, 0);
+  });
+
   test("Should return an empty array on an invalid ggshield output", () => {
     const diagnostics = parseGGShieldResults(JSON.parse("{}"));
-
     assert.strictEqual(diagnostics.length, 0);
   });
+
+  test("Should only return the 'connection_uri' match if the secret is an URI", () => {
+    const diagnostics = parseGGShieldResults(
+      JSON.parse(scanResultsWithUriIncident)
+    );
+    assert.strictEqual(diagnostics.length, 1);
+    assert.ok(diagnostics[0].message.includes("connection_uri"));
+  });
 });
