Skip to content

Commit

Permalink
fix(amazonq): update security scan languages (aws#5523)
Browse files Browse the repository at this point in the history
Sometimes security issues have a Diagnostic but not SecurityIssueHover
issue. This happens when the document's languageId is not enabled when
registering the hover/code actions provider.

Update the enabled languageIds for security scans feature.

---

<!--- REMINDER: Ensure that your PR meets the guidelines in
CONTRIBUTING.md -->

License: I confirm that my contribution is made under the terms of the
Apache 2.0 license.
  • Loading branch information
ctlai95 authored and ivikash committed Sep 4, 2024
1 parent 5073cc8 commit 0e00742
Show file tree
Hide file tree
Showing 8 changed files with 129 additions and 16 deletions.
Original file line number Diff line number Diff line change
@@ -0,0 +1,4 @@
{
"type": "Bug Fix",
"description": "Add getFeature and isEnabled utility methods to FeatureConfigProvider"
}
Original file line number Diff line number Diff line change
Expand Up @@ -39,6 +39,12 @@ describe('securityScanLanguageContext', function () {
['html', false],
['r', false],
['vb', false],
['xml', false],
['toml', false],
['pip-requirements', false],
['java-properties', false],
['go.mod', false],
['go.sum', false],
]

beforeEach(async function () {
Expand Down Expand Up @@ -100,6 +106,12 @@ describe('securityScanLanguageContext', function () {
['packer', 'tf'],
['plaintext', 'plaintext'],
['jsonc', 'json'],
['xml', 'plaintext'],
['toml', 'plaintext'],
['pip-requirements', 'plaintext'],
['java-properties', 'plaintext'],
['go.mod', 'plaintext'],
['go.sum', 'plaintext'],
]

for (const [securityScanLanguageId, expectedCwsprLanguageId] of securityScanLanguageIds) {
Expand Down
4 changes: 2 additions & 2 deletions packages/core/src/codewhisperer/activation.ts
Original file line number Diff line number Diff line change
Expand Up @@ -283,11 +283,11 @@ export async function activate(context: ExtContext): Promise<void> {
ImportAdderProvider.instance
),
vscode.languages.registerHoverProvider(
[...CodeWhispererConstants.platformLanguageIds],
[...CodeWhispererConstants.securityScanLanguageIds],
SecurityIssueHoverProvider.instance
),
vscode.languages.registerCodeActionsProvider(
[...CodeWhispererConstants.platformLanguageIds],
[...CodeWhispererConstants.securityScanLanguageIds],
SecurityIssueCodeActionProvider.instance
),
vscode.commands.registerCommand('aws.amazonq.openEditorAtRange', openEditorAtRange)
Expand Down
6 changes: 6 additions & 0 deletions packages/core/src/codewhisperer/models/constants.ts
Original file line number Diff line number Diff line change
Expand Up @@ -258,6 +258,12 @@ export const securityScanLanguageIds = [
'c',
'cpp',
'php',
'xml',
'toml',
'pip-requirements',
'java-properties',
'go.mod',
'go.sum',
] as const

export type SecurityScanLanguageId = (typeof securityScanLanguageIds)[number]
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -34,6 +34,12 @@ export class SecurityScanLanguageContext {
c: 'c',
cpp: 'cpp',
php: 'php',
xml: 'plaintext', // xml does not exist in CodewhispererLanguage
toml: 'plaintext',
'pip-requirements': 'plaintext',
'java-properties': 'plaintext',
'go.mod': 'plaintext',
'go.sum': 'plaintext',
})
}

Expand Down
31 changes: 29 additions & 2 deletions packages/core/src/shared/featureConfig.ts
Original file line number Diff line number Diff line change
Expand Up @@ -32,8 +32,11 @@ const customizationArnOverrideName = 'customizationArnOverride'
const featureConfigPollIntervalInMs = 30 * 60 * 1000 // 30 mins
const dataCollectionFeatureName = 'IDEProjectContextDataCollection'

const featureNames = [testFeatureName, customizationArnOverrideName, dataCollectionFeatureName] as const
export type FeatureName = (typeof featureNames)[number]

// TODO: add real feature later
export const featureDefinitions = new Map([
export const featureDefinitions = new Map<FeatureName, FeatureContext>([
[testFeatureName, new FeatureContext(testFeatureName, 'CONTROL', { stringValue: 'testValue' })],
[
customizationArnOverrideName,
Expand Down Expand Up @@ -181,7 +184,7 @@ export class FeatureConfigProvider {

// Get the feature value for the given key.
// In case of a misconfiguration, it will return a default feature value of Boolean true.
private getFeatureValueForKey(name: string): FeatureValue {
private getFeatureValueForKey(name: FeatureName): FeatureValue {
return this.featureConfigs.get(name)?.value ?? featureDefinitions.get(name)?.value ?? { boolValue: true }
}

Expand All @@ -193,4 +196,28 @@ export class FeatureConfigProvider {
public static getFeatureConfigs(): Map<string, FeatureContext> {
return FeatureConfigProvider.instance.featureConfigs
}

/**
* Retrieves the FeatureContext object for a given feature name.
*
* @param {string} featureName - The name of the feature.
* @returns {FeatureContext | undefined} The FeatureContext object for the specified feature, or undefined if the feature doesn't exist.
*/
public static getFeature(featureName: FeatureName): FeatureContext | undefined {
return FeatureConfigProvider.instance.featureConfigs.get(featureName)
}

/**
* Checks if a feature is active or not.
*
* @param {string} featureName - The name of the feature to check.
* @returns {boolean} False if the variation is not CONTROL, otherwise True
*/
public static isEnabled(featureName: FeatureName): boolean {
const featureContext = FeatureConfigProvider.getFeature(featureName)
if (featureContext && featureContext.variation.toLocaleLowerCase() !== 'control') {
return true
}
return false
}
}
10 changes: 10 additions & 0 deletions packages/core/src/test/fake/mockFeatureConfigData.ts
Original file line number Diff line number Diff line change
Expand Up @@ -11,4 +11,14 @@ export const mockFeatureConfigsData: FeatureEvaluation[] = [
variation: 'TREATMENT',
value: { stringValue: 'testValue' },
},
{
feature: 'featureA',
variation: 'CONTROL',
value: { stringValue: 'testValue' },
},
{
feature: 'featureB',
variation: 'TREATMENT',
value: { stringValue: 'testValue' },
},
]
72 changes: 60 additions & 12 deletions packages/core/src/test/shared/featureConfig.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -6,12 +6,26 @@
import assert from 'assert'
import sinon from 'sinon'
import { AWSError, Request } from 'aws-sdk'
import { FeatureConfigProvider, featureDefinitions } from '../../shared/featureConfig'
import { FeatureConfigProvider, featureDefinitions, FeatureName } from '../../shared/featureConfig'
import { ListFeatureEvaluationsResponse } from '../../codewhisperer'
import { createSpyClient } from '../codewhisperer/testUtil'
import { mockFeatureConfigsData } from '../fake/mockFeatureConfigData'

describe('FeatureConfigProvider', () => {
beforeEach(async () => {
const clientSpy = await createSpyClient()
sinon.stub(clientSpy, 'listFeatureEvaluations').returns({
promise: () =>
Promise.resolve({
$response: {
requestId: '',
},
featureEvaluations: mockFeatureConfigsData,
}),
} as Request<ListFeatureEvaluationsResponse, AWSError>)
await FeatureConfigProvider.instance.fetchFeatureConfigs()
})

afterEach(function () {
sinon.restore()
})
Expand All @@ -32,18 +46,52 @@ describe('FeatureConfigProvider', () => {
})

it('test getFeatureConfigsTelemetry will return expected string', async () => {
const clientSpy = await createSpyClient()
sinon.stub(clientSpy, 'listFeatureEvaluations').returns({
promise: () =>
Promise.resolve({
$response: {
requestId: '',
assert.strictEqual(
FeatureConfigProvider.instance.getFeatureConfigsTelemetry(),
`{testFeature: TREATMENT, featureA: CONTROL, featureB: TREATMENT}`
)
})

it('should should return all feature flags', async () => {
it('should should return all feature flags', async () => {
const featureConfigs = FeatureConfigProvider.getFeatureConfigs()
const expectedFeatureConfigs = {
featureA: {
name: 'featureA',
value: {
stringValue: 'testValue',
},
featureEvaluations: mockFeatureConfigsData,
}),
} as Request<ListFeatureEvaluationsResponse, AWSError>)
variation: 'CONTROL',
},
featureB: {
name: 'featureB',
value: {
stringValue: 'testValue',
},
variation: 'TREATMENT',
},
testFeature: {
name: 'testFeature',
value: {
stringValue: 'testValue',
},
variation: 'TREATMENT',
},
}

await FeatureConfigProvider.instance.fetchFeatureConfigs()
assert.strictEqual(FeatureConfigProvider.instance.getFeatureConfigsTelemetry(), `{testFeature: TREATMENT}`)
assert.deepStrictEqual(Object.fromEntries(featureConfigs), expectedFeatureConfigs)
})
})

it('should test featureA as disabled', async () => {
assert.strictEqual(FeatureConfigProvider.isEnabled('featureA' as FeatureName), false)
})

it('should test featureB as enabled', async () => {
assert.strictEqual(FeatureConfigProvider.isEnabled('featureB' as FeatureName), true)
})

it('should test feature-does-not-exist as disabled', async () => {
assert.strictEqual(FeatureConfigProvider.isEnabled('feature-does-not-exist' as FeatureName), false)
})
})

0 comments on commit 0e00742

Please sign in to comment.