Allow Git read commands in parallel hooks

.github/workflows/push.yml

@@ -17,13 +17,11 @@ jobs:
       - name: Build and test with integration coverage
         run: |
           go build -cover -v
-          mkdir -p coverage
-          GOCOVERDIR=$(pwd)/coverage go test ./... -v -count=1
-          go tool covdata textfmt -i=coverage -o coverage-integration.txt
+          mkdir -p coverage/integration
+          GOCOVERDIR="$(pwd)/coverage/integration" go test ./... -v -count=1
       - uses: actions/upload-artifact@v3
         with:
-          name: coverage
-          path: coverage-integration.txt
+          path: coverage
   test-cover-build:
     runs-on: ubuntu-latest
     steps:
@@ -34,26 +32,21 @@ jobs:
       - name: Build and test with unit coverage
         run: |
           go build -v
-          go test ./... -v -count=1 -coverprofile=coverage-unit.txt
+          mkdir -p coverage/unit
+          go test ./... -v -count=1 -cover -args -test.gocoverdir="$PWD/coverage/unit"
       - uses: actions/upload-artifact@v3
         with:
-          name: coverage
-          path: coverage-unit.txt
+          path: coverage
   coverage:
     runs-on: ubuntu-latest
     needs: [test-cover-integration, test-cover-build]
     steps:
+      - uses: actions/checkout@v3
       - uses: actions/download-artifact@v3
-        with:
-          name: coverage
       - name: Process coverage
-        # Coverage is simple text files, so we can combine the integration
-        # and unit test coverage by simply appending the latter with the
-        # first line skipped.
         run: |
-          cp coverage-integration.txt coverage.txt
-          tail -n +2 coverage-unit.txt >> coverage.txt
+          go tool covdata textfmt -i=./artifact/unit,./artifact/integration -o=coverage.txt
       - name: Upload coverage reports to Codecov
         uses: codecov/codecov-action@v3
         with:
-          files: ./coverage.txt
+          file: ./coverage.txt

hooks/pre_commit.go

@@ -1,8 +1,10 @@
 package hooks
 
 import (
+	_ "embed"
 	"fmt"
 	"os"
+	"os/exec"
 	"path"
 	"strings"
 
@@ -13,6 +15,9 @@ import (
 	"github.com/dirk/quickhook/tracing"
 )
 
+//go:embed pre_commit_git_shim.sh
+var PRE_COMMIT_GIT_SHIM string
+
 const PRE_COMMIT_HOOK = "pre-commit"
 const PRE_COMMIT_MUTATING_HOOK = "pre-commit-mutating"
 
@@ -95,6 +100,13 @@ func (hook *PreCommit) checkResult(result hookResult) bool {
 }
 
 func shimGit() (string, error) {
+	actualGit, err := exec.LookPath("git")
+	if err != nil {
+		return "", err
+	}
+	// Trusting that we didn't get a malicious path back from LookPath().
+	templated := strings.Replace(PRE_COMMIT_GIT_SHIM, "ACTUAL_GIT", actualGit, 1)
+
 	span := tracing.NewSpan("shim-git")
 	defer span.End()
 
@@ -104,12 +116,7 @@ func shimGit() (string, error) {
 	}
 
 	git := path.Join(dir, "git")
-	err = os.WriteFile(git, []byte(strings.Join([]string{
-		"#!/bin/sh",
-		"echo \"git is not allowed in parallel hooks (git $@)\"",
-		"exit 1",
-		"",
-	}, "\n")), 0755)
+	err = os.WriteFile(git, []byte(templated), 0755)
 	if err != nil {
 		return "", err
 	}

hooks/pre_commit_git_shim.sh

@@ -0,0 +1,19 @@
+#!/bin/sh
+set -e
+COMMAND=$1
+shift
+if
+    [ "$COMMAND" = "diff" ] ||
+    [ "$COMMAND" = "ls-files" ] ||
+    [ "$COMMAND" = "rev-list" ] ||
+    [ "$COMMAND" = "rev-parse" ] ||
+    [ "$COMMAND" = "show" ] ||
+    [ "$COMMAND" = "status" ];
+then
+    # The Git executable below will be replaced at runtime when shimming.
+    ACTUAL_GIT "$COMMAND" "$@"
+    exit 0
+fi
+COMBINED=$(echo "$COMMAND  $*" | xargs)
+echo "git is not allowed in parallel hooks (git $COMBINED)"
+exit 1

hooks/pre_commit_test.go

@@ -3,6 +3,8 @@ package hooks
 import (
 	"bytes"
 	"io"
+	"sort"
+	"strings"
 	"testing"
 
 	"github.com/creack/pty"
@@ -127,14 +129,34 @@ func TestHandlesDeletedFiles(t *testing.T) {
 	assert.Equal(t, "", output)
 }
 
-func TestShimsGitToDenyAccess(t *testing.T) {
+func TestGitShimAllowsReadonlyAccess(t *testing.T) {
 	tempDir := initGitForPreCommit(t)
 	tempDir.MkdirAll(".quickhook", "pre-commit")
 	tempDir.WriteFile([]string{".quickhook", "pre-commit", "accesses-git"}, "#!/bin/sh \n git status")
 
+	output, err := tempDir.ExecQuickhook("hook", "pre-commit")
+	assert.Nil(t, err)
+	assert.Empty(t, output)
+}
+
+func TestGitShimDeniesOtherAccess(t *testing.T) {
+	tempDir := initGitForPreCommit(t)
+	tempDir.MkdirAll(".quickhook", "pre-commit")
+	tempDir.WriteFile([]string{".quickhook", "pre-commit", "reset0"}, "#!/bin/sh \n git reset")
+	tempDir.WriteFile([]string{".quickhook", "pre-commit", "reset1"}, "#!/bin/sh \n git reset --hard")
+
 	output, err := tempDir.ExecQuickhook("hook", "pre-commit")
 	assert.Error(t, err)
-	assert.Equal(t, "accesses-git: git is not allowed in parallel hooks (git status)\n", output)
+	lines := strings.Split(strings.TrimSpace(output), "\n")
+	sort.Strings(lines)
+	assert.Equal(
+		t,
+		[]string{
+			"reset0: git is not allowed in parallel hooks (git reset)",
+			"reset1: git is not allowed in parallel hooks (git reset --hard)",
+		},
+		lines,
+	)
 }
 
 func TestMutatingCanAccessGit(t *testing.T) {