Skip to content

Commit

Permalink
use directory traversal in get-instance util
Browse files Browse the repository at this point in the history
  • Loading branch information
vikaspotluri123 committed Jan 29, 2022
1 parent 3a23360 commit d3fab15
Show file tree
Hide file tree
Showing 8 changed files with 63 additions and 37 deletions.
2 changes: 2 additions & 0 deletions lib/commands/doctor/index.js
Original file line number Diff line number Diff line change
Expand Up @@ -34,6 +34,8 @@ class DoctorCommand extends Command {
findValidInstall('doctor');
instance = this.system.getInstance();
instance.checkEnvironment();
} else {
instance = this.system.getInstance();
}

const context = {
Expand Down
7 changes: 6 additions & 1 deletion lib/commands/start.js
Original file line number Diff line number Diff line change
Expand Up @@ -26,7 +26,12 @@ class StartCommand extends Command {
const getInstance = require('../utils/get-instance');

const runOptions = {quiet: argv.quiet};
const instance = getInstance(argv.name, this.system, 'start');
const instance = getInstance({
name: argv.name,
system: this.system,
command: 'start',
recurse: !argv.dir
});

const isRunning = await instance.isRunning();
if (isRunning) {
Expand Down
7 changes: 6 additions & 1 deletion lib/commands/stop.js
Original file line number Diff line number Diff line change
Expand Up @@ -27,7 +27,12 @@ class StopCommand extends Command {
return this.stopAll();
}

const instance = getInstance(argv.name, this.system, 'stop');
const instance = getInstance({
name: argv.name,
system: this.system,
command: 'stop',
recurse: !argv.dir
});
const isRunning = await instance.isRunning();

if (!isRunning) {
Expand Down
25 changes: 18 additions & 7 deletions lib/utils/get-instance.js
Original file line number Diff line number Diff line change
@@ -1,20 +1,31 @@
const checkValidInstall = require('./check-valid-install');
const {SystemError} = require('../errors');
const findValidInstallation = require('./find-valid-install');

function getInstance(instanceName, system, commandName) {
const instance = system.getInstance(instanceName);

/**
* @param {object} options
* @param {string} options.name
* @param {import('../system.js')} options.system
* @param {string} options.command
* @param {boolean} options.recurse
*/
function getInstance({
name: instanceName,
system,
command: commandName,
recurse
}) {
if (instanceName) {
const instance = system.getInstance(instanceName);
if (!instance) {
throw new SystemError(`Ghost instance '${instanceName}' does not exist`);
}

process.chdir(instance.dir);
return instance;
}

checkValidInstall(commandName);

return instance;
findValidInstallation(commandName, recurse);
return system.getInstance();
}

module.exports = getInstance;
6 changes: 1 addition & 5 deletions test/unit/commands/doctor/command-spec.js
Original file line number Diff line number Diff line change
Expand Up @@ -121,7 +121,6 @@ describe('Unit: Commands > Doctor', function () {

return instance.run({skipInstanceCheck: true, local: true, argv: true}).then(() => {
expect(findValidStub.called).to.be.false;
expect(system.getInstance.called).to.be.false;
expect(system.hook.calledOnce).to.be.true;
expect(system.hook.calledWithExactly('doctor')).to.be.true;
expect(instanceStub.checkEnvironment.called).to.be.false;
Expand All @@ -136,7 +135,6 @@ describe('Unit: Commands > Doctor', function () {
const context = ui.listr.args[0][1];
expect(context.argv).to.deep.equal({skipInstanceCheck: true, local: true, argv: true});
expect(context.system).to.equal(system);
expect(context.instance).to.not.exist;
expect(context.ui).to.equal(ui);
expect(context.local).to.be.true;
expect(context.isDoctorCommand).to.be.false;
Expand Down Expand Up @@ -169,7 +167,6 @@ describe('Unit: Commands > Doctor', function () {
_: ['doctor']
}).then(() => {
expect(findValidStub.called).to.be.false;
expect(system.getInstance.called).to.be.false;
expect(system.hook.calledOnce).to.be.true;
expect(system.hook.calledWithExactly('doctor')).to.be.true;
expect(instanceStub.checkEnvironment.called).to.be.false;
Expand All @@ -184,7 +181,6 @@ describe('Unit: Commands > Doctor', function () {
_: ['doctor']
});
expect(context.system).to.equal(system);
expect(context.instance).to.not.exist;
expect(context.ui).to.equal(ui);
expect(context.local).to.be.true;
expect(context.isDoctorCommand).to.be.true;
Expand All @@ -210,7 +206,7 @@ describe('Unit: Commands > Doctor', function () {
const DoctorCommand = proxyquire(modulePath, {
'./checks': testChecks
});
instance = new DoctorCommand({listr: listrStub}, {hook: hookStub});
instance = new DoctorCommand({listr: listrStub}, {hook: hookStub, getInstance: sinon.stub()});
});

afterEach(() => {
Expand Down
35 changes: 21 additions & 14 deletions test/unit/commands/start-spec.js
Original file line number Diff line number Diff line change
Expand Up @@ -9,7 +9,6 @@ const UI = require('../../../lib/ui');
const DoctorCommand = require('../../../lib/commands/doctor');

const modulePath = '../../../lib/commands/start';
const StartCommand = require(modulePath);

function getStubs(dir, environment = undefined) {
const ui = new UI({});
Expand All @@ -22,23 +21,30 @@ function getStubs(dir, environment = undefined) {
instance._config.environment = environment;
system.environment = environment;

const getInstance = sinon.stub(system, 'getInstance').returns(instance);

return {
ui, system, instance, getInstance
ui, system, instance
};
}

describe('Unit: Commands > Start', function () {
describe('run', function () {
const oldArgv = process.argv;
let StartCommand;
let returnedInstance;

beforeEach(function () {
StartCommand = proxyquire(modulePath, {
'../utils/get-instance': sinon.stub().callsFake(() => returnedInstance),
});
})

afterEach(() => {
process.argv = oldArgv;
});

it('notifies and exits for already running instance', async function () {
const {ui, system, instance, getInstance} = getStubs('/var/www/ghost');
const {ui, system, instance} = getStubs('/var/www/ghost');
returnedInstance = instance;
const isRunning = sinon.stub(instance, 'isRunning').resolves(true);
const checkEnvironment = sinon.stub(instance, 'checkEnvironment');
const log = sinon.stub(ui, 'log');
Expand All @@ -49,7 +55,6 @@ describe('Unit: Commands > Start', function () {
const runCommand = sinon.stub(cmd, 'runCommand').resolves();

await cmd.run({});
expect(getInstance.calledOnce).to.be.true;
expect(isRunning.calledOnce).to.be.true;
expect(log.calledOnce).to.be.true;

Expand All @@ -61,6 +66,7 @@ describe('Unit: Commands > Start', function () {

it('warns of http use in production', async function () {
const {ui, system, instance} = getStubs('/var/www/ghost', 'production');
returnedInstance = instance;
const logStub = sinon.stub(ui, 'log');
const isRunning = sinon.stub(instance, 'isRunning').resolves(false);
const checkEnvironment = sinon.stub(instance, 'checkEnvironment');
Expand All @@ -85,6 +91,7 @@ describe('Unit: Commands > Start', function () {

it('no warning with ssl in production', async function () {
const {ui, system, instance} = getStubs('/var/www/ghost', 'production');
returnedInstance = instance;
const logStub = sinon.stub(ui, 'log');
const isRunning = sinon.stub(instance, 'isRunning').resolves(false);
const checkEnvironment = sinon.stub(instance, 'checkEnvironment');
Expand All @@ -105,7 +112,8 @@ describe('Unit: Commands > Start', function () {
});

it('runs startup checks and starts correctly', async function () {
const {ui, system, instance, getInstance} = getStubs('/var/www/ghost');
const {ui, system, instance} = getStubs('/var/www/ghost');
returnedInstance = instance;
const isRunning = sinon.stub(instance, 'isRunning').resolves(false);
const checkEnvironment = sinon.stub(instance, 'checkEnvironment');
const log = sinon.stub(ui, 'log');
Expand All @@ -118,24 +126,24 @@ describe('Unit: Commands > Start', function () {
instance.config.get.returns('http://localhost:2368');

await cmd.run({checkMem: false});
expect(getInstance.calledOnce).to.be.true;
expect(isRunning.calledOnce).to.be.true;
expect(checkEnvironment.calledOnce).to.be.true;
expect(runCommand.calledOnce).to.be.true;
expect(runCommand.calledWithExactly(DoctorCommand, {
expect(runCommand.args[0][1]).to.deep.equal({
categories: ['start'],
quiet: true,
checkMem: false,
skipInstanceCheck: true
})).to.be.true;
});
expect(run.calledOnce).to.be.true;
expect(start.calledOnce).to.be.true;
expect(log.calledTwice).to.be.true;
expect(instance.config.get.calledTwice).to.be.true;
});

it('doesn\'t log if quiet is set to true', async function () {
const {ui, system, instance, getInstance} = getStubs('/var/www/ghost');
const {ui, system, instance} = getStubs('/var/www/ghost');
returnedInstance = instance;
const isRunning = sinon.stub(instance, 'isRunning').resolves(false);
const checkEnvironment = sinon.stub(instance, 'checkEnvironment');
const log = sinon.stub(ui, 'log');
Expand All @@ -146,16 +154,15 @@ describe('Unit: Commands > Start', function () {
const runCommand = sinon.stub(cmd, 'runCommand').resolves();

await cmd.run({checkMem: false, quiet: true});
expect(getInstance.calledOnce).to.be.true;
expect(isRunning.calledOnce).to.be.true;
expect(checkEnvironment.calledOnce).to.be.true;
expect(runCommand.calledOnce).to.be.true;
expect(runCommand.calledWithExactly(DoctorCommand, {
expect(runCommand.args[0][1]).to.deep.equal({
categories: ['start'],
quiet: true,
checkMem: false,
skipInstanceCheck: true
})).to.be.true;
});
expect(run.calledOnce).to.be.true;
expect(start.calledOnce).to.be.true;
expect(log.called).to.be.false;
Expand Down
4 changes: 2 additions & 2 deletions test/unit/commands/stop-spec.js
Original file line number Diff line number Diff line change
Expand Up @@ -59,7 +59,7 @@ describe('Unit: Commands > Stop', function () {
await stop.run({name: 'testing'});

expect(getInstance.calledOnce).to.be.true;
expect(getInstance.args[0][0]).to.equal('testing');
expect(getInstance.args[0][0]?.name).to.equal('testing');
expect(isRunning.calledOnce).to.be.true;
expect(log.calledOnce).to.be.true;
});
Expand All @@ -78,7 +78,7 @@ describe('Unit: Commands > Stop', function () {
await cmd.run({name: 'testing'});

expect(getInstance.calledOnce).to.be.true;
expect(getInstance.args[0][0]).to.equal('testing');
expect(getInstance.args[0][0]?.name).to.equal('testing');
expect(isRunning.calledOnce).to.be.true;
expect(run.calledOnce).to.be.true;
expect(stop.calledOnce).to.be.true;
Expand Down
14 changes: 7 additions & 7 deletions test/unit/utils/get-instance-spec.js
Original file line number Diff line number Diff line change
Expand Up @@ -8,14 +8,14 @@ describe('Unit: Utils > getInstance', function () {
let getInstance, stubs, system;
beforeEach(function () {
stubs = {
checkValidInstall: sinon.stub().callsFake(a => a),
findValidInstallation: sinon.stub().callsFake(a => a),
chdir: sinon.stub(process, 'chdir'),
getInstance: sinon.stub().returns('It\'s-a Me, Mario!')
};

system = {getInstance: stubs.getInstance};
getInstance = proxyquire(modulePath, {
'./check-valid-install': stubs.checkValidInstall
'./find-valid-install': stubs.findValidInstallation
});
});

Expand All @@ -24,20 +24,20 @@ describe('Unit: Utils > getInstance', function () {
});

it('Doesn\'t change directory by default', function () {
const result = getInstance(undefined, system, 'test');
const result = getInstance({name: undefined, system, command: 'test', recurse: false});

expect(result).to.equal('It\'s-a Me, Mario!');
expect(stubs.getInstance.calledOnce).to.be.true;
expect(stubs.chdir.called).to.be.false;
expect(stubs.checkValidInstall.calledOnce).to.be.true;
expect(stubs.checkValidInstall.calledWithExactly('test')).to.be.true;
expect(stubs.findValidInstallation.calledOnce).to.be.true;
expect(stubs.findValidInstallation.calledWithExactly('test', false)).to.be.true;
});

it('Fails if the instance cannot be found', function () {
stubs.getInstance.returns(null);

try {
getInstance('ghosted', system, 'test');
getInstance({name: 'ghosted', system, command: 'test', recurse: false});
expect(false, 'Promise should have rejected').to.be.true;
} catch (error) {
expect(error).to.be.instanceof(SystemError);
Expand All @@ -49,7 +49,7 @@ describe('Unit: Utils > getInstance', function () {
const dir = '/path/to/ghost';
stubs.getInstance.returns({dir});

const result = getInstance('i', system, 'test');
const result = getInstance({name: 'i', system, command: 'test', recurse: false});

expect(result.dir).to.equal(dir);
expect(stubs.chdir.calledOnce).to.to.true;
Expand Down

0 comments on commit d3fab15

Please sign in to comment.