Merge pull request #101 from GitGuardian/agateau/insecure

Add insecure option, deprecate allowSelfSigned

Author: 6d7a

CHANGELOG.md

@@ -1,5 +1,19 @@
 # GitGuardian Secret Security Changelog
 
+## [0.16.0]
+
+### Added
+
+- Added `gitguardian.insecure` option to replace the ambiguous `gitguardian.allowSelfSigned` one.
+
+### Changed
+
+- Updated to [ggshield 1.44.1](https://github.com/GitGuardian/ggshield/releases/v1.44.1).
+
+### Deprecated
+
+- Marked `gitguardian.allowSelfSigned` as deprecated.
+
 ## [0.15.0]
 
 ### Changed

package.json

@@ -34,12 +34,21 @@
         "gitguardian.apiUrl": {
           "type": "string",
           "default": "",
-          "markdownDescription": "You can override the value here for On Premise installations"
+          "markdownDescription": "You can override the value here for On Premise installations",
+          "order": 1
+        },
+        "gitguardian.insecure": {
+          "type": "boolean",
+          "default": false,
+          "markdownDescription": "Skip all certificate verification checks.\n\nWARNING: this option makes the transfer insecure.",
+          "order": 2
         },
         "gitguardian.allowSelfSigned": {
           "type": "boolean",
           "default": false,
-          "markdownDescription": "Allow Self Signed Certificates"
+          "markdownDescription": "Allow Self Signed Certificates",
+          "markdownDeprecationMessage": "Deprecated: Please use `#gitguardian.insecure#` instead.",
+          "order": 100
         }
       }
     },

src/lib/ggshield-configuration-utils.ts

@@ -13,21 +13,20 @@ export async function getConfiguration(
 ): Promise<GGShieldConfiguration> {
   const config = workspace.getConfiguration("gitguardian");
 
-  const ggshieldPath: string | undefined = config.get("GGShieldPath");
   const apiUrl: string | undefined = config.get("apiUrl");
-  const allowSelfSigned: boolean = config.get("allowSelfSigned", false);
+  const insecure: boolean = config.get(
+    "insecure",
+    // Read allowSelfSigned for backward compatibility
+    config.get("allowSelfSigned", false),
+  );
 
   const pathToGGShield: string = await getGGShield(
     os.platform(),
     os.arch(),
     context,
     outputChannel,
-    allowSelfSigned,
+    insecure,
   );
 
-  return new GGShieldConfiguration(
-    pathToGGShield,
-    apiUrl,
-    allowSelfSigned || false,
-  );
+  return new GGShieldConfiguration(pathToGGShield, apiUrl, insecure || false);
 }

src/lib/ggshield-configuration.ts

@@ -1,15 +1,15 @@
 export class GGShieldConfiguration {
   ggshieldPath: string;
   apiUrl: string;
-  allowSelfSigned: boolean;
+  insecure: boolean;
 
   constructor(
     ggshieldPath: string = "",
     apiUrl: string = "",
-    allowSelfSigned: boolean = false,
+    insecure: boolean = false,
   ) {
     this.ggshieldPath = ggshieldPath;
     this.apiUrl = apiUrl;
-    this.allowSelfSigned = allowSelfSigned;
+    this.insecure = insecure;
   }
 }

src/lib/ggshield-resolver-utils.ts

