Add S3 etag verfication (#237)

Author: rongyamazon

src/services/S3Service.ts

@@ -1,6 +1,6 @@
 import { readFileSync } from 'fs';
 import { fileURLToPath } from 'url';
-import { S3Client, PutObjectCommand, ListBucketsCommand } from '@aws-sdk/client-s3';
+import { S3Client, PutObjectCommand, ListBucketsCommand, HeadObjectCommand } from '@aws-sdk/client-s3';
 import { Measure } from '../telemetry/TelemetryDecorator';
 import { AwsClient } from './AwsClient';
 
@@ -41,6 +41,17 @@ export class S3Service {
         });
     }
 
+    async getHeadObject(bucketName: string, key: string) {
+        return await this.withClient(async (client) => {
+            return await client.send(
+                new HeadObjectCommand({
+                    Bucket: bucketName,
+                    Key: key,
+                }),
+            );
+        });
+    }
+
     async putObject(localFilePath: string, s3Url: string) {
         const url = new URL(s3Url);
         const bucket = url.hostname;

src/stacks/actions/StackActionOperations.ts

@@ -91,6 +91,7 @@ export async function processChangeSet(
     }
     let templateBody = document.contents();
     let templateS3Url: string | undefined;
+    let expectedETag: string | undefined;
     try {
         if (params.s3Bucket) {
             const s3KeyPrefix = params.s3Key ? params.s3Key.slice(0, params.s3Key.lastIndexOf('/')) : undefined;
@@ -107,7 +108,8 @@ export async function processChangeSet(
         }
 
         if (params.s3Bucket && params.s3Key) {
-            await s3Service.putObjectContent(templateBody, params.s3Bucket, params.s3Key);
+            const putResult = await s3Service.putObjectContent(templateBody, params.s3Bucket, params.s3Key);
+            expectedETag = putResult.ETag;
             templateS3Url = `https://s3.amazonaws.com/${params.s3Bucket}/${params.s3Key}`;
         }
     } catch (error) {
@@ -125,6 +127,17 @@ export async function processChangeSet(
         params.onStackFailure,
     );
 
+    // Verify S3 object ETag before creating change set
+    if (templateS3Url && expectedETag && params.s3Bucket && params.s3Key) {
+        const headResult = await s3Service.getHeadObject(params.s3Bucket, params.s3Key);
+        if (headResult.ETag !== expectedETag) {
+            throw new ResponseError(
+                ErrorCodes.InvalidParams,
+                `S3 object ETag mismatch. Expected: ${expectedETag}, Got: ${headResult.ETag}`,
+            );
+        }
+    }
+
     await cfnService.createChangeSet({
         StackName: params.stackName,
         ChangeSetName: changeSetName,

tst/unit/stackActions/StackActionWorkflowOperations.test.ts

@@ -122,7 +122,8 @@ describe('StackActionWorkflowOperations', () => {
                 Id: 'changeset-123',
             });
 
-            mockS3Service.putObjectContent = vi.fn().mockResolvedValue({});
+            mockS3Service.putObjectContent = vi.fn().mockResolvedValue({ ETag: '"test-etag"' });
+            mockS3Service.getHeadObject = vi.fn().mockResolvedValue({ ETag: '"test-etag"' });
 
             const result = await processChangeSet(mockCfnService, mockDocumentManager, params, 'CREATE', mockS3Service);
 
@@ -132,6 +133,7 @@ describe('StackActionWorkflowOperations', () => {
                 'test-bucket',
                 'template.yaml',
             );
+            expect(mockS3Service.getHeadObject).toHaveBeenCalledWith('test-bucket', 'template.yaml');
             expect(mockCfnService.createChangeSet).toHaveBeenCalledWith({
                 StackName: 'test-stack',
                 ChangeSetName: expect.stringContaining(ExtensionName.replaceAll(' ', '-')),
@@ -144,6 +146,31 @@ describe('StackActionWorkflowOperations', () => {
             });
         });
 
+        it('should throw error when S3 ETag mismatch occurs', async () => {
+            const params: CreateValidationParams = {
+                id: 'test-id',
+                uri: 'file:///test.yaml',
+                stackName: 'test-stack',
+                s3Bucket: 'test-bucket',
+                s3Key: 'template.yaml',
+            };
+
+            const mockDocument = {
+                contents: () => 'template content',
+                documentType: 'YAML',
+            };
+            (mockDocumentManager.get as any).mockReturnValue(mockDocument);
+
+            mockS3Service.putObjectContent = vi.fn().mockResolvedValue({ ETag: '"original-etag"' });
+            mockS3Service.getHeadObject = vi.fn().mockResolvedValue({ ETag: '"different-etag"' });
+
+            await expect(
+                processChangeSet(mockCfnService, mockDocumentManager, params, 'CREATE', mockS3Service),
+            ).rejects.toThrow(ResponseError);
+
+            expect(mockS3Service.getHeadObject).toHaveBeenCalledWith('test-bucket', 'template.yaml');
+        });
+
         it('should throw error when document not found', async () => {
             const params: CreateValidationParams = {
                 id: 'test-id',