Repository: codecov/uploader (ts)

Issue:
Flag silently ignored when contains invalid characters
**Describe the bug**

>Flags Must consist only of alphanumeric characters, '_', '-', or '.' and not exceed 45 characters

If my CI suite accidentally contains a slash or some other character, the flag is silently dropped.

**To Reproduce**
Steps to reproduce the behavior:
1. Use uploader v0.2.4
2. send a flag as `-F ci/js-unit-tests`
3. The upload succeeds but the flag is blank

**Expected behavior**

I would expect it to bail out with "Invalid flag: flags Must consist only of alphanumeric characters, '_', '-', or '.' and not exceed 45 characters"

**Additional context**

If you think this is too much of a BC break, I could accept this being hidden behind a flag like `CODECOV_STRICT_FLAGS=1` or something.


Pull request:
fix: throw error when flags do not match pattern
Prior behavior was to strip out invalid flag names. The validation function will now throw.

Fixes #829

Patch:
diff --git a/src/helpers/validate.ts b/src/helpers/validate.ts
index 2938757..3bdf661 100644
--- a/src/helpers/validate.ts
+++ b/src/helpers/validate.ts
@@ -14,10 +14,20 @@ export function validateURL(url: string): boolean {
   return validator.isURL(url, { require_protocol: true })
 }
 
-export function validateFlags(flags: string): boolean {
+export function isValidFlag(flag: string): boolean {
   // eslint-disable-next-line no-useless-escape
   const mask = /^[\w\.\-]{1,45}$/
-  return mask.test(flags)
+  return flag.length === 0 || mask.test(flag)
+}
+
+export function validateFlags(flags: string[]): void {
+  const invalidFlags = flags.filter(flag => isValidFlag(flag) !== true)
+
+  if (invalidFlags.length > 0) {
+    throw new Error(
+      `Flags must consist only of alphanumeric characters, '_', '-', or '.' and not exceed 45 characters. Received ${flags}`,
+    )
+  }
 }
 
 /**
diff --git a/src/helpers/web.ts b/src/helpers/web.ts
index 159ba29..bbe8124 100644
--- a/src/helpers/web.ts
+++ b/src/helpers/web.ts
@@ -15,7 +15,6 @@ import {
   UploaderInputs,
 } from '../types'
 import { info } from './logger'
-import * as validateHelpers from './validate'
 
 /**
  *
@@ -32,15 +31,11 @@ export function populateBuildParams(
   serviceParams.name = args.name || envs.CODECOV_NAME || ''
   serviceParams.tag = args.tag || ''
 
-  let flags: string[]
-  if (typeof args.flags === 'object') {
-    flags = [...args.flags]
+  if (typeof args.flags === "string") {
+    serviceParams.flags = args.flags
   } else {
-    flags = String(args.flags || '').split(',')
+    serviceParams.flags = args.flags.join(',')
   }
-  serviceParams.flags = flags
-    .filter(flag => validateHelpers.validateFlags(flag))
-    .join(',')
 
   serviceParams.parent = args.parent || ''
   return serviceParams
diff --git a/src/index.ts b/src/index.ts
index 5807d6f..1c485fe 100644
--- a/src/index.ts
+++ b/src/index.ts
@@ -27,6 +27,7 @@ import { generateGcovCoverageFiles } from './helpers/gcov'
 import { generateXcodeCoverageFiles } from './helpers/xcode'
 import { argAsArray } from './helpers/util'
 import { checkSlug } from './helpers/checkSlug'
+import { validateFlags } from './helpers/validate'
 
 /**
  *
@@ -119,6 +120,15 @@ export async function main(
 
   info(generateHeader(getVersion()))
 
+  let flags: string[]
+  if (typeof args.flags === 'object') {
+    flags = [...args.flags]
+  } else {
+    flags = String(args.flags || '').split(',')
+  }
+
+  validateFlags(flags)
+
   // #endregion
   // #region == Step 2: detect if we are in a git repo
   const projectRoot = args.rootDir || fetchGitRoot()


Test patch:
diff --git a/test/helpers/validate.test.ts b/test/helpers/validate.test.ts
index e6cfdf2..c2cce12 100644
--- a/test/helpers/validate.test.ts
+++ b/test/helpers/validate.test.ts
@@ -35,18 +35,27 @@ describe('Input Validators', () => {
 
   describe('Flags', () => {
     it('Should pass without a dash', () => {
-      expect(validate.validateFlags('moo')).toBe(true)
+      expect(validate.isValidFlag('moo')).toBe(true)
     })
     it('Should pass with a dash', () => {
-      expect(validate.validateFlags('moo-foor')).toBe(true)
+      expect(validate.isValidFlag('moo-foor')).toBe(true)
     })
 
     it('Should pass with a period in the middle', () => {
-      expect(validate.validateFlags('moo.foor')).toBe(true)
+      expect(validate.isValidFlag('moo.foor')).toBe(true)
     })
 
     it('Should pass with a dash at the start', () => {
-      expect(validate.validateFlags('-moo-foor')).toBe(true)
+      expect(validate.isValidFlag('-moo-foor')).toBe(true)
+    })
+
+    it("should throw when they they not match the pattern", () => {
+      // arrange
+      const invalidFlagName = "flag/subflag"
+      const expectedErrorMessage = "Flags must consist only of alphanumeric characters, '_', '-', or '.' and not exceed 45 characters. Received flag/subflag"
+
+      // act
+      expect(() => validate.validateFlags([invalidFlagName])).toThrowError(expectedErrorMessage)
     })
   })