chore: add pre-commit hooks [MCP-284] #727
Conversation
There was a problem hiding this comment.
Pull Request Overview
This PR adds pre-commit hooks to the project using Husky, integrating git-secrets scanning and automated code quality checks into the development workflow.
Key Changes:
- Integrated Husky for Git hook management with a pre-commit hook that runs git-secrets, code formatting, linting, and unit tests
- Updated the npm prepare script to initialize Husky instead of running the build
- Added a clarifying comment about test certificates being self-signed
Reviewed Changes
Copilot reviewed 3 out of 4 changed files in this pull request and generated 2 comments.
| File | Description |
|---|---|
| package.json | Added Husky dependency and updated prepare script to initialize Husky |
| .husky/pre-commit | Created pre-commit hook script with git-secrets validation and automated checks |
| tests/integration/fixtures/httpsServerProxyTest.ts | Added security warning comment for self-signed certificates |
.husky/pre-commit
Outdated
| fi | ||
|
|
||
| git-secrets --register-aws > /dev/null | ||
| git-secrets --pre_commit_hook -- "$@" |
There was a problem hiding this comment.
The flag should be '--pre-commit-hook' (with hyphens) not '--pre_commit_hook' (with underscores). This is the correct flag name according to git-secrets documentation.
| git-secrets --pre_commit_hook -- "$@" | |
| git-secrets --pre-commit-hook -- "$@" |
.husky/pre-commit
Outdated
| npm run check | ||
| npm test -- --run tests/unit |
There was a problem hiding this comment.
Can we not do this - there's no reason to have this run as part of pre-commit.
There was a problem hiding this comment.
this is not the entire suite only the fast unit tests, I can remove but commits will still be red, I also moved it to pre-push, instead
There was a problem hiding this comment.
as discussed offline I've removed tests and left only git-secrets we can revisit it later
Pull Request Test Coverage Report for Build 19307245256Warning: This coverage report may be inaccurate.This pull request's base commit is no longer the HEAD commit of its target branch. This means it includes changes from outside the original pull request, including, potentially, unrelated coverage changes.
Details
💛 - Coveralls |
.github/workflows/code-health.yml
Outdated
| run: | | ||
| sudo apt-get install -y git-secrets | ||
| npm ci | ||
| - name: Run pre-commit and pre-push hooks |
There was a problem hiding this comment.
I understand the idea behind having those aligned but beyond git-secret, isn't the rest redundant and handled by other runs? We can turn this into a git-secrets check instead
|
|
||
| requests: IncomingMessage[]; | ||
|
|
||
| // note: these are self-signed certs for testing purposes, DO NOT use in production |
.github/workflows/code-health.yml
Outdated
| - name: Install dependencies | ||
| run: | | ||
| sudo apt-get install -y git-secrets | ||
| npm ci |
There was a problem hiding this comment.
Do we need to install npm packages? I don't expect we want to scan for secrets in node_modules anyway?
Proposed changes
Add pre-commit hook to run
husky, which runsgit-secrets.Checklist