From 4b7d1d3a6827691637eff750b6482042e06462d0 Mon Sep 17 00:00:00 2001 From: meredith <4412188+mariduv@users.noreply.github.com> Date: Thu, 10 Sep 2026 12:18:33 -0500 Subject: [PATCH] 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 | 5 + pkg/feature/feature.go | 1 + pkg/gitparse/gitparse.go | 276 ++++++++++++++++++++++++++++------ pkg/gitparse/gitparse_test.go | 43 +++++- pkg/sources/git/git.go | 11 +- pkg/sources/git/git_test.go | 46 ++++++ 6 files changed, 331 insertions(+), 51 deletions(-) diff --git a/main.go b/main.go index 16c16e0da..2e26dee2e 100644 --- a/main.go +++ b/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) } diff --git a/pkg/feature/feature.go b/pkg/feature/feature.go index b8d01e8a3..65f83c869 100644 --- a/pkg/feature/feature.go +++ b/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 diff --git a/pkg/gitparse/gitparse.go b/pkg/gitparse/gitparse.go index 01fe53cbb..e52f46cda 100644 --- a/pkg/gitparse/gitparse.go +++ b/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())) diff --git a/pkg/gitparse/gitparse_test.go b/pkg/gitparse/gitparse_test.go index e436c25fb..06aace3bf 100644 --- a/pkg/gitparse/gitparse_test.go +++ b/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 diff --git a/pkg/sources/git/git.go b/pkg/sources/git/git.go index 338de43b4..6e33eaedd 100644 --- a/pkg/sources/git/git.go +++ b/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...), } } diff --git a/pkg/sources/git/git_test.go b/pkg/sources/git/git_test.go index 79d6ac263..359be0d39 100644 --- a/pkg/sources/git/git_test.go +++ b/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)) + } +}