adding s3 bucket name validation (#3086)

nk1107 · nick-Ag · commit 818b51cdfc3c · 2025-07-21T12:06:40.000-07:00
diff --git a/packages/destination-actions/src/destinations/liveramp-audiences/__tests__/liveramp.test.ts b/packages/destination-actions/src/destinations/liveramp-audiences/__tests__/liveramp.test.ts
@@ -78,7 +78,7 @@ describe('Liveramp Audiences', () => {
         mapping: {
           s3_aws_access_key: 's3_aws_access_key',
           s3_aws_secret_key: 's3_aws_secret_key',
-          s3_aws_bucket_name: 's3_aws_bucket_name',
+          s3_aws_bucket_name: 's3-aws-bucket-name',
           s3_aws_region: 's3_aws_region',
           audience_key: 'audience_key',
           delimiter: ',',
@@ -106,7 +106,7 @@ describe('Liveramp Audiences', () => {
           mapping: {
             s3_aws_access_key: 's3_aws_access_key',
             s3_aws_secret_key: 's3_aws_secret_key',
-            s3_aws_bucket_name: 's3_aws_bucket_name',
+            s3_aws_bucket_name: 's3-aws-bucket-name',
             s3_aws_region: 's3_aws_region',
             audience_key: 'audience_key',
             delimiter: ',',
@@ -130,6 +130,37 @@ describe('Liveramp Audiences', () => {
         expect(e.status).toEqual(400)
       }
     })
+    it(`should throw error if S3 bucket name is invalid`, async () => {
+      try {
+        await testDestination.executeBatch('audienceEnteredS3', {
+          events: mockedEvents,
+          mapping: {
+            s3_aws_access_key: 's3_aws_access_key',
+            s3_aws_secret_key: 's3_aws_secret_key',
+            s3_aws_bucket_name: 'for-liveramp/folder01/folder_001/',
+            s3_aws_region: 's3_aws_region',
+            audience_key: 'audience_key',
+            delimiter: ',',
+            filename: 'filename.csv',
+            enable_batching: true
+          },
+          subscriptionMetadata: {
+            destinationConfigId: 'destinationConfigId',
+            actionConfigId: 'actionConfigId'
+          },
+          settings: {
+            __segment_internal_engage_force_full_sync: true,
+            __segment_internal_engage_batch_sync: true
+          }
+        })
+      } catch (e) {
+        expect(e).toBeInstanceOf(PayloadValidationError)
+        expect(e.message).toEqual(
+          `Invalid S3 bucket name: "for-liveramp/folder01/folder_001/". Bucket names cannot contain '/' characters, must be lowercase, and follow AWS naming conventions.`
+        )
+        expect(e.status).toEqual(400)
+      }
+    })
 
     it(`should throw error if S3 bucket path is invalid`, async () => {
       try {
@@ -138,7 +169,7 @@ describe('Liveramp Audiences', () => {
           mapping: {
             s3_aws_access_key: 's3_aws_access_key',
             s3_aws_secret_key: 's3_aws_secret_key',
-            s3_aws_bucket_name: 's3_aws_bucket_name',
+            s3_aws_bucket_name: 's3-aws-bucket-name',
             s3_aws_region: 's3_aws_region',
             audience_key: 'audience_key',
             delimiter: ',',
diff --git a/packages/destination-actions/src/destinations/liveramp-audiences/__tests__/s3.test.ts b/packages/destination-actions/src/destinations/liveramp-audiences/__tests__/s3.test.ts
@@ -0,0 +1,154 @@
+import { isValidS3BucketName } from '../audienceEnteredS3/s3'
+
+describe('isValidS3BucketName', () => {
+  describe('valid bucket names', () => {
+    it('should return true for valid bucket names', () => {
+      const validNames = [
+        'my-bucket',
+        'mybucket123',
+        'my.bucket.name',
+        'abc',
+        'test-bucket-123',
+        'bucket.with.dots',
+        'a1b2c3',
+        'bucket-name-with-hyphens',
+        'bucket.name.with.dots',
+        'my-bucket-123.test',
+        'validbucketname123'
+      ]
+
+      validNames.forEach((name) => {
+        expect(isValidS3BucketName(name)).toBe(true)
+      })
+    })
+  })
+
+  describe('invalid bucket names - length constraints', () => {
+    it('should return false for bucket names that are too short', () => {
+      expect(isValidS3BucketName('ab')).toBe(false)
+      expect(isValidS3BucketName('a')).toBe(false)
+      expect(isValidS3BucketName('')).toBe(false)
+    })
+
+    it('should return false for bucket names that are too long', () => {
+      const longName = 'a'.repeat(64) // 64 characters, exceeds 63 limit
+      expect(isValidS3BucketName(longName)).toBe(false)
+    })
+  })
+
+  describe('invalid bucket names - character constraints', () => {
+    it('should return false for bucket names with uppercase letters', () => {
+      expect(isValidS3BucketName('MyBucket')).toBe(false)
+      expect(isValidS3BucketName('BUCKET')).toBe(false)
+      expect(isValidS3BucketName('Test-Bucket')).toBe(false)
+    })
+
+    it('should return false for bucket names with invalid characters', () => {
+      expect(isValidS3BucketName('my_bucket')).toBe(false) // underscore
+      expect(isValidS3BucketName('my bucket')).toBe(false) // space
+      expect(isValidS3BucketName('my@bucket')).toBe(false) // @ symbol
+      expect(isValidS3BucketName('my#bucket')).toBe(false) // # symbol
+      expect(isValidS3BucketName('my$bucket')).toBe(false) // $ symbol
+      expect(isValidS3BucketName('my%bucket')).toBe(false) // % symbol
+      expect(isValidS3BucketName('my&bucket')).toBe(false) // & symbol
+      expect(isValidS3BucketName('my*bucket')).toBe(false) // * symbol
+      expect(isValidS3BucketName('my+bucket')).toBe(false) // + symbol
+      expect(isValidS3BucketName('my=bucket')).toBe(false) // = symbol
+      expect(isValidS3BucketName('my\\bucket')).toBe(false) // backslash
+      expect(isValidS3BucketName('my/bucket')).toBe(false) // forward slash
+    })
+  })
+
+  describe('invalid bucket names - begin/end constraints', () => {
+    it('should return false for bucket names that start with invalid characters', () => {
+      expect(isValidS3BucketName('.bucket')).toBe(false)
+      expect(isValidS3BucketName('-bucket')).toBe(false)
+      expect(isValidS3BucketName('.my-bucket')).toBe(false)
+      expect(isValidS3BucketName('-my-bucket')).toBe(false)
+    })
+
+    it('should return false for bucket names that end with invalid characters', () => {
+      expect(isValidS3BucketName('bucket.')).toBe(false)
+      expect(isValidS3BucketName('bucket-')).toBe(false)
+      expect(isValidS3BucketName('my-bucket.')).toBe(false)
+      expect(isValidS3BucketName('my-bucket-')).toBe(false)
+    })
+  })
+
+  describe('invalid bucket names - consecutive periods', () => {
+    it('should return false for bucket names with consecutive periods', () => {
+      expect(isValidS3BucketName('my..bucket')).toBe(false)
+      expect(isValidS3BucketName('bucket..name')).toBe(false)
+      expect(isValidS3BucketName('my...bucket')).toBe(false)
+      expect(isValidS3BucketName('a..b')).toBe(false)
+    })
+  })
+
+  describe('invalid bucket names - IP address format', () => {
+    it('should return false for bucket names formatted as IP addresses', () => {
+      expect(isValidS3BucketName('192.168.1.1')).toBe(false)
+      expect(isValidS3BucketName('10.0.0.1')).toBe(false)
+      expect(isValidS3BucketName('172.16.0.1')).toBe(false)
+      expect(isValidS3BucketName('255.255.255.255')).toBe(false)
+      expect(isValidS3BucketName('0.0.0.0')).toBe(false)
+    })
+
+    it('should return false for bucket names that look like IP addresses (even invalid ones)', () => {
+      expect(isValidS3BucketName('192.168.1.300')).toBe(false) // 300 > 255, but still IP-like
+      expect(isValidS3BucketName('999.999.999.999')).toBe(false) // invalid IP but IP-like format
+      expect(isValidS3BucketName('123.456.789.012')).toBe(false) // invalid numbers but IP-like format
+      expect(isValidS3BucketName('1.2.3.4')).toBe(false) // valid IP format
+      expect(isValidS3BucketName('001.002.003.004')).toBe(false) // leading zeros, still IP-like
+    })
+
+    it('should return true for non-IP-like names with dots and numbers', () => {
+      expect(isValidS3BucketName('192.168.1')).toBe(true) // incomplete IP (only 3 parts)
+      expect(isValidS3BucketName('192.168.1.1.1')).toBe(true) // too many octets (5 parts)
+      expect(isValidS3BucketName('bucket.123.name')).toBe(true) // clearly not IP format
+      expect(isValidS3BucketName('v1.2.3')).toBe(true) // version-like, not IP
+      expect(isValidS3BucketName('file.123')).toBe(true) // only 2 parts
+      expect(isValidS3BucketName('a.b.c.d')).toBe(true) // 4 parts but contains letters
+      expect(isValidS3BucketName('my-bucket.123.test')).toBe(true) // contains hyphens
+    })
+  })
+
+  describe('invalid bucket names - forbidden prefixes', () => {
+    it('should return false for bucket names with forbidden prefixes', () => {
+      expect(isValidS3BucketName('xn--bucket')).toBe(false)
+      expect(isValidS3BucketName('sthree-bucket')).toBe(false)
+      expect(isValidS3BucketName('amzn-s3-demo-bucket')).toBe(false)
+      expect(isValidS3BucketName('xn--test')).toBe(false)
+      expect(isValidS3BucketName('sthree-test123')).toBe(false)
+      expect(isValidS3BucketName('amzn-s3-demo-test')).toBe(false)
+    })
+  })
+
+  describe('invalid bucket names - forbidden suffixes', () => {
+    it('should return false for bucket names with forbidden suffixes', () => {
+      expect(isValidS3BucketName('bucket-s3alias')).toBe(false)
+      expect(isValidS3BucketName('bucket--ol-s3')).toBe(false)
+      expect(isValidS3BucketName('bucket.mrap')).toBe(false)
+      expect(isValidS3BucketName('bucket--x-s3')).toBe(false)
+      expect(isValidS3BucketName('bucket--table-s3')).toBe(false)
+      expect(isValidS3BucketName('my-bucket-s3alias')).toBe(false)
+      expect(isValidS3BucketName('test--ol-s3')).toBe(false)
+      expect(isValidS3BucketName('example.mrap')).toBe(false)
+    })
+  })
+
+  describe('edge cases', () => {
+    it('should handle exact length limits', () => {
+      const minValidName = 'abc' // 3 characters (minimum)
+      const maxValidName = 'a'.repeat(63) // 63 characters (maximum)
+
+      expect(isValidS3BucketName(minValidName)).toBe(true)
+      expect(isValidS3BucketName(maxValidName)).toBe(true)
+    })
+
+    it('should handle single character begin/end cases', () => {
+      expect(isValidS3BucketName('a')).toBe(false) // too short but would be valid otherwise
+      expect(isValidS3BucketName('a1a')).toBe(true) // minimum valid case
+      expect(isValidS3BucketName('1a1')).toBe(true) // numbers at begin/end
+    })
+  })
+})
diff --git a/packages/destination-actions/src/destinations/liveramp-audiences/audienceEnteredS3/index.ts b/packages/destination-actions/src/destinations/liveramp-audiences/audienceEnteredS3/index.ts
@@ -1,5 +1,5 @@
 import { ActionDefinition, PayloadValidationError } from '@segment/actions-core'
-import { isValidS3Path, normalizeS3Path, uploadS3 } from './s3'
+import { isValidS3Path, isValidS3BucketName, normalizeS3Path, uploadS3 } from './s3'
 import { generateFile } from '../operations'
 import { sendEventToAWS } from '../awsClient'
 import { LIVERAMP_MIN_RECORD_COUNT, LIVERAMP_LEGACY_FLOW_FLAG_NAME } from '../properties'
@@ -130,6 +130,12 @@ async function processData(input: ProcessDataInput<Payload>, subscriptionMetadat
       `received payload count below LiveRamp's ingestion limits. expected: >=${LIVERAMP_MIN_RECORD_COUNT} actual: ${input.payloads.length}`
     )
   }
+  //validate s3 bucket name
+  if (input.payloads[0].s3_aws_bucket_name && !isValidS3BucketName(input.payloads[0].s3_aws_bucket_name)) {
+    throw new PayloadValidationError(
+      `Invalid S3 bucket name: "${input.payloads[0].s3_aws_bucket_name}". Bucket names cannot contain '/' characters, must be lowercase, and follow AWS naming conventions.`
+    )
+  }
 
   // validate s3 path
   input.payloads[0].s3_aws_bucket_path = normalizeS3Path(input.payloads[0].s3_aws_bucket_path)
diff --git a/packages/destination-actions/src/destinations/liveramp-audiences/audienceEnteredS3/s3.ts b/packages/destination-actions/src/destinations/liveramp-audiences/audienceEnteredS3/s3.ts
@@ -88,4 +88,59 @@ function normalizeS3Path(path?: string): string | undefined {
   return path?.replace(/^\/+|\/+$/g, '')
 }
 
-export { validateS3, uploadS3, isValidS3Path, normalizeS3Path }
+/**
+ * Checks whether the provided string is a valid S3 bucket name.
+ * https://docs.aws.amazon.com/AmazonS3/latest/userguide/bucketnamingrules.html
+ *
+ * @param bucketName - The S3 bucket name to validate.
+ * @returns `true` if the bucket name is valid according to AWS naming rules, otherwise `false`.
+ */
+function isValidS3BucketName(bucketName: string): boolean {
+  // Basic checks
+  if (!bucketName || bucketName.length < 3 || bucketName.length > 63) {
+    return false
+  }
+
+  // Bucket names can consist only of lowercase letters, numbers, periods (.), and hyphens (-)
+  const validCharsRegex = /^[a-z0-9.-]+$/
+  if (!validCharsRegex.test(bucketName)) {
+    return false
+  }
+
+  // Bucket names must begin and end with a letter or number
+  const beginEndRegex = /^[a-z0-9].*[a-z0-9]$/
+  if (!beginEndRegex.test(bucketName)) {
+    return false
+  }
+
+  // Bucket names must not contain two adjacent periods
+  if (bucketName.includes('..')) {
+    return false
+  }
+
+  // Bucket names must not be formatted as an IP address (IPv4)
+  const ipRegex = /^(?:\d{1,3}\.){3}\d{1,3}$/
+  if (ipRegex.test(bucketName)) {
+    return false
+  }
+
+  // Bucket names must not start with forbidden prefixes
+  const forbiddenPrefixes = ['xn--', 'sthree-', 'amzn-s3-demo-']
+  for (const prefix of forbiddenPrefixes) {
+    if (bucketName.startsWith(prefix)) {
+      return false
+    }
+  }
+
+  // Bucket names must not end with forbidden suffixes
+  const forbiddenSuffixes = ['-s3alias', '--ol-s3', '.mrap', '--x-s3', '--table-s3']
+  for (const suffix of forbiddenSuffixes) {
+    if (bucketName.endsWith(suffix)) {
+      return false
+    }
+  }
+
+  return true
+}
+
+export { validateS3, uploadS3, isValidS3Path, normalizeS3Path, isValidS3BucketName }
