Repository navigation
fix: check for git directly instead of relying on CI=true - #2377
Merged
Merged
Conversation
CI=true isn't a safe assumption across CI systems - confirmed on bcgov/bcregistry#282 that Google Cloud Build doesn't set it at all, so a CI=true-based guard would silently keep failing there despite looking correct. Checking for git's actual presence instead doesn't depend on any CI provider's conventions.
Contributor
Author
|
/gcbrun |
pnpm/npm run lifecycle scripts through cmd.exe on Windows by default, where `command -v` and `/dev/null` do not exist. The previous guard errored and fell through to `|| true`, exiting 0 without installing the hook, so Windows developers silently got no secret scanning. `git --version && lefthook install || exit 0` has the same semantics (skip entirely when git is absent, never fail the install) and works in both cmd.exe and POSIX sh.
doug-lovett
approved these changes
Sep 28, 2026
This was referenced Sep 29, 2026
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Improves on #2364's guard, per feedback from Thor and Patrick that lefthook shouldn't be attempting to run in CI/CD at all, not just fail-safe.
What changed
Why not
CI=truelefthook's own npm postinstall already skips itself under
CI=trueby default. First tried makingpreparedo the same, but confirmed on bcgov/bcregistry#282 - via a real CD run, not assumption - that Google Cloud Build doesn't setCI=trueat all, so that check would have silently stayed broken there. Checking forgit's actual presence avoids depending on any CI provider's conventions and matches the actual failure mode.Note: this repo's CD/CI (
ppr-ui-cd.yml,ppr-ui-ci.yml) is path-scoped toppr-ui/**, so a root-only change like this one doesn't trigger either and can't be proven through a real pipeline run the way #282 was. Verified locally instead with realgit commit: hook installs when git is present, blocks a fake secret whether it's at repo root or nested under a subfolder.Update: Windows fix
The original
command -v git >/dev/null 2>&1 && lefthook install || trueguard silently did nothing on Windows. pnpm/npm run lifecycle scripts throughcmd.exethere by default,command -vand/dev/nulldon't exist, and the failure fell through to|| true, so the install exited 0 but no hook was installed. Windows devs would have had no secret scanning and no error telling them so.Replaced with:
Same semantics (skip entirely when git is absent, never fail the install), works in both
cmd.exeand POSIXsh. Verified all four combinations (cmd.exe / sh, git present / absent), plus a realgit commitwith a fake token on Windows is blocked.