-
Notifications
You must be signed in to change notification settings - Fork 2.4k
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.
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
fix(misconf): don't shift ignore rule related to code #6708
Conversation
ids: []string{"rule-2"}, | ||
}, | ||
shouldIgnore: true, | ||
}, | ||
} | ||
|
||
for _, tt := range tests { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
nit: Since we can pass a slice of ids
of rules, should we create a test case as such:
{
name: "multiple ignore rules on the same line",
src: `# trivy:ignore:rule-1
# trivy:ignore:rule-2
test #trivy:ignore:rule-3
`,
args: args{
metadata: metadataWithLine(filename, 3),
ids: []string{"rule-1", "rule-2", "rule-3", "non-existing-rule"},
},
shouldIgnore: true,
},
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
done c572cca
if _, exists := rule.sections["id"]; !exists { | ||
return Rule{}, errors.New("rule section with the `ignore` key is required") | ||
if _, exists := sections["id"]; !exists { | ||
return nil, errors.New("rule section with the `ignore` key is required") |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Are there any callers of this func that we need to add a check for when we return nil
here? I only see this and it seems like this is covered.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
if comment parsing fails, we skip it https://github.com/aquasecurity/trivy/pull/6708/files#diff-c1fe13f24e97f852991501a2535948a2ff13ef4e36564313e85754d8f9c6f35eR58-R60
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Fair enough. I guess it is subjective as I only tend to return nil
for structs that are passed around by reference. But I'm fine with this.
Description
This PR prevents a range shift for a rule that is on the same line as the code.
Related issues
Checklist