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.

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

Handle ignored cves #5804

Merged
merged 17 commits into from
Nov 29, 2024
Merged

Handle ignored cves #5804

merged 17 commits into from
Nov 29, 2024

Conversation

FestiveKyle
Copy link
Collaborator

@FestiveKyle FestiveKyle commented Oct 11, 2024

  • Exclude ignored CVEs from AdditionalFindings (frontend only)
  • Exclude ignored CVEs from CSV export
  • Log action to every verified org with claim to domain
    • Log as an "UPDATE" action with updated properties of "name: , "oldValue": "ignored", "newValue": "unignored"

@FestiveKyle FestiveKyle marked this pull request as ready for review November 27, 2024 17:08
Comment on lines +189 to +216
// Log activity for super admin logging
await logActivity({
transaction,
collections,
query,
initiatedBy: {
id: user._key,
userName: user.userName,
role: 'super_admin',
},
action: 'update',
target: {
resource: domain.domain,
resourceType: 'domain',
updatedProperties: [
{
name: ignoredCve,
oldValue: 'unignored',
newValue: 'ignored',
},
],
},
})
} catch (err) {
console.error(
`Database error occurred when user: "${userKey}" attempted to ignore CVE "${ignoredCve}" on domain "${domainId}" during activity logs, error: ${err}`,
)
}
Copy link
Collaborator

Choose a reason for hiding this comment

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

Super admins will be able to see the other logs generated. Not sure we need this.

Comment on lines +189 to +216
// Log activity for super admin logging
await logActivity({
transaction,
collections,
query,
initiatedBy: {
id: user._key,
userName: user.userName,
role: 'super_admin',
},
action: 'update',
target: {
resource: domain.domain,
resourceType: 'domain',
updatedProperties: [
{
name: ignoredCve,
oldValue: 'ignored',
newValue: 'unignored',
},
],
},
})
} catch (err) {
console.error(
`Database error occurred when user: "${userKey}" attempted to unignore CVE "${ignoredCve}" on domain "${domainId}" during activity logs, error: ${err}`,
)
}
Copy link
Collaborator

Choose a reason for hiding this comment

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

Supers can see other logs, not sure this is needed

</Text>
) : (
<SimpleGrid columns={8}>
{undetectedIgnoredCves &&
Copy link
Collaborator

Choose a reason for hiding this comment

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

empty or null array already checked for

Comment on lines -54 to 105
ignoredCves: PropTypes.array.isRequired,
undetectedIgnoredCves: PropTypes.arrayOf(PropTypes.string.isRequired).isRequired,
detectedIgnoredCves: PropTypes.object.isRequired,
setActiveCveHandler: PropTypes.func.isRequired,
Copy link
Collaborator

Choose a reason for hiding this comment

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

is it better to import the whole object or the individual prop types? if no performance/memory advantage this is fine

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

I don't think there would be any performance loss either way. We already have these separated and the undetectedIgnoredCves are just strings since we aren't getting the vuln level from the api.

@FestiveKyle FestiveKyle merged commit 88d2b29 into master Nov 29, 2024
31 checks passed
@FestiveKyle FestiveKyle deleted the handle-ignored-cves branch November 29, 2024 14:01
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants