Skip to content

Commit

Permalink
feat(doctor): detect incorrect permissions (#613)
Browse files Browse the repository at this point in the history
refs #47
closes #294

- Renames `folderPermissions` to `installFolderPermissions`
- Adds two more tasks: `filePermissions` and `folderPermissions` which check for the correct permissions inside of the ghost installation folder
	- not enabled when local processmanager is used (= local install)
	- calls `checkPermissions` util
	- `filePermissions` does not check the versions and current folder
- Refactors the `contentFolderPermissions` to be named `contentFolder` and disables task when content folder is not owned by ghost or ghost user is not found.
- Adds `checkPermissions` util which takes a property to specify which check should run:
	- `owner` checks for ownership of the content folder
	- `folder` checks if directories within the ghost installation folder have the correct permission (775)
	- `files` checks if files within the ghost installation folder have the correct permissions (664)
- Adds tests
  • Loading branch information
aileen authored and acburdine committed Feb 4, 2018
1 parent 8c68889 commit 6b614f7
Show file tree
Hide file tree
Showing 12 changed files with 407 additions and 121 deletions.
62 changes: 62 additions & 0 deletions lib/commands/doctor/checks/check-permissions.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,62 @@
'use strict';

const execa = require('execa');
const chalk = require('chalk');

const errors = require('../../../errors');

module.exports = function checkPermissions(type) {
let errMsg;

// fall back to check the owner permission of the content folder if nothing specified
type = type || 'owner';

const checkTypes = {
owner: {
command: 'find ./content ! -group ghost ! -user ghost',
help: `Run ${chalk.green('sudo chown -R ghost:ghost ./content')} and try again.`
},
folder: {
command: 'find ./ -type d ! -perm 775 ! -perm 755',
help: `Run ${chalk.green('sudo find ./ -type d -exec chmod 775 {} \\;')} and try again.`
},
files: {
command: 'find ./content ./system .ghost-cli *.json -type f ! -perm 664 ! -perm 644',
help: `Run ${chalk.green('sudo find ./content ./system .ghost-cli *.json -type f -exec chmod 664 {} \\;')} and try again.`
}
};

return execa.shell(checkTypes[type].command).then((result) => {
if (!result.stdout) {
return Promise.resolve();
}
const resultDirs = result.stdout.split('\n');
const dirWording = resultDirs.length > 1 ? 'some directories or files' : 'a directory or file';

errMsg = `Your installation folder contains ${dirWording} with incorrect permissions:\n`;

resultDirs.forEach((folder) => {
errMsg += `- ${folder}\n`
});

errMsg += checkTypes[type].help

return Promise.reject(new errors.SystemError(errMsg));
}).catch((error) => {
if (error instanceof errors.SystemError) {
return Promise.reject(error);
}

if (error.stderr && error.stderr.match(/Permission denied/i)) {
// We can't access some files or directories.
// Print the help command to fix that

errMsg = `Ghost can't access some files or directories to check for correct permissions.
${checkTypes.folder.help}`;

return Promise.reject(new errors.SystemError({message: errMsg, err: error}));
}

return Promise.reject(new errors.ProcessError(error));
});
}
40 changes: 5 additions & 35 deletions lib/commands/doctor/checks/content-folder.js
Original file line number Diff line number Diff line change
@@ -1,43 +1,13 @@
'use strict';
const execa = require('execa');

const path = require('path');
const Promise = require('bluebird');
const chalk = require('chalk');

const errors = require('../../../errors');
const checkPermissions = require('./check-permissions');
const shouldUseGhostUser = require('../../../utils/use-ghost-user');

function contentFolderPermissions() {
return execa.shell('find ./content ! -group ghost ! -user ghost').then((result) => {
let errMsg;

if (!result.stdout) {
return Promise.resolve();
}

const resultDirs = result.stdout.split('\n');
const dirWording = resultDirs.length > 1 ? 'some directories or files' : 'a directory or file';

errMsg = `Your content folder contains ${dirWording} with incorrect permissions:\n`;

resultDirs.forEach((folder) => {
errMsg += `- ${folder}\n`
});

errMsg += `Run ${chalk.blue('sudo chown -R ghost:ghost ./content')} and try again.`

return Promise.reject(new errors.SystemError(errMsg));
}).catch((error) => {
if (error instanceof errors.SystemError) {
return Promise.reject(error);
}
return Promise.reject(new errors.ProcessError(error));
});
}

module.exports = {
title: 'Content folder permissions',
title: 'Checking content folder ownership',
enabled: () => shouldUseGhostUser(path.join(process.cwd(), 'content')),
task: contentFolderPermissions,
task: () => checkPermissions('owner'),
category: ['start', 'update']
}
};
15 changes: 15 additions & 0 deletions lib/commands/doctor/checks/file-permissions.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,15 @@
'use strict';

