Skip to content

Commit

Permalink
feat: disallow installing root plugin (#888)
Browse files Browse the repository at this point in the history
* feat: disallow installing root plugin

* test: plugins command tests

* test: install unit test

* chore: update message
  • Loading branch information
mdonnalley authored Jun 12, 2024
1 parent cfb22b0 commit 0ac0ffd
Show file tree
Hide file tree
Showing 6 changed files with 167 additions and 13 deletions.
4 changes: 2 additions & 2 deletions package.json
Original file line number Diff line number Diff line change
Expand Up @@ -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"
},
Expand Down Expand Up @@ -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\"",
Expand Down
7 changes: 5 additions & 2 deletions src/commands/plugins/index.ts
Original file line number Diff line number Diff line change
Expand Up @@ -24,7 +24,7 @@ export default class PluginsIndex extends Command {

plugins!: Plugins

async run(): Promise<PluginsJson> {
public async run(): Promise<PluginsJson> {
const {flags} = await this.parse(PluginsIndex)
this.plugins = new Plugins({
config: this.config,
Expand Down Expand Up @@ -70,15 +70,18 @@ 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)
}

return tree
}

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)
Expand Down
21 changes: 21 additions & 0 deletions src/commands/plugins/install.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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', {
Expand Down
116 changes: 116 additions & 0 deletions test/commands/plugins/index.test.ts
Original file line number Diff line number Diff line change
@@ -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<Array<{name: string}>>('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'])
})
})
14 changes: 14 additions & 0 deletions test/commands/plugins/install.test.ts
Original file line number Diff line number Diff line change
@@ -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.')
})
})
18 changes: 9 additions & 9 deletions yarn.lock
Original file line number Diff line number Diff line change
Expand Up @@ -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"
Expand Down Expand Up @@ -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:
Expand Down

0 comments on commit 0ac0ffd

Please sign in to comment.