Skip to content
This repository has been archived by the owner on Aug 16, 2024. It is now read-only.

Sonarqube filter params sdk 13.x #54

Merged
merged 23 commits into from
Aug 7, 2024
Merged

Conversation

Bharatkk-metron
Copy link
Collaborator

@Bharatkk-metron Bharatkk-metron commented Jul 24, 2024

filter params added
JIra Ticket: IA-153

@Bharatkk-metron Bharatkk-metron requested a review from a team as a code owner July 24, 2024 13:53
Copy link

socket-security bot commented Jul 24, 2024

👍 Dependency issues cleared. Learn more about Socket for GitHub ↗︎

This PR previously contained dependency changes with security issues that have been resolved, removed, or ignored.

View full report↗︎

.env.example Outdated
ENABLE_FINDINGS_INGESTION=true/false

# JUST FOR INFO:

Choose a reason for hiding this comment

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

No need to mention just for info

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

removed.

Choose a reason for hiding this comment

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

Ok

Choose a reason for hiding this comment

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

This is a new file got committed. Is it required?

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Same as a yarn lock. Since we updated the package versions, we can have this.

Choose a reason for hiding this comment

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

ok

description: 'SonarQube Accounts',
defaultsToDisabled: false,
},

Choose a reason for hiding this comment

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

Please remove extra line

yarn.lock Outdated

Choose a reason for hiding this comment

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

is this file required to be modified?

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

yes. I think since we have updated the version.

Choose a reason for hiding this comment

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

Ok

