Skip to content

Commit ff5aea3

Browse files
egibswlynch
egibs
and
wlynch
authored
Remove FileReport Error field in favor of custom error type (#777)
* Remove FileReport Error field in favor of custom error type Signed-off-by: egibs <20933572+egibs@users.noreply.github.com> * Address PR comments Signed-off-by: egibs <20933572+egibs@users.noreply.github.com> * Reason is redundant Signed-off-by: egibs <20933572+egibs@users.noreply.github.com> * Update pkg/action/scan_error.go Co-authored-by: Billy Lynch <1844673+wlynch@users.noreply.github.com> Signed-off-by: Evan Gibler <20933572+egibs@users.noreply.github.com> --------- Signed-off-by: egibs <20933572+egibs@users.noreply.github.com> Signed-off-by: Evan Gibler <20933572+egibs@users.noreply.github.com> Co-authored-by: Billy Lynch <1844673+wlynch@users.noreply.github.com>
1 parent 108a013 commit ff5aea3

File tree

7 files changed

+110
-32
lines changed

7 files changed

+110
-32
lines changed

‎pkg/action/diff.go‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -96,7 +96,7 @@ func relFileReport(ctx context.Context, c malcontent.Config, fromPath string) (m
9696
}
9797
if fr, ok := value.(*malcontent.FileReport); ok {
9898
isArchive := fr.ArchiveRoot != ""
99-
if fr.Skipped != "" || fr.Error != "" {
99+
if fr.Skipped != "" {
100100
return true
101101
}
102102
rel, base, err = relPath(fromPath, fr, isArchive)

‎pkg/action/scan.go‎

Lines changed: 30 additions & 21 deletions
Original file line numberDiff line numberDiff line change
@@ -92,28 +92,24 @@ func scanSinglePath(ctx context.Context, c malcontent.Config, path string, ruleF
9292
mrs, err := yrs.Scan(fc)
9393
if err != nil {
9494
logger.Debug("skipping", slog.Any("error", err))
95-
return &malcontent.FileReport{Path: path, Error: fmt.Sprintf("scan: %v", err)}, nil
95+
return nil, err
9696
}
9797

9898
fr, err := report.Generate(ctx, path, mrs, c, archiveRoot, logger, fc)
9999
if err != nil {
100-
return nil, err
101-
}
102-
103-
if fr.Error != "" {
104-
return &malcontent.FileReport{Path: path, Error: fmt.Sprintf("generate: %v", fr.Error)}, nil
100+
return nil, NewFileReportError(err, path, TypeGenerateError)
105101
}
106102

107103
// Clean up the path if scanning an archive
108104
var clean string
109105
if isArchive {
110106
pathAbs, err := filepath.Abs(path)
111107
if err != nil {
112-
return nil, err
108+
return nil, NewFileReportError(err, path, TypeGenerateError)
113109
}
114110
archiveRootAbs, err := filepath.Abs(archiveRoot)
115111
if err != nil {
116-
return nil, err
112+
return nil, NewFileReportError(err, path, TypeGenerateError)
117113
}
118114
fr.ArchiveRoot = archiveRootAbs
119115
fr.FullPath = pathAbs
@@ -168,9 +164,6 @@ func exitIfHitOrMiss(frs *sync.Map, scanPath string, errIfHit bool, errIfMiss bo
168164
if fr.Skipped != "" {
169165
return true
170166
}
171-
if fr.Error != "" {
172-
return true
173-
}
174167
filesScanned++
175168
if len(fr.Behaviors) > 0 && match == nil {
176169
match = fr
@@ -446,7 +439,7 @@ func handleSingleFile(ctx context.Context, path string, scanInfo scanPathInfo, c
446439
}
447440

448441
fr, err := processFile(ctx, c, c.RuleFS, path, scanInfo.effectivePath, trimPath, logger)
449-
if err != nil && c.Renderer.Name() != interactive {
442+
if err != nil && (c.Renderer == nil || c.Renderer.Name() != interactive) {
450443
if len(c.TrimPrefixes) > 0 {
451444
path = report.TrimPrefixes(path, c.TrimPrefixes)
452445
}
@@ -569,26 +562,42 @@ func processArchive(ctx context.Context, c malcontent.Config, rfs []fs.FS, archi
569562
return &frs, nil
570563
}
571564

565+
// handleFileReportError returns the appropriate FileReport and error depending on the type of error.
566+
func handleFileReportError(err error, path string, logger *clog.Logger) (*malcontent.FileReport, error) {
567+
var fileErr *FileReportError
568+
if !errors.As(err, &fileErr) {
569+
return nil, fmt.Errorf("failed to handle error for path %s: error type not FileReportError: %w", path, err)
570+
}
571+
572+
switch fileErr.Type() {
573+
case TypeUnknown:
574+
return nil, fmt.Errorf("unknown error occurred while scanning path %s: %w", path, err)
575+
case TypeScanError:
576+
logger.Errorf("scan path: %v", err)
577+
return nil, fmt.Errorf("scan failed for path %s: %w", path, err)
578+
case TypeGenerateError:
579+
return &malcontent.FileReport{
580+
Path: path,
581+
Skipped: errMsgGenerateFailed,
582+
}, nil
583+
default:
584+
return nil, fmt.Errorf("unhandled error type scanning path %s: %w", path, err)
585+
}
586+
}
587+
572588
// processFile scans a single output file, rendering live output if available.
573589
func processFile(ctx context.Context, c malcontent.Config, ruleFS []fs.FS, path string, scanPath string, archiveRoot string, logger *clog.Logger) (*malcontent.FileReport, error) {
574590
logger = logger.With("path", path)
575591

576592
fr, err := scanSinglePath(ctx, c, path, ruleFS, scanPath, archiveRoot)
577-
if err != nil && c.Renderer.Name() != interactive {
578-
logger.Errorf("scan path: %v", err)
579-
return nil, err
593+
if err != nil && (c.Renderer == nil || c.Renderer.Name() != interactive) {
594+
return handleFileReportError(err, path, logger)
580595
}
581596

582597
if fr == nil {
583-
logger.Debugf("%s returned nil result", path)
584598
return nil, nil
585599
}
586600

587-
if fr.Error != "" && c.Renderer.Name() != interactive {
588-
logger.Errorf("scan error: %s", fr.Error)
589-
return nil, fmt.Errorf("report error: %v", fr.Error)
590-
}
591-
592601
return fr, nil
593602
}
594603

‎pkg/action/scan_error.go‎

Lines changed: 78 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,78 @@
1+
package action
2+
3+
import "fmt"
4+
5+
// Error message constants for NewFileReportError reasons.
6+
// If the compiled rules are invalid or the scanner malfunctions, yrs.Scan will fail.
7+
// If mrs is nil or Generate fails, the file report will be nil.
8+
const (
9+
errMsgUnknown = "unknown error"
10+
errMsgScanFailed = "scan failed"
11+
errMsgGenerateFailed = "failed to generate file report"
12+
)
13+
14+
type ErrorType int
15+
16+
// Error type iotas.
17+
const (
18+
// TypeUnknown will be the default of `0`.
19+
TypeUnknown ErrorType = iota
20+
// TypeScanError is to be used when compiled rules are invalid or the scan fails otherwise.
21+
TypeScanError
22+
// TypeGenerateError is to be used when a file's report cannot be created.
23+
TypeGenerateError
24+
)
25+
26+
// FileReportError is a custom error type to hold the error, path, and vanity reason.
27+
type FileReportError struct {
28+
err error
29+
path string
30+
reason ErrorType
31+
}
32+
33+
// NewFileReportError returns a new FileReportError.
34+
func NewFileReportError(err error, path string, reason ErrorType) *FileReportError {
35+
return &FileReportError{
36+
err: err,
37+
path: path,
38+
reason: reason,
39+
}
40+
}
41+
42+
func (e *FileReportError) errMsg() string {
43+
switch e.reason {
44+
case TypeUnknown:
45+
return errMsgUnknown
46+
case TypeScanError:
47+
return errMsgScanFailed
48+
case TypeGenerateError:
49+
return errMsgGenerateFailed
50+
default:
51+
return fmt.Sprintf("unknown error type(%d)", e.reason)
52+
}
53+
}
54+
55+
func (e *FileReportError) Error() string {
56+
if e.err == nil {
57+
return fmt.Sprintf("%s: %s", e.errMsg(), e.path)
58+
}
59+
return fmt.Sprintf("%s: %s: %v", errMsgUnknown, e.path, e.err)
60+
}
61+
62+
func (e *FileReportError) Is(target error) bool {
63+
t, ok := target.(*FileReportError)
64+
if !ok {
65+
return false
66+
}
67+
return e.path == t.path && e.reason == t.reason
68+
}
69+
70+
func (e *FileReportError) Path() string { return e.path }
71+
72+
func (e *FileReportError) Type() ErrorType {
73+
return e.reason
74+
}
75+
76+
func (e *FileReportError) Unwrap() error {
77+
return e.err
78+
}

‎pkg/malcontent/malcontent.go‎

Lines changed: 0 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -79,7 +79,6 @@ type FileReport struct {
7979
SHA256 string
8080
Size int64
8181
// compiler -> x
82-
Error string `json:",omitempty" yaml:",omitempty"`
8382
Skipped string `json:",omitempty" yaml:",omitempty"`
8483
Meta map[string]string `json:",omitempty" yaml:",omitempty"`
8584
Syscalls []string `json:",omitempty" yaml:",omitempty"`

‎pkg/render/markdown.go‎

Lines changed: 0 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -154,12 +154,6 @@ func (r Markdown) Full(ctx context.Context, _ *malcontent.Config, rep *malconten
154154
}
155155

156156
func markdownTable(_ context.Context, fr *malcontent.FileReport, w io.Writer, rc tableConfig) {
157-
path := fr.Path
158-
if fr.Error != "" {
159-
fmt.Printf("⚠️ %s - error: %s\n", path, fr.Error)
160-
return
161-
}
162-
163157
if fr.Skipped != "" {
164158
return
165159
}

‎pkg/render/tea.go‎

Lines changed: 0 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -363,8 +363,6 @@ func (r *Interactive) File(ctx context.Context, fr *malcontent.FileReport) error
363363

364364
var content string
365365
switch {
366-
case fr.Error != "":
367-
content = fmt.Sprintf("error scanning %s: %s", fr.Path, fr.Error)
368366
case fr.Skipped != "":
369367
content = fmt.Sprintf("skipped %s: %s", fr.Path, fr.Skipped)
370368
case len(fr.Behaviors) > 0:

‎pkg/report/report.go‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -367,7 +367,7 @@ func TrimPrefixes(path string, prefixes []string) string {
367367
//nolint:cyclop // ignore complexity of 64
368368
func Generate(ctx context.Context, path string, mrs *yarax.ScanResults, c malcontent.Config, expath string, _ *clog.Logger, fc []byte) (*malcontent.FileReport, error) {
369369
if mrs == nil {
370-
return &malcontent.FileReport{Path: path, Error: fmt.Sprintf("yara-x scan results are nil for %s", path)}, nil
370+
return nil, fmt.Errorf("scan failed")
371371
}
372372

373373
ignoreTags := c.IgnoreTags

0 commit comments

Comments
 (0)