Add automated secret scanning (make check-secrets, pre-commit hook) #67

2026-07-06T07:54:23+01:00

danny commented

2026-07-06 07:54:23 +01:00

(Migrated from gitlab.keyop.co.uk)

Part of #24 (https://gitlab.keyop.co.uk/keyop/go/nfq_forwarder/-/work_items/24).

Background

The bulk of issue #24 (redacting GitLab tokens accidentally committed into notes/ chat
exports) was handled directly against main via a full git filter-repo --replace-text history
rewrite - not something that can go through a normal branch/MR, since it rewrites main's own
history. That part is done: every commit SHA changed, all tags were re-pushed pointing at the
rewritten commits, and a follow-up gitleaks sweep of the rewritten history confirms zero
remaining secrets.

This MR is the preventative half of the ticket (expanded per Danny's request beyond the
originally-scoped manual redaction script): automated scanning so this can't recur silently.

Changes

make check-secrets: runs gitleaks git --log-opts="--all" (full-history sweep) and is
now part of make lint, so both local runs and CI's existing lint job catch a committed
secret with no .gitlab-ci.yml changes needed.
make install-git-hooks: installs scripts/pre-commit (gitleaks protect --staged - a
fast, staged-diff-only scan) as an opt-in pre-commit hook, catching a secret before it's ever
committed rather than only before it's merged. Not installed automatically, since git doesn't
version .git/hooks - this is a one-time manual step per clone.
make setup installs gitleaks itself if missing, matching the existing pattern for
goreleaser.
CLAUDE.md updated (Build & dev commands, Design notes, Known issues - including a note about
the history rewrite itself, for future readers).

Testing

Manually verified the hook actually blocks a commit: staged a file containing a fake token
shaped like a real GitLab PAT, confirmed git commit was rejected with the expected finding,
then removed it.
This very commit went through the installed hook successfully (see the gitleaks output in the
commit process).
make lint (now including check-secrets) is clean.

Part of #24 (https://gitlab.keyop.co.uk/keyop/go/nfq_forwarder/-/work_items/24). ## Background The bulk of issue #24 (redacting GitLab tokens accidentally committed into `notes/` chat exports) was handled directly against `main` via a full `git filter-repo --replace-text` history rewrite - not something that can go through a normal branch/MR, since it rewrites `main`'s own history. That part is done: every commit SHA changed, all tags were re-pushed pointing at the rewritten commits, and a follow-up `gitleaks` sweep of the rewritten history confirms zero remaining secrets. This MR is the preventative half of the ticket (expanded per Danny's request beyond the originally-scoped manual redaction script): automated scanning so this can't recur silently. ## Changes - **`make check-secrets`**: runs `gitleaks git --log-opts="--all"` (full-history sweep) and is now part of `make lint`, so both local runs and CI's existing `lint` job catch a committed secret with no `.gitlab-ci.yml` changes needed. - **`make install-git-hooks`**: installs `scripts/pre-commit` (`gitleaks protect --staged` - a fast, staged-diff-only scan) as an opt-in pre-commit hook, catching a secret *before* it's ever committed rather than only before it's merged. Not installed automatically, since git doesn't version `.git/hooks` - this is a one-time manual step per clone. - `make setup` installs `gitleaks` itself if missing, matching the existing pattern for `goreleaser`. - `CLAUDE.md` updated (Build & dev commands, Design notes, Known issues - including a note about the history rewrite itself, for future readers). ## Testing - Manually verified the hook actually blocks a commit: staged a file containing a fake token shaped like a real GitLab PAT, confirmed `git commit` was rejected with the expected finding, then removed it. - This very commit went through the installed hook successfully (see the gitleaks output in the commit process). - `make lint` (now including `check-secrets`) is clean.

danny (Migrated from gitlab.keyop.co.uk) approved these changes

2026-07-06 07:54:23 +01:00

danny commented

2026-07-06 07:56:13 +01:00

(Migrated from gitlab.keyop.co.uk)

mentioned in issue #24

danny commented

2026-07-06 08:06:49 +01:00

(Migrated from gitlab.keyop.co.uk)

assigned to @danny

danny commented

2026-07-06 08:06:49 +01:00

(Migrated from gitlab.keyop.co.uk)

requested review from @danny

danny commented

2026-07-06 08:06:56 +01:00

(Migrated from gitlab.keyop.co.uk)

LGTM

danny commented

2026-07-06 08:06:56 +01:00

(Migrated from gitlab.keyop.co.uk)

approved this merge request

danny (Migrated from gitlab.keyop.co.uk) scheduled this pull request to auto merge when all checks succeed

2026-07-06 08:11:18 +01:00

danny commented

2026-07-06 08:17:24 +01:00

(Migrated from gitlab.keyop.co.uk)

aborted the automatic merge because the source branch was updated. Learn more.

