feat(ecr): validate repository arn in fromRepositoryArn (#25302)

vinayak-kukreja · web-flow · commit 383cccb7ccb9 · 2023-05-31T18:13:20.000Z
This PR is adding a regex to check the `arn` structure of an ECR repository. Closes #16223 ---- *By submitting this pull request, I confirm that my contribution is made under the terms of the Apache-2.0 license*
diff --git a/packages/aws-cdk-lib/aws-ecr/lib/repository.ts b/packages/aws-cdk-lib/aws-ecr/lib/repository.ts
@@ -560,6 +560,8 @@ export class Repository extends RepositoryBase {
       throw new Error('"repositoryArn" is a late-bound value, and therefore "repositoryName" is required. Use `fromRepositoryAttributes` instead');
     }
 
+    validateRepositoryArn();
+
     const repositoryName = repositoryArn.split('/').slice(1).join('/');
 
     class Import extends RepositoryBase {
@@ -575,6 +577,14 @@ export class Repository extends RepositoryBase {
     return new Import(scope, id, {
       environmentFromArn: repositoryArn,
     });
+
+    function validateRepositoryArn() {
+      const splitArn = repositoryArn.split(':');
+
+      if (!splitArn[splitArn.length - 1].startsWith('repository/')) {
+        throw new Error(`Repository arn should be in the format 'arn:<PARTITION>:ecr:<REGION>:<ACCOUNT>:repository/<NAME>', got ${repositoryArn}.`);
+      }
+    }
   }
 
   public static fromRepositoryName(scope: Construct, id: string, repositoryName: string): IRepository {
diff --git a/packages/aws-cdk-lib/aws-ecr/test/repository.test.ts b/packages/aws-cdk-lib/aws-ecr/test/repository.test.ts
@@ -209,6 +209,19 @@ describe('repository', () => {
     expect(stack.resolve(repo2.repositoryName)).toBe('foo/bar/foo/fooo');
   });
 
+  test('import with arn without /repository', () => {
+    // GIVEN
+    const stack = new cdk.Stack();
+
+    // WHEN
+    const invalidArn = 'arn:aws:ecr:us-east-1:123456789012:foo-ecr-repo-name';
+
+    // THEN
+    expect(() => {
+      ecr.Repository.fromRepositoryArn(stack, 'repo', invalidArn);
+    }).toThrowError(`Repository arn should be in the format 'arn:<PARTITION>:ecr:<REGION>:<ACCOUNT>:repository/<NAME>', got ${invalidArn}.`);
+  });
+
   test('fails if importing with token arn and no name', () => {
     // GIVEN
     const stack = new cdk.Stack();
diff --git a/packages/aws-cdk-lib/aws-lambda/test/lambda-insights.test.ts b/packages/aws-cdk-lib/aws-lambda/test/lambda-insights.test.ts
@@ -142,7 +142,7 @@ describe('lambda-insights', () => {
     const stack = new cdk.Stack();
     new lambda.DockerImageFunction(stack, 'MyFunction', {
       code: lambda.DockerImageCode.fromEcr(ecr.Repository.fromRepositoryArn(stack, 'MyRepo',
-        'arn:aws:ecr:us-east-1:0123456789:repository/MyRepo')),
+        'arn:aws:ecr:us-east-1:123456789012:repository/MyRepo')),
       insightsVersion: lambda.LambdaInsightsVersion.VERSION_1_0_98_0,
     });
 
diff --git a/packages/aws-cdk-lib/pipelines/test/docker-credentials.test.ts b/packages/aws-cdk-lib/pipelines/test/docker-credentials.test.ts
@@ -14,7 +14,7 @@ let stack: cdk.Stack;
 beforeEach(() => {
   app = new TestApp();
   stack = new cdk.Stack(app, 'Stack', {
-    env: { account: '0123456789012', region: 'eu-west-1' },
+    env: { account: '123456789012', region: 'eu-west-1' },
   });
 });
 
@@ -44,7 +44,7 @@ describe('ExternalDockerCredential', () => {
   });
 
   test('maximum example includes all expected properties', () => {
-    const roleArn = 'arn:aws:iam::0123456789012:role/MyRole';
+    const roleArn = 'arn:aws:iam::123456789012:role/MyRole';
     const creds = cdkp.DockerCredential.customRegistry('example.com', secret, {
       secretUsernameField: 'login',
       secretPasswordField: 'pass',
@@ -86,7 +86,7 @@ describe('ExternalDockerCredential', () => {
     });
 
     test('grants read access to the secret to the assumeRole if provided', () => {
-      const assumeRole = iam.Role.fromRoleArn(stack, 'Role', 'arn:aws:iam::0123456789012:role/MyRole');
+      const assumeRole = iam.Role.fromRoleArn(stack, 'Role', 'arn:aws:iam::123456789012:role/MyRole');
       const creds = cdkp.DockerCredential.customRegistry('example.com', secret, { assumeRole });
 
       const user = new iam.User(stack, 'User');
@@ -108,7 +108,7 @@ describe('ExternalDockerCredential', () => {
           Statement: [{
             Action: 'sts:AssumeRole',
             Effect: 'Allow',
-            Resource: 'arn:aws:iam::0123456789012:role/MyRole',
+            Resource: 'arn:aws:iam::123456789012:role/MyRole',
           }],
           Version: '2012-10-17',
         },
@@ -117,7 +117,7 @@ describe('ExternalDockerCredential', () => {
     });
 
     test('does not grant any access if the usage does not match', () => {
-      const assumeRole = iam.Role.fromRoleArn(stack, 'Role', 'arn:aws:iam::0123456789012:role/MyRole');
+      const assumeRole = iam.Role.fromRoleArn(stack, 'Role', 'arn:aws:iam::123456789012:role/MyRole');
       const creds = cdkp.DockerCredential.customRegistry('example.com', secret, {
         assumeRole,
         usages: [cdkp.DockerCredentialUsage.ASSET_PUBLISHING],
@@ -138,7 +138,7 @@ describe('EcrDockerCredential', () => {
   let repo: ecr.IRepository;
 
   beforeEach(() => {
-    repo = ecr.Repository.fromRepositoryArn(stack, 'Repo', 'arn:aws:ecr:eu-west-1:0123456789012:repository/Repo');
+    repo = ecr.Repository.fromRepositoryArn(stack, 'Repo', 'arn:aws:ecr:eu-west-1:123456789012:repository/Repo');
   });
 
   test('minimal example only renders ecrRepository', () => {
@@ -150,7 +150,7 @@ describe('EcrDockerCredential', () => {
       'Fn::Select': [
         0, {
           'Fn::Split': ['/', {
-            'Fn::Join': ['', ['0123456789012.dkr.ecr.eu-west-1.', { Ref: 'AWS::URLSuffix' }, '/Repo']],
+            'Fn::Join': ['', ['123456789012.dkr.ecr.eu-west-1.', { Ref: 'AWS::URLSuffix' }, '/Repo']],
           }],
         },
       ],
@@ -161,7 +161,7 @@ describe('EcrDockerCredential', () => {
   });
 
   test('maximum example renders all fields', () => {
-    const roleArn = 'arn:aws:iam::0123456789012:role/MyRole';
+    const roleArn = 'arn:aws:iam::123456789012:role/MyRole';
     const creds = cdkp.DockerCredential.ecr([repo], {
       assumeRole: iam.Role.fromRoleArn(stack, 'Role', roleArn),
       usages: [cdkp.DockerCredentialUsage.SYNTH],
@@ -173,7 +173,7 @@ describe('EcrDockerCredential', () => {
       'Fn::Select': [
         0, {
           'Fn::Split': ['/', {
-            'Fn::Join': ['', ['0123456789012.dkr.ecr.eu-west-1.', { Ref: 'AWS::URLSuffix' }, '/Repo']],
+            'Fn::Join': ['', ['123456789012.dkr.ecr.eu-west-1.', { Ref: 'AWS::URLSuffix' }, '/Repo']],
           }],
         },
       ],
@@ -201,7 +201,7 @@ describe('EcrDockerCredential', () => {
               'ecr:BatchGetImage',
             ],
             Effect: 'Allow',
-            Resource: 'arn:aws:ecr:eu-west-1:0123456789012:repository/Repo',
+            Resource: 'arn:aws:ecr:eu-west-1:123456789012:repository/Repo',
           },
           {
             Action: 'ecr:GetAuthorizationToken',
@@ -215,7 +215,7 @@ describe('EcrDockerCredential', () => {
     });
 
     test('grants pull access to the assumed role', () => {
-      const assumeRole = iam.Role.fromRoleArn(stack, 'Role', 'arn:aws:iam::0123456789012:role/MyRole');
+      const assumeRole = iam.Role.fromRoleArn(stack, 'Role', 'arn:aws:iam::123456789012:role/MyRole');
       const creds = cdkp.DockerCredential.ecr([repo], { assumeRole });
 
       const user = new iam.User(stack, 'User');
@@ -230,7 +230,7 @@ describe('EcrDockerCredential', () => {
               'ecr:BatchGetImage',
             ],
             Effect: 'Allow',
-            Resource: 'arn:aws:ecr:eu-west-1:0123456789012:repository/Repo',
+            Resource: 'arn:aws:ecr:eu-west-1:123456789012:repository/Repo',
           },
           {
             Action: 'ecr:GetAuthorizationToken',
@@ -246,7 +246,7 @@ describe('EcrDockerCredential', () => {
           Statement: [{
             Action: 'sts:AssumeRole',
             Effect: 'Allow',
-            Resource: 'arn:aws:iam::0123456789012:role/MyRole',
+            Resource: 'arn:aws:iam::123456789012:role/MyRole',
           }],
           Version: '2012-10-17',
         },
@@ -255,7 +255,7 @@ describe('EcrDockerCredential', () => {
     });
 
     test('grants pull access to multiple repos if provided', () => {
-      const repo2 = ecr.Repository.fromRepositoryArn(stack, 'Repo2', 'arn:aws:ecr:eu-west-1:0123456789012:repository/Repo2');
+      const repo2 = ecr.Repository.fromRepositoryArn(stack, 'Repo2', 'arn:aws:ecr:eu-west-1:123456789012:repository/Repo2');
       const creds = cdkp.DockerCredential.ecr([repo, repo2]);
 
       const user = new iam.User(stack, 'User');
@@ -270,7 +270,7 @@ describe('EcrDockerCredential', () => {
               'ecr:BatchGetImage',
             ],
             Effect: 'Allow',
-            Resource: 'arn:aws:ecr:eu-west-1:0123456789012:repository/Repo',
+            Resource: 'arn:aws:ecr:eu-west-1:123456789012:repository/Repo',
           },
           {
             Action: 'ecr:GetAuthorizationToken',
@@ -284,7 +284,7 @@ describe('EcrDockerCredential', () => {
               'ecr:BatchGetImage',
             ],
             Effect: 'Allow',
-            Resource: 'arn:aws:ecr:eu-west-1:0123456789012:repository/Repo2',
+            Resource: 'arn:aws:ecr:eu-west-1:123456789012:repository/Repo2',
           }]),
           Version: '2012-10-17',
         },
@@ -324,7 +324,7 @@ describe('EcrDockerCredential', () => {
 });
 
 describe('dockerCredentialsInstallCommands', () => {
-  const secretArn = 'arn:aws:secretsmanager:eu-west-1:0123456789012:secret:mySecret-012345';
+  const secretArn = 'arn:aws:secretsmanager:eu-west-1:123456789012:secret:mySecret-012345';
   let secret: secretsmanager.ISecret;
 
   beforeEach(() => {
