Skip to content

Commit

Permalink
test: fix test-fs-access when uid is 0
Browse files Browse the repository at this point in the history
Superusers can open files with W_OK permission even though their
mode is set to 0444. This commit makes the test attempt to change
its uid to nobody on non-Windows platforms. Patch originally from
nodejs/node-v0.x-archive@28d0cbbd.

Fixes: #1031
PR-URL: #1037
Reviewed-By: Colin Ihrig <cjihrig@gmail.com>
  • Loading branch information
jbergstroem authored and cjihrig committed Mar 3, 2015
1 parent 5abfa93 commit fd3ea29
Showing 1 changed file with 46 additions and 6 deletions.
52 changes: 46 additions & 6 deletions test/parallel/test-fs-access.js
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,7 @@ var fs = require('fs');
var path = require('path');
var doesNotExist = __filename + '__this_should_not_exist';
var readOnlyFile = path.join(common.tmpDir, 'read_only_file');
var readWriteFile = path.join(common.tmpDir, 'read_write_file');

var removeFile = function(file) {
try {
Expand All @@ -13,13 +14,47 @@ var removeFile = function(file) {
}
};

var createReadOnlyFile = function(file) {
var createFileWithPerms = function(file, mode) {
removeFile(file);
fs.writeFileSync(file, '');
fs.chmodSync(file, 0444);
fs.chmodSync(file, mode);
};

createReadOnlyFile(readOnlyFile);
createFileWithPerms(readOnlyFile, 0444);
createFileWithPerms(readWriteFile, 0666);

/*
* On non-Windows supported platforms, fs.access(readOnlyFile, W_OK, ...)
* always succeeds if node runs as the super user, which is sometimes the
* case for tests running on our continuous testing platform agents.
*
* In this case, this test tries to change its process user id to a
* non-superuser user so that the test that checks for write access to a
* read-only file can be more meaningful.
*
* The change of user id is done after creating the fixtures files for the same
* reason: the test may be run as the superuser within a directory in which
* only the superuser can create files, and thus it may need superuser
* priviledges to create them.
*
* There's not really any point in resetting the process' user id to 0 after
* changing it to 'nobody', since in the case that the test runs without
* superuser priviledge, it is not possible to change its process user id to
* superuser.
*
* It can prevent the test from removing files created before the change of user
* id, but that's fine. In this case, it is the responsability of the
* continuous integration platform to take care of that.
*/
var hasWriteAccessForReadonlyFile = false;
if (process.platform !== 'win32' && process.getuid() === 0) {
hasWriteAccessForReadonlyFile = true;
try {
process.setuid('nobody');
hasWriteAccessForReadonlyFile = false;
} catch (err) {
}
}

assert(typeof fs.F_OK === 'number');
assert(typeof fs.R_OK === 'number');
Expand All @@ -45,8 +80,12 @@ fs.access(readOnlyFile, fs.F_OK | fs.R_OK, function(err) {
});

fs.access(readOnlyFile, fs.W_OK, function(err) {
assert.notEqual(err, null, 'error should exist');
assert.strictEqual(err.path, readOnlyFile);
if (hasWriteAccessForReadonlyFile) {
assert.equal(err, null, 'error should not exist');
} else {
assert.notEqual(err, null, 'error should exist');
assert.strictEqual(err.path, readOnlyFile);
}
});

assert.throws(function() {
Expand All @@ -68,7 +107,7 @@ assert.doesNotThrow(function() {
assert.doesNotThrow(function() {
var mode = fs.F_OK | fs.R_OK | fs.W_OK;

fs.accessSync(__filename, mode);
fs.accessSync(readWriteFile, mode);
});

assert.throws(function() {
Expand All @@ -79,4 +118,5 @@ assert.throws(function() {

process.on('exit', function() {
removeFile(readOnlyFile);
removeFile(readWriteFile);
});

0 comments on commit fd3ea29

Please sign in to comment.