diff --git a/home/internal/scanner/sbom.go b/home/internal/scanner/sbom.go index 86c10248..1a3299a1 100644 --- a/home/internal/scanner/sbom.go +++ b/home/internal/scanner/sbom.go @@ -5,6 +5,7 @@ import ( "fmt" "os" "path/filepath" + "strings" "time" "github.com/docker/docker/api/types/container" @@ -15,6 +16,11 @@ import ( const sbomDir = "/data/sbom" +type syftStreamResult struct { + stderr string + err error +} + // StartSBOMGeneration starts SBOM generation for an image. func (s *ScannerService) StartSBOMGeneration(imageRef, host string, format models.SBOMFormat) (*models.SBOMJob, error) { job := &models.SBOMJob{ @@ -93,44 +99,45 @@ func (s *ScannerService) runSBOMGeneration(job *models.SBOMJob) { } filePath := filepath.Join(sbomDir, job.ID+".json") - streamDone := make(chan struct { - stderr string - err error - }, 1) + streamDone := make(chan syftStreamResult, 1) go func() { stderr, err := StreamContainerStdoutToFile(ctx, apiClient, containerID, filePath, nil) - streamDone <- struct { - stderr string - err error - }{stderr, err} + streamDone <- syftStreamResult{stderr: stderr, err: err} }() statusCh, errCh := apiClient.ContainerWait(ctx, containerID, container.WaitConditionNotRunning) var exitCode int64 - select { - case err := <-errCh: - if err != nil { + var streamResult syftStreamResult + waitStatusCh := statusCh + waitErrCh := errCh + waitStreamCh := streamDone + + for waitStatusCh != nil || waitErrCh != nil || waitStreamCh != nil { + select { + case err := <-waitErrCh: + waitErrCh = nil + waitStatusCh = nil + if err != nil { + os.Remove(filePath) + s.updateSBOMStatus(job, models.ScanJobFailed, fmt.Sprintf("error waiting for syft: %v", err)) + return + } + case status := <-waitStatusCh: + waitStatusCh = nil + waitErrCh = nil + exitCode = status.StatusCode + case streamResult = <-waitStreamCh: + waitStreamCh = nil + if streamCause := syftStreamFailureCause(streamResult); streamCause != nil { + os.Remove(filePath) + s.updateSBOMStatus(job, models.ScanJobFailed, fmt.Sprintf("error streaming syft output: %v", streamCause)) + return + } + case <-ctx.Done(): os.Remove(filePath) - s.updateSBOMStatus(job, models.ScanJobFailed, fmt.Sprintf("error waiting for syft: %v", err)) + s.updateSBOMStatus(job, models.ScanJobCancelled, "cancelled") return } - case status := <-statusCh: - exitCode = status.StatusCode - case <-ctx.Done(): - os.Remove(filePath) - s.updateSBOMStatus(job, models.ScanJobCancelled, "cancelled") - return - } - - streamResult := <-streamDone - - // Surface stream errors as the primary cause before falling through to the - // exit-code branch — otherwise a broken Docker log stream gets reported - // only as "syft exited with code N" and the real failure is lost. - if streamResult.err != nil { - os.Remove(filePath) - s.updateSBOMStatus(job, models.ScanJobFailed, fmt.Sprintf("failed to read syft output: %v", streamResult.err)) - return } if exitCode != 0 { @@ -172,3 +179,13 @@ func buildSBOMCmd(imageRef string, format models.SBOMFormat) []string { } return []string{imageRef, "-o", outputFormat} } + +func syftStreamFailureCause(result syftStreamResult) any { + if result.err != nil { + return result.err + } + if stderr := strings.TrimSpace(result.stderr); stderr != "" { + return stderr + } + return nil +} diff --git a/home/internal/scanner/sbom_test.go b/home/internal/scanner/sbom_test.go new file mode 100644 index 00000000..95dffd94 --- /dev/null +++ b/home/internal/scanner/sbom_test.go @@ -0,0 +1,46 @@ +package scanner + +import ( + "errors" + "testing" +) + +func TestSyftStreamFailureCauseReturnsError(t *testing.T) { + cause := syftStreamFailureCause(syftStreamResult{ + stderr: "warning on stderr", + err: errors.New("read failure"), + }) + if cause == nil { + t.Fatal("expected non-nil stream failure cause") + } + if got := cause.(error).Error(); got != "read failure" { + t.Fatalf("expected error cause, got %q", got) + } +} + +func TestSyftStreamFailureCauseReturnsTrimmedStderr(t *testing.T) { + cause := syftStreamFailureCause(syftStreamResult{ + stderr: " syft reported an error \n", + err: nil, + }) + if cause == nil { + t.Fatal("expected non-nil stream failure cause") + } + got, ok := cause.(string) + if !ok { + t.Fatalf("expected string cause from stderr, got %T", cause) + } + if got != "syft reported an error" { + t.Fatalf("expected trimmed stderr cause, got %q", got) + } +} + +func TestSyftStreamFailureCauseReturnsNilOnCleanStream(t *testing.T) { + cause := syftStreamFailureCause(syftStreamResult{ + stderr: " \n\t ", + err: nil, + }) + if cause != nil { + t.Fatalf("expected nil cause for clean stream, got %v", cause) + } +} diff --git a/home/internal/scanner/scanner.go b/home/internal/scanner/scanner.go index 7e4a37c2..fad1e641 100644 --- a/home/internal/scanner/scanner.go +++ b/home/internal/scanner/scanner.go @@ -2,6 +2,7 @@ package scanner import ( "context" + "errors" "fmt" "log" "sync" @@ -46,9 +47,9 @@ func NewScannerService(registry *services.Registry, cfg *models.ScannerConfig, d cancels: make(map[string]context.CancelFunc), } s.config.Store(cfg) - + go s.gcWorker() - + return s } @@ -391,11 +392,8 @@ func (s *ScannerService) runScan(ctx context.Context, job *models.ScanJob, cance completedAt := time.Now() if err != nil { - if ctx.Err() != nil { - s.updateJobStatus(job, models.ScanJobCancelled, "scan cancelled") - } else { - s.updateJobStatus(job, models.ScanJobFailed, err.Error()) - } + status, message := classifyScanFailure(ctx.Err(), err) + s.updateJobStatus(job, status, message) return } @@ -648,6 +646,19 @@ func (s *ScannerService) sendBulkNotification(bulkJob *models.BulkScanJob) { } } +func classifyScanFailure(ctxErr, scanErr error) (models.ScanJobStatus, string) { + if ctxErr != nil { + if errors.Is(ctxErr, context.DeadlineExceeded) { + return models.ScanJobFailed, "scan timed out" + } + return models.ScanJobCancelled, "scan cancelled" + } + if scanErr == nil { + return models.ScanJobFailed, "scan failed" + } + return models.ScanJobFailed, scanErr.Error() +} + func computeSummary(vulns []models.Vulnerability) models.SeveritySummary { summary := models.SeveritySummary{Total: len(vulns)} for _, v := range vulns { diff --git a/home/internal/scanner/scanner_test.go b/home/internal/scanner/scanner_test.go index 1748d10c..4c71b2d5 100644 --- a/home/internal/scanner/scanner_test.go +++ b/home/internal/scanner/scanner_test.go @@ -2,6 +2,7 @@ package scanner import ( "context" + "errors" "sync/atomic" "testing" "time" @@ -425,4 +426,36 @@ func TestHeartbeatDoesNotOverwriteNonScanningStatus(t *testing.T) { if progress != "original" { t.Fatalf("expected progress to remain 'original', got %q", progress) } -} \ No newline at end of file +} + +// ─── classifyScanFailure ────────────────────────────────────────────────────── + +func TestClassifyScanFailureDeadlineExceededIsFailed(t *testing.T) { + status, msg := classifyScanFailure(context.DeadlineExceeded, errors.New("scan runner error")) + if status != models.ScanJobFailed { + t.Fatalf("expected status %q, got %q", models.ScanJobFailed, status) + } + if msg != "scan timed out" { + t.Fatalf("expected timeout message, got %q", msg) + } +} + +func TestClassifyScanFailureCanceledIsCancelled(t *testing.T) { + status, msg := classifyScanFailure(context.Canceled, errors.New("scan runner error")) + if status != models.ScanJobCancelled { + t.Fatalf("expected status %q, got %q", models.ScanJobCancelled, status) + } + if msg != "scan cancelled" { + t.Fatalf("expected cancellation message, got %q", msg) + } +} + +func TestClassifyScanFailureNoContextUsesScanError(t *testing.T) { + status, msg := classifyScanFailure(nil, errors.New("scanner exploded")) + if status != models.ScanJobFailed { + t.Fatalf("expected status %q, got %q", models.ScanJobFailed, status) + } + if msg != "scanner exploded" { + t.Fatalf("expected scanner error message, got %q", msg) + } +}