Skip to content

Commit 6aeb7b8

Browse files
aws-toolkit-automationaws-toolkit-automation
authored
Merge master into feature/auto-debug
2 parents b5bf1f6 + d40d74a commit 6aeb7b8

File tree

7 files changed

+152
-14
lines changed

7 files changed

+152
-14
lines changed

‎packages/amazonq/src/lsp/chat/messages.ts‎

Lines changed: 16 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -732,7 +732,11 @@ async function handlePartialResult<T extends ChatResult>(
732732
// This is to filter out the message containing findings from CodeReview tool to update CodeIssues panel
733733
decryptedMessage.additionalMessages = decryptedMessage.additionalMessages?.filter(
734734
(message) =>
735-
!(message.messageId !== undefined && message.messageId.endsWith(CodeWhispererConstants.findingsSuffix))
735+
!(
736+
message.messageId !== undefined &&
737+
(message.messageId.endsWith(CodeWhispererConstants.codeReviewFindingsSuffix) ||
738+
message.messageId.endsWith(CodeWhispererConstants.displayFindingsSuffix))
739+
)
736740
)
737741

738742
if (decryptedMessage.body !== undefined) {
@@ -784,7 +788,11 @@ async function handleSecurityFindings(
784788
}
785789
for (let i = decryptedMessage.additionalMessages.length - 1; i >= 0; i--) {
786790
const message = decryptedMessage.additionalMessages[i]
787-
if (message.messageId !== undefined && message.messageId.endsWith(CodeWhispererConstants.findingsSuffix)) {
791+
if (
792+
message.messageId !== undefined &&
793+
(message.messageId.endsWith(CodeWhispererConstants.codeReviewFindingsSuffix) ||
794+
message.messageId.endsWith(CodeWhispererConstants.displayFindingsSuffix))
795+
) {
788796
if (message.body !== undefined) {
789797
try {
790798
const aggregatedCodeScanIssues: AggregatedCodeScanIssue[] = JSON.parse(message.body)
@@ -803,7 +811,12 @@ async function handleSecurityFindings(
803811
issue.visible = !isIssueTitleIgnored && !isSingleIssueIgnored
804812
}
805813
}
806-
initSecurityScanRender(aggregatedCodeScanIssues, undefined, CodeAnalysisScope.PROJECT)
814+
initSecurityScanRender(
815+
aggregatedCodeScanIssues,
816+
undefined,
817+
CodeAnalysisScope.AGENTIC,
818+
message.messageId.endsWith(CodeWhispererConstants.codeReviewFindingsSuffix)
819+
)
807820
SecurityIssueTreeViewProvider.focus()
808821
} catch (e) {
809822
languageClient.info('Failed to parse findings')

‎packages/amazonq/src/lsp/client.ts‎

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -183,6 +183,8 @@ export async function startLanguageServer(
183183
modelSelection: true,
184184
workspaceFilePath: vscode.workspace.workspaceFile?.fsPath,
185185
codeReviewInChat: codeReviewInChat,
186+
// feature flag for displaying findings found not through CodeReview in the Code Issues Panel
187+
displayFindings: true,
186188
},
187189
window: {
188190
notifications: true,

‎packages/core/src/codewhisperer/commands/startSecurityScan.ts‎

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -108,6 +108,9 @@ export async function startSecurityScan(
108108
zipUtil: ZipUtil = new ZipUtil(),
109109
scanUuid?: string
110110
) {
111+
if (scope === CodeAnalysisScope.AGENTIC) {
112+
throw new CreateCodeScanFailedError('Cannot use Agentic scope')
113+
}
111114
const profile = AuthUtil.instance.regionProfileManager.activeRegionProfile
112115
const logger = getLoggerForScope(scope)
113116
/**

‎packages/core/src/codewhisperer/models/constants.ts‎

Lines changed: 5 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -841,6 +841,7 @@ export enum CodeAnalysisScope {
841841
FILE_AUTO = 'FILE_AUTO',
842842
FILE_ON_DEMAND = 'FILE_ON_DEMAND',
843843
PROJECT = 'PROJECT',
844+
AGENTIC = 'AGENTIC',
844845
}
845846

846847
export enum TestGenerationJobStatus {
@@ -907,4 +908,7 @@ export const predictionTrackerDefaultConfig = {
907908
maxSupplementalContext: 15,
908909
}
909910

910-
export const findingsSuffix = '_codeReviewFindings'
911+
export const codeReviewFindingsSuffix = '_codeReviewFindings'
912+
export const displayFindingsSuffix = '_displayFindings'
913+
914+
export const displayFindingsDetectorName = 'DisplayFindings'

‎packages/core/src/codewhisperer/service/diagnosticsProvider.ts‎

Lines changed: 13 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -26,8 +26,13 @@ export const securityScanRender: SecurityScanRender = {
2626
export function initSecurityScanRender(
2727
securityRecommendationList: AggregatedCodeScanIssue[],
2828
editor: vscode.TextEditor | undefined,
29-
scope: CodeAnalysisScope
29+
scope: CodeAnalysisScope,
30+
fromQCA: boolean = true
3031
) {
32+
// fromQCA parameter is used to determine if the findings are coming from QCA or from displayFindings tool.
33+
// if the incoming findings are from QCA review, then keep only existing findings from displayFindings
34+
// if the incoming findings are not from QCA review, then keep only the existing QCA findings
35+
securityScanRender.securityDiagnosticCollection = createSecurityDiagnosticCollection()
3136
securityScanRender.initialized = false
3237
if (scope === CodeAnalysisScope.FILE_ON_DEMAND && editor) {
3338
securityScanRender.securityDiagnosticCollection?.delete(editor.document.uri)
@@ -36,22 +41,20 @@ export function initSecurityScanRender(
3641
}
3742
for (const securityRecommendation of securityRecommendationList) {
3843
updateSecurityDiagnosticCollection(securityRecommendation)
39-
updateSecurityIssuesForProviders(securityRecommendation, scope === CodeAnalysisScope.FILE_AUTO)
44+
updateSecurityIssuesForProviders(securityRecommendation, scope === CodeAnalysisScope.FILE_AUTO, fromQCA)
4045
}
4146
securityScanRender.initialized = true
4247
}
4348

44-
function updateSecurityIssuesForProviders(securityRecommendation: AggregatedCodeScanIssue, isAutoScope?: boolean) {
49+
function updateSecurityIssuesForProviders(
50+
securityRecommendation: AggregatedCodeScanIssue,
51+
isAutoScope?: boolean,
52+
fromQCA: boolean = true
53+
) {
4554
if (isAutoScope) {
4655
SecurityIssueProvider.instance.mergeIssues(securityRecommendation)
4756
} else {
48-
const updatedSecurityRecommendationList = [
49-
...SecurityIssueProvider.instance.issues.filter(
50-
(group) => group.filePath !== securityRecommendation.filePath
51-
),
52-
securityRecommendation,
53-
]
54-
SecurityIssueProvider.instance.issues = updatedSecurityRecommendationList
57+
SecurityIssueProvider.instance.mergeIssuesDisplayFindings(securityRecommendation, fromQCA)
5558
}
5659
SecurityIssueTreeViewProvider.instance.refresh()
5760
}

‎packages/core/src/codewhisperer/service/securityIssueProvider.ts‎

Lines changed: 25 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -6,6 +6,7 @@
66
import * as vscode from 'vscode'
77
import { AggregatedCodeScanIssue, CodeScanIssue, SuggestedFix } from '../models/model'
88
import { randomUUID } from '../../shared/crypto'
9+
import { displayFindingsDetectorName } from '../models/constants'
910

1011
export class SecurityIssueProvider {
1112
static #instance: SecurityIssueProvider
@@ -161,6 +162,30 @@ export class SecurityIssueProvider {
161162
)
162163
}
163164

165+
public mergeIssuesDisplayFindings(newIssues: AggregatedCodeScanIssue, fromQCA: boolean) {
166+
const existingGroup = this._issues.find((group) => group.filePath === newIssues.filePath)
167+
if (!existingGroup) {
168+
this._issues.push(newIssues)
169+
return
170+
}
171+
172+
this._issues = this._issues.map((group) =>
173+
group.filePath !== newIssues.filePath
174+
? group
175+
: {
176+
...group,
177+
issues: [
178+
...group.issues.filter(
179+
// if the incoming findings are from QCA review, then keep only existing findings from displayFindings
180+
// if the incoming findings are not from QCA review, then keep only the existing QCA findings
181+
(issue) => fromQCA === (issue.detectorName === displayFindingsDetectorName)
182+
),
183+
...newIssues.issues,
184+
],
185+
}
186+
)
187+
}
188+
164189
private isExistingIssue(issue: CodeScanIssue, filePath: string) {
165190
return this._issues
166191
.find((group) => group.filePath === filePath)

‎packages/core/src/test/codewhisperer/mergeIssuesDisplayFindings.test.ts‎

Lines changed: 88 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,88 @@
1+
/*!
2+
* Copyright Amazon.com, Inc. or its affiliates. All Rights Reserved.
3+
* SPDX-License-Identifier: Apache-2.0
4+
*/
5+
6+
import assert from 'assert'
7+
import { SecurityIssueProvider } from '../../codewhisperer/service/securityIssueProvider'
8+
import { createCodeScanIssue } from './testUtil'
9+
import { displayFindingsDetectorName } from '../../codewhisperer/models/constants'
10+
import { AggregatedCodeScanIssue } from '../../codewhisperer/models/model'
11+
12+
describe('mergeIssuesDisplayFindings', () => {
13+
let provider: SecurityIssueProvider
14+
const testFilePath = '/test/file.py'
15+
16+
beforeEach(() => {
17+
provider = Object.create(SecurityIssueProvider.prototype)
18+
provider.issues = []
19+
})
20+
21+
it('should add new issues when no existing group', () => {
22+
const newIssues: AggregatedCodeScanIssue = {
23+
filePath: testFilePath,
24+
issues: [createCodeScanIssue({ findingId: 'new-1' })],
25+
}
26+
27+
provider.mergeIssuesDisplayFindings(newIssues, true)
28+
29+
assert.strictEqual(provider.issues.length, 1)
30+
assert.strictEqual(provider.issues[0].filePath, testFilePath)
31+
assert.strictEqual(provider.issues[0].issues.length, 1)
32+
assert.strictEqual(provider.issues[0].issues[0].findingId, 'new-1')
33+
})
34+
35+
it('should keep displayFindings when fromQCA is true', () => {
36+
provider.issues = [
37+
{
38+
filePath: testFilePath,
39+
issues: [
40+
createCodeScanIssue({ findingId: 'qca-1', detectorName: 'QCA-detector' }),
41+
createCodeScanIssue({ findingId: 'display-1', detectorName: displayFindingsDetectorName }),
42+
],
43+
},
44+
]
45+
46+
const newIssues: AggregatedCodeScanIssue = {
47+
filePath: testFilePath,
48+
issues: [createCodeScanIssue({ findingId: 'new-qca-1', detectorName: 'QCA-detector' })],
49+
}
50+
51+
provider.mergeIssuesDisplayFindings(newIssues, true)
52+
53+
assert.strictEqual(provider.issues.length, 1)
54+
assert.strictEqual(provider.issues[0].issues.length, 2)
55+
56+
const findingIds = provider.issues[0].issues.map((issue) => issue.findingId)
57+
assert.ok(findingIds.includes('display-1'))
58+
assert.ok(findingIds.includes('new-qca-1'))
59+
assert.ok(!findingIds.includes('qca-1'))
60+
})
61+
62+
it('should keep QCA findings when fromQCA is false', () => {
63+
provider.issues = [
64+
{
65+
filePath: testFilePath,
66+
issues: [
67+
createCodeScanIssue({ findingId: 'qca-1', detectorName: 'QCA-detector' }),
68+
createCodeScanIssue({ findingId: 'display-1', detectorName: displayFindingsDetectorName }),
69+
],
70+
},
71+
]
72+
73+
const newIssues: AggregatedCodeScanIssue = {
74+
filePath: testFilePath,
75+
issues: [createCodeScanIssue({ findingId: 'new-display-1', detectorName: displayFindingsDetectorName })],
76+
}
77+
78+
provider.mergeIssuesDisplayFindings(newIssues, false)
79+
80+
assert.strictEqual(provider.issues.length, 1)
81+
assert.strictEqual(provider.issues[0].issues.length, 2)
82+
83+
const findingIds = provider.issues[0].issues.map((issue) => issue.findingId)
84+
assert.ok(findingIds.includes('qca-1'))
85+
assert.ok(findingIds.includes('new-display-1'))
86+
assert.ok(!findingIds.includes('display-1'))
87+
})
88+
})

0 commit comments

Comments
 (0)