diff --git a/lib/deploy/stepFunctions/compileIamRole.js b/lib/deploy/stepFunctions/compileIamRole.js index 90ec17e..ddcabc7 100644 --- a/lib/deploy/stepFunctions/compileIamRole.js +++ b/lib/deploy/stepFunctions/compileIamRole.js @@ -634,6 +634,25 @@ function getEventBridgeSchedulerPermissions(state) { ]; } +// Because the S3 Bucket parameter can be either a literal name or a reference to an +// existing S3 bucket we need to resolve +function resolveS3BucketReference(bucket, resource) { + if (isIntrinsic(bucket)) { + return { + 'Fn::Sub': [ + resource, + { bucket }, + ], + }; + } + + return resource.replaceAll('${bucket}', bucket); +} + +function resolveS3BucketReferences(bucket, resources) { + return resources.map(resource => resolveS3BucketReference(bucket, resource)); +} + function getS3ObjectPermissions(action, state) { const bucket = state.Parameters.Bucket || '*'; const key = state.Parameters.Key || '*'; @@ -644,27 +663,29 @@ function getS3ObjectPermissions(action, state) { return [ { action: 's3:Get*', - resource: [ - `arn:aws:s3:::${bucket}`, - `arn:aws:s3:::${bucket}/*`, - ], + resource: resolveS3BucketReferences(bucket, [ + // The bucket variable is not interpolated here, or below, + // but inside resolveS3BucketReferences() to also handle intrinsics + 'arn:aws:s3:::${bucket}', + 'arn:aws:s3:::${bucket}/*', + ]), }, { action: 's3:List*', - resource: [ - `arn:aws:s3:::${bucket}`, - `arn:aws:s3:::${bucket}/*`, - ], + resource: resolveS3BucketReferences(bucket, [ + 'arn:aws:s3:::${bucket}', + 'arn:aws:s3:::${bucket}/*', + ]), }, ]; } if (prefix) { - arn = `arn:aws:s3:::${bucket}/${prefix}/${key}`; + arn = resolveS3BucketReference(bucket, `arn:aws:s3:::\${bucket}/${prefix}/${key}`); } else if (bucket === '*' && key === '*') { arn = '*'; } else { - arn = `arn:aws:s3:::${bucket}/${key}`; + arn = resolveS3BucketReference(bucket, `arn:aws:s3:::\${bucket}/${key}`); } return [{ diff --git a/lib/deploy/stepFunctions/compileIamRole.test.js b/lib/deploy/stepFunctions/compileIamRole.test.js index 0772986..4b71c39 100644 --- a/lib/deploy/stepFunctions/compileIamRole.test.js +++ b/lib/deploy/stepFunctions/compileIamRole.test.js @@ -2148,6 +2148,285 @@ describe('#compileIamRole', () => { .to.be.deep.equal('*'); }); + it('should resolve literal bucket names in S3 permissions', () => { + const literalBucket = 'my-test-bucket'; + const literalKey = 'test-key.txt'; + + serverless.service.stepFunctions = { + stateMachines: { + myStateMachine1: { + id: 'StateMachine1', + definition: { + StartAt: 'A', + States: { + A: { + Type: 'Task', + Resource: 'arn:aws:states:::aws-sdk:s3:getObject', + Parameters: { + Bucket: literalBucket, + Key: literalKey, + }, + End: true, + }, + }, + }, + }, + }, + }; + + serverlessStepFunctions.compileIamRole(); + const statements = serverlessStepFunctions.serverless.service.provider + .compiledCloudFormationTemplate.Resources.StateMachine1Role.Properties.Policies[0] + .PolicyDocument.Statement; + + expect(statements[0].Resource[0]).to.equal(`arn:aws:s3:::${literalBucket}/${literalKey}`); + }); + + it('should resolve CloudFormation reference buckets in S3 permissions', () => { + const bucketRef = { Ref: 'MyS3Bucket' }; + + serverless.service.stepFunctions = { + stateMachines: { + myStateMachine1: { + id: 'StateMachine1', + definition: { + StartAt: 'A', + States: { + A: { + Type: 'Task', + Resource: 'arn:aws:states:::aws-sdk:s3:getObject', + Parameters: { + Bucket: bucketRef, + Key: 'test-key.txt', + }, + End: true, + }, + }, + }, + }, + }, + }; + + serverlessStepFunctions.compileIamRole(); + const statements = serverlessStepFunctions.serverless.service.provider + .compiledCloudFormationTemplate.Resources.StateMachine1Role.Properties.Policies[0] + .PolicyDocument.Statement; + + expect(statements[0].Resource[0]).to.deep.equal({ + 'Fn::Sub': [ + 'arn:aws:s3:::${bucket}/test-key.txt', + { bucket: bucketRef }, + ], + }); + }); + + it('should resolve Fn::GetAtt bucket references in S3 permissions', () => { + const bucketRef = { 'Fn::GetAtt': ['MyS3Bucket', 'Arn'] }; + + serverless.service.stepFunctions = { + stateMachines: { + myStateMachine1: { + id: 'StateMachine1', + definition: { + StartAt: 'A', + States: { + A: { + Type: 'Task', + Resource: 'arn:aws:states:::aws-sdk:s3:putObject', + Parameters: { + Bucket: bucketRef, + Key: 'output.json', + Body: { result: 'success' }, + }, + End: true, + }, + }, + }, + }, + }, + }; + + serverlessStepFunctions.compileIamRole(); + const statements = serverlessStepFunctions.serverless.service.provider + .compiledCloudFormationTemplate.Resources.StateMachine1Role.Properties.Policies[0] + .PolicyDocument.Statement; + + expect(statements[0].Resource[0]).to.deep.equal({ + 'Fn::Sub': [ + 'arn:aws:s3:::${bucket}/output.json', + { bucket: bucketRef }, + ], + }); + }); + + it('should resolve bucket references in S3 listObjectsV2 permissions', () => { + const bucketRef = { Ref: 'MyS3Bucket' }; + + serverless.service.stepFunctions = { + stateMachines: { + myStateMachine1: { + id: 'StateMachine1', + definition: { + StartAt: 'A', + States: { + A: { + Type: 'Map', + ItemProcessor: { + ProcessorConfig: { + Mode: 'DISTRIBUTED', + }, + }, + StartAt: 'B', + States: { + B: { + Type: 'Task', + Resource: 'arn:aws:lambda:#{AWS::Region}:#{AWS::AccountId}:function:hello', + End: true, + }, + }, + ItemReader: { + Resource: 'arn:aws:states:::s3:listObjectsV2', + Parameters: { + Bucket: bucketRef, + Prefix: 'data', + }, + }, + End: true, + }, + }, + }, + }, + }, + }; + + serverlessStepFunctions.compileIamRole(); + const statements = serverlessStepFunctions.serverless.service.provider + .compiledCloudFormationTemplate.Resources.StateMachine1Role.Properties.Policies[0] + .PolicyDocument.Statement; + + expect(statements[3].Resource[0]).to.deep.equal({ + 'Fn::Sub': [ + 'arn:aws:s3:::${bucket}', + { bucket: bucketRef }, + ], + }); + expect(statements[3].Resource[1]).to.deep.equal({ + 'Fn::Sub': [ + 'arn:aws:s3:::${bucket}/*', + { bucket: bucketRef }, + ], + }); + }); + + it('should handle mixed literal and reference buckets correctly', () => { + const literalBucket = 'literal-bucket'; + const bucketRef = { Ref: 'ReferenceBucket' }; + const literalFile = 'file1.txt'; + + serverless.service.stepFunctions = { + stateMachines: { + myStateMachine1: { + id: 'StateMachine1', + definition: { + StartAt: 'A', + States: { + A: { + Type: 'Task', + Resource: 'arn:aws:states:::aws-sdk:s3:getObject', + Parameters: { + Bucket: literalBucket, + Key: literalFile, + }, + Next: 'B', + }, + B: { + Type: 'Task', + Resource: 'arn:aws:states:::aws-sdk:s3:putObject', + Parameters: { + Bucket: bucketRef, + Key: 'file2.txt', + Body: {}, + }, + End: true, + }, + }, + }, + }, + }, + }; + + serverlessStepFunctions.compileIamRole(); + const statements = serverlessStepFunctions.serverless.service.provider + .compiledCloudFormationTemplate.Resources.StateMachine1Role.Properties.Policies[0] + .PolicyDocument.Statement; + + // Check that we have consolidated permissions + expect(statements).to.have.lengthOf(2); + + // First statement for s3:GetObject with literal bucket + expect(statements[0].Action).to.deep.equal(['s3:GetObject']); + expect(statements[0].Resource[0]).to.equal(`arn:aws:s3:::${literalBucket}/${literalFile}`); + + // Second statement for s3:PutObject with reference bucket + expect(statements[1].Action).to.deep.equal(['s3:PutObject']); + expect(statements[1].Resource[0]).to.deep.equal({ + 'Fn::Sub': [ + 'arn:aws:s3:::${bucket}/file2.txt', + { bucket: bucketRef }, + ], + }); + }); + + it('should handle bucket references with prefixes correctly', () => { + const bucketRef = { Ref: 'MyS3Bucket' }; + + serverless.service.stepFunctions = { + stateMachines: { + myStateMachine1: { + id: 'StateMachine1', + definition: { + StartAt: 'A', + States: { + A: { + Type: 'Map', + ItemProcessor: { + StartAt: 'B', + States: { + B: { + Type: 'Task', + Resource: 'arn:aws:lambda:us-west-2:1234567890:function:foo', + End: true, + }, + }, + }, + ResultWriter: { + Resource: 'arn:aws:states:::s3:putObject', + Parameters: { + Bucket: bucketRef, + Prefix: 'results', + }, + }, + End: true, + }, + }, + }, + }, + }, + }; + + serverlessStepFunctions.compileIamRole(); + const statements = serverlessStepFunctions.serverless.service.provider + .compiledCloudFormationTemplate.Resources.StateMachine1Role.Properties.Policies[0] + .PolicyDocument.Statement; + + expect(statements[1].Resource[0]).to.deep.equal({ + 'Fn::Sub': [ + 'arn:aws:s3:::${bucket}/results/*', + { bucket: bucketRef }, + ], + }); + }); + it('should not generate any permissions for Task states not yet supported', () => { const genStateMachine = id => ({ id,