Skip to content

fix: Protect config files from declaring invalid keys #792

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 7 commits into from
Feb 8, 2017
Merged

fix: Protect config files from declaring invalid keys #792

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
7 commits
Select commit Hold shift + click to select a range
71fd8f6
throw an error for invalid keys in config
shubheksha Feb 6, 2017
f84d574
add check for camel cased options
shubheksha Feb 6, 2017
d475a0e
fix tests unnecessary errors
shubheksha Feb 6, 2017
ca9dd2e
add tests for invalid option
shubheksha Feb 6, 2017
8b3d2c5
fix nits
shubheksha Feb 6, 2017
21c6b6a
fix nits in tests
shubheksha Feb 7, 2017
6c650f0
Merge branch 'master' into fix/772
kumar303 Feb 8, 2017
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
3 changes: 2 additions & 1 deletion package.json
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -51,6 +51,7 @@
"bunyan": "1.8.5",
"camelcase": "4.0.0",
"debounce": "1.0.0",
"decamelize": "1.2.0",
"es6-error": "4.0.2",
"es6-promisify": "5.0.0",
"event-to-promise": "0.7.0",
Expand All @@ -65,9 +66,9 @@
"parse-json": "2.2.0",
"regenerator-runtime": "0.10.1",
"require-uncached": "1.0.3",
"stream-to-promise": "2.2.0",
"sign-addon": "0.2.1",
"source-map-support": "0.4.11",
"stream-to-promise": "2.2.0",
"tmp": "0.0.30",
"update-notifier": "1.0.3",
"watchpack": "1.2.0",
Expand Down
17 changes: 12 additions & 5 deletions src/config.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,8 @@
import path from 'path';

import requireUncached from 'require-uncached';
import camelCase from 'camelcase';
import decamelize from 'decamelize';

import {createLogger} from './util/logger';
import {UsageError} from './errors';
Expand All @@ -12,29 +14,34 @@ type ApplyConfigToArgvParams = {|
argv: Object,
configObject: Object,
defaultValues: Object,
configFileName: string,
|};

export function applyConfigToArgv({
argv,
configObject,
defaultValues = {},
configFileName,
}: ApplyConfigToArgvParams): Object {
const newArgv = {...argv};
for (const option in configObject) {
// we assume the value was set on the CLI if the default value is
// not the same as that on the argv object as there is a very rare chance
// this happening
// of this happening
if (camelCase(option) !== option) {
throw new UsageError(`The config option "${option}" must be ` +
`specified in camel case: "${camelCase(option)}"`);
}
const wasValueSetOnCLI = typeof(argv[option]) !== 'undefined' &&
(argv[option] !== defaultValues[option]);
if (wasValueSetOnCLI) {
log.debug(`Favoring CLI: ${option}=${argv[option]} over ` +
`configuration: ${option}=${configObject[option]}`);
continue;
}
if (!argv.hasOwnProperty(option)) {
log.debug(`Ignoring configuration: ${option}=${configObject[option]} ` +
'because this is an unknown option');
continue;
if (!argv.hasOwnProperty(decamelize(option, '-'))) {
throw new UsageError(`The config file at ${configFileName} specified ` +
`an unknown option: "${option}"`);
}
newArgv[option] = configObject[option];
}
Expand Down
83 changes: 68 additions & 15 deletions tests/unit/test.config.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -45,6 +45,11 @@ function makeArgv({
};
}

const applyConf = (params) => applyConfigToArgv({
configFileName: 'some/path/to/config.js',
...params,
});

describe('config', () => {
describe('applyConfigToArgv', () => {

Expand All @@ -64,7 +69,7 @@ describe('config', () => {
const configObject = {
sourceDir: '/configured/source/dir',
};
const newArgv = applyConfigToArgv({argv, configObject, defaultValues});
const newArgv = applyConf({argv, configObject, defaultValues});
assert.strictEqual(newArgv.sourceDir, cmdLineSrcDir);
});

Expand All @@ -82,7 +87,7 @@ describe('config', () => {
const configObject = {
sourceDir: '/configured/source/dir',
};
const newArgv = applyConfigToArgv({argv, configObject, defaultValues});
const newArgv = applyConf({argv, configObject, defaultValues});
assert.strictEqual(newArgv.sourceDir, configObject.sourceDir);
});

Expand All @@ -102,7 +107,7 @@ describe('config', () => {
const configObject = {
sourceDir: '/configured/source/dir',
};
const newArgv = applyConfigToArgv({argv, configObject, defaultValues});
const newArgv = applyConf({argv, configObject, defaultValues});
assert.strictEqual(newArgv.sourceDir, cmdLineSrcDir);
});

Expand All @@ -115,12 +120,16 @@ describe('config', () => {
demand: false,
default: 'default/value/option/definition',
},
'artifacts-dir': {
type: 'string',
demand: false,
},
},
});
const configObject = {
foo: '/configured/foo',
artifactsDir: '/configured/artifacts/dir',
};
const newArgv = applyConfigToArgv({argv, configObject, defaultValues});
const newArgv = applyConf({argv, configObject, defaultValues});
assert.strictEqual(newArgv.sourceDir, 'default/value/option/definition');
});

Expand All @@ -135,12 +144,16 @@ describe('config', () => {
demand: false,
default: 'default/value/option/definition',
},
'artifacts-dir': {
type: 'string',
demand: false,
},
},
});
const configObject = {
foo: '/configured/foo',
artifactsDir: '/configured/artifacts/dir',
};
const newArgv = applyConfigToArgv({argv, configObject, defaultValues});
const newArgv = applyConf({argv, configObject, defaultValues});
assert.strictEqual(newArgv.sourceDir, cmdLineSrcDir);
});