aborted the automatic merge because the source branch was updated. [Learn more](https://gitlab.keyop.co.uk/help/ci/pipelines/merge_trains.md#merge-request-dropped-from-the-merge-train).

danny commented

2026-07-06 08:17:25 +01:00

(Migrated from gitlab.keyop.co.uk)

added 1 commit

4a3a9b06 - Scope check-secrets to HEAD's ancestry, not every local ref

Compare with previous version

added 1 commit <ul><li>4a3a9b06 - Scope check-secrets to HEAD's ancestry, not every local ref</li></ul> [Compare with previous version](/keyop/go/nfq_forwarder/-/merge_requests/26/diffs?diff_id=288&start_sha=dec19a61194ca27262fe103182121fbd5f915290)

danny commented

2026-07-06 08:17:44 +01:00

(Migrated from gitlab.keyop.co.uk)

The lint pipeline failure just now was a real (if slightly indirect) finding, not a bad rebase: check-secrets used gitleaks git --log-opts="--all", which follows every local ref in whatever checked out the repo - and the CI runner's reused workspace still had local refs/objects from before the issue #24 history rewrite, even though the actual GitLab remote (confirmed via the API - only main and this branch exist) has already moved past them. So it was flagging already-redacted history via a stale local cache, not a real leak.

Fixed by scoping the scan to HEAD's own ancestry instead of every local ref - every commit that matters ends up in main's history via a merged MR anyway, so this loses no real coverage. Verified clean locally; pushed as a follow-up commit.

The lint pipeline failure just now was a real (if slightly indirect) finding, not a bad rebase: `check-secrets` used `gitleaks git --log-opts="--all"`, which follows *every local ref* in whatever checked out the repo - and the CI runner's reused workspace still had local refs/objects from before the issue #24 history rewrite, even though the actual GitLab remote (confirmed via the API - only `main` and this branch exist) has already moved past them. So it was flagging already-redacted history via a stale local cache, not a real leak. Fixed by scoping the scan to `HEAD`'s own ancestry instead of every local ref - every commit that matters ends up in `main`'s history via a merged MR anyway, so this loses no real coverage. Verified clean locally; pushed as a follow-up commit.

danny commented

2026-07-06 08:27:29 +01:00

(Migrated from gitlab.keyop.co.uk)

added 1 commit

7b19ec3b - Actually scope check-secrets to HEAD (previous fix was a no-op)

Compare with previous version

added 1 commit <ul><li>7b19ec3b - Actually scope check-secrets to HEAD (previous fix was a no-op)</li></ul> [Compare with previous version](/keyop/go/nfq_forwarder/-/merge_requests/26/diffs?diff_id=290&start_sha=4a3a9b06ffbaa92f60f745627f1113ef5faea3e5)

danny commented

2026-07-06 08:27:43 +01:00

(Migrated from gitlab.keyop.co.uk)

Correction to my last comment: dropping the explicit --log-opts="--all" didn't actually change anything, and the pipeline failed identically for that reason - gitleaks' own default when --log-opts is empty is git log ... --full-history --all, i.e. still every local ref. An empty override falls back to that same default rather than meaning "no extra scope."

Fixed properly this time with an explicit --log-opts="HEAD". Verified by creating a local ref pointing at one of the old pre-rewrite commits and confirming --log-opts="HEAD" correctly ignores it while the bare default reproduces the exact CI failure - so this is the real fix, not another guess. HEAD's ancestry is a property of the commit graph itself (content-addressed), independent of whatever stale refs a given checkout happens to be carrying, which I also confirmed directly: git merge-base --is-ancestor <old-sha> HEAD returns false, i.e. that commit was never reachable from HEAD in the first place.

Correction to my last comment: dropping the explicit `--log-opts="--all"` didn't actually change anything, and the pipeline failed identically for that reason - **gitleaks' own default when `--log-opts` is empty is `git log ... --full-history --all`**, i.e. still every local ref. An empty override falls back to that same default rather than meaning "no extra scope." Fixed properly this time with an explicit `--log-opts="HEAD"`. Verified by creating a local ref pointing at one of the old pre-rewrite commits and confirming `--log-opts="HEAD"` correctly ignores it while the bare default reproduces the exact CI failure - so this is the real fix, not another guess. HEAD's ancestry is a property of the commit graph itself (content-addressed), independent of whatever stale refs a given checkout happens to be carrying, which I also confirmed directly: `git merge-base --is-ancestor <old-sha> HEAD` returns false, i.e. that commit was never reachable from HEAD in the first place.

danny commented

2026-07-06 08:42:01 +01:00

(Migrated from gitlab.keyop.co.uk)

mentioned in commit cdeebc121a

mentioned in commit cdeebc121a1bd3b51c1609a72f1da0b140af6dd1