tsconfig.json Outdated
@@ -1,7 +1,8 @@
{
"extends": "./node_modules/@jupiterone/integration-sdk-dev-tools/config/typescript",
"compilerOptions": {
"outDir": "dist"
"outDir": "dist",
"lib": ["dom"]

Choose a reason for hiding this comment

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

Why this change?

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Without this change yarn test:ci was failing. Was not able to find some global variables. Which was basically coming from Dom Library.

Choose a reason for hiding this comment

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

Okay

Copy link

⚠️  1 New Security Finding

The latest commit contains 1 new security finding.

Findings
Dependency: npm / fast-xml-parser@ 4.2.5

Vulnerability Information
Vulnerability Severity CVSS EPSS Affected
Versions
Fixed
Versions
Contains
Malware
Description
CVE-2024-41818 Critical 7.5 4.4.1
no fast-xml-parser vulnerable to ReDOS at currency parsing
Dependency Location
https://github.com/JupiterOne/graph-sonarqube/blob/3f17bce98971aa6a7a0c795b354fa7f058b8c67b/yarn.lock

Not a finding? Ignore it by adding a comment on the line with just the word noboost.

Scanner: boostsecurity - BoostSecurity osv-scanner

Comment on lines 5 to 9
[INGESTION_SOURCE_IDS.ACCOUNT]: {
title: 'Account',
description: 'SonarQube Accounts',
defaultsToDisabled: false,
},
Copy link
Contributor

Choose a reason for hiding this comment

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

This ingestion source is not required, in general without account the rest of the steps wouldn't run, and we don't fetch anything so it's ok to always have it

Comment on lines 16 to 35
severities: {
type: 'string',
mask: false,
optional: true,
},
status: {
type: 'string',
mask: false,
optional: true,
},
createdInLast: {
type: 'string',
mask: false,
optional: true,
},
types: {
type: 'string',
mask: false,
optional: true,
},
Copy link
Contributor

Choose a reason for hiding this comment

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

Suggested change
severities: {
type: 'string',
mask: false,
optional: true,
},
status: {
type: 'string',
mask: false,
optional: true,
},
createdInLast: {
type: 'string',
mask: false,
optional: true,
},
types: {
type: 'string',
mask: false,
optional: true,
},
findingSeverities: {
type: 'string',
mask: false,
optional: true,
},
findingStatus: {
type: 'string',
mask: false,
optional: true,
},
findingsIngestSinceDays: {
type: 'string',
mask: false,
optional: true,
},
findingTypes: {
type: 'string',
mask: false,
optional: true,
},

.env.example Outdated
#To retrieve issues created during a time span before the current time (exclusive). Accepted units are 'y' for year, 'm' for month, 'w' for week and 'd' for day. If this parameter is set, createdAfter must not be set
# Example value: 1m2w (1 month 2 weeks)
CREATED_IN_LAST=10d # default value is 10 days
Copy link
Contributor

Choose a reason for hiding this comment

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

Please change this name to FINDINGS_INGEST_SINCE_DAYS and make it a number where options are 90, 180, 275, 365 (days). Then you can convert those to the format sonarqube expects when you send the request. This way we are not attached to using their format in our config.
Github example:
image

Comment on lines 31 to 44
function getSeverities(instanceConfig: SonarqubeIntegrationConfig) {
const { severities, apiVersion } = instanceConfig;
const severitiesSet = new Set(
severities?.split(',').map((severity) => FINDINGS_SEVERITIES[severity]),
);

// V2 -> 10.4 or above version
if (apiVersion == APIVersion.V1) {
return severities ? severities.split(',') : V1_SEVERITIES_VALUES;
}
return severitiesSet.size === 0
? V2_SEVERITIES_VALUES
: Array.from(severitiesSet);
}
Copy link
Contributor

Choose a reason for hiding this comment

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

Can you validate the config for this in validateInvocation and parse it there instead? So when this step starts the config value is ready to use
Example: https://github.com/JupiterOne/integrations/blob/main/graph/graph-gitlab/src/config.ts#L70

@@ -34,24 +95,38 @@ export async function fetchFindings({
// We need to further filter our API calls in order to minimize the chances
// we'll hit the 10,000 limit impose by the API. We're currently filtering
// by project and severity.
for (const severity of severityList) {
for (const severity of severities) {
Copy link
Contributor

Choose a reason for hiding this comment

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

👍

Comment on lines 46 to 73
function getFilterParams(
instanceConfig: SonarqubeIntegrationConfig,
): NodeJS.Dict<string | string[]> {
const { apiVersion, status, createdInLast, types } = instanceConfig;

let filterParams: NodeJS.Dict<string | string[]>;

if (apiVersion === APIVersion.V1) {
// V1 -> below 10.4 version
filterParams = {
status,
types,
};
} else {
const statusesSet = new Set(
status?.split(',').map((status) => FINDING_STATUSES[status]),
);
const typesSet = new Set(
types?.split(',').map((type) => FINDING_TYPES[type]),
);
filterParams = {
issueStatuses: Array.from(statusesSet).join(','),
impactSoftwareQualities: Array.from(typesSet).join(','),
};
}
filterParams['createdInLast'] = createdInLast || DEFAULT_CREATED_IN_LAST;
return filterParams;
}
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 with severities, it should be validated and parsed in validateInvocation

src/types.ts Outdated
Comment on lines 17 to 20
severities?: string;
status?: string;
createdInLast?: string;
types?: string;
Copy link
Contributor

Choose a reason for hiding this comment

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

Suggested change
severities?: string;
status?: string;
createdInLast?: string;
types?: string;
findingSeverities?: string[];
findingStatus?: string[];
findingsIngestSinceDays?: number;
findingTypes?: string[];

@@ -21,6 +79,9 @@ export async function fetchFindings({
}: IntegrationStepExecutionContext<SonarqubeIntegrationConfig>) {
const client = createSonarqubeClient(instance.config, logger);

const severities = getSeverities(instance.config);
const filterParams = getFilterParams(instance.config);
Copy link
Contributor

Choose a reason for hiding this comment

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

can you make sure we don't send any of these if they come undefined in instanceConfig instead of using defaults? so only new instances use those filters

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 have removed the function getSeverities. For getFilterParams, we will use the values only if they are present; otherwise, we will not include anything in filterParams.

@@ -19,6 +24,7 @@ export const accountSteps: IntegrationStep<SonarqubeIntegrationConfig>[] = [
{
id: Steps.ACCOUNT,
name: 'Fetch Account',
ingestionSourceId: INGESTION_SOURCE_IDS.ACCOUNT,
Copy link
Contributor

Choose a reason for hiding this comment

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

remove this

};

export const INGESTION_SOURCE_IDS = {
ACCOUNT: 'accounts',
Copy link
Contributor

Choose a reason for hiding this comment

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

remove this one

Comment on lines 47 to 48
filterParams['createdInLast'] =
`${findingsIngestSinceDays || DEFAULT_FINDING_INGEST_SINCE_DAYS}d`;
Copy link
Contributor

Choose a reason for hiding this comment

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

we should also not filter by this if there's no config set for it

Copy link

Report too large to display inline

View full report↗︎

@RonaldEAM RonaldEAM merged commit 192c035 into main Aug 7, 2024
8 checks passed
@RonaldEAM RonaldEAM deleted the sonarqube-filter-params-sdk-13.x branch August 7, 2024 20:55
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants