Skip to content

Commit

Permalink
Prevent loading plugins for incorrect module #626 (#653)
Browse files Browse the repository at this point in the history
* fix: do not load plugin when they patch a different module than defined in config #626
  • Loading branch information
vmarchaud authored Feb 12, 2020
1 parent 56a9745 commit b1fe2c8
Show file tree
Hide file tree
Showing 11 changed files with 46 additions and 2 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -28,6 +28,11 @@ export interface Plugin<T = any> {
*/
supportedVersions?: string[];

/**
* Name of the module that the plugin instrument.
*/
moduleName: string;

/**
* Method that enables the instrumentation patch.
* @param moduleExports The value of the `module.exports` property that would
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -29,7 +29,7 @@ import * as path from 'path';
/** This class represent the base to patch plugin. */
export abstract class BasePlugin<T> implements Plugin<T> {
supportedVersions?: string[];
readonly moduleName?: string; // required for internalFilesExports
abstract readonly moduleName: string; // required for internalFilesExports
readonly version?: string; // required for internalFilesExports
protected readonly _basedir?: string; // required for internalFilesExports

Expand Down
10 changes: 9 additions & 1 deletion packages/opentelemetry-node/src/instrumentation/PluginLoader.ts
Original file line number Diff line number Diff line change
Expand Up @@ -132,8 +132,16 @@ export class PluginLoader {
// Expecting a plugin from module;
try {
const plugin: Plugin = require(modulePath).plugin;

if (!utils.isSupportedVersion(version, plugin.supportedVersions)) {
this.logger.error(
`PluginLoader#load: Plugin ${name} only supports module ${plugin.moduleName} with the versions: ${plugin.supportedVersions}`
);
return exports;
}
if (plugin.moduleName !== name) {
this.logger.error(
`PluginLoader#load: Entry ${name} use a plugin that instruments ${plugin.moduleName}`
);
return exports;
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -93,6 +93,13 @@ const alreadyRequiredPlugins: Plugins = {
},
};

const differentNamePlugins: Plugins = {
'random-module': {
enabled: true,
path: '@opentelemetry/plugin-http-module',
},
};

describe('PluginLoader', () => {
const provider = new NoopTracerProvider();
const logger = new NoopLogger();
Expand Down Expand Up @@ -243,6 +250,16 @@ describe('PluginLoader', () => {
pluginLoader.load(alreadyRequiredPlugins);
pluginLoader.unload();
});

it('should not load a plugin that patches a different module that the one configured', () => {
const pluginLoader = new PluginLoader(provider, logger);
assert.strictEqual(pluginLoader['_plugins'].length, 0);
pluginLoader.load(differentNamePlugins);
// @ts-ignore only to trigger the loading of the plugin
const randomModule = require('random-module');
assert.strictEqual(pluginLoader['_plugins'].length, 0);
pluginLoader.unload();
});
});

describe('.unload()', () => {
Expand Down

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

2 changes: 2 additions & 0 deletions packages/opentelemetry-web/test/WebTracer.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -27,6 +27,8 @@ class DummyPlugin extends BasePlugin<unknown> {
constructor() {
super('dummy');
}
moduleName = 'dummy';

patch() {}
unpatch() {}
}
Expand Down

0 comments on commit b1fe2c8

Please sign in to comment.