const checkPermissions = require('./check-permissions');

module.exports = {
title: 'Checking file permissions',
enabled: (ctx) => {
const instance = ctx.system.getInstance();
const isLocal = instance.process.name === 'local' ? true : false;

return !isLocal;
},
task: () => checkPermissions('files'),
category: ['start', 'update']
};
30 changes: 10 additions & 20 deletions lib/commands/doctor/checks/folder-permissions.js
Original file line number Diff line number Diff line change
@@ -1,25 +1,15 @@
'use strict';
const fs = require('fs-extra');
const constants = require('constants');

const errors = require('../../../errors');
const checkDirectoryAndAbove = require('./check-directory');

function folderPermissions(ctx) {
return fs.access(process.cwd(), constants.R_OK | constants.W_OK).catch(() => {
return Promise.reject(new errors.SystemError(`The current directory is not writable.
Please fix your directory permissions.`));
}).then(() => {
if (ctx.local || !ctx.system.platform.linux || (ctx.argv && ctx.argv['setup-linux-user'] === false)) {
return Promise.resolve();
}

return checkDirectoryAndAbove(process.cwd(), 'run `ghost install`');
});
}
const checkPermissions = require('./check-permissions');

module.exports = {
title: 'Checking current folder permissions',
task: folderPermissions,
category: ['install', 'update']
title: 'Checking folder permissions',
enabled: (ctx) => {
const instance = ctx.system.getInstance();
const isLocal = instance.process.name === 'local' ? true : false;

return !isLocal;
},
task: () => checkPermissions('folder'),
category: ['start', 'update']
};
12 changes: 8 additions & 4 deletions lib/commands/doctor/checks/index.js
Original file line number Diff line number Diff line change
@@ -1,16 +1,20 @@
'use strict';
const nodeVersion = require('./node-version');
const folderPermissions = require('./folder-permissions');
const installFolderPermissions = require('./install-folder-permissions');
const systemStack = require('./system-stack');
const mysqlCheck = require('./mysql');
const validateConfig = require('./validate-config');
const contentFolderPermissions = require('./content-folder');
const folderPermissions = require('./folder-permissions');
const filePermissions = require('./file-permissions');
const contentFolder = require('./content-folder');

module.exports = [
nodeVersion,
folderPermissions,
installFolderPermissions,
systemStack,
mysqlCheck,
validateConfig,
contentFolderPermissions
folderPermissions,
filePermissions,
contentFolder
];
25 changes: 25 additions & 0 deletions lib/commands/doctor/checks/install-folder-permissions.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,25 @@
'use strict';
const fs = require('fs-extra');
const constants = require('constants');

const errors = require('../../../errors');
const checkDirectoryAndAbove = require('./check-directory');

function installFolderPermissions(ctx) {
return fs.access(process.cwd(), constants.R_OK | constants.W_OK).catch(() => {
return Promise.reject(new errors.SystemError(`The current directory is not writable.
Please fix your directory permissions.`));
}).then(() => {
if (ctx.local || !ctx.system.platform.linux || (ctx.argv && ctx.argv['setup-linux-user'] === false)) {
return Promise.resolve();
}

return checkDirectoryAndAbove(process.cwd(), 'run `ghost install`');
});
}

