git_commit_check.py over-blocks: scans staged file content (not diff); `console.log(...token)` pattern false-positives on the word 'token' in log labels

[michael-wojcik]

Summary

hooks/git_commit_check.py has two coupled issues that together block legitimate commits, including the very commits that remove an offending line.

Issue 1 — scans full staged file content, not the diff

get_staged_file_content (git show :<file>, lines ~82-86) returns the entire staged content of each staged file, and check_security runs re.search(pattern, content) (line ~129-130) against that full content. The hook does not restrict scanning to added lines (^+ from git diff --cached).

Consequence: a pre-existing line matching a secret pattern blocks any commit that stages that file — even a commit whose only change is removing the offending line. Removing the line requires staging the removal in a separate git add (which doesn't trigger the hook, since it only fires on git commit per lines ~367-370) and only then committing. This catch-22 is non-obvious and surfaced as a real blocker during a dependency migration.

Issue 2 — console\.log\s*\(.*token (line ~120) is over-broad

The pattern matches the word "token" anywhere after console.log(, including inside log message labels. Real-world false positive:

console.log("User groups from ID token:", userGroups);

This logs userGroups (a cognito:groups claim) — not a token. The regex matched the substring "token" in the label "User groups from ID token:". The line was pre-existing (not introduced by the migration).

Reproduction

1. Have a tracked file containing console.log("X from ID token:", value) (pre-existing).
2. Stage any change to that file (git add <file>).
3. git commit → hook blocks with "SECURITY: Potential secret exposure in log ... matches pattern 'console.log\s*\(.*token'".
4. Even staging the removal of that line blocks (the staged content still contains it until the removal is staged).

Suggested fixes

• Scan added lines only: derive scan input from git diff --cached --unified=0 (filter ^+), not git show :<file>. This makes the hook additive-only — removing an offending line no longer triggers, and pre-existing content doesn't block unrelated commits to the same file.
• Tighten the token pattern: match actual token values/variables rather than the word "token" in string literals, e.g. console\.log\s*\([^)]*\b(tokenSet|access_token|id_token|refresh_token|authorization|bearer)\b and/or a secret-shape regex (eyJ[a-zA-Z0-9_-]+\.eyJ...). The label-word match is the main false-positive source.
• Optional: an allowlist/escape-hatch comment (e.g. // pact-allow: non-secret log) for confirmed false positives, auditable via grep.

Context

Encountered during a webapp-frontend openid-client v5→v6 migration (PACT orchestrate). The blocked commit was a 1-line removal of a pre-existing debug console.log (recommended by the security-engineer + user-confirmed). Workaround was: git restore --staged . && git add <clean files> (no git commit in the command → hook doesn't fire), then the commit bash proceeded cleanly.

The fail-closed posture + .env/--diff-filter=d design is good; this is narrowly about the content-vs-diff scan scope and the label-word false positive.