Skip to content

v7.x Update - Add ability to append repo configurations, and user supplied configurations to the specified or default configuration#487

Merged
zricethezav merged 4 commits intogitleaks:masterfrom
StateFarmIns:merge-configs-v7
Mar 5, 2021
Merged

v7.x Update - Add ability to append repo configurations, and user supplied configurations to the specified or default configuration#487
zricethezav merged 4 commits intogitleaks:masterfrom
StateFarmIns:merge-configs-v7

Conversation

@eddie-northcutt-wfp0
Copy link
Contributor

@eddie-northcutt-wfp0 eddie-northcutt-wfp0 commented Dec 10, 2020 •
edited
Loading

Description:

Note: this is an update of #458 with changes that are compatible with v7.x version of Gitleaks.

Currently, users can either define a configuration, or define a repo config, but not both as one would be overwritten by the other. Now users can merge configurations together. For example, a repo can be ran against a master configuration, but say there is a one-off false positive in that repo. Instead of changing the master config, users can now include a repo config and merge it with the master config, allow listing the file in question. Additionally, users can define an additional config outside or inside of the context of a repo. This allows for merging up to three configurations at once.

New Options:

--append-repo-config: appends config stored in repo with the config located at --config-path , or the default config if one is not specified.
--additional-config: path to an additional config to be appended to the config located at --config-path , and/or the repo config , and/or the default config.

Checklist:

  • Does your PR pass tests?
  • Have you written new tests for your changes?
  • Have you lint your code locally prior to submission?

resolves: #429

@eddie-northcutt-wfp0 eddie-northcutt-wfp0 changed the title Merge configs v7 v7.x Update - Add ability to merge repo configurations, and user supplied configurations with specified or default configuration Dec 10, 2020
@eddie-northcutt-wfp0 eddie-northcutt-wfp0 mentioned this pull request Dec 10, 2020
Closed
3 tasks
@zricethezav zricethezav self-requested a review December 10, 2020 19:38
@zricethezav
Copy link
Collaborator

zricethezav commented Dec 11, 2020

@eddie-northcutt-wfp0 thanks for opening a new MR for v7.x.x. Hopefully the code was a little easier to understand.

I agree with the ability to merge in a repo-config, that seems useful. But I'm failing to see the need for --additional-config. Can you give me a specific use case where that would be needed? I'm hesitant to add 2 more config options as that would bring the total to 4.

I can see how --config-path=/path/to/main/config --repo-config-path=/repo/specific/config --merge-repo-config makes sense but not really seeing where --additional-config would be useful.

I'm also wondering if maybe --merge-repo-config should be renamed --append-repo-config as it doesn't actually resolve merge conflicts like name rule description collisions

zricethezav
zricethezav reviewed Dec 11, 2020
View reviewed changes
zricethezav
zricethezav reviewed Dec 11, 2020
View reviewed changes
zricethezav
zricethezav reviewed Dec 11, 2020
View reviewed changes
@eddie-northcutt-wfp0
Copy link
Contributor Author

eddie-northcutt-wfp0 commented Dec 11, 2020

@zricethezav Thanks for taking some time to review this! The --additional-config option was added to enable the option of applying an extra configuration on top of the default config, or the config supplied to --config-path. This also allows for combining config files outside of the context of a repo since it is read from the file system. This idea came from #429. If you think the cli complexity is too much of a trade off for this feature, I am not opposed to removing it as I understand where you are coming from. Let me know what you think.

As for the --append-repo-config suggestion, you make a good point. I will get that verbiage corrected, in addition to your other suggestions.

@eddie-northcutt-wfp0 eddie-northcutt-wfp0 changed the title v7.x Update - Add ability to merge repo configurations, and user supplied configurations with specified or default configuration v7.x Update - Add ability to append repo configurations, and user supplied configurations to the specified or default configuration Dec 11, 2020
@zricethezav zricethezav merged commit a10ae91 into gitleaks:master Mar 5, 2021
@zricethezav
Copy link
Collaborator

zricethezav commented Mar 5, 2021

@eddie-northcutt-wfp0 sorry for taking so long on this. Approved and merged, this will be shipped in the next release, v7.3.0

@ubergesundheit ubergesundheit mentioned this pull request Jul 27, 2021
Merged
alayne222 pushed a commit to alayne222/gitleaks that referenced this pull request May 28, 2025
@northcutted
v7.x Update - Add ability to append repo configurations, and user sup…
3af2a91 
…plied configurations to the specified or default configuration (gitleaks#487)

* Update merge configuration changes for  Gitleaks 7.x

* Change merge to append, and did some clean up
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@zricethezav zricethezav zricethezav approved these changes

Assignees

No one assigned

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

Allow custom config on top of the default config

3 participants

@eddie-northcutt-wfp0 @zricethezav @northcutted