Secrets in Git, PR size and a review checklist Intermediate

This page answers: what to do, in what order, after committing a secret by accident; how to prevent it; and how small a PR should be to get a real review.

Never commit secrets

.env, private keys, service account JSON, tokens in test files — once pushed, consider it leaked.

If it leaked: rotate first, rewrite later

Loading diagram…

1. Revoke/rotate the secret immediately (change the DB password, revoke the API key, create a new key). GitHub docs: once rotated, the old secret is useless, and sometimes you no longer need to rewrite history at all.
2. Check the access logs of the exposed system for the period the secret was public.
3. If you still need to remove it from history (e.g. personal data that cannot be rotated): use git filter-repo as the GitHub docs describe.

# needs git-filter-repo ≥ 2.47 for --sensitive-data-removal
git filter-repo --sensitive-data-removal --invert-paths --path config/prod.env
# or replace strings across the whole history
git filter-repo --sensitive-data-removal --replace-text ../passwords.txt

Why a rewrite is not enough (per the GitHub docs):

• It cannot remove the data from other people’s clones and forks.
• Old commits remain accessible by SHA in cached views on GitHub, and through pull requests that reference them (refs/pull/* is read-only and needs GitHub Support).
• A teammate who pulls and then pushes from an old clone, or a merge commit, can bring the data back.
• Every hash from that point changes; commit signatures are lost; open PRs get scrambled.

Attackers scan public repos continuously; the time from push to exploitation can be very short. The order is always: rotate → then clean up history.

Prevention

Layer	What to do	Note
.gitignore	Ignore .env*, *.pem, node_modules/, dist/, coverage/	Files already tracked are unaffected — you must git rm --cached <file> first
Personal ignores	Editor and OS patterns go in core.excludesFile (default ~/.config/git/ignore)	Do not clutter the project’s .gitignore
.env.example	Commit a sample file with empty keys instead of the real .env
Pre-commit hook	Secret scanner (e.g. gitleaks), lint, format on staged files	.git/hooks is not versioned; use Husky/lefthook or core.hooksPath to share hooks
CI	Rerun the same checks, plus server-side secret scanning if the platform supports it	Local hooks can be bypassed with --no-verify

Signed commits (briefly)

Anyone can set user.email to your email. A signature proves the commit really came from your key. GitHub supports GPG, SSH and S/MIME; the simplest is to sign with an SSH key you already have:

git config --global gpg.format ssh
git config --global user.signingkey ~/.ssh/id_ed25519.pub
git config --global commit.gpgSign true

• Register that key as a “signing key” on GitHub, then enable the “require signed commits” branch protection if the team needs it.
• Note: “Rebase and merge” on GitHub creates new commits that GitHub cannot sign on your behalf.

PRs: size and how to review

The view of someone who reviews dozens of PRs every week:

Rule	Why
Small PRs: aim for under ~400 lines of real change (excluding lockfiles and generated code)	Can be reviewed carefully in one sitting; a 2,000-line PR gets “LGTM” because the reviewer gives up
One PR, one purpose	Easy to revert or cherry-pick into a hotfix
Split refactor/rename PRs out first	The feature PR that follows only contains real logic
PR description: problem, solution, how it was tested, risks, rollback	The reviewer does not have to guess; on-call rereads it during an incident
Multi-step DB/API changes: split PRs following expand-contract	Each PR is independently deployable
Review your own diff before requesting review	Catches debug logs, stray files, secrets

The 400-line figure is a rule of thumb, not a standard. What matters is that the PR is small enough for the reviewer to understand all of it.

When reviewing:

• Mark severity clearly: blocking (bugs, security, data loss) vs nit: (style, naming). Do not block a PR over nits.
• Ask instead of command: “What happens if items is empty?” works better than “This is wrong”.
• Comment on the code, not the author. Give praise where a good approach deserves it.
• Leave style and formatting to tools (Prettier, ESLint, CI); humans review design, correctness and operability.
• Respond within one working day. A PR left waiting 3 days grows and conflicts.

More on reviewing: the Code review cluster.

Squash merge and a hotfix that could not be cherry-picked

A team of 8, a NestJS payment service, GitHub flow with squash merge. One 1,900-line PR contained: a module rename, a Prisma upgrade, and a new partial refund flow. After the merge, the refund flow computed fees wrongly for multi-currency transactions.

Rolling back just the refund feature: impossible. The squash had combined the rename and the Prisma upgrade into one commit, and reverting it touched migrations. The team had to write a forward hotfix under pressure.

After the incident, they changed the rules:

1. Renames/dependency upgrades are always separate PRs, merged first.
2. New features sit behind a feature flag, so “rollback” just means turning off the flag.
3. A PR over ~400 lines of logic must justify it in the description.

When to apply / exceptions

Rule	Reasonable exception
Do not rebase shared branches	The whole team agrees and everyone resets together (e.g. to remove a secret)
Squash merge	The PR has deliberate atomic commits (refactor → feat) you want to keep: use rebase-merge or a merge commit
Trunk-based	A product that must maintain several versions in parallel: long-lived release branches or GitFlow
Conventional Commits	Personal repos, throwaway prototypes. For team repos and published libraries, use them — especially when generating changelogs automatically
Revert instead of reset on main	No exceptions on shared branches

Code review checklist for Git

1. Each commit is atomic, builds and passes tests; refactors are separate from behavior changes.
2. Messages follow Conventional Commits; subject ≤ ~50 characters; the body says why; breaking changes have ! or BREAKING CHANGE:.
3. Branches live no longer than 1-2 days; long work uses feature flags.
4. No stray wip/fix review commits: already --fixup + rebase -i --autosquashed.
5. Force push only on your own branch, always with --force-with-lease; branch protection blocks force pushes to main.
6. No secrets, .env or build files in the diff; pre-commit hooks and CI both scan for secrets.
7. Small PRs with one purpose, whose description covers how to test and how to roll back.
8. Incidents on main are handled with revert, not reset + force push.

Next: Common mistakes.