From 0c2d079543eacc7f22b9fff48921d496e91ca47c Mon Sep 17 00:00:00 2001 From: Aaron Franks Date: Mon, 14 Aug 2017 06:29:03 -0700 Subject: [PATCH 1/5] In strict mode, wrap output in a proxy that throws for missing keys --- index.js | 2 ++ lib/strictProxy.js | 30 ++++++++++++++++++++++++++++++ tests/test_basics.js | 8 -------- tests/test_dotenv.js | 8 -------- tests/test_strict.js | 40 ++++++++++++++++++++++++++++++++++++++++ 5 files changed, 72 insertions(+), 16 deletions(-) create mode 100644 lib/strictProxy.js create mode 100644 tests/test_strict.js diff --git a/index.js b/index.js index 6ebe9bd..a9d15d9 100644 --- a/index.js +++ b/index.js @@ -117,6 +117,8 @@ function cleanEnv(inputEnv, specs = {}, options = {}) { const reporter = options.reporter || require('./lib/reporter') reporter({ errors, env: output }) + if (options.strict) output = require('./lib/strictProxy')(output) + return Object.freeze(output) } diff --git a/lib/strictProxy.js b/lib/strictProxy.js new file mode 100644 index 0000000..dab84cf --- /dev/null +++ b/lib/strictProxy.js @@ -0,0 +1,30 @@ +/** +* Wrap the environment object with a Proxy that throws when: +* a) trying to mutate an env var +* b) trying to access an invalid (unset) env var +* +* @return {Object} - Proxied environment object with get/set traps +*/ +module.exports = envObj => new Proxy(envObj, { + get(target, name) { + // These checks are needed because calling console.log on a + // proxy that throws crashes the entire process. This whitelists + // the necessary properties for `console.log(envObj)` to work. + if (['inspect', Symbol.toStringTag].includes(name)) return envObj[name] + if (name.toString() === 'Symbol(util.inspect.custom)') return envObj[name] + + // if (name === 'prototype') return {} + // if (name === 'nodeType') return undefined + + const val = envObj[name] + if (val === undefined) { + throw new Error(`Environment var not found: ${name}`) + } else { + return val + } + }, + + set(name) { + throw new Error(`[envalid] Attempt to mutate environment value: ${name}`) + }, +}) diff --git a/tests/test_basics.js b/tests/test_basics.js index 9c3f284..ebc3d6c 100644 --- a/tests/test_basics.js +++ b/tests/test_basics.js @@ -8,14 +8,6 @@ test('string passthrough', () => { assertPassthrough({ FOO: 'bar' }, { FOO: str() }) }) -test('strict option: only specified fields are passed through', () => { - const opts = { strict: true } - const env = cleanEnv({ FOO: 'bar', BAZ: 'baz' }, { - FOO: str() - }, opts) - assert.deepEqual(env, { FOO: 'bar' }) -}) - test('transformer option: allow transformation of keys', () => { const lowerCaseKey = (acc, [key, value]) => Object.assign(acc, { [key.toLowerCase()]: value }) const opts = { diff --git a/tests/test_dotenv.js b/tests/test_dotenv.js index e6c29d3..18ea86a 100644 --- a/tests/test_dotenv.js +++ b/tests/test_dotenv.js @@ -18,14 +18,6 @@ test('.env contents are cleaned', () => { assert.deepEqual(env, { FOO: 'bar', BAR: 'asdfasdf', MYNUM: 4 }) }) -test('.env test in strict mode', () => { - const opts = { strict: true } - const env = cleanEnv({ FOO: 'bar', BAZ: 'baz' }, { - MYNUM: num() - }, opts) - assert.deepEqual(env, { MYNUM: 4 }) -}) - test('can opt out of dotenv with dotEnvPath=null', () => { const env = cleanEnv({ FOO: 'bar' }, {}, { dotEnvPath: null }) assert.deepEqual(env, { FOO: 'bar' }) diff --git a/tests/test_strict.js b/tests/test_strict.js new file mode 100644 index 0000000..e04daf4 --- /dev/null +++ b/tests/test_strict.js @@ -0,0 +1,40 @@ +const fs = require('fs') +const { createGroup, assert } = require('painless') +const { cleanEnv, str, num } = require('..') +const test = createGroup() +const strictOption = { strict: true, reporter: null } + + +// assert.deepEqual() substitute for assertions on proxied strict-mode env objects +// Chai's deepEqual() performs a few checks that the Proxy chokes on, so rather than +// adding special-case code inside the proxy's get() trap, we use this custom assert +// function +const objStrictDeepEqual = (actual, desired) => { + const desiredKeys = Object.keys(desired) + assert.deepEqual(Object.keys(actual), desiredKeys) + for (const k of desiredKeys) { + assert.strictEqual(actual[k], desired[k]) + } +} + +test.beforeEach(() => fs.writeFileSync('.env', ` +BAR=asdfasdf +MYNUM=4 +`)) +test.afterEach(() => fs.unlinkSync('.env')) + + +test('strict option: only specified fields are passed through', () => { + const env = cleanEnv({ FOO: 'bar', BAZ: 'baz' }, { + FOO: str() + }, strictOption) + objStrictDeepEqual(env, { FOO: 'bar' }) +}) + +test('.env test in strict mode', () => { + const env = cleanEnv({ FOO: 'bar', BAZ: 'baz' }, { + MYNUM: num() + }, strictOption) + objStrictDeepEqual(env, { MYNUM: 4 }) +}) + From 0cea66663e797f402bbab687cbe9a36e8dd285a1 Mon Sep 17 00:00:00 2001 From: Aaron Franks Date: Mon, 14 Aug 2017 07:05:57 -0700 Subject: [PATCH 2/5] Add tests for new strict mode behavior --- tests/test_strict.js | 17 ++++++++++++++++- 1 file changed, 16 insertions(+), 1 deletion(-) diff --git a/tests/test_strict.js b/tests/test_strict.js index e04daf4..fbf8b20 100644 --- a/tests/test_strict.js +++ b/tests/test_strict.js @@ -2,7 +2,7 @@ const fs = require('fs') const { createGroup, assert } = require('painless') const { cleanEnv, str, num } = require('..') const test = createGroup() -const strictOption = { strict: true, reporter: null } +const strictOption = { strict: true } // assert.deepEqual() substitute for assertions on proxied strict-mode env objects @@ -38,3 +38,18 @@ test('.env test in strict mode', () => { objStrictDeepEqual(env, { MYNUM: 4 }) }) +test('strict mode objects throw when invalid attrs are accessed', () => { + const env = cleanEnv({ FOO: 'bar', BAZ: 'baz' }, { + FOO: str() + }, strictOption) + assert.strictEqual(env.FOO, 'bar') + assert.throws(() => env.ASDF) +}) + +test('strict mode objects throw when attempting to mutate', () => { + const env = cleanEnv({ FOO: 'bar', BAZ: 'baz' }, { + FOO: str() + }, strictOption) + assert.throws(() => env.FOO = 'foooooo') +}) + From 0a91f08a2eb2c8877f0c3a2b2eb4c1a8d607305f Mon Sep 17 00:00:00 2001 From: Aaron Franks Date: Mon, 14 Aug 2017 07:24:36 -0700 Subject: [PATCH 3/5] Document new strict mode behavior in the readme --- README.md | 13 +++++++++++-- 1 file changed, 11 insertions(+), 2 deletions(-) diff --git a/README.md b/README.md index afbbf0c..0b90d8d 100644 --- a/README.md +++ b/README.md @@ -21,8 +21,7 @@ positional arguments: * `environment` - An object containing your env vars (eg. `process.env`) * `validators` - An object that specifies the format of required vars. * `options` - An (optional) object, which supports the following keys: - * `strict` - (default: `false`) If true, the output of `cleanEnv` will *only* - contain the env vars that were specified in the `validators` argument. + * `strict` - (default: `false`) Enable more rigorous behavior. See "Strict Mode" below * `reporter` - Pass in a function to override the default error handling and console output. See `lib/reporter.js` for the default implementation. * `transformer` - A function used to transform the cleaned environment object @@ -135,6 +134,16 @@ const env = cleanEnv(process.env, myValidators, { ``` +## Strict mode + +By passing the `{ strict: true }` option, envalid gives you extra tight guarantees +about the cleaned env object: + +* The env object will *only* contain the env vars that were specified by your `validators`. +* Any attempt to access an invalid/missing property on the env object will cause a thrown error. +* Any attempt to mutate the cleaned env object will cause a thrown error. + + ## `.env` File Support Envalid wraps the very handy [dotenv](https://www.npmjs.com/package/dotenv) package, From 300acbe32af45cfb3c3f8f2743008d08c8d8b2d0 Mon Sep 17 00:00:00 2001 From: Aaron Franks Date: Mon, 14 Aug 2017 08:42:08 -0700 Subject: [PATCH 4/5] Improvements from code review --- lib/strictProxy.js | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/lib/strictProxy.js b/lib/strictProxy.js index dab84cf..e55d76e 100644 --- a/lib/strictProxy.js +++ b/lib/strictProxy.js @@ -16,11 +16,11 @@ module.exports = envObj => new Proxy(envObj, { // if (name === 'prototype') return {} // if (name === 'nodeType') return undefined - const val = envObj[name] - if (val === undefined) { - throw new Error(`Environment var not found: ${name}`) + const varExists = envObj.hasOwnProperty(name) + if (!varExists) { + throw new Error(`[envalid] Environment var not found: ${name}`) } else { - return val + return envObj[name] } }, From beef35e8e65a4c83347617a7afb8135defae3bf0 Mon Sep 17 00:00:00 2001 From: Aaron Franks Date: Mon, 14 Aug 2017 09:00:17 -0700 Subject: [PATCH 5/5] Proxy code simplifications --- lib/strictProxy.js | 11 +++-------- 1 file changed, 3 insertions(+), 8 deletions(-) diff --git a/lib/strictProxy.js b/lib/strictProxy.js index e55d76e..72cd836 100644 --- a/lib/strictProxy.js +++ b/lib/strictProxy.js @@ -13,15 +13,10 @@ module.exports = envObj => new Proxy(envObj, { if (['inspect', Symbol.toStringTag].includes(name)) return envObj[name] if (name.toString() === 'Symbol(util.inspect.custom)') return envObj[name] - // if (name === 'prototype') return {} - // if (name === 'nodeType') return undefined - const varExists = envObj.hasOwnProperty(name) - if (!varExists) { - throw new Error(`[envalid] Environment var not found: ${name}`) - } else { - return envObj[name] - } + if (!varExists) throw new Error(`[envalid] Environment var not found: ${name}`) + + return envObj[name] }, set(name) {