Repository: codecov/uploader
Pull request #325 (2021-08-23 13:13:37)

fix: do not validate token when using `-u`


Patch:
diff --git a/.eslintignore b/.eslintignore
index 46d8de1..5aee132 100644
--- a/.eslintignore
+++ b/.eslintignore
@@ -1,1 +1,2 @@
 test/providers/provider_template.test.js
+dist/**/*
diff --git a/CHANGELOG.md b/CHANGELOG.md
new file mode 100644
index 0000000..93c37b6
--- /dev/null
+++ b/CHANGELOG.md
@@ -0,0 +1,10 @@
+# Changes Since
+
+- fix: throw if token validation fails
+- fix: do not validate token if uploader url is changed with `-u`
+
+* refactor: move default url out of function for flexibility
+
+- fix: remove unused function (fetchToken)
+
+# 0.1.x (Beta)
diff --git a/src/ci_providers/provider_bitbucket.ts b/src/ci_providers/provider_bitbucket.ts
index f5c0c34..161713b 100644
--- a/src/ci_providers/provider_bitbucket.ts
+++ b/src/ci_providers/provider_bitbucket.ts
@@ -1,5 +1,4 @@
 import childProcess from 'child_process'
-import { runExternalProgram } from '../helpers/util'
 import { validateSHA } from '../helpers/validate'
 import { IServiceParams, UploaderEnvs, UploaderInputs } from '../types'
 
