-
Notifications
You must be signed in to change notification settings - Fork 18
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Feat: exclude flag #199
Feat: exclude flag #199
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Hi! Many thanks!!
Sorry for the delay, I'm on reserve duty.
Thank you for joining to our project.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Hi! Many thanks!!
Sorry for the delay, I'm on reserve duty.
Thank you for joining to our project.
still WIP, I have to add actual support on glob and comma-separated list and fix the "totalitemsscanned" bug |
Converted to a draft PR. |
…d support in 'Glob' pattern
@baruchiro Hey, I made the changes you suggested |
I will be back next week, sorry. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thank you for your work. This feature is more complicated than we thought.
I played with your implementation, and first, I didn't manage to make it work, I tried these options:
$ go run . filesystem --path . --exclude-paths secrets
$ go run . filesystem --path . --exclude-paths ./secrets
$ go run . filesystem --path . --exclude-paths ./secrets/*
$ go run . filesystem --path . --exclude-paths ./secrets/**/*
The main issue here, I think, is that you're testing the filepath.Glob(filename)
on the current working directory, which is not always the scanning directory.
If you think about that more, you will understand that every plugin will consider the "working directory" differently, or even will not use a directory at all (such as the Confluence plugin, which is accessing through API).
It is what I mentioned here. I think you need to start with implementing it only for the filesystem
and the git
plugins, where we know for sure this filter needs to work.
@@ -89,7 +92,7 @@ func Execute() (int, error) { | |||
rootCmd.PersistentFlags().StringSliceVar(&secretsConfigVar.SpecialList, specialRulesFlagName, []string{}, "special (non-default) rules to apply.\nThis list is not affected by the --rule and --ignore-rule flags.") | |||
rootCmd.PersistentFlags().Var(&ignoreOnExitVar, ignoreOnExitFlagName, "defines which kind of non-zero exits code should be ignored\naccepts: all, results, errors, none\nexample: if 'results' is set, only engine errors will make 2ms exit code different from 0") | |||
rootCmd.PersistentFlags().IntVar(&secretsConfigVar.MaxTargetMegabytes, maxTargetMegabytesFlagName, 0, "files larger than this will be skipped.\nOmit or set to 0 to disable this check.") | |||
|
|||
rootCmd.PersistentFlags().StringSliceVar(&excludedPathsVar, excludedPathsFlagName, []string{}, "exclude paths from scan. supports glob and can be provided multiple times or as a quoted comma separated string example: './shouldNotScan/*,somefile.txt'") | |||
rootCmd.AddCommand(secrets.GetRulesCommand(&secretsConfigVar)) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
rootCmd.AddCommand(secrets.GetRulesCommand(&secretsConfigVar)) | |
rootCmd.PersistentFlags().StringSliceVar(&excludedPathsVar, excludedPathsFlagName, []string{}, "exclude paths from scan.\nsupports glob and can be provided multiple times or as a quoted comma separated string\nexample: './shouldNotScan/*,somefile.txt'") |
Make it multiline
func resolveGlobExpression(excludedFileNames []string) map[string]bool { | ||
var matchedFileNames = make(map[string]bool) | ||
for _, filename := range excludedFileNames { | ||
matched, err := filepath.Glob(filename) | ||
if err != nil { | ||
log.Error().Msgf("failed to get exclude path %s: %s", filename, err) | ||
} else { | ||
for _, value := range matched { | ||
matchedFileNames[value] = true | ||
} | ||
} | ||
} | ||
return matchedFileNames | ||
} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Please move it to a new file called glob.go
or something under the lib
folder.
Hi @Tzurrr, as we discussed on #194, this feature should be implemented for each relevant plugin. It will not be a global argument. It is already implemented for the Let me know what you think, thank you very much for your work, and sorry about the changes. You can contact me at [email protected] or on Discord. |
Closing this, thank you @Tzurrr. I'm inviting you to continue the work in a new PR, and implement the filter for the |
Closes #194
Proposed Changes
I've added the option to use "--exclude" flag. this flag allows the user to write file/folder name to exclude.
I was debating whether to do seperated flags for folders and files, but it seems to me like too much duplicated code.
Because I didnt seperated it - now if a user have a file within a folder that named the same and the he wants to exclude only the file - the whole folder will be excluded. I would like to hear your opinion about the case I mentioned above
I submit this contribution under the Apache-2.0 license.