-
Notifications
You must be signed in to change notification settings - Fork 77
feat(secrets scanning): support globs #5594
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
Conversation
This pull request adds or modifies JavaScript ( |
␊ | ||
Secret env var "ENV_VAR_2"'s value detected:␊ | ||
found value at line 1 in dist/static-files/notsafefile.js␊ | ||
found value at line 1 in src/static-files/notsafefile.js␊ |
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.
context: the tests omit our globbed path but not the file next to it by design.
const omitPathMatches = (relativePath, omitPaths) => { | ||
return omitPaths.some((oPath) => { | ||
// check if the substring matches or glob pattern | ||
return relativePath.startsWith(oPath) || minimatch(relativePath, oPath, { dot: true }) |
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.
the actual change is using minimatch for globs in addition to substring support
The change itself looks good. I'm wondering if there's a different matching library we could use that's already part of the dependencies, so we don't increase bundlesize unneccessarily. |
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.
In zip-it-and-ship-it, which is a dep of build, we're using fast-glob
for matching:
Could we also use that in here, to save on some bundle size?
@Skn0tt I used minimatch because it's used in the build-info package |
Oh, good point! I still think we should concentrate on one, but that's a separate issue then :D |
Agreed! |
🎉 Thanks for submitting a pull request! 🎉
Summary
Support for globs support for secrets scanning. Requested by enterprise team: https://netlify.slack.com/archives/C023PC3D08J/p1713347068392949
Fixes https://linear.app/netlify/issue/COM-665/glob-support-in-secrets-scan-omit-paths
For us to review and ship your PR efficiently, please perform the following steps:
we can discuss the changes and get feedback from everyone that should be involved. If you`re fixing a typo or
something that`s on fire 🔥 (e.g. incident related), you can skip this step.
your code follows our style guide and passes our tests.
A picture of a cute animal (not mandatory, but encouraged)