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

chore: install secret detection on pre-commit hooks #3606

Open
wants to merge 1 commit into
base: main
Choose a base branch
from

Conversation

zondervancalvez
Copy link
Contributor

@zondervancalvez zondervancalvez commented Oct 29, 2024

Commit to be reviewed

chore: install secret detection on pre-commit hooks

Primary Changes
----------------
1. Installed gitleaks for secret detection.
2. This pre-commit checker detects any secrets
or crypto so that it doesn't get pushed to
the github repo.
3. Added script to run install and uninstall
the pre-commit hooks in package.json

Fixes #2290

Pull Request Requirements

  • Rebased onto upstream/main branch and squashed into single commit to help maintainers review it more efficient and to avoid spaghetti git commit graphs that obfuscate which commit did exactly what change, when and, why.
  • Have git sign off at the end of commit message to avoid being marked red. You can add -s flag when using git commit command. You may refer to this link for more information.
  • Follow the Commit Linting specification. You may refer to this link for more information.

Character Limit

  • Pull Request Title and Commit Subject must not exceed 72 characters (including spaces and special characters).
  • Commit Message per line must not exceed 80 characters (including spaces and special characters).

A Must Read for Beginners
For rebasing and squashing, here's a must read guide for beginners.

@zondervancalvez
Copy link
Contributor Author

image

Copy link
Contributor

@petermetz petermetz left a comment

Choose a reason for hiding this comment

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

@zondervancalvez A good start, but please add an npm script in the top level package.json that coldstarts the pre-commit hook. The idea is that on-boarding new contributors could run that script ones and have the commit hook enabled after it finished running.

Please also add another npm script that removes the pre-commit hook and cleans up after itself (files downloaded/created during the enabler script )

You could call the scripts tools:install-pre-commit-secret-detection and tools:uninstall-pre-commit-secret-detection

@petermetz
Copy link
Contributor

@zondervancalvez Are you still working on this?

@zondervancalvez
Copy link
Contributor Author

zondervancalvez commented Nov 12, 2024

@zondervancalvez Are you still working on this?

@petermetz Yes. I just prioritized the #3614

Primary Changes
----------------
1. Installed gitleaks for secret detection.
2. This pre-commit checker detects any secrets
or crypto so that it doesn't get pushed to
the github repo.
3. Added script to run install and uninstall
the pre-commit hooks in package.json

Fixes hyperledger-cacti#2290

Signed-off-by: bado <[email protected]>
@jagpreetsinghsasan
Copy link
Contributor

@hyperledger-cacti/cacti-maintainers requesting review on this PR

Comment on lines +36 to +37
"tools:install-pre-commit-secret-detection": "pre-commit install && pre-commit autoupdate",
"tools:uninstall-pre-commit-secret-detection": "pre-commit uninstall",
Copy link
Contributor

Choose a reason for hiding this comment

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

Could you also update our CONTRIBUTING.md and / or BUILD.md to recommend installing these pre-commits when setting up the environment?

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.

chore: install secret detection on pre-commit hooks
4 participants