Skip to content

Commit 3503a3b

Browse files
ivyivy
authored
fix: prevent lefthook run from overwriting global hooks (#1371)
1 parent afac466 commit 3503a3b

2 files changed

Lines changed: 80 additions & 12 deletions

File tree

‎internal/command/install.go‎

Lines changed: 16 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -41,10 +41,6 @@ type InstallArgs struct {
4141
}
4242

4343
func (l *Lefthook) Install(ctx context.Context, args InstallArgs, hooks []string) error {
44-
if err := l.ensureHooksPathUnset(args.Force, args.ResetHooksPath); err != nil {
45-
return err
46-
}
47-
4844
cfg, err := l.readOrCreateConfig()
4945
if err != nil {
5046
return err
@@ -70,6 +66,17 @@ func (l *Lefthook) Install(ctx context.Context, args InstallArgs, hooks []string
7066
}
7167
}
7268

69+
return l.installHooks(cfg, hooks, args)
70+
}
71+
72+
// installHooks is the single entry point for creating hook files.
73+
// It ensures core.hooksPath is checked before any hooks are written,
74+
// preventing silent overwrites of global hook directories.
75+
func (l *Lefthook) installHooks(cfg *config.Config, hooks []string, args InstallArgs) error {
76+
if err := l.ensureHooksPathUnset(args.Force, args.ResetHooksPath); err != nil {
77+
return err
78+
}
79+
7380
return l.createHooksIfNeeded(cfg, hooks, args.Force)
7481
}
7582

@@ -196,7 +203,11 @@ func (l *Lefthook) syncHooks(cfg *config.Config, fetchRemotes bool) (*config.Con
196203
}
197204

198205
// Don't rely on config checksum if remotes were refetched
199-
return cfg, l.createHooksIfNeeded(cfg, hooks, false)
206+
if err := l.installHooks(cfg, hooks, InstallArgs{}); err != nil {
207+
log.Warnf("Skipping hook sync: %s", err)
208+
return cfg, nil
209+
}
210+
return cfg, nil
200211
}
201212

202213
func (l *Lefthook) shouldRefetch(remote *config.Remote) bool {

‎internal/command/install_test.go‎

Lines changed: 64 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -317,14 +317,14 @@ remotes:
317317
t.Run(fmt.Sprintf("%d: %s", n, tt.name), func(t *testing.T) {
318318
assert := assert.New(t)
319319

320-
// Prepend git config commands required by getHooksPathConfig() in install.go.
321-
// These commands are always called at the start of Install() to detect core.hooksPath conflicts.
320+
// Append git config commands required by ensureHooksPathUnset() via installHooks().
321+
// These commands are called after remote syncing, when hooks are about to be created.
322322
gitCmds := tt.git
323323
if len(gitCmds) == 0 || gitCmds[0].Command != "git config --local core.hooksPath" {
324-
gitCmds = append([]cmdtest.Out{
325-
{Command: "git config --local core.hooksPath"},
326-
{Command: "git config --global core.hooksPath"},
327-
}, gitCmds...)
324+
gitCmds = append(gitCmds,
325+
cmdtest.Out{Command: "git config --local core.hooksPath"},
326+
cmdtest.Out{Command: "git config --global core.hooksPath"},
327+
)
328328
}
329329

330330
repo := gittest.NewRepositoryBuilder().
@@ -506,6 +506,52 @@ commit-msg:
506506
hookPath(config.GhostHookName),
507507
},
508508
},
509+
{
510+
// Reproduces the bug: when core.hooksPath is set globally (e.g. for
511+
// credential leak detection), syncHooks should skip hook creation
512+
// rather than overwriting the user's global hooks directory.
513+
// See: reproduce-bug.sh
514+
name: "unsynchronized with global core.hooksPath set",
515+
config: `
516+
pre-commit:
517+
commands:
518+
tests:
519+
run: yarn test
520+
`,
521+
checksum: "00000000f706df65f379a9ff5ce0119b 1555894311\n",
522+
git: []cmdtest.Out{
523+
{Command: "git config --local core.hooksPath"},
524+
{Command: "git config --global core.hooksPath", Output: "/home/user/.config/git/hooks"},
525+
},
526+
wantExist: []string{
527+
configPath,
528+
},
529+
wantNotExist: []string{
530+
hookPath("pre-commit"),
531+
hookPath(config.GhostHookName),
532+
},
533+
},
534+
{
535+
name: "unsynchronized with local core.hooksPath set",
536+
config: `
537+
pre-commit:
538+
commands:
539+
tests:
540+
run: yarn test
541+
`,
542+
checksum: "00000000f706df65f379a9ff5ce0119b 1555894311\n",
543+
git: []cmdtest.Out{
544+
{Command: "git config --local core.hooksPath", Output: ".custom-hooks"},
545+
{Command: "git config --global core.hooksPath"},
546+
},
547+
wantExist: []string{
548+
configPath,
549+
},
550+
wantNotExist: []string{
551+
hookPath("pre-commit"),
552+
hookPath(config.GhostHookName),
553+
},
554+
},
509555
{
510556
name: "with unfetched remote",
511557
config: `
@@ -566,7 +612,18 @@ remotes:
566612
t.Run(fmt.Sprintf("%d: %s", n, tt.name), func(t *testing.T) {
567613
assert := assert.New(t)
568614

569-
repo := gittest.NewRepositoryBuilder().Root(root).Fs(fs).Cmd(cmdtest.NewOrdered(t, tt.git)).Build()
615+
// Append git config commands required by ensureHooksPathUnset() via installHooks().
616+
// These commands are called when syncHooks needs to create/update hooks,
617+
// which happens after any remote fetching.
618+
gitCmds := tt.git
619+
if len(gitCmds) == 0 || gitCmds[0].Command != "git config --local core.hooksPath" {
620+
gitCmds = append(gitCmds,
621+
cmdtest.Out{Command: "git config --local core.hooksPath"},
622+
cmdtest.Out{Command: "git config --global core.hooksPath"},
623+
)
624+
}
625+
626+
repo := gittest.NewRepositoryBuilder().Root(root).Fs(fs).Cmd(cmdtest.NewOrdered(t, gitCmds)).Build()
570627
lefthook := &Lefthook{
571628
fs: fs,
572629
repo: repo,

0 commit comments

Comments
 (0)