From 0ac0ffda71f9e30a36ff28877b9f8a0f7415ce34 Mon Sep 17 00:00:00 2001 From: Mike Donnalley Date: Wed, 12 Jun 2024 13:59:33 -0600 Subject: [PATCH] feat: disallow installing root plugin (#888) * feat: disallow installing root plugin * test: plugins command tests * test: install unit test * chore: update message --- package.json | 4 +- src/commands/plugins/index.ts | 7 +- src/commands/plugins/install.ts | 21 +++++ test/commands/plugins/index.test.ts | 116 ++++++++++++++++++++++++++ test/commands/plugins/install.test.ts | 14 ++++ yarn.lock | 18 ++-- 6 files changed, 167 insertions(+), 13 deletions(-) create mode 100644 test/commands/plugins/index.test.ts create mode 100644 test/commands/plugins/install.test.ts diff --git a/package.json b/package.json index 5245cea3..36bff5a7 100644 --- a/package.json +++ b/package.json @@ -43,7 +43,7 @@ "oclif": "^4.13.1", "prettier": "^3.3.1", "shx": "^0.3.4", - "sinon": "^17", + "sinon": "^18", "ts-node": "^10.9.2", "typescript": "^5.4.5" }, @@ -86,7 +86,7 @@ "posttest": "yarn lint", "prepack": "yarn build && oclif manifest && oclif readme", "prepare": "husky && yarn build", - "pretest": "yarn build --noEmit && tsc -p test --noEmit", + "pretest": "tsc -p test --noEmit", "test:integration:install": "mocha \"test/**/install.integration.ts\" --timeout 1200000", "test:integration:link": "mocha \"test/**/link.integration.ts\"", "test:integration:sf": "mocha \"test/**/sf.integration.ts\"", diff --git a/src/commands/plugins/index.ts b/src/commands/plugins/index.ts index 594b655f..8cefa80c 100644 --- a/src/commands/plugins/index.ts +++ b/src/commands/plugins/index.ts @@ -24,7 +24,7 @@ export default class PluginsIndex extends Command { plugins!: Plugins - async run(): Promise { + public async run(): Promise { const {flags} = await this.parse(PluginsIndex) this.plugins = new Plugins({ config: this.config, @@ -70,7 +70,7 @@ export default class PluginsIndex extends Command { private createTree(plugin: Plugin) { const tree: RecursiveTree = {} - for (const p of plugin.children) { + for (const p of plugin.children ?? []) { tree[this.formatPlugin(p)] = this.createTree(p) } @@ -78,7 +78,10 @@ export default class PluginsIndex extends Command { } private display(plugins: Plugin[]) { + const rootPlugin = plugins.find((p) => p.root === this.config.root) for (const plugin of plugins.filter((p: Plugin) => !p.parent)) { + // don't log the root plugin + if (plugin.name === rootPlugin?.name) continue this.log(this.formatPlugin(plugin)) if (plugin.children && plugin.children.length > 0) { const tree = this.createTree(plugin) diff --git a/src/commands/plugins/install.ts b/src/commands/plugins/install.ts index 5e5cf5b2..8b876f99 100644 --- a/src/commands/plugins/install.ts +++ b/src/commands/plugins/install.ts @@ -150,9 +150,30 @@ Use the <%= config.scopedEnvVarKey('NPM_REGISTRY') %> environment variable to se logLevel: determineLogLevel(this.config, this.flags, 'notice'), }) const aliases = this.config.pjson.oclif.aliases || {} + + const count = argv.length for (let name of argv as string[]) { if (aliases[name] === null) this.error(`${name} is blocked`) name = aliases[name] || name + + // Ignore the root plugin. Error if it's the only plugin being installed. + if (name === this.config.name) { + const updateInstructions = + // this.config.binPath is set when the CLI is installed from an installer but not when it's installed from npm + this.config.binPath && this.config.plugins.get('@oclif/plugin-update') + ? ` Use "${this.config.bin} update" to update ${this.config.name}.` + : '' + const msg = `Ignoring top-level CLI plugin ${this.config.name}.${updateInstructions}` + + if (count === 1) { + this.error(msg) + } else { + this.warn(msg) + } + + continue + } + const p = await this.parsePlugin(plugins, name) let plugin await this.config.runHook('plugins:preinstall', { diff --git a/test/commands/plugins/index.test.ts b/test/commands/plugins/index.test.ts new file mode 100644 index 00000000..0be6ac38 --- /dev/null +++ b/test/commands/plugins/index.test.ts @@ -0,0 +1,116 @@ +import {Config} from '@oclif/core' +import {runCommand} from '@oclif/test' +import {expect} from 'chai' +import sinon from 'sinon' + +describe('plugins', () => { + afterEach(() => { + sinon.restore() + }) + + it('shows no plugins installed', async () => { + const config = await Config.load(import.meta.url) + sinon.stub(config, 'getPluginsList').returns([]) + + const {stdout} = await runCommand('plugins', config, {print: true}) + expect(stdout).to.equal('No plugins installed.\n') + }) + + it('lists user plugins in stdout', async () => { + const config = await Config.load(import.meta.url) + const plugins = [ + ...config.getPluginsList(), + {name: 'user-plugin-1', type: 'user', version: '1.0.0'}, + {name: 'user-plugin-2', type: 'user', version: '1.0.0'}, + ] + // @ts-expect-error because we aren't stubbing the entire Plugin instance + sinon.stub(config, 'getPluginsList').returns(plugins) + + const {stdout} = await runCommand('plugins', config) + expect(stdout).to.equal('user-plugin-1 1.0.0\nuser-plugin-2 1.0.0\n') + }) + + it('lists nested user plugins in stdout', async () => { + const config = await Config.load(import.meta.url) + const plugins = [ + ...config.getPluginsList(), + { + children: [ + { + name: 'user-plugin-2', + type: 'user', + version: '1.0.0', + }, + ], + name: 'user-plugin-1', + type: 'user', + version: '1.0.0', + }, + ] + // @ts-expect-error because we aren't stubbing the entire Plugin instance + sinon.stub(config, 'getPluginsList').returns(plugins) + + const {stdout} = await runCommand('plugins', config) + expect(stdout).to.equal(`user-plugin-1 1.0.0 +└─ user-plugin-2 1.0.0 +`) + }) + + it('lists dev plugins in stdout with --core', async () => { + const config = await Config.load(import.meta.url) + const plugins = [ + ...config.getPluginsList(), + {name: 'dev-plugin-1', type: 'dev', version: '1.0.0'}, + {name: 'user-plugin-1', type: 'user', version: '1.0.0'}, + ] + // @ts-expect-error because we aren't stubbing the entire Plugin instance + sinon.stub(config, 'getPluginsList').returns(plugins) + + const {stdout} = await runCommand('plugins --core', config) + expect(stdout).to.equal('dev-plugin-1 1.0.0 (dev)\nuser-plugin-1 1.0.0\n') + }) + + it('lists uninstalled jit plugins in stdout with --core', async () => { + const config = await Config.load(import.meta.url) + sinon.stub(config.pjson, 'oclif').value({ + ...config.pjson.oclif, + jitPlugins: { + 'jit-plugin-1': '1.0.0', + 'jit-plugin-2': '1.0.0', + 'jit-plugin-3': '1.0.0', + }, + }) + const plugins = [ + ...config.getPluginsList(), + {name: 'user-plugin-1', type: 'user', version: '1.0.0'}, + {name: 'jit-plugin-1', type: 'user', version: '1.0.0'}, + ] + // @ts-expect-error because we aren't stubbing the entire Plugin instance + sinon.stub(config, 'getPluginsList').returns(plugins) + + const {stdout} = await runCommand('plugins --core', config) + expect(stdout).to.equal(`jit-plugin-1 1.0.0 +user-plugin-1 1.0.0 + +Uninstalled JIT Plugins: +jit-plugin-2 1.0.0 +jit-plugin-3 1.0.0 +`) + }) + + it('returns all plugin types and root plugin in json', async () => { + const config = await Config.load(import.meta.url) + const plugins = [ + ...config.getPluginsList(), + {name: 'user-plugin-1', type: 'user', version: '1.0.0'}, + {name: 'dev-plugin-1', type: 'dev', version: '1.0.0'}, + {name: 'core-plugin-1', type: 'core', version: '1.0.0'}, + ] + // @ts-expect-error because we aren't stubbing the entire Plugin instance + sinon.stub(config, 'getPluginsList').returns(plugins) + + const {result} = await runCommand>('plugins', config) + const pluginNames = result?.map((p) => p.name).sort() + expect(pluginNames).to.deep.equal(['@oclif/plugin-plugins', 'core-plugin-1', 'dev-plugin-1', 'user-plugin-1']) + }) +}) diff --git a/test/commands/plugins/install.test.ts b/test/commands/plugins/install.test.ts new file mode 100644 index 00000000..ea8b4001 --- /dev/null +++ b/test/commands/plugins/install.test.ts @@ -0,0 +1,14 @@ +import {runCommand} from '@oclif/test' +import {expect} from 'chai' +import sinon from 'sinon' + +describe('plugins:install', () => { + afterEach(() => { + sinon.restore() + }) + + it('fails if you install root plugin', async () => { + const {error} = await runCommand('plugins install @oclif/plugin-plugins') + expect(error?.message).to.equal('Ignoring top-level CLI plugin @oclif/plugin-plugins.') + }) +}) diff --git a/yarn.lock b/yarn.lock index 47120db0..a0ca03be 100644 --- a/yarn.lock +++ b/yarn.lock @@ -5521,10 +5521,10 @@ negotiator@^0.6.3: resolved "https://registry.npmjs.org/negotiator/-/negotiator-0.6.3.tgz" integrity sha512-+EUsqGPLsM+j/zdChZjsnX51g4XrHFOIXwfnCVPGlQk/k5giakcKsuxCObBRu6DSm9opw/O6slWbJdghQM4bBg== -nise@^5.1.9: - version "5.1.9" - resolved "https://registry.yarnpkg.com/nise/-/nise-5.1.9.tgz#0cb73b5e4499d738231a473cd89bd8afbb618139" - integrity sha512-qOnoujW4SV6e40dYxJOb3uvuoPHtmLzIk4TFo+j0jPJoC+5Z9xja5qH5JZobEPsa8+YYphMrOSwnrshEhG2qww== +nise@^6.0.0: + version "6.0.0" + resolved "https://registry.yarnpkg.com/nise/-/nise-6.0.0.tgz#ae56fccb5d912037363c3b3f29ebbfa28bde8b48" + integrity sha512-K8ePqo9BFvN31HXwEtTNGzgrPpmvgciDsFz8aztFjt4LqKO/JeFD8tBOeuDiCMXrIl/m1YvfH8auSpxfaD09wg== dependencies: "@sinonjs/commons" "^3.0.0" "@sinonjs/fake-timers" "^11.2.2" @@ -6522,16 +6522,16 @@ simple-swizzle@^0.2.2: dependencies: is-arrayish "^0.3.1" -sinon@^17: - version "17.0.2" - resolved "https://registry.yarnpkg.com/sinon/-/sinon-17.0.2.tgz#470894bcc2d24b01bad539722ea46da949892405" - integrity sha512-uihLiaB9FhzesElPDFZA7hDcNABzsVHwr3YfmM9sBllVwab3l0ltGlRV1XhpNfIacNDLGD1QRZNLs5nU5+hTuA== +sinon@^18: + version "18.0.0" + resolved "https://registry.yarnpkg.com/sinon/-/sinon-18.0.0.tgz#69ca293dbc3e82590a8b0d46c97f63ebc1e5fc01" + integrity sha512-+dXDXzD1sBO6HlmZDd7mXZCR/y5ECiEiGCBSGuFD/kZ0bDTofPYc6JaeGmPSF+1j1MejGUWkORbYOLDyvqCWpA== dependencies: "@sinonjs/commons" "^3.0.1" "@sinonjs/fake-timers" "^11.2.2" "@sinonjs/samsam" "^8.0.0" diff "^5.2.0" - nise "^5.1.9" + nise "^6.0.0" supports-color "^7" slash@^3.0.0: