From 3aa03681382da57b29cf6d54fb5f5a6d80d9de57 Mon Sep 17 00:00:00 2001 From: David Hasani <60020664+dhasani23@users.noreply.github.com> Date: Wed, 27 Dec 2023 03:13:14 -0800 Subject: [PATCH] fix(transform): error handling, add test, remove dep #4191 - 'he' dependency used but not necessary: https://sim.amazon.com/issues/EGCP-40 - stopJob error not caught - not all constants were in the constants file - no integration test for verifying that the pre-signed upload URL uses HTTPS and sets a 60 second timeout (needed for AppSec) --- .../commands/startTransformByQ.ts | 20 +++++++++++++------ src/codewhisperer/models/constants.ts | 9 +++++++++ .../transformationResultsViewProvider.ts | 9 ++++----- src/shared/utilities/textUtilities.ts | 7 +++++++ .../amazonqGumby/transformByQ.test.ts | 12 +++++++++++ 5 files changed, 46 insertions(+), 11 deletions(-) diff --git a/src/codewhisperer/commands/startTransformByQ.ts b/src/codewhisperer/commands/startTransformByQ.ts index 8f099021b9a..e419022a723 100644 --- a/src/codewhisperer/commands/startTransformByQ.ts +++ b/src/codewhisperer/commands/startTransformByQ.ts @@ -27,7 +27,7 @@ import { QuickPickItem } from 'vscode' import { MultiStepInputFlowController } from '../../shared//multiStepInputFlowController' import path from 'path' import { sleep } from '../../shared/utilities/timeoutUtils' -import * as he from 'he' +import { encodeHTML } from '../../shared/utilities/textUtilities' import { CodeTransformJavaSourceVersionsAllowed, CodeTransformJavaTargetVersionsAllowed, @@ -94,7 +94,7 @@ async function pickProject( shouldResume: () => Promise.resolve(false), }) state.project = pick - transformByQState.setProjectName(he.encode(state.project.label)) // encode to avoid HTML injection risk + transformByQState.setProjectName(encodeHTML(state.project.label)) // encode to avoid HTML injection risk } export async function startTransformByQ() { @@ -195,7 +195,7 @@ export async function startTransformByQ() { getLogger().error(errorMessage, error) throw new ToolkitError(errorMessage, { cause: error as Error }) } - transformByQState.setJobId(jobId) + transformByQState.setJobId(encodeHTML(jobId)) await vscode.commands.executeCommand('aws.amazonq.refresh') await sleep(2000) // sleep before polling job to prevent ThrottlingException @@ -255,8 +255,12 @@ export async function startTransformByQ() { sessionPlanProgress['returnCode'] = StepProgress.Succeeded } catch (error) { if (transformByQState.isCancelled()) { - stopJob(transformByQState.getJobId()) - vscode.window.showErrorMessage(CodeWhispererConstants.transformByQCancelledMessage) + try { + await stopJob(transformByQState.getJobId()) + vscode.window.showErrorMessage(CodeWhispererConstants.transformByQCancelledMessage) + } catch { + vscode.window.showErrorMessage(CodeWhispererConstants.errorStoppingJobMessage) + } } else { transformByQState.setToFailed() let displayedErrorMessage = CodeWhispererConstants.transformByQFailedMessage @@ -344,7 +348,11 @@ export async function confirmStopTransformByQ( transformByQState.setToCancelled() await vscode.commands.executeCommand('aws.amazonq.refresh') vscode.commands.executeCommand('setContext', 'gumby.isStopButtonAvailable', false) - stopJob(jobId) + try { + await stopJob(jobId) + } catch { + vscode.window.showErrorMessage(CodeWhispererConstants.errorStoppingJobMessage) + } telemetry.codeTransform_jobIsCancelledByUser.emit({ codeTransformCancelSrcComponents: cancelSrc, codeTransformSessionId: codeTransformTelemetryState.getSessionId(), diff --git a/src/codewhisperer/models/constants.ts b/src/codewhisperer/models/constants.ts index b0cddc1eede..fb1e9645c4b 100644 --- a/src/codewhisperer/models/constants.ts +++ b/src/codewhisperer/models/constants.ts @@ -314,6 +314,15 @@ export const jobInProgressMessage = 'Job is already in-progress' export const cancellationInProgressMessage = 'Cancellation is in-progress' +export const errorStoppingJobMessage = 'Error stopping job' + +export const errorDownloadingDiffMessage = 'Transform by Q experienced an error when downloading the diff' + +export const viewProposedChangesMessage = + 'Transformation job completed. You can view the transformation summary along with the proposed changes and accept or reject them in the Proposed Changes panel.' + +export const changesAppliedMessage = 'Changes applied' + export const noSupportedJavaProjectsFoundMessage = 'We could not find an upgrade-eligible application. We currently support upgrade of Java applications of version 8 and 11. Be sure to also build your project.' diff --git a/src/codewhisperer/service/transformationResultsViewProvider.ts b/src/codewhisperer/service/transformationResultsViewProvider.ts index 3f77259ad7b..15c85272528 100644 --- a/src/codewhisperer/service/transformationResultsViewProvider.ts +++ b/src/codewhisperer/service/transformationResultsViewProvider.ts @@ -20,6 +20,7 @@ import { telemetry } from '../../shared/telemetry/telemetry' import { codeTransformTelemetryState } from '../../amazonqGumby/telemetry/codeTransformTelemetryState' import { calculateTotalLatency } from '../../amazonqGumby/telemetry/codeTransformTelemetry' import { MetadataResult } from '../../shared/telemetry/telemetryClient' +import * as CodeWhispererConstants from '../models/constants' export abstract class ProposedChangeNode { abstract readonly resourcePath: string @@ -278,7 +279,7 @@ export class ProposedTransformationExplorer { ) } catch (e: any) { // This allows the customer to retry the download - vscode.window.showErrorMessage('Transform by Q experienced an error when downloading the diff') + vscode.window.showErrorMessage(CodeWhispererConstants.errorDownloadingDiffMessage) vscode.commands.executeCommand('setContext', 'gumby.reviewState', TransformByQReviewStatus.NotStarted) const errorMessage = 'There was a problem fetching the transformed code.' getLogger().error('CodeTransform: ExportResultArchive error = ', errorMessage) @@ -331,9 +332,7 @@ export class ProposedTransformationExplorer { }) } - await vscode.window.showInformationMessage( - 'Transformation job completed. You can view the transformation summary along with the proposed changes and accept or reject them in the Proposed Changes panel.' - ) + await vscode.window.showInformationMessage(CodeWhispererConstants.viewProposedChangesMessage) await vscode.commands.executeCommand('aws.amazonq.transformationHub.summary.reveal') telemetry.codeTransform_vcsDiffViewerVisible.emit({ codeTransformSessionId: codeTransformTelemetryState.getSessionId(), @@ -352,7 +351,7 @@ export class ProposedTransformationExplorer { TransformByQReviewStatus.NotStarted ) transformDataProvider.refresh() - await vscode.window.showInformationMessage('Changes applied') + await vscode.window.showInformationMessage(CodeWhispererConstants.changesAppliedMessage) telemetry.codeTransform_vcsViewerSubmitted.emit({ codeTransformSessionId: codeTransformTelemetryState.getSessionId(), codeTransformJobId: transformByQState.getJobId(), diff --git a/src/shared/utilities/textUtilities.ts b/src/shared/utilities/textUtilities.ts index d3a09367523..9deb7ce9465 100644 --- a/src/shared/utilities/textUtilities.ts +++ b/src/shared/utilities/textUtilities.ts @@ -283,3 +283,10 @@ export function formatDateTimestamp(forceUTC: boolean, d: Date = new Date()): st // trim 'Z' (last char of iso string) and add offset string return `${iso.substring(0, iso.length - 1)}${offsetString}` } + +/** + * To satisfy a pentesting concern, encodes HTML to mitigate risk of HTML injection + */ +export function encodeHTML(str: string) { + return str.replace(//g, '>') +} diff --git a/src/testInteg/amazonqGumby/transformByQ.test.ts b/src/testInteg/amazonqGumby/transformByQ.test.ts index d1f9d4f82e9..372667947ab 100644 --- a/src/testInteg/amazonqGumby/transformByQ.test.ts +++ b/src/testInteg/amazonqGumby/transformByQ.test.ts @@ -66,4 +66,16 @@ describe('transformByQ', async function () { }) ) }) + + it('WHEN createUploadUrl then URL uses HTTPS and sets 60 second expiration', async function () { + const sha256 = getSha256(zippedCodePath) + const response = await codeWhisperer.codeWhispererClient.createUploadUrl({ + contentChecksum: sha256, + contentChecksumType: CodeWhispererConstants.contentChecksumType, + uploadIntent: CodeWhispererConstants.uploadIntent, + }) + const uploadUrl = response.uploadUrl + const usesHttpsAndExpiresAfter60Seconds = uploadUrl.includes('https') && uploadUrl.includes('X-Amz-Expires=60') + assert.strictEqual(usesHttpsAndExpiresAfter60Seconds, true) + }) })