Skip to content

Commit

Permalink
Fix: Use explicit chmod to ensure setgid permission can be set & acco…
Browse files Browse the repository at this point in the history 
…unt for umask with default mode (closes #183, closes #185)
  • Loading branch information
@phated
phated committed Nov 30, 2017
1 parent 79c7216 commit 8eb33dc
Show file tree
Hide file tree
Showing 3 changed files with 78 additions and 74 deletions.
12 changes: 8 additions & 4 deletions lib/file-operations.js
View file
Original file line number Diff line number Diff line change
Expand Up @@ -201,19 +201,19 @@ function mkdirp(dirpath, customMode, callback) {
customMode = undefined;
}

var mode = customMode || constants.DEFAULT_DIR_MODE;
var mode = customMode || (constants.DEFAULT_DIR_MODE & ~process.umask());
dirpath = path.resolve(dirpath);

fs.mkdir(dirpath, mode, onMkdir);

function onMkdir(mkdirErr) {
if (!mkdirErr) {
return callback();
return fs.stat(dirpath, onStat);
}

switch (mkdirErr.code) {
case 'ENOENT': {
return mkdirp(path.dirname(dirpath), mode, onRecurse);
return mkdirp(path.dirname(dirpath), onRecurse);
}

case 'EEXIST': {
Expand All @@ -234,11 +234,15 @@ function mkdirp(dirpath, customMode, callback) {
return callback(mkdirErr);
}

if (stats.mode === mode) {
return callback();
}

if (!customMode) {
return callback();
}

fs.chmod(dirpath, customMode, callback);
fs.chmod(dirpath, mode, callback);
}
}

Expand Down
7 changes: 1 addition & 6 deletions test/dest-modes.js
View file
Original file line number Diff line number Diff line change
Expand Up @@ -336,12 +336,7 @@ describe('.dest() with custom modes', function() {
var inputPath = path.join(__dirname, './fixtures/wow/suchempty');
var expectedBase = path.join(__dirname, './out-fixtures/wow');
var expectedPath = path.join(__dirname, './out-fixtures/wow/suchempty');
// NOTE: Darwin does not set setgid
var expectedDirMode = constants.DEFAULT_DIR_MODE;
if (!isDarwin) {
expectedDirMode |= parseInt('2000', 8);
}
expectedDirMode &= ~process.umask();
var expectedDirMode = parseInt('2777', 8) & ~process.umask();
var expectedFileMode = constants.DEFAULT_FILE_MODE & ~process.umask();

var firstFile = new File({
Expand Down
133 changes: 69 additions & 64 deletions test/file-operations.js
View file
Original file line number Diff line number Diff line change
Expand Up @@ -11,6 +11,7 @@ var buffer = require('buffer');
var defaultResolution = require('default-resolution');

var fo = require('../lib/file-operations');
var constants = require('../lib/constants');

var mkdirp = fo.mkdirp;
var closeFd = fo.closeFd;
Expand All @@ -22,10 +23,15 @@ var updateMetadata = fo.updateMetadata;

var resolution = defaultResolution();

var MASK_MODE = parseInt('7777', 8);
var DEFAULT_DIR_MODE = masked(constants.DEFAULT_DIR_MODE & ~process.umask()).toString(8);

function masked(mode) {
return mode & MASK_MODE;
return mode & constants.MASK_MODE;
}

function statHuman(filepath) {
var stats = fs.lstatSync(filepath);
return masked(stats.mode).toString(8);
}

function noop() {}
Expand Down Expand Up @@ -914,10 +920,15 @@ describe('updateMetadata', function() {
});

describe('mkdirp', function() {
var DEFAULT_DIR_MODE = parseInt('0777', 8);
var MODE_MASK = parseInt('0777', 8);

var dir = path.join(__dirname, './fixtures/bar');
var fixtures = path.join(__dirname, './fixtures');
var dir = path.join(fixtures, './bar');

beforeEach(function(done) {
// Linux inherits the setgid of the directory and it messes up our assertions
// So we explixitly set the mode to 777 before each test
fs.chmod(fixtures, '777', done);
});

afterEach(function() {
return del(dir);
Expand All @@ -926,49 +937,37 @@ describe('mkdirp', function() {
it('makes a single directory', function(done) {
mkdirp(dir, function(err) {
expect(err).toNotExist();
expect(statHuman(dir)).toExist();

fs.stat(dir, function(err2, stats) {
expect(err2).toNotExist();
expect(stats).toExist();

done();
});
done();
});
});

it('makes single directory w/ correct permissions', function(done) {
it('makes single directory w/ default mode', function(done) {
if (isWindows) {
this.skip();
return;
}

mkdirp(dir, function(err) {
expect(err).toNotExist();
expect(statHuman(dir)).toEqual(DEFAULT_DIR_MODE);

fs.stat(dir, function(err2, stats) {
expect(err2).toNotExist();
expect(stats.mode & MODE_MASK).toEqual(DEFAULT_DIR_MODE & ~process.umask());

done();
});
done();
});
});

it('makes multiple directories', function(done) {
var nestedDir = path.join(dir, './baz/foo');
mkdirp(nestedDir, function(err) {
expect(err).toNotExist();
expect(statHuman(nestedDir)).toExist();

fs.stat(nestedDir, function(err2, stats) {
expect(err2).toNotExist();
expect(stats).toExist();

done();
});
done();
});
});

it('makes multiple directories w/ correct permissions', function(done) {
it('makes multiple directories w/ default mode', function(done) {
if (isWindows) {
this.skip();
return;
Expand All @@ -977,13 +976,9 @@ describe('mkdirp', function() {
var nestedDir = path.join(dir, './baz/foo');
mkdirp(nestedDir, function(err) {
expect(err).toNotExist();
expect(statHuman(nestedDir)).toEqual(DEFAULT_DIR_MODE);

fs.stat(nestedDir, function(err2, stats) {
expect(err2).toNotExist();
expect(stats.mode & MODE_MASK).toEqual(DEFAULT_DIR_MODE & ~process.umask());

done();
});
done();
});
});

Expand All @@ -996,13 +991,24 @@ describe('mkdirp', function() {
var mode = parseInt('0700', 8);
mkdirp(dir, mode, function(err) {
expect(err).toNotExist();
expect(statHuman(dir)).toEqual(masked(mode).toString(8));

fs.stat(dir, function(err2, stats) {
expect(err2).toNotExist();
expect(stats.mode & MODE_MASK).toEqual(mode & ~process.umask());
done();
});
});

done();
});
it('can create a directory with setgid permission', function(done) {
if (isWindows) {
this.skip();
return;
}

var mode = parseInt('2700', 8);
mkdirp(dir, mode, function(err) {
expect(err).toNotExist();
expect(statHuman(dir)).toEqual(masked(mode).toString(8));

done();
});
});

Expand All @@ -1018,13 +1024,9 @@ describe('mkdirp', function() {

mkdirp(dir, function(err2) {
expect(err2).toNotExist();
expect(statHuman(dir)).toEqual(masked(mode).toString(8));

fs.stat(dir, function(err3, stats) {
expect(err3).toNotExist();
expect(stats.mode & MODE_MASK).toEqual(mode & ~process.umask());

done();
});
done();
});
});
});
Expand All @@ -1039,13 +1041,27 @@ describe('mkdirp', function() {
var mode = parseInt('0700',8);
mkdirp(nestedDir, mode, function(err) {
expect(err).toNotExist();
expect(statHuman(nestedDir)).toEqual(masked(mode).toString(8));

fs.stat(nestedDir, function(err2, stats) {
expect(err2).toNotExist();
expect(stats.mode & MODE_MASK).toEqual(mode & ~process.umask());
done();
});
});

done();
});
it('uses default mode on intermediate directories', function(done) {
if (isWindows) {
this.skip();
return;
}

var intermediateDir = path.join(dir, './baz');
var nestedDir = path.join(intermediateDir, './foo');
var mode = parseInt('0700',8);
mkdirp(nestedDir, mode, function(err) {
expect(err).toNotExist();
expect(statHuman(dir)).toEqual(DEFAULT_DIR_MODE);
expect(statHuman(intermediateDir)).toEqual(DEFAULT_DIR_MODE);

done();
});
});

Expand All @@ -1058,21 +1074,13 @@ describe('mkdirp', function() {
var mode = parseInt('0700',8);
mkdirp(dir, function(err) {
expect(err).toNotExist();
expect(statHuman(dir)).toEqual(DEFAULT_DIR_MODE);

fs.stat(dir, function(err2, stats) {
mkdirp(dir, mode, function(err2) {
expect(err2).toNotExist();
expect(stats.mode & MODE_MASK).toEqual(DEFAULT_DIR_MODE & ~process.umask());

mkdirp(dir, mode, function(err3) {
expect(err3).toNotExist();
expect(statHuman(dir)).toEqual(masked(mode).toString(8));

fs.stat(dir, function(err4, stats) {
expect(err2).toNotExist();
expect(stats.mode & MODE_MASK).toEqual(mode & ~process.umask());

done();
});
});
done();
});
});
});
Expand Down Expand Up @@ -1109,14 +1117,11 @@ describe('mkdirp', function() {
fs.writeFile(file, 'hello world', function(err2) {
expect(err2).toNotExist();

var stats = fs.statSync(file);
var expectedMode = stats.mode & MODE_MASK;
var expectedMode = statHuman(file);

mkdirp(file, mode, function(err3) {
expect(err3).toExist();

var stats = fs.statSync(file);
expect(stats.mode & MODE_MASK).toEqual(expectedMode);
expect(statHuman(file)).toEqual(expectedMode);

done();
});
Expand Down

0 comments on commit 8eb33dc

Please sign in to comment.