feat: improve permissions support (#1059)

* test: Add tests for permissions

* feat: Add permissions implementations to Volume

* fix: Side effects test pollution in chmod() test

* fix: Improper handling of O_EXCL in openFile()

* fix: No longer check execute permission on files in mkdirp

While files are not a permitted place to create subdirectories in, the proper error should be ENOTDIR, not EACCES

* test: Add tests for recursive mkdir

* test: Add tests for chmodSync

* fix: No longer require permissions to chmod

* test: Add more extensive tests for mkdir recursive mode

* refactor: Inline tree walking

* chore: cleanup

* style: Run prettier

* test: Improve testing for missing permissions on intermediate directories

* chore: cleanup

* style: run prettier

* fix: utimes should not require permissions

src/__tests__/promises.test.ts

@@ -38,18 +38,22 @@ describe('Promises API', () => {
       });
     });
     describe('chmod(mode)', () => {
-      const vol = new Volume();
-      const { promises } = vol;
-      vol.fromJSON({
-        '/foo': 'bar',
+      let vol;
+      beforeEach(() => {
+        vol = new Volume();
+        vol.fromJSON({
+          '/foo': 'bar',
+        });
       });
       it('Change mode of existing file', async () => {
+        const { promises } = vol;
         const fileHandle = await promises.open('/foo', 'a');
         await fileHandle.chmod(0o444);
         expect(vol.statSync('/foo').mode & 0o777).toEqual(0o444);
         await fileHandle.close();
       });
       it('Reject when the file handle was closed', async () => {
+        const { promises } = vol;
         const fileHandle = await promises.open('/foo', 'a');
         await fileHandle.close();
         return expect(fileHandle.chmod(0o666)).rejects.toBeInstanceOf(Error);

src/__tests__/util.ts

@@ -1,6 +1,18 @@
 import { createFsFromVolume, Volume } from '..';
 import { Link, Node } from '../node';
 
+// Turn the done callback into an incremental one that will only fire after being called
+// `times` times, failing with the first reported error if such exists.
+// Useful for testing callback-style functions with several different fixtures without
+// having to clutter the test suite with a multitude of individual tests (like it.each would).
+export const multitest = (_done: (err?: Error) => void, times: number) => {
+  let err;
+  return function done(_err?: Error) {
+    err ??= _err;
+    if (!--times) _done(_err);
+  };
+};
+
 export const create = (json: { [s: string]: string } = { '/foo': 'bar' }) => {
   const vol = Volume.fromJSON(json);
   return vol;

src/__tests__/volume/ReadStream.test.ts

@@ -18,4 +18,46 @@ describe('ReadStream', () => {
       done();
     });
   });
+
+  it('should emit EACCES error when file has insufficient permissions', done => {
+    const fs = createFs({ '/test': 'test' });
+    fs.chmodSync('/test', 0o333); // wx
+    new fs.ReadStream('/test')
+      .on('error', err => {
+        expect(err).toBeInstanceOf(Error);
+        expect(err).toHaveProperty('code', 'EACCES');
+        done();
+      })
+      .on('open', () => {
+        done(new Error("Expected ReadStream to emit EACCES but it didn't"));
+      });
+  });
+
+  it('should emit EACCES error when containing directory has insufficient permissions', done => {
+    const fs = createFs({ '/foo/test': 'test' });
+    fs.chmodSync('/foo', 0o666); // rw
+    new fs.ReadStream('/foo/test')
+      .on('error', err => {
+        expect(err).toBeInstanceOf(Error);
+        expect(err).toHaveProperty('code', 'EACCES');
+        done();
+      })
+      .on('open', () => {
+        done(new Error("Expected ReadStream to emit EACCES but it didn't"));
+      });
+  });
+
+  it('should emit EACCES error when intermediate directory has insufficient permissions', done => {
+    const fs = createFs({ '/foo/test': 'test' });
+    fs.chmodSync('/', 0o666); // rw
+    new fs.ReadStream('/foo/test')
+      .on('error', err => {
+        expect(err).toBeInstanceOf(Error);
+        expect(err).toHaveProperty('code', 'EACCES');
+        done();
+      })
+      .on('open', () => {
+        done(new Error("Expected ReadStream to emit EACCES but it didn't"));
+      });
+  });
 });

src/__tests__/volume/WriteStream.test.ts

@@ -19,4 +19,60 @@ describe('WriteStream', () => {
       done();
     });
   });
+
+  it('should emit EACCES error when file has insufficient permissions', done => {
+    const fs = createFs({ '/test': 'test' });
+    fs.chmodSync('/test', 0o555); // rx
+    new fs.WriteStream('/test')
+      .on('error', err => {
+        expect(err).toBeInstanceOf(Error);
+        expect(err).toHaveProperty('code', 'EACCES');
+        done();
+      })
+      .on('open', () => {
+        done(new Error("Expected WriteStream to emit EACCES but it didn't"));
+      });
+  });
+
+  it('should emit EACCES error for an existing file when containing directory has insufficient permissions', done => {
+    const fs = createFs({ '/foo/test': 'test' });
+    fs.chmodSync('/foo', 0o666); // rw
+    new fs.WriteStream('/foo/test')
+      .on('error', err => {
+        expect(err).toBeInstanceOf(Error);
+        expect(err).toHaveProperty('code', 'EACCES');
+        done();
+      })
+      .on('open', () => {
+        done(new Error("Expected WriteStream to emit EACCES but it didn't"));
+      });
+  });
+
+  it('should emit EACCES error for when intermediate directory has insufficient permissions', done => {
+    const fs = createFs({ '/foo/test': 'test' });
+    fs.chmodSync('/', 0o666); // rw
+    new fs.WriteStream('/foo/test')
+      .on('error', err => {
+        expect(err).toBeInstanceOf(Error);
+        expect(err).toHaveProperty('code', 'EACCES');
+        done();
+      })
+      .on('open', () => {
+        done(new Error("Expected WriteStream to emit EACCES but it didn't"));
+      });
+  });
+
+  it('should emit EACCES error for a non-existent file when containing directory has insufficient permissions', done => {
+    const fs = createFs({});
+    fs.mkdirSync('/foo', { mode: 0o555 }); // rx
+    new fs.WriteStream('/foo/test')
+      .on('error', err => {
+        expect(err).toBeInstanceOf(Error);
+        expect(err).toHaveProperty('code', 'EACCES');
+        done();
+      })
+      .on('open', () => {
+        done(new Error("Expected WriteStream to emit EACCES but it didn't"));
+      });
+  });
 });

