fix(git): apply timeout and fatal-error wrapping consistently (audit #14.1)

core/git/git.go

@@ -156,24 +156,29 @@ func (c *impl) ApplyPatch(ctx context.Context, patch []byte) error {
 
 // RevParse returns the revision hash of a reference.
 func (c *impl) RevParse(ctx context.Context, ref string) (string, error) {
+	ctx, cancel := context.WithTimeout(ctx, _gitTimeout)
+	defer cancel()
 	args := []string{"rev-parse", ref}
 	out, err := c.runner.output(ctx, c.directory, "git", args...)
 	if err != nil {
-		return "", err
+		return "", wrapError(ctx, args, err)
 	}
 	return strings.TrimSpace(string(out)), nil
 }
 
 // IsAncestor reports whether ancestorRef is an ancestor of descendantRef.
 func (c *impl) IsAncestor(ctx context.Context, ancestorRef, descendantRef string) (bool, error) {
-	_, err := c.runner.output(ctx, c.directory, "git", "merge-base", "--is-ancestor", ancestorRef, descendantRef)
+	ctx, cancel := context.WithTimeout(ctx, _gitTimeout)
+	defer cancel()
+	args := []string{"merge-base", "--is-ancestor", ancestorRef, descendantRef}
+	_, err := c.runner.output(ctx, c.directory, "git", args...)
 	if err != nil {
 		if exitErr, ok := err.(*exec.ExitError); ok && exitErr.ExitCode() == 1 {
 			return false, nil
 		}
 		// an exit code other than 1, or a non-ExitError failure (context canceled,
 		// git binary missing, I/O error), indicates the check itself failed.
-		return false, fmt.Errorf("check if ref %s is ancestor of %s: %w", ancestorRef, descendantRef, err)
+		return false, wrapError(ctx, args, err)
 	}
 	return true, nil
 }
@@ -226,18 +231,24 @@ func (c *impl) DiffWithStatus(ctx context.Context, baseRef, targetRef string) ([
 
 // GetCommitTimeSecond returns the commit timestamp of the given ref in Unix seconds.
 func (c *impl) GetCommitTimeSecond(ctx context.Context, ref string) (int64, error) {
-	out, err := c.runner.output(ctx, c.directory, "git", "log", "-1", "--format=%ct", ref)
+	ctx, cancel := context.WithTimeout(ctx, _gitTimeout)
+	defer cancel()
+	args := []string{"log", "-1", "--format=%ct", ref}
+	out, err := c.runner.output(ctx, c.directory, "git", args...)
 	if err != nil {
-		return 0, err
+		return 0, wrapError(ctx, args, err)
 	}
 	return strconv.ParseInt(strings.TrimSpace(string(out)), 10, 64)
 }
 
 // FileHashes gets a mapping of files to their hashes based on `git ls-tree --full-tree -r <ref>`.
 func (c *impl) FileHashes(ctx context.Context, ref string) (map[string][]byte, error) {
-	out, err := c.runner.output(ctx, c.directory, "git", "ls-tree", "--full-tree", "-r", "-z", ref)
+	ctx, cancel := context.WithTimeout(ctx, _gitTimeout)
+	defer cancel()
+	args := []string{"ls-tree", "--full-tree", "-r", "-z", ref}
+	out, err := c.runner.output(ctx, c.directory, "git", args...)
 	if err != nil {
-		return nil, err
+		return nil, wrapError(ctx, args, err)
 	}
 
 	fileHashes := make(map[string][]byte)

core/git/git_test.go

@@ -280,10 +280,11 @@ func TestGetCommitTimeSecond_parsesUnixTimestamp(t *testing.T) {
 }
 
 func TestGetCommitTimeSecond_errorPropagates(t *testing.T) {
-	m := &mockRunner{err: errors.New("git error")}
+	m := &mockRunner{err: assert.AnError}
 	g := &impl{directory: "/repo", runner: m}
 	_, err := g.GetCommitTimeSecond(context.Background(), "HEAD")
 	require.Error(t, err)
+	assert.ErrorIs(t, err, assert.AnError)
 }
 
 func TestDefaultGit_FileHashes(t *testing.T) {
@@ -314,7 +315,7 @@ func TestDefaultGit_FileHashes(t *testing.T) {
 		},
 		{
 			name:      "git error",
-			wantError: errors.New(""),
+			wantError: assert.AnError,
 		},
 	}
 
@@ -330,7 +331,12 @@ func TestDefaultGit_FileHashes(t *testing.T) {
 			m.out = tt.giveOutput
 			m.err = tt.wantError
 			gotHashes, err := g.FileHashes(ctx, tt.name)
-			require.Equal(t, tt.wantError, err)
+			if tt.wantError != nil {
+				require.Error(t, err)
+				assert.ErrorIs(t, err, tt.wantError)
+			} else {
+				require.NoError(t, err)
+			}
 			assert.Equal(t, tt.wantHashes, gotHashes)
 		})
 	}