Low-memory Git Scanning (#5257)

* Refactor gitparse for reduced peak memory use

Instead of one single `git log` that grows huge, get a commit list with
"log" then request `git show` groups of those commits.

This is currently slightly slower than main, and parallelizing this part
further doesn't improve things because of the diffChan consumer in the
caller.  It does pipeline just enough that one git show can run while
scanning consumes the prior set of diffs.

* Group commits while reading instead of slurp and split

* Make the low-memory scan mode a non-default option

main.go

@@ -91,6 +91,7 @@ var (
 	forceSkipBinaries        = cli.Flag("force-skip-binaries", "Force skipping binaries.").Bool()
 	forceSkipArchives        = cli.Flag("force-skip-archives", "Force skipping archives.").Bool()
 	gitCloneTimeout          = cli.Flag("git-clone-timeout", "Maximum time to spend cloning a repository, as a duration.").Hidden().Duration()
+	gitLowMemoryScan         = cli.Flag("git-low-memory-scan", "Reduce memory use for git scanning.").Hidden().Bool()
 	skipAdditionalRefs       = cli.Flag("skip-additional-refs", "Skip additional references.").Bool()
 	userAgentSuffix          = cli.Flag("user-agent-suffix", "Suffix to add to User-Agent.").String()
 	dropUnverifiedJWTResults = cli.Flag("drop-unverified-jwt-results", "Drop unverified results without any verification errors from the JWT detector.").Bool()
@@ -518,6 +519,10 @@ func run(state overseer.State, logSync func() error) {
 		feature.GitCloneTimeoutDuration.Store(int64(*gitCloneTimeout))
 	}
 
+	if *gitLowMemoryScan {
+		feature.UseGitLowMemoryScan.Store(true)
+	}
+
 	if *skipAdditionalRefs {
 		feature.SkipAdditionalRefs.Store(true)
 	}

pkg/feature/feature.go

@@ -13,6 +13,7 @@ var (
 	UserAgentSuffix                          AtomicString
 	UseSimplifiedGitlabEnumeration           atomic.Bool
 	UseGitMirror                             atomic.Bool
+	UseGitLowMemoryScan                      atomic.Bool
 	GitlabProjectsPerPage                    atomic.Int64
 	UseGithubGraphQLAPI                      atomic.Bool // use github graphql api to fetch issues, pr's and comments
 	HTMLDecoderEnabled                       atomic.Bool

pkg/gitparse/gitparse.go

@@ -3,11 +3,13 @@ package gitparse
 import (
 	"bufio"
 	"bytes"
+	"cmp"
 	"fmt"
 	"io"
 	"os"
 	"os/exec"
 	"path/filepath"
+	"slices"
 	"strconv"
 	"strings"
 	"time"
@@ -33,6 +35,14 @@ const (
 
 	// defaultWaitDelay is the default time to wait after context cancellation before forcefully killing git processes.
 	defaultWaitDelay = 5 * time.Second
+
+	// abbrevCommit is the git sha abbreviation length to use for `git show` invocations in the lower-memory scan mode.
+	abbrevCommit = 20
+
+	// showGroupSize is the number of commits per `git show` in the lower-memory scan mode.
+	//
+	// Windows has a command length limit of 32767, so at these values we should only be using a tiny part of of that for the commit list ((abbrevCommit + 1) * showGroupSize).  We don't target any platforms with shorter limits.
+	showGroupSize = 75
 )
 
 // contentWriter defines a common interface for writing, reading, and managing diff content.
@@ -130,6 +140,7 @@ type Parser struct {
 	waitDelay     time.Duration
 
 	useCustomContentWriter bool
+	lowMemoryScan          bool
 }
 
 type ParseState int
@@ -191,6 +202,13 @@ func UseCustomContentWriter() Option {
 	return func(parser *Parser) { parser.useCustomContentWriter = true }
 }
 
+// UseLowMemoryScan sets the scan to optimize for limited memory at the cost of speed (currently up to 9%)
+func UseLowMemoryScan() Option {
+	return func(parser *Parser) {
+		parser.lowMemoryScan = true
+	}
+}
+
 // WithMaxDiffSize sets maxDiffSize option. Diffs larger than maxDiffSize will
 // be truncated.
 func WithMaxDiffSize(maxDiffSize int64) Option {
@@ -233,8 +251,17 @@ func NewParser(options ...Option) *Parser {
 	return parser
 }
 
+type gitArgs struct {
+	env    []string
+	global []string
+	log    []string
+	show   []string
+	paths  []string
+}
+
 // RepoPath parses the output of the `git log` command for the `source` path.
-// The Diff chan will return diffs in the order they are parsed from the log.
+// The Diff chan will return diffs in the order they are parsed from the log,
+// though the diffs are generated using `git show` in groups.
 func (c *Parser) RepoPath(
 	ctx context.Context,
 	source string,
@@ -242,51 +269,212 @@ func (c *Parser) RepoPath(
 	abbreviatedLog bool,
 	excludedGlobs []string,
 	isBare bool,
-	additionalArgs ...string,
 ) (chan *Diff, error) {
-	args := []string{
-		"-C", source,
-		"log",
-		"--patch", // https://git-scm.com/docs/git-log#Documentation/git-log.txt---patch
-		"--full-history",
-		"--date=iso-strict",
-		"--pretty=fuller", // https://git-scm.com/docs/git-log#_pretty_formats
-		"--notes",         // https://git-scm.com/docs/git-log#Documentation/git-log.txt---notesltrefgt
-	}
-	if abbreviatedLog {
-		args = append(args, "--diff-filter=AM")
-	}
-	if head != "" {
-		args = append(args, head)
-	} else {
-		args = append(args, "--all")
-	}
-	args = append(args, additionalArgs...) // These need to come before --
-	for _, glob := range excludedGlobs {
-		args = append(args, "--", ".", ":(exclude)"+glob)
+	args := c.prepGitArgs(source, head, abbreviatedLog, excludedGlobs, isBare)
+
+	if c.lowMemoryScan {
+		return c.repoPathLowMemory(ctx, args)
 	}
 
-	cmd := exec.CommandContext(ctx, "git", args...)
-	absPath, err := filepath.Abs(source)
-	if err == nil {
-		if !isBare {
-			cmd.Env = append(cmd.Env, "GIT_DIR="+filepath.Join(absPath, ".git"))
-		} else {
-			cmd.Env = append(cmd.Env,
-				"GIT_DIR="+absPath,
+	showCmd := exec.CommandContext(ctx,
+		"git",
+		slices.Concat(args.global, []string{"log"}, args.show, args.log, args.paths)...,
+	)
+	showCmd.Env = args.env
+
+	return c.executeCommand(ctx, showCmd, false)
+}
+
+func (c *Parser) repoPathLowMemory(ctx context.Context, args gitArgs) (chan *Diff, error) {
+	commitGroups, err := c.gatherGitLog(ctx, args)
+	if err != nil {
+		return nil, err
+	}
+
+	// c.executeCommand returns a channel that is later closed by a
+	// different goroutine after the command finishes, but we're not
+	// running a single command anymore. we'll use a channel of channels to
+	// reduce back to one channel we return to our caller.  Unbuffered so
+	// we have at most one git show running and one git show draining.
+	diffGroups := make(chan chan *Diff)
+	go func() {
+		defer common.RecoverWithExit(ctx)
+		defer close(diffGroups)
+
+		for group := range commitGroups {
+			if common.IsDone(ctx) {
+				return
+			}
+
+			showCmd := exec.CommandContext(ctx,
+				"git",
+				slices.Concat(args.global, []string{"show"}, args.show, group, args.paths)...,
 			)
-			// We need those variables to handle incoming commits
-			// while using trufflehog in pre-receive hooks
-			if dir := os.Getenv("GIT_OBJECT_DIRECTORY"); dir != "" {
-				cmd.Env = append(cmd.Env, "GIT_OBJECT_DIRECTORY="+dir)
+			showCmd.Env = args.env
+
+			diffGroup, err := c.executeCommand(ctx, showCmd, false)
+			if err != nil {
+				ctx.Logger().Error(err, "Error executing git show for commit group.")
+				return
 			}
-			if dir := os.Getenv("GIT_ALTERNATE_OBJECT_DIRECTORIES"); dir != "" {
-				cmd.Env = append(cmd.Env, "GIT_ALTERNATE_OBJECT_DIRECTORIES="+dir)
+			err = common.CancellableWrite(ctx, diffGroups, diffGroup)
+			if err != nil {
+				ctx.Logger().Error(err, "git show interation cancelled")
+				return
 			}
 		}
+	}()
+
+	// and this is the single channel we're responsible for returning and
+	// closing.
+	diffChan := make(chan *Diff)
+	go func() {
+		defer common.RecoverWithExit(ctx)
+		defer close(diffChan)
+
+		var err error
+		for groupdiffs := range diffGroups {
+			for diff := range groupdiffs {
+				err = common.CancellableWrite(ctx, diffChan, diff)
+				if err != nil {
+					return // context cancel
+				}
+			}
+		}
+	}()
+
+	return diffChan, nil
+}
+
+// Ask git for a list of all relevant commit hashes but only hashes.  Git takes
+// on the work of linearizing history for us, then we work through the commit
+// list.  Returns a channel of groups of commit IDs, so scanning can start asap
+// even if git log is taking a bit for large repos.
+func (c *Parser) gatherGitLog(ctx context.Context, args gitArgs) (chan []string, error) {
+	cmd := exec.CommandContext(ctx,
+		"git", slices.Concat(
+			args.global, []string{"log"},
+			args.log, []string{
+				// https://git-scm.com/docs/git-log#_pretty_formats
+				"--pretty=format:%h",
+				// https://git-scm.com/docs/git-log#Documentation/git-log.txt---abbrevn
+				fmt.Sprintf("--abbrev=%d", abbrevCommit),
+			},
+			args.paths,
+		)...)
+	cmd.WaitDelay = c.waitDelay
+	cmd.Env = args.env
+
+	stdOut, err := cmd.StdoutPipe()
+	if err != nil {
+		return nil, err
+	}
+
+	err = cmd.Start()
+	if err != nil {
+		return nil, fmt.Errorf("failed to execute git log: %w", err)
+	}
+
+	commitGroups := make(chan []string)
+	go func() {
+		defer close(commitGroups)
+		defer func() {
+			err := cmd.Wait()
+			if err != nil {
+				ctx.Logger().Error(err, "git log exited with error", "stderr", cmd.Stderr)
+			}
+		}()
+
+		s := bufio.NewScanner(stdOut)
+		commitGroup := make([]string, 0, showGroupSize)
+
+		var err error
+		for s.Scan() && !common.IsDone(ctx) {
+			commitGroup = append(commitGroup, s.Text())
+
+			if len(commitGroup) == showGroupSize {
+				err = common.CancellableWrite(ctx, commitGroups, commitGroup)
+				if err != nil {
+					ctx.Logger().Error(err, "git log stopping early")
+					return
+				}
+				commitGroup = make([]string, 0, showGroupSize)
+			}
+		}
+		if len(commitGroup) != 0 {
+			err = common.CancellableWrite(ctx, commitGroups, commitGroup)
+			if err != nil {
+				ctx.Logger().Error(err, "failed to flush last git log group")
+			}
+
+		}
+
+		if err := s.Err(); err != nil {
+			ctx.Logger().Error(err, "error reading git log")
+		}
+	}()
+
+	return commitGroups, nil
+}
+
+func (c *Parser) prepGitArgs(source string, head string, abbreviatedLog bool, excludedGlobs []string, isBare bool) gitArgs {
+	args := gitArgs{
+		global: []string{
+			"-C", source,
+		},
+		log: []string{
+			// https://git-scm.com/docs/git-log#Documentation/git-log.txt---full-history
+			"--full-history",
+		},
+		show: []string{
+			// https://git-scm.com/docs/git-show#Documentation/git-show.txt---patch
+			"--patch",
+			// https://git-scm.com/docs/git-log#Documentation/git-log.txt---dateformat
+			"--date=iso-strict",
+			// https://git-scm.com/docs/git-show#_pretty_formats
+			"--pretty=fuller",
+			// https://git-scm.com/docs/git-show#Documentation/git-show.txt---notesref
+			"--notes",
+		},
+		paths: []string{},
+	}
+
+	if abbreviatedLog {
+		// https://git-scm.com/docs/git-show#Documentation/git-show.txt---diff-filterACDMRTUXB
+		args.log = append(args.log, "--diff-filter=AM")
+		args.show = append(args.show, "--diff-filter=AM")
+	}
+
+	// Keep head or all last, before the --, not required but sensible
+	// https://git-scm.com/docs/git-log#Documentation/git-log.txt---all
+	args.log = append(args.log, cmp.Or(head, "--all"))
+
+	// And then potentially add -- to args here
+	if len(excludedGlobs) != 0 {
+		args.paths = []string{"--", "."}
+		for _, glob := range excludedGlobs {
+			// This is not directly doc'd but added in git 1.9.0 and found in pathspec.c
+			args.paths = append(args.paths, ":(exclude)"+glob)
+		}
 	}
 
-	return c.executeCommand(ctx, cmd, false, c.waitDelay)
+	absPath, err := filepath.Abs(source)
+	if err == nil {
+		if !isBare {
+			args.env = append(args.env, "GIT_DIR="+filepath.Join(absPath, ".git"))
+		} else {
+			args.env = append(args.env, "GIT_DIR="+absPath)
+			// We need those variables to handle incoming commits
+			// while using trufflehog in pre-receive hooks
+			if dir := os.Getenv("GIT_OBJECT_DIRECTORY"); dir != "" {
+				args.env = append(args.env, "GIT_OBJECT_DIRECTORY="+dir)
+			}
+			if dir := os.Getenv("GIT_ALTERNATE_OBJECT_DIRECTORIES"); dir != "" {
+				args.env = append(args.env, "GIT_ALTERNATE_OBJECT_DIRECTORIES="+dir)
+			}
+		}
+	}
+	return args
 }
 
 // Staged parses the output of the `git diff` command for the `source` path.
@@ -301,29 +489,29 @@ func (c *Parser) Staged(ctx context.Context, source string) (chan *Diff, error)
 		cmd.Env = append(cmd.Env, "GIT_DIR="+filepath.Join(absPath, ".git"))
 	}
 
-	return c.executeCommand(ctx, cmd, true, c.waitDelay)
+	return c.executeCommand(ctx, cmd, true)
 }
 
 // executeCommand runs an exec.Cmd, reads stdout and stderr, and waits for the Cmd to complete.
 // waitDelay specifies how long to wait after context cancellation before forcefully killing the process.
-func (c *Parser) executeCommand(ctx context.Context, cmd *exec.Cmd, isStaged bool, waitDelay time.Duration) (chan *Diff, error) {
+func (c *Parser) executeCommand(ctx context.Context, cmd *exec.Cmd, isStaged bool) (chan *Diff, error) {
 	diffChan := make(chan *Diff, 64)
 
 	stdOut, err := cmd.StdoutPipe()
 	if err != nil {
-		return diffChan, err
+		return nil, err
 	}
 	stdErr, err := cmd.StderrPipe()
 	if err != nil {
-		return diffChan, err
+		return nil, err
 	}
 
 	// Set WaitDelay to allow the command additional time to exit after context cancellation
-	cmd.WaitDelay = waitDelay
+	cmd.WaitDelay = c.waitDelay
 
 	err = cmd.Start()
 	if err != nil {
-		return diffChan, err
+		return nil, err
 	}
 
 	go func() {
@@ -355,7 +543,7 @@ func (c *Parser) FromReader(ctx context.Context, stdOut io.Reader, diffChan chan
 
 		totalLogSize int
 	)
-	var latestState = Initial
+	latestState := Initial
 
 	diff := func(c *Commit, opts ...diffOption) *Diff {
 		opts = append(opts, withCustomContentWriter(bufferwriter.New()))

pkg/gitparse/gitparse_test.go

@@ -2,6 +2,7 @@ package gitparse
 
 import (
 	"bytes"
+	"path/filepath"
 	"strings"
 	"testing"
 	"time"
@@ -17,6 +18,45 @@ import (
 	bufferedfilewriter "github.com/trufflesecurity/trufflehog/v3/pkg/writers/buffered_file_writer"
 )
 
+func TestPrepGitArgs(t *testing.T) {
+	t.Setenv("GIT_OBJECT_DIRECTORY", "")
+	t.Setenv("GIT_ALTERNATE_OBJECT_DIRECTORIES", "")
+	repopath := t.TempDir()
+
+	p := Parser{}
+
+	args := p.prepGitArgs(repopath, "", false, nil, false)
+	assert.Equal(t, []string{"-C", repopath}, args.global)
+	assert.Contains(t, args.log, "--all")
+	assert.NotContains(t, args.log, "--diff-filter=AM")
+	assert.Equal(t, []string{"GIT_DIR=" + filepath.Join(repopath, ".git")}, args.env)
+	assert.Empty(t, args.paths)
+
+	args = p.prepGitArgs(repopath, "branchname", true, []string{"some/file.txt", "bloated.dat"}, true)
+	// head
+	assert.Contains(t, args.log, "branchname")
+	assert.NotContains(t, args.log, "--all")
+	// abbreviatedLog
+	assert.Contains(t, args.log, "--diff-filter=AM")
+	assert.Contains(t, args.show, "--diff-filter=AM")
+	// excludedGlobs
+	assert.Contains(t, args.paths, "--")
+	assert.Contains(t, args.paths, ":(exclude)bloated.dat")
+	// isBare
+	assert.Equal(t, []string{"GIT_DIR=" + repopath}, args.env)
+
+	// test env passthrough used for pre-receive
+	t.Setenv("GIT_OBJECT_DIRECTORY", "foo")
+	t.Setenv("GIT_ALTERNATE_OBJECT_DIRECTORIES", "bar")
+
+	args = p.prepGitArgs(repopath, "", false, nil, true)
+	assert.Equal(t, []string{
+		"GIT_DIR=" + repopath,
+		"GIT_OBJECT_DIRECTORY=foo",
+		"GIT_ALTERNATE_OBJECT_DIRECTORIES=bar",
+	}, args.env)
+}
+
 type testCaseLine struct {
 	latestState ParseState
 	line        []byte
@@ -750,7 +790,6 @@ func TestToFileLinePathParse(t *testing.T) {
 // `asserts` on `Diff`'s _structure_, giving better test output than just comparing two
 // diffs.
 func assertDiffEqualToExpected(t *testing.T, expected *Diff, actual *Diff) {
-
 	// Use `cmp.Diff` to automatically compare all the exported fields.  This allows this test to grow automatically if
 	// new exported fields are added to these structs.  However, the most important field we want to test is unexpected
 	// (i.e. contentWriter) which is where the actual content of the diff is stored.  We break this out next.
@@ -1476,8 +1515,8 @@ func TestMaxCommitSize(t *testing.T) {
 	if diffCount != 2 {
 		t.Errorf("Commit count does not match. Got: %d, expected: %d", diffCount, 2)
 	}
-
 }
+
 func TestWaitDelay(t *testing.T) {
 	// Test that WithWaitDelay sets the waitDelay correctly
 	customDelay := 10 * time.Second

pkg/sources/git/git.go

@@ -127,11 +127,12 @@ type Config struct {
 // NewGit creates a new Git instance with the provided configuration. The Git instance is used to interact with
 // Git repositories.
 func NewGit(config *Config) *Git {
-	var parser *gitparse.Parser
+	parserOpts := []gitparse.Option{}
 	if config.UseCustomContentWriter {
-		parser = gitparse.NewParser(gitparse.UseCustomContentWriter())
-	} else {
-		parser = gitparse.NewParser()
+		parserOpts = append(parserOpts, gitparse.UseCustomContentWriter())
+	}
+	if feature.UseGitLowMemoryScan.Load() {
+		parserOpts = append(parserOpts, gitparse.UseLowMemoryScan())
 	}
 
 	return &Git{
@@ -145,7 +146,7 @@ func NewGit(config *Config) *Git {
 		concurrency:        semaphore.NewWeighted(int64(config.Concurrency)),
 		skipBinaries:       config.SkipBinaries,
 		skipArchives:       config.SkipArchives,
-		parser:             parser,
+		parser:             gitparse.NewParser(parserOpts...),
 	}
 }
 

pkg/sources/git/git_test.go

@@ -1545,3 +1545,49 @@ func TestGitChunk_LongLine(t *testing.T) {
 	// one chunk for the commit/file metadata, and at least one chunk for the file content
 	assert.Equal(t, 2, count, "expected two chunks from a file with a 100 KB line")
 }
+
+func TestGitLowMemoryScan(t *testing.T) {
+	feature.UseGitLowMemoryScan.Store(true)
+	t.Cleanup(func() { feature.UseGitLowMemoryScan.Store(false) })
+
+	ctx, cancel := context.WithCancel(context.Background())
+	defer cancel()
+
+	thisrepo := &sourcespb.Git{
+		Directories: []string{"../../../"},
+		Credential: &sourcespb.Git_Unauthenticated{
+			Unauthenticated: &credentialspb.Unauthenticated{},
+		},
+	}
+	wantChunk := &sources.Chunk{
+		SourceType:   sourcespb.SourceType_SOURCE_TYPE_GIT,
+		SourceName:   "this repo, low memory",
+		SourceVerify: false,
+	}
+
+	s := Source{}
+	conn, err := anypb.New(thisrepo)
+	if err != nil {
+		t.Fatal(err)
+	}
+
+	err = s.Init(ctx, "this repo, low memory", 0, 0, false, conn, 1)
+	if err != nil {
+		t.Errorf("Source.Init() error = %v", err)
+		return
+	}
+
+	chunksCh := make(chan *sources.Chunk, 1)
+	go func() {
+		assert.NoError(t, s.Chunks(ctx, chunksCh))
+	}()
+
+	gotChunk := <-chunksCh
+	gotChunk.Data = nil
+	// Commits don't come in a deterministic order, so remove metadata comparison
+	gotChunk.SourceMetadata = nil
+	if diff := pretty.Compare(gotChunk, wantChunk); diff != "" {
+		t.Errorf("Source.Chunks() UseGitLowMemoryScan diff: (-got +want)\n%s", diff)
+		t.Errorf("Data: %s", string(gotChunk.Data))
+	}
+}