feat(result_parser): Better parsing of URI secrets

salome-voltz · salome-voltz · commit 11ce4e8cc9ee · 2024-11-14T09:26:13.000+01:00
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
@@ -173,15 +173,13 @@ export async function scanFile(
   ]);
 
   if (proc.status === 128 || proc.status === 3) {
-    let errorMessage = "";
-    proc.stderr.split("\n").forEach((stderrLine) => {
-      if (
-        stderrLine.length > 0 &&
-        !stderrLine.includes("Scanning Path...") // ggshield outputs this info message on stderr, ignore it
-      ) {
-        errorMessage += stderrLine + "\n";
-      }
-    });
+    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;
@@ -203,11 +201,11 @@ export async function scanFile(
     updateStatusBarItem(StatusBarStatus.noSecretFound);
     return;
   } else {
-    const results = JSON.parse(proc.stdout);
-    let incidentsDiagnostics: Diagnostic[] = parseGGShieldResults(results);
     updateStatusBarItem(StatusBarStatus.secretFound);
-    diagnosticCollection.set(fileUri, incidentsDiagnostics);
   }
+  const results = JSON.parse(proc.stdout);
+  let incidentsDiagnostics: Diagnostic[] = parseGGShieldResults(results);
+  diagnosticCollection.set(fileUri, incidentsDiagnostics);
 }
 
 export async function loginGGShield(
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
@@ -78,7 +78,7 @@ suite("scanFile", () => {
       statusBar.StatusBarStatus.ignoredFile
     );
   });
-  
+
   const errorCodes = [128, 3];
   errorCodes.forEach((code) => {
     test(`displays an error message if the scan command fails with error code ${code}`, async () => {
@@ -88,11 +88,15 @@ suite("scanFile", () => {
         stderr: "Error",
       });
 
-      await scanFile("test.py", Uri.file("test.py"), {} as GGShieldConfiguration);
+      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");
+      assert.strictEqual(errorMessageMock.lastCall.args[0], "ggshield: Error");
     });
   });
 
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"));
+  });
 });
