Repository: codecov/uploader
Pull request #326 (2021-08-23 15:05:50)

Release: Token handling fixes
- #325 
- #321 
- #315 
- #314 


Patch:
diff --git a/.circleci/config.yml b/.circleci/config.yml
index af2f86a..d01500e 100644
--- a/.circleci/config.yml
+++ b/.circleci/config.yml
@@ -5,7 +5,7 @@
 version: 2.1
 
 orbs:
-  win: circleci/windows@2.4.0
+  win: circleci/windows@2.4.1
 
 jobs:
   build-linux-and-osx:
@@ -403,7 +403,7 @@ jobs:
             - release_tag
   release:
     docker:
-      - image: google/cloud-sdk:352.0.0-slim
+      - image: google/cloud-sdk:353.0.0-slim
     steps:
       - attach_workspace:
           # Must be absolute path or relative path from working_directory
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 161713b..f5c0c34 100644
--- a/src/ci_providers/provider_bitbucket.ts
+++ b/src/ci_providers/provider_bitbucket.ts
@@ -1,4 +1,5 @@
 import childProcess from 'child_process'
+import { runExternalProgram } from '../helpers/util'
 import { validateSHA } from '../helpers/validate'
 import { IServiceParams, UploaderEnvs, UploaderInputs } from '../types'
 
@@ -44,7 +45,7 @@ function _getSHA(inputs: UploaderInputs): string {
   let commit = envs.BITBUCKET_COMMIT || ''
 
   if (commit && validateSHA(commit, 12)) {
-    commit = childProcess.execFileSync('git', ['rev-parse', commit])
+    commit = runExternalProgram('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 9a30d97..c5087c0 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().trimRight()
+  return result.stdout.toString().trim()
 }
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 5f92966..b1ac811 100644
--- a/test/helpers/util.test.ts
+++ b/test/helpers/util.test.ts
@@ -31,7 +31,7 @@ describe('runExternalProgram()', () => {
 
     })
 
-    it('should throw an error when program returns an error', () => {
+    it('should return stdout on success', () => {
         const spawnSync = td.replace(childProcess, 'spawnSync')
         td.when(spawnSync('ls', ['-geewhiz'])).thenReturn({
           stdout: 'I am output',
@@ -39,4 +39,19 @@ 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 085b70b..7d69ae8 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, 'execFileSync')
+    const execFileSync = td.replace(childProcess, 'spawnSync')
     td.when(execFileSync('git', ['rev-parse', 'testingsha12'])).thenReturn(
-      'newtestsha',
+      { stdout: 'newtestsha'},
     )
     const params = providerBitbucket.getServiceParams(inputs)
     expect(params).toMatchObject(expected)