pre-push hooks are broken because lefthook does not forward STDIN

[marques-l-GILD]

🔧 Summary

Lefthook

Lefthook version

1.4.3 41b1cf139772b322400bfcb7754b6cb1c220e6be

Steps to reproduce

Add a pre-push script that relies on STDIN provided by git. For context, git passes the following information via STDIN to pre-push hooks (per official docs):

# If pushing without using a named remote those arguments will be equal.
#
# Information about the commits which are being pushed is supplied as lines to
# the standard input in the form:
#
#   <local ref> <local oid> <remote ref> <remote oid>

You can simulate this via CLI by running:

$ echo 'refs/heads/main 3a236f32fd701ffda70dd22ea65e61d2670bc1a9 refs/heads/main 0000000000000000000000000000000000000000' | LEFTHOOK_VERBOSE=true lefthook run pre-push origin [email protected]:<my-org/my-repo>.git

And of course, substitute the SHAs and remote URL above with valid ones.

If your installed pre-push hook is reading STDIN for these values, the script will hang indefinitely because lefthook does not forward this to each hook.

Expected results

pre-push hooks should receive STDIN. Valid use cases are tools that scan for secrets being pushed in git history (e.g., talisman.

Actual results

pre-push hooks that rely on STDIN provided by git hang/fail.

[mrexox]

Hey! Have you tried interactive and follow options?

[marques-l-GILD]

I have not - the docs didn't imply they would do what I was looking to do, but I'll give that I try and let you know if it works.

[marques-l-GILD]

@mrexox sadly, it does not work. I tried with the following:

pre-push:
  scripts:
    talisman_hook_script:
      runner: bash
      interactive: true
      follow: true

Looking at the entry hook that lefthook installs, it doesn't even read STDIN to pass them downstream.

For example, in this snippet within call_lefthook():

  if lefthook -h >/dev/null 2>&1
  then
    eval lefthook $@

I would have expected conceptually something more like:

hookname="$(basename $0)"

if [ 'pre-push' = "$hookname" ]; then
  cat - | eval lefthook "$@"
else
  eval lefthook "$@"
fi

And not to mention any golang code that needs to glue STDIN to each pre-push.scripts.[*] in lefthook.yml.

[mrexox]

@marques-l-GILD, can you please send a link to a test repo where the bug can be reproduced? Or maybe a lefthook.yml?

Lefthook is not supposed to work with pipes (as long as I know, STDIN and pipes have different mechanisms). For example:

# lefthook.yml

test:
  commands:
    read-and-print:
      interactive: true
      run: |
        read line
        echo Got: $line

$ lefthook run test
Lefthook v1.4.3

  EXECUTE > read-and-print
a line
Got: a line

SUMMARY: (done in 3.30 seconds)
✔️  read-and-print

$ echo "won't go to stdin" | lefthook run test
Lefthook v1.4.3
RUNNING HOOK: test

  EXECUTE > read-and-print

<hangs, waiting for input from STDIN>

I think if you send your lefthook.yml content and scripts you use for pre-push hook (if you have them) - that would help me find the issue (probably with lefthook).

[marques-l-GILD]

@mrexox sure, I might need some time to pull out of a private repo into a public test account with things set up. I probably won't get to that until after my workday, but I'll try to get that done tonight (I'm in pacific time zone).

And just to clarify the use case, the behavior I'm looking for is to be non-interactive -- the user doesn't supply any input themselves. It's that git puts 4 parameters in a single line on STDIN to the pre-push hook. I need this STDIN forwarded to the scripts lefthook manages for pre-push.

That said, you might be right that pipes and STDIN may have different nuances. Pipes aside, this would achieve the same thing without a pipe (assuming your shell is bash):

lefthook run test <<< "also doesn't work"

To be fully compatible with git's pre-push hook API, this needs to work.

This is the documented behavior: https://git-scm.com/docs/githooks#_pre_push

[marques-l-GILD]

@mrexox Ok I created a separate repo with a very simple test case. Instructions are in the README. Do note that you should fork this so you can test the pre-push by pushing to a repo you have write access to.

https://github.com/marques-l-GILD/lefthook-pre-push-test

Also, please let me know how I can help, whether it's clarifications, or writing code to help implement/fix this (I just might need some direction on where in the lefthook codebase to look).

Thanks for your help!

[marques-l-GILD]

It's also worth mentioning there a few other hooks that also use STDIN, but probably uncommon (even pre-push is relatively uncommon).

AFAIK, here are the hooks that receive information from git on STDIN:

• pre-push
• pre-receive
• post-receive
• post-rewrite
• reference-transaction

[mrexox]

Got it, thank you for the repo example. I have checked how it works, and I could make it work with the following config:

# lefthook.yml

---
pre-push:
  follow: true
  scripts:
    # This is an example script, adapted from .git/hooks/pre-push.sample
    "pre-push.sh":
      runner: sh
    # This is the "real" script I want to run on pre-push
    talisman_hook_script:
      runner: bash

However it fails to do while read ..., so it supports reading only once (I guess because STDIN is not being closed or whatever). I will dig into this issue and notify you about the progress. Unfortunately, I don't guarantee to do it very soon.

[marques-l-GILD]

@mrexox I totally understand you have priorities on top of supporting OSS, so I'm still thankful for the effort of just looking into it. 🙏

[shining-mind]

+1 we need this

[mrexox]

Hey everyone, I found a cause for this bug – lefthook assumes that interactive: true must always connect with tty which is true only when you need to use the tools like commitzen. For scripts and commands that just want to get an input from stdin interactive will just make it hang, since tty will remain open.

I am going to add a use_stdin option (false by default) to pass stdin but do not use tty. Feel free to suggest a better naming. I am planning to release the fix this week.

[mrexox]

Hey! Please, check out v1.4.11 of lefthook I have recently released. Add use_stdin: true option to your script configuration. I hope this helps, but if not – I'll be here for further fixes.