Add gitleaks to default config or move it to a plugin

[kriswest]

Is your feature request related to a problem? Please describe.
The Git leaks configuration should either be added to the default config, or the gitleaks processor moved out to a plugin. If moved to a plugin, we need to think about where plugins are configured - it sporobably shouldn't be via the main configuration elements (such apis) as at present.

Original comment thread: #1243 (comment)

@jescalada

Should gitLeaks from the api be present with a default value instead? 🤔

@kriswest

I'm wondering that too. I don't see a default value for the configPath variable in the defaults in the code:

git-proxy/src/proxy/processors/push-action/gitleaks.ts

Lines 42 to 47 in 84d2563

	type ConfigOptions = {
	enabled: boolean;
	ignoreGitleaksAllow: boolean;
	noColor: boolean;
	configPath: string | undefined;
	};

and you can't set an undefined value in JSON so I guess it would be:

{
  "enabled": false,
  "ignoreGitleaksAllow": true,
  "noColor": false,
}

However, I also note the comment:

  // adding gitleaks into main git-proxy for now as default off
  // in the future will likely be moved to a plugin where it'll be default on

Are we going to move it to a plugin or keep it where it is? That should probably decide if and where it goes into the schema.

If it goes into a plugin, are thre thoughts on configuring plugins? plugins is just an array of strings at present - should we be creating something new or is there an existing pattern I missed @coopernetes?

Describe the solution you'd like
The default gitleaks settings appear in the default config file OR in a defined area of the configuration for plugins.

[06kellyjac]

I can try moving it to a plugin but IIRC the main blocker is the plugin can't specify where it wants to be in the chain, they all go after parsePush, but the .git dir isn't realized until after pullRemote.
As part of the work I can allow plugins to specify dependency steps & also add configuration for plugins. ATM for our plugins we've been putting values into "api" in the config.

E.g.

{
  "...": "stuff",
  "api": {
    "some-plugin": {
      "key1": "data",
      "key2": "data"
    },
    "other-plugin": {
      "key1": "data",
      "key2": "data"
    }
  },
  "....": "otherstuff"
}

[jescalada]

Relevant discussion here: #1149

Should we go ahead with the removal/refactoring of the plugin system, or perhaps deprecate CommonJS plugins as they're no longer compatible? @kriswest @06kellyjac

[06kellyjac]

I'd be against removing it with plugin use @ citi. We would either have to go back to forking or adjust peoples AD access somewhere else to maintain custom/additional policy/rules.
Having it ESM only is fine.