Skip to content

Commit 707ce6a

Browse files
committed
fix(local-process): error if content folder owned by other user
closes #501 - if the content folder is owned by a separate user, the local process manager will fail - improve local process manager errors - add local process manager tests
1 parent 3db21d6 commit 707ce6a

File tree

2 files changed

+418
-9
lines changed

2 files changed

+418
-9
lines changed

lib/utils/local-process.js

+41-9
Original file line numberDiff line numberDiff line change
@@ -1,8 +1,9 @@
11
'use strict';
22
const fs = require('fs-extra');
3+
const os = require('os');
34
const path = require('path');
45
const fkill = require('fkill');
5-
const spawn = require('child_process').spawn;
6+
const childProcess = require('child_process');
67
const isRunning = require('is-running');
78

89
const errors = require('../errors');
@@ -28,8 +29,14 @@ class LocalProcess extends ProcessManager {
2829
* @public
2930
*/
3031
start(cwd, environment) {
32+
// Check that content folder is owned by the current user
33+
if (!this._checkContentFolder(cwd)) {
34+
return Promise.reject(new errors.SystemError(`The content folder is not owned by the current user.
35+
Please ensure the content folder has correct permissions and try again.`));
36+
}
37+
3138
return new Promise((resolve, reject) => {
32-
const cp = spawn('node', [process.argv[1] , 'run'], {
39+
const cp = childProcess.spawn('node', [process.argv[1] , 'run'], {
3340
cwd: cwd,
3441
detached: true,
3542
stdio: ['ignore', 'ignore', 'ignore', 'ipc'],
@@ -39,7 +46,12 @@ class LocalProcess extends ProcessManager {
3946
// Stick the pid into the pidfile so we can stop the process later
4047
fs.writeFileSync(path.join(cwd, PID_FILE), cp.pid);
4148

42-
cp.on('error', reject);
49+
cp.on('error', (error) => {
50+
reject(new errors.CliError({
51+
message: 'An error occurred while starting Ghost.',
52+
err: error
53+
}));
54+
});
4355

4456
cp.on('exit', (code) => {
4557
fs.removeSync(path.join(cwd, PID_FILE));
@@ -50,8 +62,7 @@ class LocalProcess extends ProcessManager {
5062
cp.on('message', (msg) => {
5163
if (msg.error) {
5264
fs.removeSync(path.join(cwd, PID_FILE));
53-
54-
return reject(new errors.GhostError(msg));
65+
return reject(new errors.GhostError(msg.error));
5566
}
5667

5768
if (msg.started) {
@@ -80,18 +91,24 @@ class LocalProcess extends ProcessManager {
8091
} catch (e) {
8192
if (e.code === 'ENOENT') {
8293
// pid was not found, exit
83-
return;
94+
return Promise.resolve();
8495
}
8596

86-
throw e;
97+
return Promise.reject(new errors.CliError({
98+
message: 'An unexpected error occurred when reading the pidfile.',
99+
error: e
100+
}));
87101
}
88102

89-
const isWindows = process.platform === 'win32';
103+
const isWindows = os.platform() === 'win32';
90104

91105
return fkill(pid, {force: isWindows}).catch((error) => {
92106
// TODO: verify windows outputs same error message as mac/linux
93107
if (!error.message.match(/No such process/)) {
94-
throw error;
108+
return Promise.reject(new errors.CliError({
109+
message: 'An unexpected error occurred while stopping Ghost.',
110+
err: error
111+
}));
95112
}
96113
}).then(() => {
97114
fs.removeSync(path.join(cwd, PID_FILE));
@@ -151,6 +168,21 @@ class LocalProcess extends ProcessManager {
151168
return running;
152169
}
153170

171+
/**
172+
* Check that the content folder is owned by the current user
173+
*
174+
* @param {String} cwd current working directory
175+
* @return {Boolean} true if ownership is correct, otherwise false
176+
*/
177+
_checkContentFolder(cwd) {
178+
if (os.platform() === 'win32') {
179+
return true;
180+
}
181+
182+
const stat = fs.lstatSync(path.join(cwd, 'content'));
183+
return stat.uid === process.getuid();
184+
}
185+
154186
/**
155187
* Because this process manager should work on every system,
156188
* just return true here

0 commit comments

Comments
 (0)