Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

feat(cli): error out when ignore file cannot be found #7624

Merged
merged 2 commits into from
Oct 3, 2024
Merged

feat(cli): error out when ignore file cannot be found #7624

Show file tree
Hide file tree
Changes from 1 commit
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
f0d051d
feat(cli): error out when ignore file cannot be found
sgaist Sep 30, 2024
cc2e22a
refactor: move ignore file check to report flag handler
sgaist Oct 2, 2024
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
17 changes: 16 additions & 1 deletion pkg/result/filter_test.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1016,15 +1016,30 @@ func TestFilter(t *testing.T) {
})
}

ignoreFile := tt.args.ignoreFile
if ignoreFile == "" {
ignoreFile = result.DefaultIgnoreFile
}
err := result.Filter(ctx, tt.args.report, result.FilterOptions{
Severities: tt.args.severities,
VEXSources: vexSources,
IgnoreStatuses: tt.args.ignoreStatuses,
IgnoreFile: tt.args.ignoreFile,
IgnoreFile: ignoreFile,
PolicyFile: tt.args.policyFile,
})
require.NoError(t, err)
assert.Equal(t, tt.want, tt.args.report)
})
}

t.Run("Error on existent ignore file", func(t *testing.T) {
fakeTime := time.Date(2020, 8, 10, 7, 28, 17, 958601, time.UTC)
ctx := clock.With(context.Background(), fakeTime)
test := tests[0]

err := result.Filter(ctx, test.args.report, result.FilterOptions{
IgnoreFile: "invalid",
})
assert.ErrorContains(t, err, "invalid error: invalid does not exist")
})
}
3 changes: 3 additions & 0 deletions pkg/result/ignore.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -184,6 +184,9 @@ func (c *IgnoreConfig) MatchLicense(licenseID, filePath string) *IgnoreFinding {
func ParseIgnoreFile(ctx context.Context, ignoreFile string) (IgnoreConfig, error) {
var conf IgnoreConfig
if _, err := os.Stat(ignoreFile); errors.Is(err, fs.ErrNotExist) {
if ignoreFile != DefaultIgnoreFile {
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This check occurs at the end of scanning. We may want to fail on that at an earlier stage. Something like:

diff --git a/pkg/flag/report_flags.go b/pkg/flag/report_flags.go
index 67d553b65..d69443e89 100644
--- a/pkg/flag/report_flags.go
+++ b/pkg/flag/report_flags.go
@@ -6,6 +6,7 @@ import (

        "github.com/mattn/go-shellwords"
        "github.com/samber/lo"
+       "github.com/spf13/viper"
        "golang.org/x/xerrors"

        dbTypes "github.com/aquasecurity/trivy-db/pkg/types"
@@ -14,6 +15,7 @@ import (
        "github.com/aquasecurity/trivy/pkg/log"
        "github.com/aquasecurity/trivy/pkg/result"
        "github.com/aquasecurity/trivy/pkg/types"
+       "github.com/aquasecurity/trivy/pkg/utils/fsutils"
        xstrings "github.com/aquasecurity/trivy/pkg/x/strings"
 )

@@ -238,6 +240,10 @@ func (f *ReportFlagGroup) ToOptions() (ReportOptions, error) {
                }
        }

+       if viper.IsSet(f.IgnoreFile.ConfigName) && !fsutils.FileExists(f.IgnoreFile.Value()) {
+               return ReportOptions{}, xerrors.Errorf("ignore file not found: %s", f.IgnoreFile.Value())
+       }
+
        return ReportOptions{
                Format:           format,
                ReportFormat:     f.ReportFormat.Value(),

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Even better !

Thanks for improving my viper knowledge

return IgnoreConfig{}, xerrors.Errorf("%s does not exist", ignoreFile)
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Should we error out or just simply WARN the user that their ignorefile doesn't exist? @knqyf263 WDYT?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

If I may, while a warning might be enough when a user calls trivy explicitly, in an automated context, unless there's some warning detection put in place, it might get missed and thus the scan output will not match expectation.

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Fair enough. It looks better to return an error.

}
// .trivyignore doesn't necessarily exist
return IgnoreConfig{}, nil
} else if filepath.Ext(ignoreFile) == ".yml" || filepath.Ext(ignoreFile) == ".yaml" {
Expand Down