src/__tests__/volume/appendFile.test.ts

@@ -1,4 +1,4 @@
-import { create } from '../util';
+import { create, multitest } from '../util';
 
 describe('appendFile(file, data[, options], callback)', () => {
   it('Simple write to non-existing file', done => {
@@ -15,4 +15,55 @@ describe('appendFile(file, data[, options], callback)', () => {
       done();
     });
   });
+
+  it('Appending gives EACCES without sufficient permissions on the file', done => {
+    const vol = create({ '/foo': 'foo' });
+    vol.chmodSync('/foo', 0o555); // rx across the board
+    vol.appendFile('/foo', 'bar', err => {
+      try {
+        expect(err).toBeInstanceOf(Error);
+        expect(err).toHaveProperty('code', 'EACCES');
+        done();
+      } catch (failure) {
+        done(failure);
+      }
+    });
+  });
+
+  it('Appending gives EACCES if file does not exist and containing directory has insufficient permissions', _done => {
+    const perms = [
+      0o555, // rx across the board
+      0o666, // rw across the board
+    ];
+    const done = multitest(_done, perms.length);
+
+    perms.forEach(perm => {
+      const vol = create({});
+      vol.mkdirSync('/foo', { mode: perm });
+      vol.appendFile('/foo/test', 'bar', err => {
+        try {
+          expect(err).toBeInstanceOf(Error);
+          expect(err).toHaveProperty('code', 'EACCES');
+          done();
+        } catch (failure) {
+          done(failure);
+        }
+      });
+    });
+  });
+
+  it('Appending gives EACCES if intermediate directory has insufficient permissions', done => {
+    const vol = create({});
+    vol.mkdirSync('/foo');
+    vol.chmodSync('/', 0o666); // rw
+    vol.appendFile('/foo/test', 'bar', err => {
+      try {
+        expect(err).toBeInstanceOf(Error);
+        expect(err).toHaveProperty('code', 'EACCES');
+        done();
+      } catch (failure) {
+        done(failure);
+      }
+    });
+  });
 });

