fix: flatten errors correctly for directory based CSAF providers (#76)

## What

Flatten errors correctly for directory based CSAF poviders. 

Aligned error structure for directory based CSAF poviders. Previously
`errors.FlattenError` did only support ROLIE feed based csaf providers
as this feature was only targeted at restricted CSAF sources which are
always ROLIE feed based.

This was not clearly enough stated in the function doc comment which can
easily lead to incorrect results when using this function with directory
based CSAF providers. In this case `errors.FlattenError` was a no-op
just returning the original error.

## Why

We want to support fine grained error handling also for directory based
CSAF providers and avoid traps for the caller of `errors.FlattenError`.
The caller usually won't know if the CSAF provider is ROLIE or directory
based, unless it is a restricted CSAF provider which rules out the
latter case. Thus this would make it hard to rely on the output.

Author: llugin

cmd/csaf_downloader/downloader.go

@@ -492,11 +492,15 @@ func (dc *downloadContext) downloadAdvisory(
 		switch {
 		case resp.StatusCode == http.StatusUnauthorized:
 			errorCh <- csafErrs.ErrInvalidCredentials{Message: fmt.Sprintf("invalid credentials to retrieve CSAF document %s at URL %s: %s", filename, file.URL(), resp.Status)}
+		case resp.StatusCode == http.StatusForbidden:
+			// if we have access to the feed containing the document, we also must have access to itself, otherwise this indicates a problem with the provider
+			errorCh <- csafErrs.ErrCsafProviderIssue{Message: fmt.Sprintf("access denied to CSAF document %s at URL %s: %s", filename, file.URL(), resp.Status)}
 		case resp.StatusCode == http.StatusNotFound:
 			errorCh <- csafErrs.ErrCsafProviderIssue{Message: fmt.Sprintf("could not find CSAF document %s listed in table of content at URL %s: %s ", filename, file.URL(), resp.Status)}
 		case resp.StatusCode >= 500:
-			errorCh <- fmt.Errorf("could not retrieve CSAF document %s at URL %s: %s %w", filename, file.URL(), resp.Status, csafErrs.ErrRetryable) // mark as retryable error
-		default:
+			providerErr := csafErrs.ErrCsafProviderIssue{Message: fmt.Sprintf("could not retrieve CSAF document %s at URL %s: %s", filename, file.URL(), resp.Status)}
+			errorCh <- fmt.Errorf("%w %w", providerErr, csafErrs.ErrRetryable) // mark error as retryable as failure for server side errors are often temporary
+		default: // client error or fringe case
 			errorCh <- fmt.Errorf("could not retrieve CSAF document %s at URL %s: %s", filename, file.URL(), resp.Status)
 		}
 		dc.stats.downloadFailed++

csaf/advisories.go

@@ -187,6 +187,7 @@ func (afp *AdvisoryFileProcessor) Process(
 			dirURLs = []string{baseURL}
 		}
 
+		feedErrs := []error{} // errors encountered while processing directories/feeds
 		for _, base := range dirURLs {
 			if base == "" {
 				continue
@@ -195,13 +196,18 @@ func (afp *AdvisoryFileProcessor) Process(
 			// Use changes.csv to be able to filter by age.
 			files, err := afp.loadChanges(base, lg)
 			if err != nil {
-				return err
+				feedErrs = append(feedErrs, err)
+				continue
 			}
 			// XXX: Is treating as white okay? better look into the advisories?
 			if err := fn(TLPLabelWhite, files); err != nil {
-				return err
+				feedErrs = append(feedErrs, err)
 			}
 		}
+
+		if len(feedErrs) > 0 {
+			return &errs.CompositeErrFeed{Errs: feedErrs}
+		}
 	} // TODO: else scan directories?
 	return nil
 }
@@ -214,23 +220,37 @@ func (afp *AdvisoryFileProcessor) loadChanges(
 ) ([]AdvisoryFile, error) {
 	base, err := url.Parse(baseURL)
 	if err != nil {
-		return nil, err
+		return nil, errs.ErrCsafProviderIssue{Message: fmt.Sprintf("invalid directory url %s: %v", baseURL, err)}
 	}
 	changesURL := base.JoinPath("changes.csv").String()
 
 	resp, err := afp.client.Get(changesURL)
 	if err != nil {
-		return nil, err
+		return nil, errs.ErrNetwork{Message: fmt.Sprintf("failed get request for url %s: %v", changesURL, err)}
 	}
 
 	if resp.StatusCode != http.StatusOK {
-		return nil, fmt.Errorf("fetching %s failed. Status code %d (%s)",
-			changesURL, resp.StatusCode, resp.Status)
+		switch { // we don't expect 401 and 403, as directory based feeds are supposed to be public, but just to be on the safe side
+		case resp.StatusCode == http.StatusUnauthorized:
+			return nil, errs.ErrInvalidCredentials{Message: fmt.Sprintf("invalid credentials for accessing %s: %s", changesURL, resp.Status)}
+		case resp.StatusCode == http.StatusForbidden:
+			return []AdvisoryFile{}, nil // user has insufficient permissions to access feed, no error
+		case resp.StatusCode == http.StatusNotFound:
+			return nil, errs.ErrCsafProviderIssue{Message: fmt.Sprintf("could not find changes.csv at %s: %s", changesURL, resp.Status)}
+		case resp.StatusCode >= 500:
+			providerErr := errs.ErrCsafProviderIssue{Message: fmt.Sprintf("could not retrieve changes.csv at %s: %s", changesURL, resp.Status)}
+			return nil, fmt.Errorf("%w %w", providerErr, errs.ErrRetryable) // mark error as retryable as failure for server side errors are often temporary
+		default: // client error or fringe case
+			return nil, fmt.Errorf("could not retrieve changes.csv at %s: %s", changesURL, resp.Status)
+		}
 	}
 
 	defer resp.Body.Close()
 	var files []AdvisoryFile
 	c := csv.NewReader(resp.Body)
+	// format specification:
+	// https://docs.oasis-open.org/csaf/csaf/v2.0/os/csaf-v2.0-os.html#7113-requirement-13-changescsv
+	c.FieldsPerRecord = 2
 	const (
 		pathColumn = 0
 		timeColumn = 1
@@ -241,16 +261,12 @@ func (afp *AdvisoryFileProcessor) loadChanges(
 			break
 		}
 		if err != nil {
-			return nil, err
-		}
-		if len(r) < 2 {
-			lg(slog.LevelError, "Not enough columns", "line", line)
-			continue
+			return nil, errs.ErrCsafProviderIssue{Message: fmt.Sprintf("could not read record from changes.csv: %v", err)}
 		}
 		t, err := time.Parse(time.RFC3339, r[timeColumn])
 		if err != nil {
 			lg(slog.LevelError, "Invalid time stamp in line", "url", changesURL, "line", line, "err", err)
-			continue
+			return nil, errs.ErrCsafProviderIssue{Message: fmt.Sprintf("could not read timestamp from changes.csv: %v", err)}
 		}
 		// Apply date range filtering.
 		if afp.AgeAccept != nil && !afp.AgeAccept(t) {
@@ -259,7 +275,7 @@ func (afp *AdvisoryFileProcessor) loadChanges(
 		path := r[pathColumn]
 		if _, err := url.Parse(path); err != nil {
 			lg(slog.LevelError, "Contains an invalid URL", "url", changesURL, "path", path, "line", line)
-			continue
+			return nil, errs.ErrCsafProviderIssue{Message: fmt.Sprintf("could not read url from changes.csv: %v", err)}
 		}
 
 		files = append(files,
@@ -320,13 +336,14 @@ func (afp *AdvisoryFileProcessor) processROLIE(
 			switch {
 			case res.StatusCode == http.StatusUnauthorized:
 				feedErrs = append(feedErrs, errs.ErrInvalidCredentials{Message: fmt.Sprintf("invalid credentials for TLP:%s ROLIE feed at %s: %s", label, feedURL.String(), res.Status)})
-			case res.StatusCode == http.StatusNotFound:
-				feedErrs = append(feedErrs, errs.ErrCsafProviderIssue{Message: fmt.Sprintf("could not find TLP:%s ROLIE feed at %s: %s", label, feedURL.String(), res.Status)})
 			case res.StatusCode == http.StatusForbidden:
 				// user has insufficient permissions to access feed, no error
-			case res.StatusCode > 500:
-				feedErrs = append(feedErrs, fmt.Errorf("could not retrieve TLP:%s ROLIE feed at %s: %s %w", label, feedURL.String(), res.Status, errs.ErrRetryable)) // mark error as retryable
-			default:
+			case res.StatusCode == http.StatusNotFound:
+				feedErrs = append(feedErrs, errs.ErrCsafProviderIssue{Message: fmt.Sprintf("could not find TLP:%s ROLIE feed at %s: %s", label, feedURL.String(), res.Status)})
+			case res.StatusCode >= 500:
+				providerErr := errs.ErrCsafProviderIssue{Message: fmt.Sprintf("could not retrieve TLP:%s ROLIE feed at %s: %s", label, feedURL.String(), res.Status)}
+				feedErrs = append(feedErrs, fmt.Errorf("%w %w", providerErr, errs.ErrRetryable)) // mark error as retryable as failure for server side errors are often temporary
+			default: // client error or fringe case
 				feedErrs = append(feedErrs, fmt.Errorf("could not retrieve TLP:%s ROLIE feed at %s: %s", label, feedURL.String(), res.Status))
 			}
 			continue
@@ -427,7 +444,7 @@ func (afp *AdvisoryFileProcessor) processROLIE(
 		}
 	}
 	if len(feedErrs) > 0 {
-		return &errs.CompositeErrRolieFeed{Errs: feedErrs}
+		return &errs.CompositeErrFeed{Errs: feedErrs}
 	}
 	return nil
 }

pkg/errs/errors.go

@@ -47,14 +47,14 @@ func (e ErrInvalidCredentials) Error() string {
 
 var ErrRetryable = errors.New("(retryable error)")
 
-// CompositeErrRolieFeed holds an array of errors which encountered during processing rolie feeds
-type CompositeErrRolieFeed struct {
+// CompositeErrFeed holds an array of errors which encountered during processing rolie feeds
+type CompositeErrFeed struct {
 	Errs []error
 }
 
-func (e *CompositeErrRolieFeed) Error() string {
+func (e *CompositeErrFeed) Error() string {
 	if len(e.Errs) == 0 {
-		return "empty CompositeErrRolieFeed"
+		return "empty CompositeErrFeed"
 	}
 
 	messages := make([]string, 0, len(e.Errs))
@@ -64,7 +64,7 @@ func (e *CompositeErrRolieFeed) Error() string {
 	return strings.Join(messages, "\n")
 }
 
-func (e *CompositeErrRolieFeed) Unwrap() []error {
+func (e *CompositeErrFeed) Unwrap() []error {
 	return e.Errs
 }
 
@@ -89,10 +89,10 @@ func (e *CompositeErrCsafDownload) Unwrap() []error {
 	return e.Errs
 }
 
-// FlattenError flattens out all composite errors (note: discards the errors wrapped around [CompositeErrRolieFeed] or [CompositeErrCsafDownload])
-// The assumed structure is CompositeErrRolieFeed{Errs: []error{...,CompositeErrCsafDownload,...,CompositeErrCsafDownload,...}}.
+// FlattenError flattens out all composite errors (note: discards the errors wrapped around [CompositeErrFeed] or [CompositeErrCsafDownload])
+// The assumed structure is CompositeErrFeed{Errs: []error{...,CompositeErrCsafDownload,...,CompositeErrCsafDownload,...}}.
 func FlattenError(err error) (flattenedErrors []error) {
-	var rolieErrs *CompositeErrRolieFeed
+	var rolieErrs *CompositeErrFeed
 	if errors.As(err, &rolieErrs) {
 		for _, rolieErr := range rolieErrs.Unwrap() {
 			var csafDlErrs *CompositeErrCsafDownload

pkg/errs/errors_test.go

@@ -26,21 +26,21 @@ func TestFlattenError(t *testing.T) {
 
 		compositeErrCsafDownload := &CompositeErrCsafDownload{Errs: csafDownloadErrsFlat}
 
-		singleRolieFeedErrs := []error{
-			errors.New("single error rolie feed 1"),
-			errors.New("single error rolie feed 2"),
+		singleFeedErrs := []error{
+			errors.New("single error feed 1"),
+			errors.New("single error feed 2"),
 		}
 
-		rolieFeedCompositeErr := CompositeErrRolieFeed{
+		feedCompositeErr := CompositeErrFeed{
 			Errs: append(
-				singleRolieFeedErrs,
-				fmt.Errorf("issues during downloader of rolie: %w", compositeErrCsafDownload),
+				singleFeedErrs,
+				fmt.Errorf("issues during download of feed: %w", compositeErrCsafDownload),
 				compositeErrCsafDownload,
 			),
 		}
-		wantFlattenedErrors := slices.Concat(singleRolieFeedErrs, csafDownloadErrsFlat, csafDownloadErrsFlat)
+		wantFlattenedErrors := slices.Concat(singleFeedErrs, csafDownloadErrsFlat, csafDownloadErrsFlat)
 
-		gotFlattenedErrors := FlattenError(fmt.Errorf("wrap rolie feed composite err: %w", &rolieFeedCompositeErr))
+		gotFlattenedErrors := FlattenError(fmt.Errorf("wrap feed composite err: %w", &feedCompositeErr))
 
 		assert.ElementsMatch(t, wantFlattenedErrors, gotFlattenedErrors)
 	})