@@ -45,7 +44,7 @@ function _getSHA(inputs: UploaderInputs): string {
   let commit = envs.BITBUCKET_COMMIT || ''
 
   if (commit && validateSHA(commit, 12)) {
-    commit = runExternalProgram('git', ['rev-parse', commit])
+    commit = childProcess.execFileSync('git', ['rev-parse', commit])
   }
 
   return args.sha || commit || ''
diff --git a/src/helpers/constansts.ts b/src/helpers/constansts.ts
new file mode 100644
index 0000000..841860c
--- /dev/null
+++ b/src/helpers/constansts.ts
@@ -0,0 +1,1 @@
+export const DEFAULT_UPLOAD_HOST = 'https://codecov.io'
diff --git a/src/helpers/token.ts b/src/helpers/token.ts
index 6413705..03983d7 100644
--- a/src/helpers/token.ts
+++ b/src/helpers/token.ts
@@ -2,6 +2,7 @@ import fs from 'fs'
 import yaml from 'js-yaml'
 import path from 'path'
 import { UploaderArgs, UploaderInputs } from '../types'
+import { DEFAULT_UPLOAD_HOST } from './constansts'
 import { info, logError, verbose } from './logger'
 import { validateToken } from './validate'
 
@@ -19,10 +20,23 @@ export function getToken(inputs: UploaderInputs, projectRoot: string): string {
     [getTokenFromYaml(projectRoot, args), 'Codecov yaml config'],
   ]
 
-  for (const option of options) {
-    if (option[0] && validateToken(option[0])) {
-      info(`->  Token set by ${option[1]}`)
-      return option[0]
+  for (const [token, source] of options) {
+    if (token) {
+      info(`->  Token found by ${source}`)
+      // If this is self-hosted (-u is set), do not validate
+      // This is because self-hosted can use a global upload token
+      if (args.url !== DEFAULT_UPLOAD_HOST) {
+        verbose('Self-hosted install detected due to -u flag')
+        info(`->  Token set by ${source}`)
+        return token
+      }
+      if (validateToken(token) !== true) {
+        throw new Error(
+          `Token found by ${source} with length ${token?.length} did not pass validation`,
+        )
+      }
+
+      return token
     }
   }
 
diff --git a/src/helpers/util.ts b/src/helpers/util.ts
index c5087c0..9a30d97 100644
--- a/src/helpers/util.ts
+++ b/src/helpers/util.ts
@@ -12,5 +12,5 @@ export function runExternalProgram(
   if (result.error) {
     throw new Error(`Error running external program: ${result.error}`)
   }
-  return result.stdout.toString().trim()
+  return result.stdout.toString().trimRight()
 }
diff --git a/src/helpers/validate.ts b/src/helpers/validate.ts
index 8826d13..37ac56e 100644
--- a/src/helpers/validate.ts
+++ b/src/helpers/validate.ts
@@ -7,6 +7,7 @@ import { UploaderArgs } from '../types'
  * @returns boolean
  */
 export function validateToken(token: string): boolean {
+  // TODO: this should be refactored to check against format and length
   return validator.isAlphanumeric(token) || validator.isUUID(token)
 }
 
@@ -42,23 +43,6 @@ export function validateSHA(
   )
 }
 
-export function fetchToken(args: UploaderArgs): string {
-  // Token gets set in the following order:
-  // * args.token
-  // * process.env.CODECOV_TOKEN
-  // * ''
-  let token = ''
-  if (args.token && validateToken(args.token)) {
-    token = args.token
-  } else if (
-    process.env.CODECOV_TOKEN &&
-    validateToken(process.env.CODECOV_TOKEN)
-  ) {
-    token = process.env.CODECOV_TOKEN
-  }
-  return token
-}
-
 export function checkValueType(
   name: string,
   value: unknown,


Test patch:
diff --git a/test/helpers/token.test.ts b/test/helpers/token.test.ts
index 196afed..f04c264 100644
--- a/test/helpers/token.test.ts
+++ b/test/helpers/token.test.ts
@@ -3,6 +3,7 @@ import path from 'path'
 import * as fileHelpers from '../../src/helpers/files'
 import * as tokenHelpers from '../../src/helpers/token'
 import { UploaderInputs } from '../../src/types'
+import { DEFAULT_UPLOAD_HOST } from '../../src/helpers/constansts'
 
 describe('Get tokens', () => {
   const fixturesDir = path.join(
@@ -84,4 +85,41 @@ describe('Get tokens', () => {
       expect(tokenHelpers.getToken(inputs, '.')).toBe('')
     })
   })
+
+  it('should return token correctly from args when `-u` differs from default host', () => {
+    const inputs: UploaderInputs = {
+      args: {
+        url: 'dummy.local',
+        token: 'goodToken'
+      },
+      environment: {
+        CODECOV_TOKEN: 'badToken'
+      }
+    }
+    expect(tokenHelpers.getToken(inputs, fixturesDir)).toBe('goodToken')
+  })
+
+  it('should return token correctly from env when `-u` differs from default host', () => {
+    const inputs: UploaderInputs = {
+      args: {
+        url: 'dummy.local',
+      },
+      environment: {
+        CODECOV_TOKEN: 'goodT----oken'
+      }
+    }
+    expect(tokenHelpers.getToken(inputs, fixturesDir)).toBe('goodT----oken')
+  })
+
+  it('should fail validation when an invalid token is passed and host is not changed', () => {
+    const inputs: UploaderInputs = {
+      args: {
+        url: DEFAULT_UPLOAD_HOST
+      },
+      environment: {
+        CODECOV_TOKEN: 'bad------Token'
+      }
+    }
+    expect(() => tokenHelpers.getToken(inputs, fixturesDir)).toThrowError(/Token found by environment variables with length 14 did not pass validation/)
+  })
 })
diff --git a/test/helpers/util.test.ts b/test/helpers/util.test.ts
index b1ac811..5f92966 100644
--- a/test/helpers/util.test.ts
+++ b/test/helpers/util.test.ts
@@ -31,7 +31,7 @@ describe('runExternalProgram()', () => {
 
     })
 
-    it('should return stdout on success', () => {
+    it('should throw an error when program returns an error', () => {
         const spawnSync = td.replace(childProcess, 'spawnSync')
         td.when(spawnSync('ls', ['-geewhiz'])).thenReturn({
           stdout: 'I am output',
@@ -39,19 +39,4 @@ describe('runExternalProgram()', () => {
         expect(runExternalProgram('ls', ['-geewhiz'])).toEqual("I am output")
 
     })
-
-    it('should return a trimmed string with no newlines', () => {
-        const spawnSync = td.replace(childProcess, 'spawnSync')
-        td.when(spawnSync('ls', ['-geewhiz'])).thenReturn({
-          stdout: `
-          
-          
-          I am output
-          
-          
-          `,
-        })
-        expect(runExternalProgram('ls', ['-geewhiz'])).toEqual("I am output")
-
-    })
 })
\ No newline at end of file
diff --git a/test/helpers/validate.test.ts b/test/helpers/validate.test.ts
index 0d7c760..8b00c89 100644
--- a/test/helpers/validate.test.ts
+++ b/test/helpers/validate.test.ts
@@ -1,14 +1,20 @@
 import * as validate from '../../src/helpers/validate'
 import { checkValueType } from '../../src/helpers/validate'
+import { DEFAULT_UPLOAD_HOST } from '../../src/helpers/constansts'
+import Module from 'module'
 
 // Backup the env
 const realEnv = { ...process.env }
 
 describe('Input Validators', () => {
   describe('checkValueType()', () => {
-it('throws an error when the value is not the correct type', () => {
-  expect(() => { checkValueType('testValue', {}, 'number')}).toThrowError(/The value of testValue is not of type number, can not continue, please review/)
-})
+    it('throws an error when the value is not the correct type', () => {
+      expect(() => {
+        checkValueType('testValue', {}, 'number')
+      }).toThrowError(
+        /The value of testValue is not of type number, can not continue, please review/,
+      )
+    })
   })
 
   describe('Tokens', () => {
@@ -88,34 +94,4 @@ it('throws an error when the value is not the correct type', () => {
       ).toBe(true)
     })
   })
-
-  describe('fetchToken()', () => {
-    beforeEach(() => {
-      delete process.env.CODECOV_TOKEN
-    })
-
-    afterEach(() => {
-      process.env = realEnv
-    })
-
-    it('should return an empty string if neither args or env are set', () => {
-      const args = { flags: '' }
-      expect(validate.fetchToken(args)).toEqual('')
-    })
-
-    it('should return the value of the env if env is set, and args are not set', () => {
-      process.env.CODECOV_TOKEN = 'testingEnvToken'
-      const args = { flags: '' }
-      expect(validate.fetchToken(args)).toEqual('testingEnvToken')
-    })
-
-    it('should return the value of the arg if env is set, and args are set', () => {
-      process.env.CODECOV_TOKEN = 'testingEnvToken'
-      const args = {
-        token: 'testingArgToken',
-        flags: '',
-      }
-      expect(validate.fetchToken(args)).toEqual('testingArgToken')
-    })
-  })
 })
diff --git a/test/providers/provider_bitbucket.test.ts b/test/providers/provider_bitbucket.test.ts
index 7d69ae8..085b70b 100644
--- a/test/providers/provider_bitbucket.test.ts
+++ b/test/providers/provider_bitbucket.test.ts
@@ -158,9 +158,9 @@ describe('Bitbucket Params', () => {
       slug: 'testOwner/testSlug',
     }
 
-    const execFileSync = td.replace(childProcess, 'spawnSync')
+    const execFileSync = td.replace(childProcess, 'execFileSync')
     td.when(execFileSync('git', ['rev-parse', 'testingsha12'])).thenReturn(
-      { stdout: 'newtestsha'},
+      'newtestsha',
     )
     const params = providerBitbucket.getServiceParams(inputs)
     expect(params).toMatchObject(expected)