module.exports = {
title: 'Checking current folder permissions',
task: installFolderPermissions,
category: ['install', 'update']
};
48 changes: 48 additions & 0 deletions test/unit/commands/doctor/checks/check-permissions-spec.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,48 @@
'use strict';
const expect = require('chai').expect;
const sinon = require('sinon');

const execa = require('execa');
const errors = require('../../../../../lib/errors');

const checkPermissions = require('../../../../../lib/commands/doctor/checks/check-permissions');

describe('Unit: Doctor Checks > Util > checkPermissions', function () {
const sandbox = sinon.sandbox.create();

afterEach(function () {
sandbox.restore();
});

it('falls back to check owner permissions if not specified', function () {
const execaStub = sandbox.stub(execa, 'shell').resolves({stdout: ''});

return checkPermissions().then(() => {
expect(execaStub.calledWithExactly('find ./content ! -group ghost ! -user ghost')).to.be.true;
});
});

it('rejects with error if no Ghost can\'t access files', function () {
const execaStub = sandbox.stub(execa, 'shell').rejects({stderr: 'Permission denied'});

return checkPermissions('folder').then(() => {
expect(false, 'error should have been thrown').to.be.true;
}).catch((error) => {
expect(error).to.be.an.instanceof(errors.SystemError);
expect(error.message).to.match(/Ghost can't access some files or directories to check for correct permissions./);
expect(execaStub.calledWithExactly('find ./ -type d ! -perm 775 ! -perm 755')).to.be.true;
});
});

it('rejects with error if execa command fails', function () {
const execaStub = sandbox.stub(execa, 'shell').rejects(new Error('oops, cmd could not be executed'));

return checkPermissions('files').then(() => {
expect(false, 'error should have been thrown').to.be.true;
}).catch((error) => {
expect(error).to.be.an.instanceof(errors.ProcessError);
expect(error.message).to.match(/oops, cmd could not be executed/);
expect(execaStub.calledWithExactly('find ./content ./system .ghost-cli *.json -type f ! -perm 664 ! -perm 644')).to.be.true;
});
});
});
15 changes: 12 additions & 3 deletions test/unit/commands/doctor/checks/content-folder-spec.js
Original file line number Diff line number Diff line change
Expand Up @@ -7,14 +7,23 @@ const errors = require('../../../../../lib/errors');

const contentFolderPermissions = require('../../../../../lib/commands/doctor/checks/content-folder');

describe('Unit: Doctor Checks > Content folder permissions', function () {
describe('Unit: Doctor Checks > Checking content folder ownership', function () {
const sandbox = sinon.sandbox.create();
const shouldUseGhostUserStub = sinon.stub();

afterEach(() => {
sandbox.restore();
});

it('exports tasks', function () {
expect(contentFolderPermissions).to.be.an.instanceof(Object);
expect(contentFolderPermissions.title).to.match(/Checking content folder ownership/);
expect(contentFolderPermissions.task).to.be.an.instanceof(Function);
expect(contentFolderPermissions.enabled).to.be.an.instanceof(Function);
expect(contentFolderPermissions.category).to.be.an.instanceof(Array);
expect(contentFolderPermissions.category).to.have.length(2);
});

it('skips when content folder is not owned by ghost', function () {
shouldUseGhostUserStub.returns(false);
const execaStub = sandbox.stub(execa, 'shell').resolves();
Expand All @@ -33,7 +42,7 @@ describe('Unit: Doctor Checks > Content folder permissions', function () {
expect(false, 'error should have been thrown').to.be.true;
}).catch((error) => {
expect(error).to.be.an.instanceof(errors.SystemError);
expect(error.message).to.match(/Your content folder contains some directories or files with incorrect permissions:/);
expect(error.message).to.match(/Your installation folder contains some directories or files with incorrect permissions:/);
expect(error.message).to.match(/- \.\/content\/images/);
expect(execaStub.called).to.be.true;
});
Expand All @@ -48,7 +57,7 @@ describe('Unit: Doctor Checks > Content folder permissions', function () {
expect(false, 'error should have been thrown').to.be.true;
}).catch((error) => {
expect(error).to.be.an.instanceof(errors.SystemError);
expect(error.message).to.match(/Your content folder contains a directory or file with incorrect permissions/);
expect(error.message).to.match(/Your installation folder contains a directory or file with incorrect permissions:/);
expect(error.message).to.match(/- .\/content\/images\/test.jpg/);
expect(execaStub.called).to.be.true;
});
Expand Down
82 changes: 82 additions & 0 deletions test/unit/commands/doctor/checks/file-permissions-spec.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,82 @@
'use strict';
const expect = require('chai').expect;
const sinon = require('sinon');

const execa = require('execa');
const errors = require('../../../../../lib/errors');

const filePermissions = require('../../../../../lib/commands/doctor/checks/file-permissions');

describe('Unit: Doctor Checks > Checking file permissions', function () {
const sandbox = sinon.sandbox.create();

afterEach(() => {
sandbox.restore();
});

it('exports tasks', function () {
expect(filePermissions).to.be.an.instanceof(Object);
expect(filePermissions.title).to.match(/Checking file permissions/);
expect(filePermissions.task).to.be.an.instanceof(Function);
expect(filePermissions.enabled).to.be.an.instanceof(Function);
expect(filePermissions.category).to.be.an.instanceof(Array);
expect(filePermissions.category).to.have.length(2);
});

it('skips when content when ghost is locally installed', function () {
const getInstanceStub = sinon.stub().returns({process: {name: 'local'}});
const execaStub = sandbox.stub(execa, 'shell').resolves();

expect(filePermissions).to.exist;
expect(filePermissions.enabled({system: {getInstance: getInstanceStub}}), 'skips if no Ghost user should be used').to.be.false;
expect(execaStub.called).to.be.false;
});

it('rejects with error if folders have incorrect permissions', function () {
const execaStub = sandbox.stub(execa, 'shell').resolves({stdout: './content/images\n./system/apps\n./content/themes'});
const getInstanceStub = sinon.stub().returns({process: {name: ''}});

expect(filePermissions.enabled({system: {getInstance: getInstanceStub}}), 'skips if no Ghost user should be used').to.be.true;
return filePermissions.task({}).then(() => {
expect(false, 'error should have been thrown').to.be.true;
}).catch((error) => {
expect(error).to.be.an.instanceof(errors.SystemError);
expect(error.message).to.match(/Your installation folder contains some directories or files with incorrect permissions:/);
expect(error.message).to.match(/- \.\/system\/apps/);
expect(execaStub.called).to.be.true;
});
});

it('rejects with error if files have incorrect permissions', function () {
const execaStub = sandbox.stub(execa, 'shell').resolves({stdout: './content/images/test.jpg'});

return filePermissions.task({}).then(() => {
expect(false, 'error should have been thrown').to.be.true;
}).catch((error) => {
expect(error).to.be.an.instanceof(errors.SystemError);
expect(error.message).to.match(/Your installation folder contains a directory or file with incorrect permissions:/);
expect(error.message).to.match(/- .\/content\/images\/test.jpg/);
expect(execaStub.called).to.be.true;
});
});

it('passes if all folders have the correct permissions', function () {
const execaStub = sandbox.stub(execa, 'shell').resolves({stdout: ''});

return filePermissions.task({}).then(() => {
expect(execaStub.called).to.be.true;
});
});

it('rejects with error if execa command fails', function () {
const execaStub = sandbox.stub(execa, 'shell').rejects(new Error('oops, cmd could not be executed'));

return filePermissions.task({}).then(() => {
expect(false, 'error should have been thrown').to.be.true;
}).catch((error) => {
expect(error).to.be.an.instanceof(errors.ProcessError);
expect(error.message).to.match(/oops, cmd could not be executed/);
expect(execaStub.called).to.be.true;
});
});
});
Loading

0 comments on commit 6b614f7

Please sign in to comment.