From 927af2703aeb1adbe2688d9bdab099a1d5a781e8 Mon Sep 17 00:00:00 2001 From: Lorenzo Gabriele Date: Mon, 29 Jun 2026 16:41:14 +0200 Subject: [PATCH] Refactor opengrep runner to stream JSON output Switched command execution from cmd.Output() buffering to pipe-based streaming decode with json.Decoder, reducing peak memory usage during large scans. Added bounded stderr tail buffering and updated tests to cover streaming execution and buffer truncation behavior. --- internal/tool/command.go | 99 +++++++++++++++++++++++++++-------- internal/tool/command_test.go | 48 +++++++++++++---- 2 files changed, 114 insertions(+), 33 deletions(-) diff --git a/internal/tool/command.go b/internal/tool/command.go index 71af0b3..375fe76 100644 --- a/internal/tool/command.go +++ b/internal/tool/command.go @@ -50,14 +50,9 @@ type SemgrepErrorLocation struct { func executeCommandForFiles(configurationFile *os.File, toolExecution codacy.ToolExecution, patternDescriptions *[]codacy.PatternDescription, language string, files []string) ([]codacy.Result, error) { semgrepCmd := createCommand(configurationFile, toolExecution.SourceDir, language, files) - semgrepOutput, semgrepError, err := runCommand(semgrepCmd) + output, semgrepError, err := runAndParseCommand(semgrepCmd, patternDescriptions) if err != nil { - return nil, errors.New("Error running semgrep: " + *semgrepError + "\n" + err.Error()) - } - - output, err := parseCommandOutput(patternDescriptions, *semgrepOutput) - if err != nil { - return nil, err + return nil, errors.New("Error running semgrep: " + semgrepError + "\n" + err.Error()) } return output, nil } @@ -104,32 +99,58 @@ func createCommandParameters(language string, configurationFile *os.File, filesT return cmdParams } -func runCommand(cmd *exec.Cmd) (*string, *string, error) { - var stderr bytes.Buffer - cmd.Stderr = &stderr - cmdOutput, err := cmd.Output() +func runAndParseCommand(cmd *exec.Cmd, patternDescriptions *[]codacy.PatternDescription) ([]codacy.Result, string, error) { + stdoutPipe, err := cmd.StdoutPipe() if err != nil { - stderrString := stderr.String() - return nil, &stderrString, err + return nil, "", err + } + + stderrPipe, err := cmd.StderrPipe() + if err != nil { + return nil, "", err + } + + stderrTail := &limitedBuffer{max: maxStderrBytes} + stderrDone := make(chan error, 1) + go func() { + _, copyErr := io.Copy(stderrTail, stderrPipe) + stderrDone <- copyErr + }() + + if err := cmd.Start(); err != nil { + return nil, "", err + } + + results, parseErr := parseCommandOutput(patternDescriptions, stdoutPipe) + if parseErr != nil && cmd.Process != nil { + _ = cmd.Process.Kill() + } + + waitErr := cmd.Wait() + stderrCopyErr := <-stderrDone + if stderrCopyErr != nil && !isBenignStreamClose(stderrCopyErr) { + return nil, stderrTail.String(), stderrCopyErr + } + if parseErr != nil { + return nil, stderrTail.String(), parseErr } - cmdOutputString := string(cmdOutput) - return &cmdOutputString, nil, nil + if waitErr != nil { + return nil, stderrTail.String(), waitErr + } + + return results, "", nil } -func parseCommandOutput(patternDescriptions *[]codacy.PatternDescription, commandOutput string) ([]codacy.Result, error) { +func parseCommandOutput(patternDescriptions *[]codacy.PatternDescription, stream io.Reader) ([]codacy.Result, error) { var result []codacy.Result - // Convert the JSON string to a []byte slice - jsonData := []byte(commandOutput) - // Create a bytes.Reader from the []byte slice - reader := bytes.NewReader(jsonData) // Create a JSON decoder - decoder := json.NewDecoder(reader) + decoder := json.NewDecoder(stream) // Read and process the JSON stream for { var semgrepOutput SemgrepOutput // or a struct that matches your JSON structure if err := decoder.Decode(&semgrepOutput); err != nil { - if err == io.EOF { + if isBenignStreamClose(err) { break // End of input } return nil, err @@ -143,6 +164,40 @@ func parseCommandOutput(patternDescriptions *[]codacy.PatternDescription, comman return result, nil } +const maxStderrBytes = 64 * 1024 + +type limitedBuffer struct { + buf bytes.Buffer + max int +} + +func (l *limitedBuffer) Write(p []byte) (int, error) { + if l.max <= 0 { + return len(p), nil + } + if len(p) >= l.max { + l.buf.Reset() + _, _ = l.buf.Write(p[len(p)-l.max:]) + return len(p), nil + } + if l.buf.Len()+len(p) > l.max { + drop := l.buf.Len() + len(p) - l.max + current := l.buf.Bytes() + l.buf.Reset() + _, _ = l.buf.Write(current[drop:]) + } + _, _ = l.buf.Write(p) + return len(p), nil +} + +func (l *limitedBuffer) String() string { + return l.buf.String() +} + +func isBenignStreamClose(err error) bool { + return errors.Is(err, io.EOF) || errors.Is(err, os.ErrClosed) || strings.Contains(err.Error(), "file already closed") +} + func appendIssueToResult(result []codacy.Result, patternDescriptions *[]codacy.PatternDescription, semgrepOutput SemgrepOutput) []codacy.Result { for _, semgrepRes := range semgrepOutput.Results { if semgrepRes.Extra.IsIgnored { diff --git a/internal/tool/command_test.go b/internal/tool/command_test.go index c0a6fd4..f45e04e 100644 --- a/internal/tool/command_test.go +++ b/internal/tool/command_test.go @@ -4,6 +4,7 @@ import ( "os" "os/exec" "path/filepath" + "strings" "testing" codacy "github.com/codacy/codacy-engine-golang-seed/v6" @@ -56,30 +57,32 @@ func TestCreateCommandParameters(t *testing.T) { assert.Subset(t, cmdParams, expectedParams) } -func TestRunCommand(t *testing.T) { +func TestRunAndParseCommand(t *testing.T) { // Arrange - mockCmd := exec.Command("echo", "Testing runCommand()") + mockCmd := exec.Command("echo", "{\"results\":[],\"errors\":[]}") + patternDescriptions := []codacy.PatternDescription{} // Act - stdout, stderr, err := runCommand(mockCmd) + results, stderr, err := runAndParseCommand(mockCmd, &patternDescriptions) // Assert assert.NoError(t, err) - assert.Nil(t, stderr) - assert.Equal(t, "Testing runCommand()\n", *stdout) + assert.Empty(t, stderr) + assert.Empty(t, results) } -func TestRunCommand_Error(t *testing.T) { +func TestRunAndParseCommand_Error(t *testing.T) { // Arrange mockCmd := exec.Command("invalid_command_name") + patternDescriptions := []codacy.PatternDescription{} // Act - stdout, stderr, err := runCommand(mockCmd) + results, stderr, err := runAndParseCommand(mockCmd, &patternDescriptions) // Assert assert.Error(t, err, "Expected an error running an invalid command") - assert.Empty(t, stdout, "Expected empty stdout for a failed command") - assert.Empty(t, stderr, "Expected empty stderr for a failed command") + assert.Empty(t, results, "Expected empty results for a failed command") + assert.Empty(t, stderr, "Expected empty stderr for a failed command start failure") } func TestParseCommandOutput(t *testing.T) { @@ -93,7 +96,7 @@ func TestParseCommandOutput(t *testing.T) { commandOutput := "{\"version\": \"1.49.0\", \"results\": [{\"check_id\": \"bash.curl.security.curl-eval.curl-eval\", \"path\": \"src/bash/curl-eval.bash\", \"start\": {\"line\": 5}, \"end\": {\"line\": 5}, \"extra\": {\"message\": \"Sample message\"}}], \"errors\": []}" // Act - result, err := parseCommandOutput(&mockPatternDescriptions, commandOutput) + result, err := parseCommandOutput(&mockPatternDescriptions, strings.NewReader(commandOutput)) // Assert assert.NoError(t, err, "Expected no error during parsing command output") @@ -162,7 +165,7 @@ func TestAppendToResultWithIgnore(t *testing.T) { }` // Act - result, _ := parseCommandOutput(&mockPatternDescriptions, validSemgrepOutput) + result, _ := parseCommandOutput(&mockPatternDescriptions, strings.NewReader(validSemgrepOutput)) // Assert assert.Len(t, result, 2, "Expected length of the result slice to be 2") @@ -338,3 +341,26 @@ func TestWriteMessageWithInvalidPatternID(t *testing.T) { // Assert assert.Equal(t, docgen.GetFirstSentence(nonEmptyMessage), description, "Expected first sentence of non-empty message for invalid pattern ID") } + +func TestLimitedBufferKeepsOnlyTail(t *testing.T) { + // Arrange + buffer := &limitedBuffer{max: 5} + + // Act + _, _ = buffer.Write([]byte("123")) + _, _ = buffer.Write([]byte("4567")) + + // Assert + assert.Equal(t, "34567", buffer.String()) +} + +func TestLimitedBufferLargeWriteTruncatesToMax(t *testing.T) { + // Arrange + buffer := &limitedBuffer{max: 4} + + // Act + _, _ = buffer.Write([]byte("abcdef")) + + // Assert + assert.Equal(t, "cdef", buffer.String()) +}