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

FAI-12709: Refactor Vanta Source to pull generic vulns + remediations #1759

Open
wants to merge 18 commits into
base: main
Choose a base branch
from

Conversation

matiaslcoulougian
Copy link
Contributor

@matiaslcoulougian matiaslcoulougian commented Oct 29, 2024

Description

Replace GraphQL Queries by integration with generic vulnerability endpoints. Add vulnerability update to resolved status.

TODO:

  • Add cicd artifacts query to create cicd_ArtifactVulnerability*
  • Implement cutoff days and date filtering in source
  • Update tests

Provide description here

Type of change

  • Bug fix
  • New feature
  • Breaking change

Related issues

Fix #1

Migration notes

Describe migration notes if any

Extra info

Add any additional information

@matiaslcoulougian matiaslcoulougian marked this pull request as ready for review November 13, 2024 14:30
Copy link

sonarcloud bot commented Nov 15, 2024

Copy link
Contributor

@chalenge chalenge left a comment

Choose a reason for hiding this comment

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

let's use the axios instance from faros-js-utils

@@ -1,5 +1,5 @@
query cicdArtifactQueryByCommitSha($commitShas: [String], $limit: Int) {
cicd_Artifact(where: {uid: {_in: $commitShas}}, limit: $limit, distinct_on: uid) {
Copy link
Contributor

Choose a reason for hiding this comment

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

isn't this more efficient instead of querying one by one?

query vcsRepositoryQuery($vcsRepoNames: [String], $limit: Int) {
vcs_Repository(where: {name: {_in: $vcsRepoNames}}, limit: $limit, distinct_on: name) {
query vcsRepositoryQuery($vcsRepoName: String!) {
vcs_Repository(where: {name: {_eq: $vcsRepoName}}, distinct_on: name) {
Copy link
Contributor

Choose a reason for hiding this comment

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

same as above

export abstract class VulnerabilityRemediations extends Converter {
source = 'vanta';
readonly destinationModels: ReadonlyArray<DestinationModel> = [];
severityMap: {[key: string]: number} = {
Copy link
Contributor

Choose a reason for hiding this comment

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

can we document where these category values are coming from?

import {getQueryFromName} from './utils';

export abstract class VulnerabilityRemediations extends Converter {
source = 'vanta';
Copy link
Contributor

Choose a reason for hiding this comment

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

is it vanta or Vanta?

"$schema": "http://json-schema.org/draft-04/schema#",
"type": "object",
"properties": {
"data": {}
Copy link
Contributor

Choose a reason for hiding this comment

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

add a proper spec

@@ -65,47 +110,180 @@ export class Vanta {
};
try {
const packed_response: AxiosResponse = await this.getAxiosResponse(
this.apiUrl,
this.apiUrl + '/graphql',
Copy link
Contributor

Choose a reason for hiding this comment

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

unless we are using this.apiUrl for other things, let's remove it as class property and add baseUrl when we create the axiosInstance

cursor: string | null,
slaDeadlineAfterDate: Date
): Promise<any> {
const url = `${this.apiUrl}v1/vulnerabilities`;
Copy link
Contributor

Choose a reason for hiding this comment

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

see comment above to attach this.apiUrl as baseUrl of instance

}

return assetMap;
}

async getAxiosResponse(
Copy link
Contributor

Choose a reason for hiding this comment

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

Don't think we need this function if we use the faros axiosInstance. It already has retry logic backed in and/or we can pass logic specific to this API.

Even if we need this function it will become cleaner


for (const vulnerability of data) {
const asset = assetMap.get(vulnerability.targetId);
// Image tage scan won't be available if asset is a repository: https://developer.vanta.com/reference/listvulnerableassets#:~:text=has%20been%20scanned.-,imageScanTag,-string%20%7C%20null
Copy link
Contributor

Choose a reason for hiding this comment

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

some weird linting or line breaks

if (relatedEntity.repository) {
return [
{
model: 'vcs_RepositoryVulnerability__Update',
Copy link
Contributor

Choose a reason for hiding this comment

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

Do we really need to query the graph for this? Would a blanket update without relatedEntity.repository.name, work? Is there any real risk when we do that?

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.

3 participants