fix(api): Variables with empty names (keyshade-xyz#745)

Co-authored-by: Rajdip Bhattacharya <[email protected]>

apps/api/src/decorators/non-empty-trimmed-string.decorator.ts

@@ -0,0 +1,7 @@
+import { IsNotEmpty } from 'class-validator'
+import { TrimmedString } from '@/decorators/trimmed-string.decorator'
+import { applyDecorators } from '@nestjs/common'
+
+export function NonEmptyTrimmedString() {
+  return applyDecorators(TrimmedString(), IsNotEmpty())
+}

apps/api/src/decorators/trim-string.decorator.ts

@@ -1,7 +1,7 @@
-import { Transform } from 'class-transformer'
-
-export function TrimString() {
-  return Transform(({ value }) =>
-    typeof value === 'string' ? value?.trim() : value
-  )
-}
+import { Transform } from 'class-transformer'
+
+export function TrimString() {
+  return Transform(({ value }) =>
+    typeof value === 'string' ? value.trim() : value
+  )
+}

apps/api/src/decorators/trimmed-string.decorator.ts

@@ -0,0 +1,7 @@
+import { IsString } from 'class-validator'
+import { TrimString } from '@/decorators/trim-string.decorator'
+import { applyDecorators } from '@nestjs/common'
+
+export function TrimmedString() {
+  return applyDecorators(TrimString(), IsString())
+}

apps/api/src/secret/dto/create.secret/create.secret.ts

@@ -8,17 +8,15 @@ import {
   Length,
   ValidateNested
 } from 'class-validator'
-import { TrimString } from '@/decorators/trim-string.decorator'
+import { NonEmptyTrimmedString } from '@/decorators/non-empty-trimmed-string.decorator'
 
 export class CreateSecret {
-  @IsString()
-  @IsNotEmpty()
-  @TrimString()
+  @NonEmptyTrimmedString()
   name: string
 
-  @IsString()
   @IsOptional()
   @Length(0, 100)
+  @NonEmptyTrimmedString()
   note?: string
 
   @IsString()
@@ -33,13 +31,9 @@ export class CreateSecret {
 }
 
 class Entry {
-  @IsString()
-  @IsNotEmpty()
-  @TrimString()
+  @NonEmptyTrimmedString()
   environmentSlug: string
 
-  @IsString()
-  @IsNotEmpty()
-  @TrimString()
+  @NonEmptyTrimmedString()
   value: string
 }

apps/api/src/variable/dto/create.variable/create.variable.ts

@@ -7,16 +7,15 @@ import {
   Length,
   ValidateNested
 } from 'class-validator'
+import { NonEmptyTrimmedString } from '@/decorators/non-empty-trimmed-string.decorator'
 
 export class CreateVariable {
-  @IsString()
-  @Transform(({ value }) => value.trim())
+  @NonEmptyTrimmedString()
   name: string
 
-  @IsString()
   @IsOptional()
   @Length(0, 100)
-  @Transform(({ value }) => (value ? value.trim() : null))
+  @NonEmptyTrimmedString()
   note?: string
 
   @IsOptional()
@@ -27,11 +26,9 @@ export class CreateVariable {
 }
 
 class Entry {
-  @IsString()
-  @Transform(({ value }) => value.trim())
+  @NonEmptyTrimmedString()
   environmentSlug: string
 
-  @IsString()
-  @Transform(({ value }) => value.trim())
+  @NonEmptyTrimmedString()
   value: string
 }

apps/api/src/variable/variable.e2e.spec.ts

@@ -37,6 +37,7 @@ import { UserService } from '@/user/service/user.service'
 import { UserModule } from '@/user/user.module'
 import { QueryTransformPipe } from '@/common/pipes/query.transform.pipe'
 import { fetchEvents } from '@/common/event'
+import { ValidationPipe } from '@nestjs/common'
 
 describe('Variable Controller Tests', () => {
   let app: NestFastifyApplication
@@ -84,7 +85,13 @@ describe('Variable Controller Tests', () => {
     eventService = moduleRef.get(EventService)
     userService = moduleRef.get(UserService)
 
-    app.useGlobalPipes(new QueryTransformPipe())
+    app.useGlobalPipes(
+      new ValidationPipe({
+        whitelist: true,
+        transform: true
+      }),
+      new QueryTransformPipe()
+    )
 
     await app.init()
     await app.getHttpAdapter().getInstance().ready()
@@ -186,7 +193,7 @@ describe('Variable Controller Tests', () => {
           entries: [
             {
               value: 'Variable 3 value',
-              environmentId: environment2.id
+              environmentSlug: environment2.slug
             }
           ]
         },
@@ -228,6 +235,26 @@ describe('Variable Controller Tests', () => {
       expect(variableVersion.version).toBe(1)
     })
 
+    it('should not be able to create variable with empty name', async () => {
+      const response = await app.inject({
+        method: 'POST',
+        url: `/variable/${project1.slug}`,
+        payload: {
+          name: ' '
+        },
+        headers: {
+          'x-e2e-user-email': user1.email
+        }
+      })
+
+      expect(response.statusCode).toBe(400)
+
+      const messages = response.json().message
+
+      expect(messages).toHaveLength(1)
+      expect(messages[0]).toEqual('name should not be empty')
+    })
+
     it('should not be able to create a variable with a non-existing environment', async () => {
       const response = await app.inject({
         method: 'POST',
@@ -345,6 +372,26 @@ describe('Variable Controller Tests', () => {
       )
     })
 
+    it('should not be able to update variable with empty name', async () => {
+      const response = await app.inject({
+        method: 'PUT',
+        url: `/variable/${variable1.slug}`,
+        payload: {
+          name: ' '
+        },
+        headers: {
+          'x-e2e-user-email': user1.email
+        }
+      })
+
+      expect(response.statusCode).toBe(400)
+
+      const messages = response.json().message
+
+      expect(messages).toHaveLength(1)
+      expect(messages[0]).toEqual('name should not be empty')
+    })
+
     it('should not be able to update a variable with same name in the same project', async () => {
       const response = await app.inject({
         method: 'PUT',