Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

feat(fs): Scope checks for filesystem functions #16511

Merged
merged 16 commits into from
Jul 14, 2022
1 change: 1 addition & 0 deletions lib/constants/error-messages.ts
Original file line number Diff line number Diff line change
Expand Up @@ -41,6 +41,7 @@ export const NO_VULNERABILITY_ALERTS = 'no-vulnerability-alerts';

// Manager Error
export const MANAGER_LOCKFILE_ERROR = 'lockfile-error';
export const FILE_ACCESS_VIOLATION_ERROR = 'file-access-violation-error';

// Host error
export const EXTERNAL_HOST_ERROR = 'external-host-error';
Expand Down
5 changes: 2 additions & 3 deletions lib/modules/manager/npm/post-update/yarn.spec.ts
Original file line number Diff line number Diff line change
Expand Up @@ -300,9 +300,8 @@ describe('modules/manager/npm/post-update/yarn', () => {
// subdirectory isolated workspaces to work with Yarn 2+.
expect(res.lockFile).toBe('');
expect(fs.outputFile).toHaveBeenCalledTimes(1);
expect(fs.outputFile).toHaveBeenCalledWith(
'some-dir/sub_workspace/yarn.lock',
''
expect(mockedFunction(fs.outputFile).mock.calls[0][0]).toEndWith(
'some-dir/sub_workspace/yarn.lock'
);
expect(fixSnapshots(execSnapshots)).toMatchSnapshot();
}
Expand Down
135 changes: 52 additions & 83 deletions lib/util/fs/index.spec.ts
Original file line number Diff line number Diff line change
Expand Up @@ -2,8 +2,7 @@ import _findUp from 'find-up';
import fs from 'fs-extra';
import tmp, { DirectoryResult } from 'tmp-promise';
import { join } from 'upath';
import { envMock } from '../../../test/exec-util';
import { env, mockedFunction } from '../../../test/util';
import { mockedFunction } from '../../../test/util';
import { GlobalConfig } from '../../config/global';
import {
chmodLocalFile,
Expand Down Expand Up @@ -37,17 +36,32 @@ jest.mock('find-up');
const findUp = mockedFunction(_findUp);

describe('util/fs/index', () => {
let dirResult: DirectoryResult;
let localDirResult: DirectoryResult;
let localDir: string;

let cacheDirResult: DirectoryResult;
let cacheDir: string;

let tmpDirResult: DirectoryResult;
let tmpDir: string;

beforeEach(async () => {
GlobalConfig.set({ localDir: '' });
dirResult = await tmp.dir({ unsafeCleanup: true });
tmpDir = dirResult.path;
localDirResult = await tmp.dir({ unsafeCleanup: true });
localDir = localDirResult.path;

cacheDirResult = await tmp.dir({ unsafeCleanup: true });
cacheDir = cacheDirResult.path;

tmpDirResult = await tmp.dir({ unsafeCleanup: true });
tmpDir = tmpDirResult.path;

GlobalConfig.set({ localDir, cacheDir });
});

afterEach(async () => {
await dirResult.cleanup();
await localDirResult?.cleanup();
await cacheDirResult?.cleanup();
await tmpDirResult?.cleanup();
});

describe('getParentDir', () => {
Expand Down Expand Up @@ -87,23 +101,24 @@ describe('util/fs/index', () => {

describe('readLocalFile', () => {
it('reads buffer', async () => {
expect(await readLocalFile(__filename)).toBeInstanceOf(Buffer);
await fs.outputFile(`${localDir}/file.txt`, 'foobar');
const res = await readLocalFile('file.txt');
expect(res).toBeInstanceOf(Buffer);
});

it('reads string', async () => {
expect(typeof (await readLocalFile(__filename, 'utf8'))).toBe('string');
await fs.outputFile(`${localDir}/file.txt`, 'foobar');
const res = await readLocalFile('file.txt', 'utf8');
expect(res).toBe('foobar');
});

it('does not throw', async () => {
// Does not work on FreeBSD: https://nodejs.org/docs/latest-v10.x/api/fs.html#fs_fs_readfile_path_options_callback
expect(await readLocalFile(__dirname)).toBeNull();
it('returns null if file is not found', async () => {
expect(await readLocalFile('foobar')).toBeNull();
});
});

describe('writeLocalFile', () => {
it('outputs file', async () => {
const localDir = tmpDir;
GlobalConfig.set({ localDir });
await writeLocalFile('foo/bar/file.txt', 'foobar');

const path = `${localDir}/foo/bar/file.txt`;
Expand All @@ -114,8 +129,6 @@ describe('util/fs/index', () => {

describe('deleteLocalFile', () => {
it('deletes file', async () => {
const localDir = tmpDir;
GlobalConfig.set({ localDir });
const filePath = `${localDir}/foo/bar/file.txt`;
await fs.outputFile(filePath, 'foobar');

Expand All @@ -127,8 +140,6 @@ describe('util/fs/index', () => {

describe('renameLocalFile', () => {
it('renames file', async () => {
const localDir = tmpDir;
GlobalConfig.set({ localDir });
const sourcePath = `${localDir}/foo.txt`;
const targetPath = `${localDir}/bar.txt`;
await fs.outputFile(sourcePath, 'foobar');
Expand All @@ -143,8 +154,6 @@ describe('util/fs/index', () => {

describe('ensureDir', () => {
it('creates directory', async () => {
const localDir = tmpDir;
GlobalConfig.set({ localDir });
const path = `${localDir}/foo/bar`;

await ensureDir(path);
Expand All @@ -155,8 +164,6 @@ describe('util/fs/index', () => {

describe('ensureLocalDir', () => {
it('creates local directory', async () => {
const localDir = tmpDir;
GlobalConfig.set({ localDir });
const path = `${localDir}/foo/bar`;

await ensureLocalDir('foo/bar');
Expand All @@ -166,30 +173,11 @@ describe('util/fs/index', () => {
});

describe('ensureCacheDir', () => {
function setupMock(root: string): {
dirFromEnv: string;
dirFromConfig: string;
} {
const dirFromEnv = join(root, join('/bar/others/bundler'));
const dirFromConfig = join(root, join('/bar'));

jest.resetAllMocks();
env.getChildProcessEnv.mockReturnValueOnce({
...envMock.basic,
});

GlobalConfig.set({
cacheDir: join(dirFromConfig),
});

return { dirFromEnv, dirFromConfig };
}

it('prefers environment variables over global config', async () => {
const { dirFromEnv } = setupMock(tmpDir);
const res = await ensureCacheDir('bundler');
expect(res).toEqual(dirFromEnv);
expect(await fs.pathExists(dirFromEnv)).toBeTrue();
const path = join(cacheDir, 'others/bundler');
expect(res).toEqual(path);
expect(await fs.pathExists(path)).toBeTrue();
});
});

Expand All @@ -202,25 +190,22 @@ describe('util/fs/index', () => {

describe('localPathExists', () => {
it('returns true for file', async () => {
expect(await localPathExists(__filename)).toBeTrue();
const path = `${localDir}/file.txt`;
await fs.outputFile(path, 'foobar');
expect(await localPathExists('file.txt')).toBeTrue();
});

it('returns true for directory', async () => {
expect(await localPathExists(getParentDir(__filename))).toBeTrue();
expect(await localPathExists('.')).toBeTrue();
});

it('returns false', async () => {
expect(await localPathExists(__filename.replace('.ts', '.txt'))).toBe(
false
);
expect(await localPathExists('file.txt')).toBe(false);
});
});

describe('findLocalSiblingOrParent', () => {
it('returns path for file', async () => {
const localDir = tmpDir;
GlobalConfig.set({ localDir });

await writeLocalFile('crates/one/Cargo.toml', 'foo');
await writeLocalFile('Cargo.lock', 'bar');

Expand Down Expand Up @@ -252,8 +237,6 @@ describe('util/fs/index', () => {

describe('readLocalDirectory', () => {
it('returns dir content', async () => {
const localDir = tmpDir;
GlobalConfig.set({ localDir });
await writeLocalFile('test/Cargo.toml', '');
await writeLocalFile('test/Cargo.lock', '');

Expand All @@ -272,8 +255,6 @@ describe('util/fs/index', () => {
});

it('return empty array for non existing directory', async () => {
const localDir = tmpDir;
GlobalConfig.set({ localDir });
await expect(readLocalDirectory('somedir')).rejects.toThrow();
});

Expand All @@ -287,10 +268,10 @@ describe('util/fs/index', () => {

describe('createCacheWriteStream', () => {
it('creates write stream', async () => {
const path = `${tmpDir}/file.txt`;
const path = `${cacheDir}/file.txt`;
await fs.outputFile(path, 'foo');

const stream = createCacheWriteStream(path);
const stream = createCacheWriteStream('file.txt');
expect(stream).toBeInstanceOf(fs.WriteStream);

const write = new Promise((resolve, reject) => {
Expand All @@ -304,17 +285,19 @@ describe('util/fs/index', () => {

describe('localPathIsFile', () => {
it('returns true for file', async () => {
expect(await localPathIsFile(__filename)).toBeTrue();
const path = `${localDir}/file.txt`;
await fs.outputFile(path, 'foo');
expect(await localPathIsFile('file.txt')).toBeTrue();
});

it('returns false for directory', async () => {
expect(await localPathIsFile(__dirname)).toBeFalse();
const path = `${localDir}/foobar`;
await fs.mkdir(path);
expect(await localPathIsFile(path)).toBeFalse();
});

it('returns false for non-existing path', async () => {
expect(
await localPathIsFile(__filename.replace('.ts', '.txt'))
).toBeFalse();
expect(await localPathIsFile(`${localDir}/foobar`)).toBeFalse();
});
});

Expand Down Expand Up @@ -344,8 +327,6 @@ describe('util/fs/index', () => {

describe('chmodLocalFile', () => {
it('changes file mode', async () => {
const localDir = tmpDir;
GlobalConfig.set({ localDir });
await writeLocalFile('foo', 'bar');
let stat = await statLocalFile('foo');
const oldMode = stat!.mode & 0o777;
Expand All @@ -363,9 +344,6 @@ describe('util/fs/index', () => {

describe('statLocalFile', () => {
it('returns stat object', async () => {
const localDir = tmpDir;
GlobalConfig.set({ localDir });

expect(await statLocalFile('foo')).toBeNull();

await writeLocalFile('foo', 'bar');
Expand All @@ -377,43 +355,34 @@ describe('util/fs/index', () => {

describe('listCacheDir', () => {
it('lists directory', async () => {
const cacheDir = tmpDir;
GlobalConfig.set({ cacheDir });
await fs.outputFile(`${cacheDir}/foo/bar.txt`, 'foobar');
expect(await listCacheDir(`${cacheDir}/foo`)).toEqual(['bar.txt']);
expect(await listCacheDir('foo')).toEqual(['bar.txt']);
});
});

describe('rmCache', () => {
it('removes cache dir', async () => {
const cacheDir = tmpDir;
GlobalConfig.set({ cacheDir });
await fs.outputFile(`${cacheDir}/foo/bar/file.txt`, 'foobar');
await rmCache(`${cacheDir}/foo/bar`);
await rmCache(`foo/bar`);
expect(await fs.pathExists(`${cacheDir}/foo/bar/file.txt`)).toBeFalse();
expect(await fs.pathExists(`${cacheDir}/foo/bar`)).toBeFalse();
});
});

describe('readCacheFile', () => {
it('reads file', async () => {
const cacheDir = tmpDir;
GlobalConfig.set({ cacheDir });
await fs.outputFile(`${cacheDir}/foo/bar/file.txt`, 'foobar');
expect(await readCacheFile(`${cacheDir}/foo/bar/file.txt`, 'utf8')).toBe(
'foobar'
);
expect(await readCacheFile(`${cacheDir}/foo/bar/file.txt`)).toEqual(
expect(await readCacheFile(`foo/bar/file.txt`, 'utf8')).toBe('foobar');
expect(await readCacheFile(`foo/bar/file.txt`)).toEqual(
Buffer.from('foobar')
);
});
});

describe('outputCacheFile', () => {
it('outputs file', async () => {
const file = join(tmpDir, 'some-file');
await outputCacheFile(file, 'foobar');
const res = await fs.readFile(file, 'utf8');
await outputCacheFile('file.txt', 'foobar');
const res = await fs.readFile(`${cacheDir}/file.txt`, 'utf8');
expect(res).toBe('foobar');
});
});
Expand Down
Loading