Expand All @@ -157,7 +170,7 @@ describe('config', () => {
const configObject = {
overwriteFiles: true,
};
const newArgv = applyConfigToArgv({argv, configObject, defaultValues});
const newArgv = applyConf({argv, configObject, defaultValues});
assert.strictEqual(newArgv.overwriteFiles, true);
});

Expand All @@ -173,7 +186,7 @@ describe('config', () => {
const configObject = {
overwriteFiles: true,
};
const newArgv = applyConfigToArgv({argv, configObject, defaultValues});
const newArgv = applyConf({argv, configObject, defaultValues});
assert.strictEqual(newArgv.overwriteFiles, true);
});

Expand All @@ -189,7 +202,7 @@ describe('config', () => {
const configObject = {
overwriteFiles: false,
};
const newArgv = applyConfigToArgv({argv, configObject, defaultValues});
const newArgv = applyConf({argv, configObject, defaultValues});
assert.strictEqual(newArgv.overwriteFiles, true);
});

Expand All @@ -201,12 +214,15 @@ describe('config', () => {
'source-dir': {
type: 'string',
},
'verbose': {
type: 'boolean',
},
},
});
const configObject = {
foo: true,
verbose: true,
};
const newArgv = applyConfigToArgv({argv, configObject, defaultValues});
const newArgv = applyConf({argv, configObject, defaultValues});
assert.strictEqual(newArgv.sourceDir, cmdLineSrcDir);
});

Expand All @@ -223,7 +239,7 @@ describe('config', () => {
const configObject = {
numberOfRetries: 1,
};
const newArgv = applyConfigToArgv({argv, configObject, defaultValues});
const newArgv = applyConf({argv, configObject, defaultValues});
assert.strictEqual(newArgv.numberOfRetries, 1);
});

Expand All @@ -240,7 +256,7 @@ describe('config', () => {
const configObject = {
numberOfRetries: 1,
};
const newArgv = applyConfigToArgv({argv, configObject, defaultValues});
const newArgv = applyConf({argv, configObject, defaultValues});
assert.strictEqual(newArgv.numberOfRetries, 0);
});

Expand All @@ -256,9 +272,46 @@ describe('config', () => {
const configObject = {
sourceDir: '/configured/directory',
};
const newArgv = applyConfigToArgv({argv, configObject, defaultValues});
const newArgv = applyConf({argv, configObject, defaultValues});
assert.strictEqual(newArgv.sourceDir, '/configured/directory');
});

it('throws an error when an option is not camel cased', () => {
const {argv, defaultValues} = makeArgv({
globalOpt: {
'source-dir': {
type: 'string',
demand: false,
},
},
});
const configObject = {
'source-dir': 'fake/value/',
};
assert.throws(() => {
applyConf({argv, configObject, defaultValues});
}, UsageError, 'UsageError: The config option "source-dir" must be ' +
'specified in camel case: "sourceDir"');
});

it('throws an error when an option is invalid', () => {
const {argv, defaultValues} = makeArgv({
globalOpt: {
'source-dir': {
type: 'string',
demand: false,
},
},
});
const configFileName = 'fake/path/to/config';
const configObject = {
artifactsDir: 'fake/artifacts/dir',
};
assert.throws(() => {
applyConf({argv, configObject, defaultValues, configFileName});
}, UsageError, 'UsageError: The config file at fake/path/to/config ' +
'specified an unknown option: "artifactsDir"');
});
});

describe('loadJSConfigFile', () => {
Expand Down