Skip to content

Commit

Permalink
fix(amazonq): duplicate security issues on same line #4895
Browse files Browse the repository at this point in the history
Problem
Security issues appear multiple times in the same line if the ListCodeScanFindings response is paginated.

Solution
Create the issues list from issues map only once per scan instead of on every page response.
  • Loading branch information
ctlai95 authored May 2, 2024
1 parent 82548be commit 1dc36ed
Show file tree
Hide file tree
Showing 3 changed files with 136 additions and 25 deletions.
Original file line number Diff line number Diff line change
@@ -0,0 +1,4 @@
{
"type": "Bug Fix",
"description": "Security Scan: Addresses a bug where security issues sometimes appear multiple times"
}
44 changes: 19 additions & 25 deletions packages/core/src/codewhisperer/service/securityScanHandler.ts
Original file line number Diff line number Diff line change
Expand Up @@ -48,32 +48,8 @@ export async function listScanResults(
})
.promise()
issues.forEach(issue => {
mapToAggregatedList(codeScanIssueMap, aggregatedCodeScanIssueList, issue, projectPath)
mapToAggregatedList(codeScanIssueMap, issue)
})
return aggregatedCodeScanIssueList
}

function mapToAggregatedList(
codeScanIssueMap: Map<string, RawCodeScanIssue[]>,
aggregatedCodeScanIssueList: AggregatedCodeScanIssue[],
json: string,
projectPath: string
) {
const codeScanIssues: RawCodeScanIssue[] = JSON.parse(json)
codeScanIssues.forEach(issue => {
if (codeScanIssueMap.has(issue.filePath)) {
const list = codeScanIssueMap.get(issue.filePath)
if (list === undefined) {
codeScanIssueMap.set(issue.filePath, [issue])
} else {
list.push(issue)
codeScanIssueMap.set(issue.filePath, list)
}
} else {
codeScanIssueMap.set(issue.filePath, [issue])
}
})

codeScanIssueMap.forEach((issues, key) => {
const filePath = path.join(projectPath, '..', key)
if (existsSync(filePath) && statSync(filePath).isFile()) {
Expand All @@ -100,6 +76,24 @@ function mapToAggregatedList(
aggregatedCodeScanIssueList.push(aggregatedCodeScanIssue)
}
})
return aggregatedCodeScanIssueList
}

function mapToAggregatedList(codeScanIssueMap: Map<string, RawCodeScanIssue[]>, json: string) {
const codeScanIssues: RawCodeScanIssue[] = JSON.parse(json)
codeScanIssues.forEach(issue => {
if (codeScanIssueMap.has(issue.filePath)) {
const list = codeScanIssueMap.get(issue.filePath)
if (list === undefined) {
codeScanIssueMap.set(issue.filePath, [issue])
} else {
list.push(issue)
codeScanIssueMap.set(issue.filePath, list)
}
} else {
codeScanIssueMap.set(issue.filePath, [issue])
}
})
}

export async function pollScanJobStatus(
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,113 @@
/*!
* Copyright Amazon.com, Inc. or its affiliates. All Rights Reserved.
* SPDX-License-Identifier: Apache-2.0
*/

import { PromiseResult } from 'aws-sdk/lib/request'
import { DefaultCodeWhispererClient, ListCodeScanFindingsResponse } from '../../../codewhisperer/client/codewhisperer'
import { listScanResults } from '../../../codewhisperer/service/securityScanHandler'
import { Stub, stub } from '../../utilities/stubber'
import { AWSError, HttpResponse } from 'aws-sdk'
import { RawCodeScanIssue } from '../../../codewhisperer/models/model'
import { CodeAnalysisScope } from '../../../codewhisperer/models/constants'
import assert from 'assert'
import sinon from 'sinon'
import fs from 'fs'

const mockCodeScanFindings = JSON.stringify([
{
filePath: '/workspaceFolder/python3.7-plain-sam-app/hello_world/app.py',
startLine: 1,
endLine: 1,
title: 'title',
description: {
text: 'text',
markdown: 'markdown',
},
detectorId: 'detectorId',
detectorName: 'detectorName',
findingId: 'findingId',
relatedVulnerabilities: [],
severity: 'High',
remediation: {
recommendation: {
text: 'text',
url: 'url',
},
suggestedFixes: [],
},
} satisfies RawCodeScanIssue,
])

const mockListCodeScanFindingsResponse: Awaited<Promise<PromiseResult<ListCodeScanFindingsResponse, AWSError>>> = {
$response: {
hasNextPage: () => false,
nextPage: () => undefined,
data: undefined,
error: undefined,
requestId: '',
redirectCount: 0,
retryCount: 0,
httpResponse: new HttpResponse(),
},
codeScanFindings: mockCodeScanFindings,
}

// eslint-disable-next-line id-length
const mockListCodeScanFindingsPaginatedResponse: Awaited<
Promise<PromiseResult<ListCodeScanFindingsResponse, AWSError>>
> = {
...mockListCodeScanFindingsResponse,
nextToken: 'nextToken',
}

describe('securityScanHandler', function () {
describe('listScanResults', function () {
let mockClient: Stub<DefaultCodeWhispererClient>
beforeEach(function () {
mockClient = stub(DefaultCodeWhispererClient)
sinon.stub(fs, 'existsSync').returns(true)
sinon.stub(fs, 'statSync').returns({ isFile: () => true } as fs.Stats)
})

afterEach(function () {
sinon.restore()
})

it('should make ListCodeScanFindings request and aggregate findings by file path', async function () {
mockClient.listCodeScanFindings.resolves(mockListCodeScanFindingsResponse)

const aggregatedCodeScanIssueList = await listScanResults(
mockClient,
'jobId',
'codeScanFindingsSchema',
'projectPath',
CodeAnalysisScope.PROJECT
)

assert.equal(aggregatedCodeScanIssueList.length, 1)
assert.equal(aggregatedCodeScanIssueList[0].issues.length, 1)
})

it('should handle ListCodeScanFindings request with paginated response', async function () {
mockClient.listCodeScanFindings
.onFirstCall()
.resolves(mockListCodeScanFindingsPaginatedResponse)
.onSecondCall()
.resolves(mockListCodeScanFindingsPaginatedResponse)
.onThirdCall()
.resolves(mockListCodeScanFindingsResponse)

const aggregatedCodeScanIssueList = await listScanResults(
mockClient,
'jobId',
'codeScanFindingsSchema',
'projectPath',
CodeAnalysisScope.PROJECT
)

assert.equal(aggregatedCodeScanIssueList.length, 1)
assert.equal(aggregatedCodeScanIssueList[0].issues.length, 3)
})
})
})

0 comments on commit 1dc36ed

Please sign in to comment.