src/__tests__/volume/appendFileSync.test.ts

@@ -11,4 +11,33 @@ describe('appendFileSync(file, data, options)', () => {
     vol.appendFileSync('/a', 'c');
     expect(vol.readFileSync('/a', 'utf8')).toEqual('bc');
   });
+  it('Appending throws EACCES without sufficient permissions on the file', () => {
+    const vol = create({ '/foo': 'foo' });
+    vol.chmodSync('/foo', 0o555); // rx across the board
+    expect(() => {
+      vol.appendFileSync('/foo', 'bar');
+    }).toThrowError(/EACCES/);
+  });
+  it('Appending throws EACCES if file does not exist and containing directory has insufficient permissions', () => {
+    const perms = [
+      0o555, // rx across the board
+      // 0o666, // rw across the board
+      // 0o111, // x
+      // 0o222  // w
+    ];
+    perms.forEach(perm => {
+      const vol = create({});
+      vol.mkdirSync('/foo', perm);
+      expect(() => {
+        vol.appendFileSync('/foo/test', 'bar');
+      }).toThrowError(/EACCES/);
+    });
+  });
+  it('Appending throws EACCES if intermediate directory has insufficient permissions', () => {
+    const vol = create({ '/foo/test': 'test' });
+    vol.chmodSync('/', 0o666); // rw
+    expect(() => {
+      vol.appendFileSync('/foo/test', 'bar');
+    }).toThrowError(/EACCES/);
+  });
 });

src/__tests__/volume/chmodSync.test.ts

@@ -0,0 +1,76 @@
+import { create } from '../util';
+
+describe('chmodSync', () => {
+  it('should be able to chmod files and directories owned by the UID regardless of their permissions', () => {
+    const perms = [
+      0o777, // rwx
+      0o666, // rw
+      0o555, // rx
+      0o444, // r
+      0o333, // wx
+      0o222, // w
+      0o111, // x
+      0o000, // none
+    ];
+    // Check for directories
+    perms.forEach(perm => {
+      const vol = create({});
+      vol.mkdirSync('/foo', { mode: perm });
+      expect(() => {
+        vol.chmodSync('/foo', 0o777);
+      }).not.toThrow();
+    });
+    // Check for files
+    perms.forEach(perm => {
+      const vol = create({ '/foo': 'foo' });
+      expect(() => {
+        vol.chmodSync('/foo', 0o777);
+      }).not.toThrow();
+    });
+  });
+
+  it('should chmod the target of a symlink, not the symlink itself', () => {
+    const vol = create({ '/target': 'contents' });
+    vol.symlinkSync('/target', '/link');
+    const expectedLink = vol.lstatSync('/link').mode;
+    const expectedTarget = vol.statSync('/target').mode & ~0o777;
+    vol.chmodSync('/link', 0);
+
+    expect(vol.lstatSync('/link').mode).toEqual(expectedLink);
+    expect(vol.statSync('/target').mode).toEqual(expectedTarget);
+  });
+
+  it.skip('should throw EPERM when trying to chmod targets not owned by the uid', () => {
+    const uid = process.getuid() + 1;
+    // Check for directories
+    const vol = create({});
+    vol.mkdirSync('/foo');
+    vol.chownSync('/foo', uid, process.getgid());
+    expect(() => {
+      vol.chmodSync('/foo', 0o777);
+    }).toThrow(/PERM/);
+  });
+
+  it("should throw ENOENT when target doesn't exist", () => {
+    const vol = create({});
+    expect(() => {
+      vol.chmodSync('/foo', 0o777);
+    }).toThrow(/ENOENT/);
+  });
+
+  it('should throw EACCES when containing directory has insufficient permissions', () => {
+    const vol = create({ '/foo/test': 'test' });
+    vol.chmodSync('/foo', 0o666); // rw
+    expect(() => {
+      vol.chmodSync('/foo/test', 0o777);
+    }).toThrow(/EACCES/);
+  });
+
+  it('should throw EACCES when intermediate directory has insufficient permissions', () => {
+    const vol = create({ '/foo/test': 'test' });
+    vol.chmodSync('/', 0o666); // rw
+    expect(() => {
+      vol.chmodSync('/foo/test', 0o777);
+    }).toThrow(/EACCES/);
+  });
+});