@@ -36,7 +36,7 @@ export async function getGGShield(
   arch: string,
   context: ExtensionContext,
   outputChannel: OutputChannel,
-  allowSelfSigned: boolean,
+  insecure: boolean,
 ): Promise<string> {
   const version = getGGShieldVersion(context);
   console.log(`Latest GGShield version: ${version}`);
@@ -65,13 +65,7 @@ export async function getGGShield(
   }
   fs.mkdirSync(ggshieldFolder);
   // install GGShield
-  await installGGShield(
-    platform,
-    arch,
-    ggshieldFolder,
-    version,
-    allowSelfSigned,
-  );
+  await installGGShield(platform, arch, ggshieldFolder, version, insecure);
   outputChannel.appendLine(
     `Updated to GGShield v${version}. Checkout https://github.com/GitGuardian/ggshield for more info.`,
   );
@@ -138,7 +132,7 @@ export async function installGGShield(
   arch: string,
   ggshieldFolder: string,
   version: string,
-  allowSelfSigned: boolean,
+  insecure: boolean,
 ): Promise<void> {
   let extension: string = "";
   switch (platform) {
@@ -163,7 +157,7 @@ export async function installGGShield(
     fileName,
     downloadUrl,
     ggshieldFolder,
-    allowSelfSigned,
+    insecure,
   );
   extractGGShieldBinary(path.join(ggshieldFolder, fileName), ggshieldFolder);
 }
@@ -201,11 +195,11 @@ async function downloadGGShieldFromGitHub(
   fileName: string,
   downloadUrl: string,
   ggshieldFolder: string,
-  allowSelfSigned: boolean,
+  insecure: boolean,
 ): Promise<void> {
   console.log(`Downloading GGShield from ${downloadUrl}`);
 
-  const instance = allowSelfSigned
+  const instance = insecure
     ? new Agent({
         rejectUnauthorized: false,
       })

src/lib/run-ggshield.ts

@@ -37,9 +37,8 @@ export function runGGShieldCommand(
   if (workspace.workspaceFolders?.length || 0 > 0) {
     options["cwd"] = workspace.workspaceFolders![0].uri.fsPath;
   }
-  // if allowSelfSigned is enabled, add the --allow-self-signed flag
-  if (configuration.allowSelfSigned) {
-    args = ["--allow-self-signed"].concat(args);
+  if (configuration.insecure) {
+    args = ["--insecure"].concat(args);
   }
 
   if (configuration.apiUrl && !args.includes("--version")) {

src/test/suite/lib/ggshield-configuration-utils.test.ts

@@ -27,21 +27,35 @@ suite("getConfiguration", () => {
     simple.restore();
   });
 
+  /**
+   * Helper class to fake different configurations of the extension
+   */
+  class FakeConfiguration {
+    records: Record<string, any>;
+
+    constructor(records: Record<string, any>) {
+      this.records = records;
+    }
+
+    public get(section: string, defaultValue: any): any {
+      if (this.records.hasOwnProperty(section)) {
+        return this.records[section];
+      }
+      return defaultValue;
+    }
+  }
+
   test("Vscode settings are correctly read", async () => {
     const context = {} as ExtensionContext;
     const outputChannel = window.createOutputChannel("GitGuardian");
     simple.mock(context, "asAbsolutePath").returnWith("");
 
-    getConfigurationMock.returnWith({
-      get: (key: string) => {
-        if (key === "apiUrl") {
-          return "https://custom-url.com";
-        }
-        if (key === "allowSelfSigned") {
-          return true;
-        }
-      },
-    });
+    getConfigurationMock.returnWith(
+      new FakeConfiguration({
+        apiUrl: "https://custom-url.com",
+        insecure: true,
+      } as Record<string, any>),
+    );
     const configuration = await getConfiguration(context, outputChannel);
 
     // Assert both workspace.getConfiguration and GGShieldConfiguration constructor were called
@@ -52,6 +66,21 @@ suite("getConfiguration", () => {
 
     // Assert that the configuration has the expected values
     assert.strictEqual(configuration.apiUrl, "https://custom-url.com");
-    assert.strictEqual(configuration.allowSelfSigned, true);
+    assert.strictEqual(configuration.insecure, true);
+  });
+  test("insecure falls back on allowSelfSigned", async () => {
+    const context = {} as ExtensionContext;
+    const outputChannel = window.createOutputChannel("GitGuardian");
+    simple.mock(context, "asAbsolutePath").returnWith("");
+
+    getConfigurationMock.returnWith(
+      new FakeConfiguration({
+        allowSelfSigned: true,
+      } as Record<string, any>),
+    );
+    const configuration = await getConfiguration(context, outputChannel);
+
+    // Assert that the configuration has the expected values
+    assert.strictEqual(configuration.insecure, true);
   });
 });

src/test/suite/lib/run-ggshield.test.ts

@@ -36,28 +36,28 @@ suite("runGGShieldCommand", () => {
     delete process.env.TEST_GLOBAL_VAR;
   });
 
-  const testCasesAllowSelfSigned = [
+  const testCasesInsecure = [
     {
-      allowSelfSigned: true,
+      insecure: true,
       description:
-        "GGshield is called with flag --allow-self-signed when allowSelfSigned is true",
+        "GGshield is called with flag --insecure when insecure is true",
     },
     {
-      allowSelfSigned: false,
+      insecure: false,
       description:
-        "GGshield is not called with flag --allow-self-signed when allowSelfSigned is false",
+        "GGshield is not called with flag --insecure when insecure is false",
     },
   ];
 
-  testCasesAllowSelfSigned.forEach(({ allowSelfSigned, description }) => {
+  testCasesInsecure.forEach(({ insecure: insecure, description }) => {
     test(description, () => {
       process.env.TEST_GLOBAL_VAR = "GlobalValue";
 
       runGGShield.runGGShieldCommand(
         {
           ggshieldPath: "path/to/ggshield",
           apiUrl: "",
-          allowSelfSigned: allowSelfSigned,
+          insecure: insecure,
         } as GGShieldConfiguration,
         ["test"],
       );
@@ -67,7 +67,7 @@ suite("runGGShieldCommand", () => {
       const spawnSyncArgs = spawnSyncMock.lastCall.args;
       const args = spawnSyncArgs[1];
 
-      assert.strictEqual(args[0] === "--allow-self-signed", allowSelfSigned);
+      assert.strictEqual(args[0] === "--insecure", insecure);
     });
   });