From 9f082d97ca45d4512478f8b212a2abe89bb30fb9 Mon Sep 17 00:00:00 2001 From: Gleb Bahmutov Date: Mon, 23 Sep 2019 11:59:49 -0400 Subject: [PATCH] Catch env variable with reserved name CYPRESS_ENV 1621 (#1626) * server: check CYPRESS_ENV variable when merging configs * catch invalid CYPRESS_ENV value in CLI, close #1621 * linting * sanitize platform in test snapshot * linting * update error message text * add missing comma * fix finally merge in JS code * pass CLI linter * fix log reference, should be debug * use correct sinon reference * update message, show first part in red * update error message text --- cli/__snapshots__/cli_spec.js | 29 +++++ cli/__snapshots__/errors_spec.js | 1 + cli/index.js | 2 +- cli/lib/cli.js | 107 +++++++++++++++---- cli/lib/errors.js | 79 ++++++++++---- cli/lib/util.js | 21 +++- cli/package.json | 3 +- cli/test/lib/cli_spec.js | 52 +++++++++ packages/server/lib/config.coffee | 12 ++- packages/server/lib/errors.coffee | 8 ++ packages/server/test/unit/config_spec.coffee | 30 +++++- 11 files changed, 293 insertions(+), 51 deletions(-) diff --git a/cli/__snapshots__/cli_spec.js b/cli/__snapshots__/cli_spec.js index d471f77b5da0..5465cc92f7d9 100644 --- a/cli/__snapshots__/cli_spec.js +++ b/cli/__snapshots__/cli_spec.js @@ -354,3 +354,32 @@ exports['shows help for run --foo 1'] = ` ------- ` + +exports['cli CYPRESS_ENV allows staging environment 1'] = ` + code: 0 + stderr: + ------- + + ------- + +` + +exports['cli CYPRESS_ENV catches environment "foo" 1'] = ` + code: 11 + stderr: + ------- + The environment variable with the reserved name "CYPRESS_ENV" is set. + + Unset the "CYPRESS_ENV" environment variable and run Cypress again. + + ---------- + + CYPRESS_ENV=foo + + ---------- + + Platform: xxx + Cypress Version: 1.2.3 + ------- + +` diff --git a/cli/__snapshots__/errors_spec.js b/cli/__snapshots__/errors_spec.js index f8333d37acba..44b1a631d786 100644 --- a/cli/__snapshots__/errors_spec.js +++ b/cli/__snapshots__/errors_spec.js @@ -32,6 +32,7 @@ exports['errors individual has the following errors 1'] = [ "failedDownload", "failedUnzip", "invalidCacheDirectory", + "invalidCypressEnv", "invalidSmokeTestDisplayError", "missingApp", "missingDependency", diff --git a/cli/index.js b/cli/index.js index 7da84f9ec87b..250d3d4b52b1 100644 --- a/cli/index.js +++ b/cli/index.js @@ -23,6 +23,6 @@ switch (args.exec) { break default: - // export our node module interface + debug('exporting Cypress module interface') module.exports = require('./lib/cypress') } diff --git a/cli/lib/cli.js b/cli/lib/cli.js index dc6a6e82e7c8..fb94e25eebc6 100644 --- a/cli/lib/cli.js +++ b/cli/lib/cli.js @@ -5,6 +5,7 @@ const logSymbols = require('log-symbols') const debug = require('debug')('cypress:cli') const util = require('./util') const logger = require('./logger') +const errors = require('./errors') const cache = require('./tasks/cache') // patch "commander" method called when a user passed an unknown option @@ -27,7 +28,9 @@ const coerceFalse = (arg) => { const spaceDelimitedSpecsMsg = (files) => { logger.log() logger.warn(stripIndent` - ${logSymbols.warning} Warning: It looks like you're passing --spec a space-separated list of files: + ${ + logSymbols.warning +} Warning: It looks like you're passing --spec a space-separated list of files: "${files.join(' ')}" @@ -54,7 +57,8 @@ const parseVariableOpts = (fnArgs, args) => { const nextOptOffset = _.findIndex(_.slice(args, argIndex), (arg) => { return _.startsWith(arg, '--') }) - const endIndex = nextOptOffset !== -1 ? argIndex + nextOptOffset : args.length + const endIndex = + nextOptOffset !== -1 ? argIndex + nextOptOffset : args.length const maybeSpecs = _.slice(args, argIndex, endIndex) const extraSpecs = _.intersection(maybeSpecs, fnArgs) @@ -70,11 +74,34 @@ const parseVariableOpts = (fnArgs, args) => { } const parseOpts = (opts) => { - opts = _.pick(opts, - 'project', 'spec', 'reporter', 'reporterOptions', 'path', 'destination', - 'port', 'env', 'cypressVersion', 'config', 'record', 'key', - 'browser', 'detached', 'headed', 'global', 'dev', 'force', 'exit', - 'cachePath', 'cacheList', 'cacheClear', 'parallel', 'group', 'ciBuildId') + opts = _.pick( + opts, + 'project', + 'spec', + 'reporter', + 'reporterOptions', + 'path', + 'destination', + 'port', + 'env', + 'cypressVersion', + 'config', + 'record', + 'key', + 'browser', + 'detached', + 'headed', + 'global', + 'dev', + 'force', + 'exit', + 'cachePath', + 'cacheList', + 'cacheClear', + 'parallel', + 'group', + 'ciBuildId' + ) if (opts.exit) { opts = _.omit(opts, 'exit') @@ -86,16 +113,23 @@ const parseOpts = (opts) => { } const descriptions = { - record: 'records the run. sends test results, screenshots and videos to your Cypress Dashboard.', - key: 'your secret Record Key. you can omit this if you set a CYPRESS_RECORD_KEY environment variable.', + record: + 'records the run. sends test results, screenshots and videos to your Cypress Dashboard.', + key: + 'your secret Record Key. you can omit this if you set a CYPRESS_RECORD_KEY environment variable.', spec: 'runs a specific spec file. defaults to "all"', - reporter: 'runs a specific mocha reporter. pass a path to use a custom reporter. defaults to "spec"', + reporter: + 'runs a specific mocha reporter. pass a path to use a custom reporter. defaults to "spec"', reporterOptions: 'options for the mocha reporter. defaults to "null"', port: 'runs Cypress on a specific port. overrides any value in cypress.json.', - env: 'sets environment variables. separate multiple values with a comma. overrides any value in cypress.json or cypress.env.json', - config: 'sets configuration values. separate multiple values with a comma. overrides any value in cypress.json.', - browserRunMode: 'runs Cypress in the browser with the given name. if a filesystem path is supplied, Cypress will attempt to use the browser at that path.', - browserOpenMode: 'path to a custom browser to be added to the list of available browsers in Cypress', + env: + 'sets environment variables. separate multiple values with a comma. overrides any value in cypress.json or cypress.env.json', + config: + 'sets configuration values. separate multiple values with a comma. overrides any value in cypress.json.', + browserRunMode: + 'runs Cypress in the browser with the given name. if a filesystem path is supplied, Cypress will attempt to use the browser at that path.', + browserOpenMode: + 'path to a custom browser to be added to the list of available browsers in Cypress', detached: 'runs Cypress application in detached mode', project: 'path to the project', global: 'force Cypress into global mode as if its globally installed', @@ -108,11 +142,25 @@ const descriptions = { cacheList: 'list cached binary versions', cacheClear: 'delete all cached binaries', group: 'a named group for recorded runs in the Cypress dashboard', - parallel: 'enables concurrent runs and automatic load balancing of specs across multiple machines or processes', - ciBuildId: 'the unique identifier for a run on your CI provider. typically a "BUILD_ID" env var. this value is automatically detected for most CI providers', + parallel: + 'enables concurrent runs and automatic load balancing of specs across multiple machines or processes', + ciBuildId: + 'the unique identifier for a run on your CI provider. typically a "BUILD_ID" env var. this value is automatically detected for most CI providers', } -const knownCommands = ['version', 'run', 'open', 'install', 'verify', '-v', '--version', 'help', '-h', '--help', 'cache'] +const knownCommands = [ + 'version', + 'run', + 'open', + 'install', + 'verify', + '-v', + '--version', + 'help', + '-h', + '--help', + 'cache', +] const text = (description) => { if (!descriptions[description]) { @@ -123,9 +171,11 @@ const text = (description) => { } function includesVersion (args) { - return _.includes(args, 'version') || + return ( + _.includes(args, 'version') || _.includes(args, '--version') || _.includes(args, '-v') + ) } function showVersions () { @@ -147,6 +197,14 @@ module.exports = { args = process.argv } + if (!util.isValidCypressEnvValue(process.env.CYPRESS_ENV)) { + debug('invalid CYPRESS_ENV value', process.env.CYPRESS_ENV) + + return errors.exitWithError(errors.errors.invalidCypressEnv)( + `CYPRESS_ENV=${process.env.CYPRESS_ENV}` + ) + } + const program = new commander.Command() // bug in commaner not printing name @@ -177,7 +235,10 @@ module.exports = { .option('-k, --key ', text('key')) .option('-s, --spec ', text('spec')) .option('-r, --reporter ', text('reporter')) - .option('-o, --reporter-options ', text('reporterOptions')) + .option( + '-o, --reporter-options ', + text('reporterOptions') + ) .option('-p, --port ', text('port')) .option('-e, --env ', text('env')) .option('-c, --config ', text('config')) @@ -218,7 +279,9 @@ module.exports = { program .command('install') .usage('[options]') - .description('Installs the Cypress executable matching this package\'s version') + .description( + 'Installs the Cypress executable matching this package\'s version' + ) .option('-f, --force', text('forceInstall')) .action((opts) => { require('./tasks/install') @@ -229,7 +292,9 @@ module.exports = { program .command('verify') .usage('[options]') - .description('Verifies that Cypress is installed correctly and executable') + .description( + 'Verifies that Cypress is installed correctly and executable' + ) .option('--dev', text('dev'), coerceFalse) .action((opts) => { const defaultOpts = { force: true, welcomeMessage: false } diff --git a/cli/lib/errors.js b/cli/lib/errors.js index 1a83b96ce434..ee45a1c21c4a 100644 --- a/cli/lib/errors.js +++ b/cli/lib/errors.js @@ -36,7 +36,9 @@ const failedUnzip = { const missingApp = (binaryDir) => { return { - description: `No version of Cypress is installed in: ${chalk.cyan(binaryDir)}`, + description: `No version of Cypress is installed in: ${chalk.cyan( + binaryDir + )}`, solution: stripIndent` \nPlease reinstall Cypress by running: ${chalk.cyan('cypress install')} `, @@ -59,7 +61,8 @@ const binaryNotExecutable = (executable) => { const notInstalledCI = (executable) => { return { - description: 'The cypress npm package is installed, but the Cypress binary is missing.', + description: + 'The cypress npm package is installed, but the Cypress binary is missing.', solution: stripIndent`\n We expected the binary to be installed here: ${chalk.cyan(executable)} @@ -114,7 +117,7 @@ const smokeTestFailure = (smokeTestCommand, timedOut) => { const invalidSmokeTestDisplayError = { code: 'INVALID_SMOKE_TEST_DISPLAY_ERROR', description: 'Cypress verification failed.', - solution (msg) { + solution (msg) { return stripIndent` Cypress failed to start after spawning a new Xvfb server. @@ -152,7 +155,8 @@ const missingDependency = { } const invalidCacheDirectory = { - description: 'Cypress cannot write to the cache directory due to file permissions', + description: + 'Cypress cannot write to the cache directory due to file permissions', solution: stripIndent` See discussion and possible solutions at ${chalk.blue(util.getGitHubIssueUrl(1281))} @@ -165,7 +169,8 @@ const versionMismatch = { } const unexpected = { - description: 'An unexpected error occurred while verifying the Cypress executable.', + description: + 'An unexpected error occurred while verifying the Cypress executable.', solution: stripIndent` Please search Cypress documentation for possible solutions: @@ -179,10 +184,19 @@ const unexpected = { `, } +const invalidCypressEnv = { + description: + chalk.red('The environment variable with the reserved name "CYPRESS_ENV" is set.'), + solution: chalk.red('Unset the "CYPRESS_ENV" environment variable and run Cypress again.'), + exitCode: 11, +} + const removed = { CYPRESS_BINARY_VERSION: { description: stripIndent` - The environment variable CYPRESS_BINARY_VERSION has been renamed to CYPRESS_INSTALL_BINARY as of version ${chalk.green('3.0.0')} + The environment variable CYPRESS_BINARY_VERSION has been renamed to CYPRESS_INSTALL_BINARY as of version ${chalk.green( + '3.0.0' + )} `, solution: stripIndent` You should set CYPRESS_INSTALL_BINARY instead. @@ -190,7 +204,9 @@ const removed = { }, CYPRESS_SKIP_BINARY_INSTALL: { description: stripIndent` - The environment variable CYPRESS_SKIP_BINARY_INSTALL has been removed as of version ${chalk.green('3.0.0')} + The environment variable CYPRESS_SKIP_BINARY_INSTALL has been removed as of version ${chalk.green( + '3.0.0' + )} `, solution: stripIndent` To skip the binary install, set CYPRESS_INSTALL_BINARY=0 @@ -210,8 +226,7 @@ const CYPRESS_RUN_BINARY = { } function getPlatformInfo () { - return util.getOsVersionAsync() - .then((version) => { + return util.getOsVersionAsync().then((version) => { return stripIndent` Platform: ${os.platform()} (${version}) Cypress Version: ${util.pkgVersion()} @@ -220,8 +235,7 @@ function getPlatformInfo () { } function addPlatformInformation (info) { - return getPlatformInfo() - .then((platform) => { + return getPlatformInfo().then((platform) => { return merge(info, { platform }) }) } @@ -231,18 +245,18 @@ function addPlatformInformation (info) { * and if possible a way to solve it. Resolves with a string. */ function formErrorText (info, msg, prevMessage) { - return addPlatformInformation(info) - .then((obj) => { + return addPlatformInformation(info).then((obj) => { const formatted = [] function add (msg) { - formatted.push( - stripIndents(msg) - ) + formatted.push(stripIndents(msg)) } - la(is.unemptyString(obj.description), - 'expected error description to be text', obj.description) + la( + is.unemptyString(obj.description), + 'expected error description to be text', + obj.description + ) // assuming that if there the solution is a function it will handle // error message and (optional previous error message) @@ -258,8 +272,11 @@ function formErrorText (info, msg, prevMessage) { `) } else { - la(is.unemptyString(obj.solution), - 'expected error solution to be text', obj.solution) + la( + is.unemptyString(obj.solution), + 'expected error solution to be text', + obj.solution + ) add(` ${obj.description} @@ -312,13 +329,30 @@ const raise = (info) => { const throwFormErrorText = (info) => { return (msg, prevMessage) => { - return formErrorText(info, msg, prevMessage) - .then(raise(info)) + return formErrorText(info, msg, prevMessage).then(raise(info)) + } +} + +/** + * Forms full error message with error and OS details, prints to the error output + * and then exits the process. + * @param {ErrorInformation} info Error information {description, solution} + * @example return exitWithError(errors.invalidCypressEnv)('foo') + */ +const exitWithError = (info) => { + return (msg) => { + return formErrorText(info, msg).then((text) => { + // eslint-disable-next-line no-console + console.error(text) + process.exit(info.exitCode || 1) + }) } } module.exports = { raise, + exitWithError, + // formError, formErrorText, throwFormErrorText, hr, @@ -334,6 +368,7 @@ module.exports = { unexpected, failedDownload, failedUnzip, + invalidCypressEnv, invalidCacheDirectory, removed, CYPRESS_RUN_BINARY, diff --git a/cli/lib/util.js b/cli/lib/util.js index 9d0044835675..029c0bb1893c 100644 --- a/cli/lib/util.js +++ b/cli/lib/util.js @@ -119,6 +119,25 @@ function stdoutLineMatches (expectedLine, stdout) { return lines.some(lineMatches) } +/** + * Confirms if given value is a valid CYPRESS_ENV value. Undefined values + * are valid, because the system can set the default one. + * + * @param {string} value + * @example util.isValidCypressEnvValue(process.env.CYPRESS_ENV) + */ +function isValidCypressEnvValue (value) { + if (_.isUndefined(value)) { + // will get default value + return true + } + + // names of config environments, see "packages/server/config/app.yml" + const names = ['development', 'test', 'staging', 'production'] + + return _.includes(names, value) +} + /** * Prints NODE_OPTIONS using debug() module, but only * if DEBUG=cypress... is set @@ -158,7 +177,7 @@ const dequote = (str) => { const util = { normalizeModuleOptions, - + isValidCypressEnvValue, printNodeOptions, isCi () { diff --git a/cli/package.json b/cli/package.json index 30e1e8fa1780..37ff7f30be79 100644 --- a/cli/package.json +++ b/cli/package.json @@ -86,7 +86,8 @@ "shelljs": "0.8.3", "sinon": "7.2.2", "snap-shot-it": "7.8.0", - "spawn-mock": "1.0.0" + "spawn-mock": "1.0.0", + "strip-ansi": "4.0.0" }, "files": [ "bin", diff --git a/cli/test/lib/cli_spec.js b/cli/test/lib/cli_spec.js index 952441ad1132..42ed40e53a41 100644 --- a/cli/test/lib/cli_spec.js +++ b/cli/test/lib/cli_spec.js @@ -73,6 +73,58 @@ describe('cli', () => { }) }) + context('CYPRESS_ENV', () => { + /** + * Replaces line "Platform: ..." with "Platform: xxx" + * @param {string} s + */ + const replacePlatform = (s) => { + return s.replace(/Platform: .+/, 'Platform: xxx') + } + + /** + * Replaces line "Cypress Version: ..." with "Cypress Version: 1.2.3" + * @param {string} s + */ + const replaceCypressVersion = (s) => { + return s.replace(/Cypress Version: .+/, 'Cypress Version: 1.2.3') + } + + const sanitizePlatform = (text) => { + return text + .split(os.eol) + .map(replacePlatform) + .map(replaceCypressVersion) + .join(os.eol) + } + + it('allows staging environment', () => { + const options = { + env: { + CYPRESS_ENV: 'staging', + }, + // we are only interested in the exit code + filter: ['code', 'stderr'], + } + + return execa('bin/cypress', ['help'], options).then(snapshot) + }) + + it('catches environment "foo"', () => { + const options = { + env: { + CYPRESS_ENV: 'foo', + }, + // we are only interested in the exit code + filter: ['code', 'stderr'], + } + + return execa('bin/cypress', ['help'], options) + .then(sanitizePlatform) + .then(snapshot) + }) + }) + context('cypress version', () => { const binaryDir = '/binary/dir' diff --git a/packages/server/lib/config.coffee b/packages/server/lib/config.coffee index 7777795fec20..96f9a36bd1a8 100644 --- a/packages/server/lib/config.coffee +++ b/packages/server/lib/config.coffee @@ -10,7 +10,7 @@ origin = require("./util/origin") coerce = require("./util/coerce") settings = require("./util/settings") v = require("./util/validation") -debug = require("debug")("cypress:server:config") +debug = require("debug")("cypress:server:config") pathHelpers = require("./util/path_helpers") CYPRESS_ENV_PREFIX = "CYPRESS_" @@ -207,6 +207,11 @@ hideSpecialVals = (val, key) -> module.exports = { getConfigKeys: -> configKeys + isValidCypressEnvValue: (value) -> + # names of config environments, see "config/app.yml" + names = ["development", "test", "staging", "production"] + _.includes(names, value) + whitelist: (obj = {}) -> _.pick(obj, configKeys.concat(breakingConfigKeys)) @@ -265,7 +270,12 @@ module.exports = { ## split out our own app wide env from user env variables ## and delete envFile config.env = @parseEnv(config, options.env, resolved) + config.cypressEnv = process.env["CYPRESS_ENV"] + debug("using CYPRESS_ENV %s", config.cypressEnv) + if not @isValidCypressEnvValue(config.cypressEnv) + errors.throw("INVALID_CYPRESS_ENV", config.cypressEnv) + delete config.envFile ## when headless diff --git a/packages/server/lib/errors.coffee b/packages/server/lib/errors.coffee index 02ff1114092f..b5c329493b0c 100644 --- a/packages/server/lib/errors.coffee +++ b/packages/server/lib/errors.coffee @@ -826,6 +826,14 @@ getMsgByType = (type, arg1 = {}, arg2) -> """ Cypress detected policy settings on your computer that may cause issues with using this browser. For more information, see https://on.cypress.io/bad-browser-policy """ + when "INVALID_CYPRESS_ENV" + """ + We have detected unknown or unsupported CYPRESS_ENV value + + #{chalk.yellow(arg1)} + + Please do not modify CYPRESS_ENV value. + """ get = (type, arg1, arg2) -> msg = getMsgByType(type, arg1, arg2) diff --git a/packages/server/test/unit/config_spec.coffee b/packages/server/test/unit/config_spec.coffee index 8880304be789..57e4f20e47a6 100644 --- a/packages/server/test/unit/config_spec.coffee +++ b/packages/server/test/unit/config_spec.coffee @@ -3,10 +3,11 @@ require("../spec_helper") _ = require("lodash") path = require("path") R = require("ramda") -config = require("#{root}lib/config") -configUtil = require("#{root}lib/util/config") -scaffold = require("#{root}lib/scaffold") -settings = require("#{root}lib/util/settings") +config = require("#{root}lib/config") +errors = require("#{root}lib/errors") +configUtil = require("#{root}lib/util/config") +scaffold = require("#{root}lib/scaffold") +settings = require("#{root}lib/util/settings") describe "lib/config", -> beforeEach -> @@ -17,6 +18,27 @@ describe "lib/config", -> afterEach -> process.env = @env + context "environment name check", -> + it "throws an error for unknown CYPRESS_ENV", -> + sinon.stub(errors, "throw").withArgs("INVALID_CYPRESS_ENV", "foo-bar") + process.env.CYPRESS_ENV = "foo-bar" + cfg = { + projectRoot: "/foo/bar/" + } + options = {} + config.mergeDefaults(cfg, options) + expect(errors.throw).have.been.calledOnce + + it "allows known CYPRESS_ENV", -> + sinon.stub(errors, "throw") + process.env.CYPRESS_ENV = "test" + cfg = { + projectRoot: "/foo/bar/" + } + options = {} + config.mergeDefaults(cfg, options) + expect(errors.throw).not.to.be.called + context ".get", -> beforeEach -> @projectRoot = "/_test-output/path/to/project"