Skip to content
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

Added --name-only option #229

Open
wants to merge 2 commits into
base: master
Choose a base branch
from
Open

Conversation

pzarfostph
Copy link

Description of changes:

  • Added --name-only command line option
  • Added unit tests

The PR adds a --name-only option, to print only the file name containing the secret, not the secret value itself.

This is for useful for CICD processes, in the case that someone actually commits and pushes a credential that gets caught by the CICD script. You want the file to get flagged, but you don't want the actual credential echoed into the CICD log files.

This is a potential solution for Issue #187

By submitting this pull request, I confirm that my contribution is made under the terms of the Apache 2.0 license.

repo_run git-secrets --scan --name-only
[ $status -eq 1 ]
}

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This or another test should confirm that the output doesn't include the secret

@sparr
Copy link
Contributor

sparr commented Jun 15, 2023

This PR doesn't seem to actually include the new functionality. It adds the option, but doesn't use the option. Is there a commit missing?

@@ -13,7 +13,7 @@
# permissions and limitations under the License.

NONGIT_OK=1 OPTIONS_SPEC="\
git secrets --scan [-r|--recursive] [--cached] [--no-index] [--untracked] [<files>...]
git secrets --scan [-r|--recursive] [--cached] [--name-only] [--no-index] [--untracked] [<files>...]
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This should also be mentioned at README.rst#synopsis

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

And an update to README.rst requires make man.

@@ -13,7 +13,7 @@
# permissions and limitations under the License.

NONGIT_OK=1 OPTIONS_SPEC="\
git secrets --scan [-r|--recursive] [--cached] [--no-index] [--untracked] [<files>...]
git secrets --scan [-r|--recursive] [--cached] [--name-only] [--no-index] [--untracked] [<files>...]
git secrets --scan-history
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Should this also apply to --scan-history?

@sparr
Copy link
Contributor

sparr commented Jun 20, 2023

If you rebase this on master you can skip the say check and then tests will run. Or if you give maintainers permission to push to your branch then I can do it.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants