From f6b134949c9752312fff8f8914bdea6eff57c47e Mon Sep 17 00:00:00 2001 From: Sergei Zharinov Date: Sun, 10 Jul 2022 22:56:48 +0300 Subject: [PATCH 01/12] feat(fs): Scope checks for filesystem functions --- lib/constants/error-messages.ts | 1 + lib/util/fs/index.spec.ts | 122 +++++++++++++------------------- lib/util/fs/index.ts | 83 +++++++++++----------- lib/util/fs/util.spec.ts | 31 ++++++++ lib/util/fs/util.ts | 32 +++++++++ 5 files changed, 156 insertions(+), 113 deletions(-) create mode 100644 lib/util/fs/util.spec.ts create mode 100644 lib/util/fs/util.ts diff --git a/lib/constants/error-messages.ts b/lib/constants/error-messages.ts index 79ca38ddb9fe8f..41dff572a5a93b 100644 --- a/lib/constants/error-messages.ts +++ b/lib/constants/error-messages.ts @@ -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'; diff --git a/lib/util/fs/index.spec.ts b/lib/util/fs/index.spec.ts index 525c1cdf2ad32c..73eb58d6a8833f 100644 --- a/lib/util/fs/index.spec.ts +++ b/lib/util/fs/index.spec.ts @@ -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, @@ -37,17 +36,30 @@ 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(); }); describe('getParentDir', () => { @@ -87,23 +99,30 @@ describe('util/fs/index', () => { describe('readLocalFile', () => { it('reads buffer', async () => { - expect(await readLocalFile(__filename)).toBeInstanceOf(Buffer); + const path = `${localDir}/file.txt`; + await fs.outputFile(path, 'foobar'); + + const res = await readLocalFile(path); + + expect(res).toBeInstanceOf(Buffer); }); it('reads string', async () => { - expect(typeof (await readLocalFile(__filename, 'utf8'))).toBe('string'); + const path = `${localDir}/file.txt`; + await fs.outputFile(path, 'foobar'); + + const res = await readLocalFile(path, 'utf8'); + + expect(typeof res).toBe('string'); }); 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(); + expect(await readLocalFile('')).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`; @@ -114,8 +133,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'); @@ -127,8 +144,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'); @@ -143,8 +158,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); @@ -155,8 +168,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'); @@ -166,30 +177,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(); }); }); @@ -202,25 +194,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(path)).toBeTrue(); }); it('returns true for directory', async () => { - expect(await localPathExists(getParentDir(__filename))).toBeTrue(); + expect(await localPathExists(localDir)).toBeTrue(); }); it('returns false', async () => { - expect(await localPathExists(__filename.replace('.ts', '.txt'))).toBe( - false - ); + expect(await localPathExists(`${localDir}/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'); @@ -252,8 +241,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', ''); @@ -272,8 +259,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(); }); @@ -287,7 +272,7 @@ 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); @@ -304,17 +289,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(path)).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(); }); }); @@ -344,8 +331,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; @@ -363,9 +348,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'); @@ -377,8 +359,6 @@ 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']); }); @@ -386,8 +366,6 @@ describe('util/fs/index', () => { 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`); expect(await fs.pathExists(`${cacheDir}/foo/bar/file.txt`)).toBeFalse(); @@ -397,8 +375,6 @@ describe('util/fs/index', () => { 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' @@ -411,7 +387,7 @@ describe('util/fs/index', () => { describe('outputCacheFile', () => { it('outputs file', async () => { - const file = join(tmpDir, 'some-file'); + const file = join(cacheDir, 'some-file'); await outputCacheFile(file, 'foobar'); const res = await fs.readFile(file, 'utf8'); expect(res).toBe('foobar'); diff --git a/lib/util/fs/index.ts b/lib/util/fs/index.ts index dbf3e7a23ea2c2..94ea854ac63d09 100644 --- a/lib/util/fs/index.ts +++ b/lib/util/fs/index.ts @@ -7,6 +7,7 @@ import type { WriteFileOptions } from 'fs-extra'; import upath from 'upath'; import { GlobalConfig } from '../../config/global'; import { logger } from '../../logger'; +import { ensureCachePath, ensureLocalPath } from './util'; export const pipeline = util.promisify(stream.pipeline); @@ -31,8 +32,7 @@ export async function readLocalFile( fileName: string, encoding?: string ): Promise { - const { localDir } = GlobalConfig.get(); - const localFileName = upath.join(localDir, fileName); + const localFileName = ensureLocalPath(fileName); try { const fileContent = encoding ? await fs.readFile(localFileName, encoding) @@ -48,15 +48,14 @@ export async function writeLocalFile( fileName: string, fileContent: string | Buffer ): Promise { - const { localDir } = GlobalConfig.get(); - const localFileName = upath.join(localDir, fileName); + const localFileName = ensureLocalPath(fileName); await fs.outputFile(localFileName, fileContent); } export async function deleteLocalFile(fileName: string): Promise { - const { localDir } = GlobalConfig.get(); + const localDir = GlobalConfig.get('localDir'); if (localDir) { - const localFileName = upath.join(localDir, fileName); + const localFileName = ensureLocalPath(fileName); await fs.remove(localFileName); } } @@ -65,8 +64,9 @@ export async function renameLocalFile( fromFile: string, toFile: string ): Promise { - const { localDir } = GlobalConfig.get(); - await fs.move(upath.join(localDir, fromFile), upath.join(localDir, toFile)); + const fromPath = ensureLocalPath(fromFile); + const toPath = ensureLocalPath(toFile); + await fs.move(fromPath, toPath); } export async function ensureDir(dirName: string): Promise { @@ -75,17 +75,14 @@ export async function ensureDir(dirName: string): Promise { } } -export async function ensureLocalDir(dirName: string): Promise { - const { localDir } = GlobalConfig.get(); - const localDirName = upath.join(localDir, dirName); - await fs.ensureDir(localDirName); +export async function ensureLocalDir(dirName: string): Promise { + const fullPath = ensureLocalPath(dirName); + await fs.ensureDir(fullPath); + return fullPath; } export async function ensureCacheDir(name: string): Promise { - const cacheDirName = upath.join( - GlobalConfig.get('cacheDir'), - `others/${name}` - ); + const cacheDirName = ensureCachePath(`others/${name}`); await fs.ensureDir(cacheDirName); return cacheDirName; } @@ -96,17 +93,19 @@ export async function ensureCacheDir(name: string): Promise { * without risk of that information leaking to other repositories/users. */ export function privateCacheDir(): string { - const { cacheDir } = GlobalConfig.get(); + const cacheDir = GlobalConfig.get('cacheDir'); return upath.join(cacheDir, '__renovate-private-cache'); } -export function localPathExists(pathName: string): Promise { - const { localDir } = GlobalConfig.get(); +export async function localPathExists(pathName: string): Promise { // Works for both files as well as directories - return fs - .stat(upath.join(localDir, pathName)) - .then((s) => !!s) - .catch(() => false); + const path = ensureLocalPath(pathName); + try { + const s = await fs.stat(path); + return !!s; + } catch (_) { + return false; + } } /** @@ -142,22 +141,24 @@ export async function findLocalSiblingOrParent( * Get files by name from directory */ export async function readLocalDirectory(path: string): Promise { - const { localDir } = GlobalConfig.get(); - const localPath = upath.join(localDir, path); + const localPath = ensureLocalPath(path); const fileList = await fs.readdir(localPath); return fileList; } export function createCacheWriteStream(path: string): fs.WriteStream { - return fs.createWriteStream(path); + const fullPath = ensureCachePath(path); + return fs.createWriteStream(fullPath); } -export function localPathIsFile(pathName: string): Promise { - const { localDir } = GlobalConfig.get(); - return fs - .stat(upath.join(localDir, pathName)) - .then((s) => s.isFile()) - .catch(() => false); +export async function localPathIsFile(pathName: string): Promise { + const path = ensureLocalPath(pathName); + try { + const s = await fs.stat(path); + return s.isFile(); + } catch (_) { + return false; + } } /** @@ -196,16 +197,14 @@ export function chmodLocalFile( fileName: string, mode: string | number ): Promise { - const localDir = GlobalConfig.get('localDir'); - const fullFileName = upath.join(localDir, fileName); + const fullFileName = ensureLocalPath(fileName); return fs.chmod(fullFileName, mode); } export async function statLocalFile( fileName: string ): Promise { - const localDir = GlobalConfig.get('localDir'); - const fullFileName = upath.join(localDir, fileName); + const fullFileName = ensureLocalPath(fileName); try { return await fs.stat(fullFileName); } catch (_) { @@ -214,11 +213,13 @@ export async function statLocalFile( } export function listCacheDir(path: string): Promise { - return fs.readdir(path); + const fullPath = ensureCachePath(path); + return fs.readdir(fullPath); } export async function rmCache(path: string): Promise { - await fs.rm(path, { recursive: true }); + const fullPath = ensureCachePath(path); + await fs.rm(fullPath, { recursive: true }); } export async function readCacheFile(fileName: string): Promise; @@ -230,7 +231,8 @@ export function readCacheFile( fileName: string, encoding?: string ): Promise { - return encoding ? fs.readFile(fileName, encoding) : fs.readFile(fileName); + const fullPath = ensureCachePath(fileName); + return encoding ? fs.readFile(fullPath, encoding) : fs.readFile(fullPath); } export function outputCacheFile( @@ -238,7 +240,8 @@ export function outputCacheFile( data: unknown, options?: WriteFileOptions | string ): Promise { - return fs.outputFile(file, data, options ?? {}); + const filePath = ensureCachePath(file); + return fs.outputFile(filePath, data, options ?? {}); } export async function readSystemFile(fileName: string): Promise; diff --git a/lib/util/fs/util.spec.ts b/lib/util/fs/util.spec.ts new file mode 100644 index 00000000000000..bd2f5d7ad7a962 --- /dev/null +++ b/lib/util/fs/util.spec.ts @@ -0,0 +1,31 @@ +import { GlobalConfig } from '../../config/global'; +import { FILE_ACCESS_VIOLATION_ERROR } from '../../constants/error-messages'; +import { ensureCachePath, ensureLocalPath } from './util'; + +describe('util/fs/util', () => { + test.each` + path | fullPath + ${''} | ${'/foo'} + ${'baz'} | ${'/foo/baz'} + ${'/baz'} | ${'/foo/baz'} + `(`ensureLocalPath('$path', '$fullPath')`, ({ path, fullPath }) => { + GlobalConfig.set({ localDir: '/foo' }); + expect(ensureLocalPath(path)).toBe(fullPath); + }); + + test.each` + path | fullPath + ${''} | ${'/foo'} + ${'baz'} | ${'/foo/baz'} + ${'/baz'} | ${'/foo/baz'} + `(`ensureCachePath('$path', '$fullPath')`, ({ path, fullPath }) => { + GlobalConfig.set({ cacheDir: '/foo' }); + expect(ensureCachePath(path)).toBe(fullPath); + }); + + it('throws FILE_ACCESS_VIOLATION_ERROR', () => { + GlobalConfig.set({ localDir: '/foo', cacheDir: '/bar' }); + expect(() => ensureLocalPath('..')).toThrow(FILE_ACCESS_VIOLATION_ERROR); + expect(() => ensureCachePath('..')).toThrow(FILE_ACCESS_VIOLATION_ERROR); + }); +}); diff --git a/lib/util/fs/util.ts b/lib/util/fs/util.ts new file mode 100644 index 00000000000000..3c0458282e2675 --- /dev/null +++ b/lib/util/fs/util.ts @@ -0,0 +1,32 @@ +import upath from 'upath'; +import { GlobalConfig } from '../../config/global'; +import { FILE_ACCESS_VIOLATION_ERROR } from '../../constants/error-messages'; +import { logger } from '../../logger'; + +function assertBaseDir(path: string, baseDir: string): void { + if (!path.startsWith(upath.resolve(baseDir))) { + logger.warn( + { path, baseDir }, + 'Preventing access to file outside the base directory' + ); + throw new Error(FILE_ACCESS_VIOLATION_ERROR); + } +} + +export function ensureLocalPath(path: string): string { + const localDir = GlobalConfig.get('localDir')!; + const fullPath = path.startsWith(localDir) + ? path + : upath.join(localDir, path); + assertBaseDir(fullPath, localDir); + return fullPath; +} + +export function ensureCachePath(path: string): string { + const cacheDir = GlobalConfig.get('cacheDir')!; + const fullPath = path.startsWith(cacheDir) + ? path + : upath.join(cacheDir, path); + assertBaseDir(fullPath, cacheDir); + return fullPath; +} From e5388f9fe52dea705e5914d8d0a87a6f33a5c13d Mon Sep 17 00:00:00 2001 From: Sergei Zharinov Date: Sun, 10 Jul 2022 23:19:20 +0300 Subject: [PATCH 02/12] Fix tests --- lib/modules/manager/batect/extract.ts | 2 ++ lib/modules/manager/cocoapods/extract.ts | 2 ++ lib/modules/manager/flux/extract.spec.ts | 2 ++ lib/modules/manager/gitlabci/extract.spec.ts | 2 ++ lib/modules/manager/mix/extract.ts | 2 ++ lib/modules/manager/npm/post-update/yarn.spec.ts | 1 + lib/util/fs/access.ts | 13 +++++++++++++ lib/util/fs/util.ts | 13 +------------ 8 files changed, 25 insertions(+), 12 deletions(-) create mode 100644 lib/util/fs/access.ts diff --git a/lib/modules/manager/batect/extract.ts b/lib/modules/manager/batect/extract.ts index dea4284291e9f2..8d504d97158373 100644 --- a/lib/modules/manager/batect/extract.ts +++ b/lib/modules/manager/batect/extract.ts @@ -16,6 +16,8 @@ import type { ExtractionResult, } from './types'; +jest.mock('../../../util/fs/access'); + function loadConfig(content: string): BatectConfig { const config = load(content); diff --git a/lib/modules/manager/cocoapods/extract.ts b/lib/modules/manager/cocoapods/extract.ts index d392d7ede1dd2d..bf8c2a0f9962b4 100644 --- a/lib/modules/manager/cocoapods/extract.ts +++ b/lib/modules/manager/cocoapods/extract.ts @@ -8,6 +8,8 @@ import { PodDatasource } from '../../datasource/pod'; import type { PackageDependency, PackageFile } from '../types'; import type { ParsedLine } from './types'; +jest.mock('../../../util/fs/access'); + const regexMappings = [ regEx(`^\\s*pod\\s+(['"])(?[^'"/]+)(\\/(?[^'"]+))?(['"])`), regEx( diff --git a/lib/modules/manager/flux/extract.spec.ts b/lib/modules/manager/flux/extract.spec.ts index 331bd4c3ea23e2..c0f94a2f36afd3 100644 --- a/lib/modules/manager/flux/extract.spec.ts +++ b/lib/modules/manager/flux/extract.spec.ts @@ -4,6 +4,8 @@ import type { RepoGlobalConfig } from '../../../config/types'; import type { ExtractConfig } from '../types'; import { extractAllPackageFiles, extractPackageFile } from '.'; +jest.mock('../../../util/fs/access'); + const config: ExtractConfig = {}; const adminConfig: RepoGlobalConfig = { localDir: '' }; diff --git a/lib/modules/manager/gitlabci/extract.spec.ts b/lib/modules/manager/gitlabci/extract.spec.ts index 96a720b30eeae8..63806b01bd32b6 100644 --- a/lib/modules/manager/gitlabci/extract.spec.ts +++ b/lib/modules/manager/gitlabci/extract.spec.ts @@ -9,6 +9,8 @@ import { } from './extract'; import { extractAllPackageFiles, extractPackageFile } from '.'; +jest.mock('../../../util/fs/access'); + const config: ExtractConfig = {}; const adminConfig: RepoGlobalConfig = { localDir: '' }; diff --git a/lib/modules/manager/mix/extract.ts b/lib/modules/manager/mix/extract.ts index c8b0815f65356a..2ab2ce9c66aeab 100644 --- a/lib/modules/manager/mix/extract.ts +++ b/lib/modules/manager/mix/extract.ts @@ -4,6 +4,8 @@ import { newlineRegex, regEx } from '../../../util/regex'; import { HexDatasource } from '../../datasource/hex'; import type { PackageDependency, PackageFile } from '../types'; +jest.mock('../../../util/fs/access'); + const depSectionRegExp = regEx(/defp\s+deps.*do/g); const depMatchRegExp = regEx( /{:(?\w+),\s*(?[^:"]+)?:?\s*"(?[^"]+)",?\s*(?:organization: "(?.*)")?.*}/gm diff --git a/lib/modules/manager/npm/post-update/yarn.spec.ts b/lib/modules/manager/npm/post-update/yarn.spec.ts index f368baa4b57899..daca86267c79db 100644 --- a/lib/modules/manager/npm/post-update/yarn.spec.ts +++ b/lib/modules/manager/npm/post-update/yarn.spec.ts @@ -22,6 +22,7 @@ jest.mock('child_process'); jest.mock('../../../../util/exec/env'); jest.mock('./node-version'); jest.mock('../../../datasource'); +jest.mock('../../../../util/fs/access'); delete process.env.NPM_CONFIG_CACHE; diff --git a/lib/util/fs/access.ts b/lib/util/fs/access.ts new file mode 100644 index 00000000000000..91aac5bdb610a3 --- /dev/null +++ b/lib/util/fs/access.ts @@ -0,0 +1,13 @@ +import upath from 'upath'; +import { FILE_ACCESS_VIOLATION_ERROR } from '../../constants/error-messages'; +import { logger } from '../../logger'; + +export function assertBaseDir(path: string, baseDir: string): void { + if (!path.startsWith(upath.resolve(baseDir))) { + logger.warn( + { path, baseDir }, + 'Preventing access to file outside the base directory' + ); + throw new Error(FILE_ACCESS_VIOLATION_ERROR); + } +} diff --git a/lib/util/fs/util.ts b/lib/util/fs/util.ts index 3c0458282e2675..8a6c85483cd649 100644 --- a/lib/util/fs/util.ts +++ b/lib/util/fs/util.ts @@ -1,17 +1,6 @@ import upath from 'upath'; import { GlobalConfig } from '../../config/global'; -import { FILE_ACCESS_VIOLATION_ERROR } from '../../constants/error-messages'; -import { logger } from '../../logger'; - -function assertBaseDir(path: string, baseDir: string): void { - if (!path.startsWith(upath.resolve(baseDir))) { - logger.warn( - { path, baseDir }, - 'Preventing access to file outside the base directory' - ); - throw new Error(FILE_ACCESS_VIOLATION_ERROR); - } -} +import { assertBaseDir } from './access'; export function ensureLocalPath(path: string): string { const localDir = GlobalConfig.get('localDir')!; From 90678c354d745e00fef925e4d4333c388d64712b Mon Sep 17 00:00:00 2001 From: Sergei Zharinov Date: Sun, 10 Jul 2022 23:21:05 +0300 Subject: [PATCH 03/12] fix --- lib/modules/manager/mix/extract.spec.ts | 2 ++ lib/modules/manager/mix/extract.ts | 2 -- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/lib/modules/manager/mix/extract.spec.ts b/lib/modules/manager/mix/extract.spec.ts index 530b2267aa358a..8f27d815873dae 100644 --- a/lib/modules/manager/mix/extract.spec.ts +++ b/lib/modules/manager/mix/extract.spec.ts @@ -2,6 +2,8 @@ import { Fixtures } from '../../../../test/fixtures'; import { GlobalConfig } from '../../../config/global'; import { extractPackageFile } from '.'; +jest.mock('../../../util/fs/access'); + describe('modules/manager/mix/extract', () => { beforeEach(() => { GlobalConfig.set({ localDir: '' }); diff --git a/lib/modules/manager/mix/extract.ts b/lib/modules/manager/mix/extract.ts index 2ab2ce9c66aeab..c8b0815f65356a 100644 --- a/lib/modules/manager/mix/extract.ts +++ b/lib/modules/manager/mix/extract.ts @@ -4,8 +4,6 @@ import { newlineRegex, regEx } from '../../../util/regex'; import { HexDatasource } from '../../datasource/hex'; import type { PackageDependency, PackageFile } from '../types'; -jest.mock('../../../util/fs/access'); - const depSectionRegExp = regEx(/defp\s+deps.*do/g); const depMatchRegExp = regEx( /{:(?\w+),\s*(?[^:"]+)?:?\s*"(?[^"]+)",?\s*(?:organization: "(?.*)")?.*}/gm From 1b8b4c34ee97a42faaa916bf27c58de5fd7b1bd9 Mon Sep 17 00:00:00 2001 From: Sergei Zharinov Date: Sun, 10 Jul 2022 23:22:14 +0300 Subject: [PATCH 04/12] fix --- lib/modules/manager/batect/extract.spec.ts | 2 ++ lib/modules/manager/batect/extract.ts | 2 -- lib/modules/manager/cocoapods/extract.spec.ts | 2 ++ lib/modules/manager/cocoapods/extract.ts | 2 -- 4 files changed, 4 insertions(+), 4 deletions(-) diff --git a/lib/modules/manager/batect/extract.spec.ts b/lib/modules/manager/batect/extract.spec.ts index e36c85ece5e344..891bbe4025dd27 100644 --- a/lib/modules/manager/batect/extract.spec.ts +++ b/lib/modules/manager/batect/extract.spec.ts @@ -7,6 +7,8 @@ import { getDep } from '../dockerfile/extract'; import type { ExtractConfig, PackageDependency } from '../types'; import { extractAllPackageFiles } from '.'; +jest.mock('../../../util/fs/access'); + const fixturesDir = 'lib/modules/manager/batect/__fixtures__'; function createDockerDependency(tag: string): PackageDependency { diff --git a/lib/modules/manager/batect/extract.ts b/lib/modules/manager/batect/extract.ts index 8d504d97158373..dea4284291e9f2 100644 --- a/lib/modules/manager/batect/extract.ts +++ b/lib/modules/manager/batect/extract.ts @@ -16,8 +16,6 @@ import type { ExtractionResult, } from './types'; -jest.mock('../../../util/fs/access'); - function loadConfig(content: string): BatectConfig { const config = load(content); diff --git a/lib/modules/manager/cocoapods/extract.spec.ts b/lib/modules/manager/cocoapods/extract.spec.ts index ff7b06b5c2f553..d6f61884d4ee7b 100644 --- a/lib/modules/manager/cocoapods/extract.spec.ts +++ b/lib/modules/manager/cocoapods/extract.spec.ts @@ -3,6 +3,8 @@ import { GlobalConfig } from '../../../config/global'; import type { RepoGlobalConfig } from '../../../config/types'; import { extractPackageFile } from '.'; +jest.mock('../../../util/fs/access'); + const simplePodfile = Fixtures.get('Podfile.simple'); const complexPodfile = Fixtures.get('Podfile.complex'); diff --git a/lib/modules/manager/cocoapods/extract.ts b/lib/modules/manager/cocoapods/extract.ts index bf8c2a0f9962b4..d392d7ede1dd2d 100644 --- a/lib/modules/manager/cocoapods/extract.ts +++ b/lib/modules/manager/cocoapods/extract.ts @@ -8,8 +8,6 @@ import { PodDatasource } from '../../datasource/pod'; import type { PackageDependency, PackageFile } from '../types'; import type { ParsedLine } from './types'; -jest.mock('../../../util/fs/access'); - const regexMappings = [ regEx(`^\\s*pod\\s+(['"])(?[^'"/]+)(\\/(?[^'"]+))?(['"])`), regEx( From eb424ebf28eded4812c8b9b7a5f9807f99a9a0ef Mon Sep 17 00:00:00 2001 From: Sergei Zharinov Date: Mon, 11 Jul 2022 20:12:38 +0300 Subject: [PATCH 05/12] fixes --- lib/util/fs/index.spec.ts | 39 ++++++++++++++----------------------- lib/util/fs/util.spec.ts | 41 +++++++++++++++++++++++++++------------ lib/util/fs/util.ts | 21 +++++++++----------- 3 files changed, 53 insertions(+), 48 deletions(-) diff --git a/lib/util/fs/index.spec.ts b/lib/util/fs/index.spec.ts index 73eb58d6a8833f..e4751c24681c10 100644 --- a/lib/util/fs/index.spec.ts +++ b/lib/util/fs/index.spec.ts @@ -99,20 +99,14 @@ describe('util/fs/index', () => { describe('readLocalFile', () => { it('reads buffer', async () => { - const path = `${localDir}/file.txt`; - await fs.outputFile(path, 'foobar'); - - const res = await readLocalFile(path); - + await fs.outputFile(`${localDir}/file.txt`, 'foobar'); + const res = await readLocalFile('file.txt'); expect(res).toBeInstanceOf(Buffer); }); it('reads string', async () => { - const path = `${localDir}/file.txt`; - await fs.outputFile(path, 'foobar'); - - const res = await readLocalFile(path, 'utf8'); - + await fs.outputFile(`${localDir}/file.txt`, 'foobar'); + const res = await readLocalFile('file.txt', 'utf8'); expect(typeof res).toBe('string'); }); @@ -196,15 +190,15 @@ describe('util/fs/index', () => { it('returns true for file', async () => { const path = `${localDir}/file.txt`; await fs.outputFile(path, 'foobar'); - expect(await localPathExists(path)).toBeTrue(); + expect(await localPathExists('file.txt')).toBeTrue(); }); it('returns true for directory', async () => { - expect(await localPathExists(localDir)).toBeTrue(); + expect(await localPathExists('.')).toBeTrue(); }); it('returns false', async () => { - expect(await localPathExists(`${localDir}/file.txt`)).toBe(false); + expect(await localPathExists('file.txt')).toBe(false); }); }); @@ -275,7 +269,7 @@ describe('util/fs/index', () => { 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) => { @@ -291,7 +285,7 @@ describe('util/fs/index', () => { it('returns true for file', async () => { const path = `${localDir}/file.txt`; await fs.outputFile(path, 'foo'); - expect(await localPathIsFile(path)).toBeTrue(); + expect(await localPathIsFile('file.txt')).toBeTrue(); }); it('returns false for directory', async () => { @@ -360,14 +354,14 @@ describe('util/fs/index', () => { describe('listCacheDir', () => { it('lists directory', async () => { 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 () => { 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(); }); @@ -376,10 +370,8 @@ describe('util/fs/index', () => { describe('readCacheFile', () => { it('reads file', async () => { 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') ); }); @@ -387,9 +379,8 @@ describe('util/fs/index', () => { describe('outputCacheFile', () => { it('outputs file', async () => { - const file = join(cacheDir, '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'); }); }); diff --git a/lib/util/fs/util.spec.ts b/lib/util/fs/util.spec.ts index bd2f5d7ad7a962..3dc4b831570bd9 100644 --- a/lib/util/fs/util.spec.ts +++ b/lib/util/fs/util.spec.ts @@ -3,29 +3,46 @@ import { FILE_ACCESS_VIOLATION_ERROR } from '../../constants/error-messages'; import { ensureCachePath, ensureLocalPath } from './util'; describe('util/fs/util', () => { + const localDir = '/foo'; + const cacheDir = '/bar'; + + beforeAll(() => { + GlobalConfig.set({ localDir, cacheDir }); + }); + test.each` path | fullPath - ${''} | ${'/foo'} - ${'baz'} | ${'/foo/baz'} - ${'/baz'} | ${'/foo/baz'} + ${''} | ${`${localDir}`} + ${'baz'} | ${`${localDir}/baz`} + ${'/baz'} | ${`${localDir}/baz`} `(`ensureLocalPath('$path', '$fullPath')`, ({ path, fullPath }) => { - GlobalConfig.set({ localDir: '/foo' }); expect(ensureLocalPath(path)).toBe(fullPath); }); + test.each` + path + ${'..'} + ${'../etc/passwd'} + ${'/foo/../../etc/passwd'} + `(`ensureLocalPath('$path', '${localDir}') - throws`, ({ path }) => { + expect(() => ensureLocalPath(path)).toThrow(FILE_ACCESS_VIOLATION_ERROR); + }); + test.each` path | fullPath - ${''} | ${'/foo'} - ${'baz'} | ${'/foo/baz'} - ${'/baz'} | ${'/foo/baz'} + ${''} | ${`${cacheDir}`} + ${'baz'} | ${`${cacheDir}/baz`} + ${'/baz'} | ${`${cacheDir}/baz`} `(`ensureCachePath('$path', '$fullPath')`, ({ path, fullPath }) => { - GlobalConfig.set({ cacheDir: '/foo' }); expect(ensureCachePath(path)).toBe(fullPath); }); - it('throws FILE_ACCESS_VIOLATION_ERROR', () => { - GlobalConfig.set({ localDir: '/foo', cacheDir: '/bar' }); - expect(() => ensureLocalPath('..')).toThrow(FILE_ACCESS_VIOLATION_ERROR); - expect(() => ensureCachePath('..')).toThrow(FILE_ACCESS_VIOLATION_ERROR); + test.each` + path + ${'..'} + ${'../etc/passwd'} + ${'/bar/../../etc/passwd'} + `(`ensureCachePath('$path', '${cacheDir}') - throws`, ({ path }) => { + expect(() => ensureCachePath(path)).toThrow(FILE_ACCESS_VIOLATION_ERROR); }); }); diff --git a/lib/util/fs/util.ts b/lib/util/fs/util.ts index 8a6c85483cd649..721efbab69274d 100644 --- a/lib/util/fs/util.ts +++ b/lib/util/fs/util.ts @@ -2,20 +2,17 @@ import upath from 'upath'; import { GlobalConfig } from '../../config/global'; import { assertBaseDir } from './access'; -export function ensureLocalPath(path: string): string { - const localDir = GlobalConfig.get('localDir')!; - const fullPath = path.startsWith(localDir) - ? path - : upath.join(localDir, path); - assertBaseDir(fullPath, localDir); +function ensurePath(path: string, key: 'localDir' | 'cacheDir'): string { + const baseDir = upath.resolve(GlobalConfig.get(key)!); + const fullPath = upath.resolve(upath.join(baseDir, path)); + assertBaseDir(fullPath, baseDir); return fullPath; } +export function ensureLocalPath(path: string): string { + return ensurePath(path, 'localDir'); +} + export function ensureCachePath(path: string): string { - const cacheDir = GlobalConfig.get('cacheDir')!; - const fullPath = path.startsWith(cacheDir) - ? path - : upath.join(cacheDir, path); - assertBaseDir(fullPath, cacheDir); - return fullPath; + return ensurePath(path, 'cacheDir'); } From 0f05e1598d0b804f51632ee63375ae10ad597007 Mon Sep 17 00:00:00 2001 From: Sergei Zharinov Date: Mon, 11 Jul 2022 20:15:56 +0300 Subject: [PATCH 06/12] More tests --- lib/util/fs/util.spec.ts | 18 ++++++++++-------- 1 file changed, 10 insertions(+), 8 deletions(-) diff --git a/lib/util/fs/util.spec.ts b/lib/util/fs/util.spec.ts index 3dc4b831570bd9..b552320f293ed8 100644 --- a/lib/util/fs/util.spec.ts +++ b/lib/util/fs/util.spec.ts @@ -11,10 +11,11 @@ describe('util/fs/util', () => { }); test.each` - path | fullPath - ${''} | ${`${localDir}`} - ${'baz'} | ${`${localDir}/baz`} - ${'/baz'} | ${`${localDir}/baz`} + path | fullPath + ${''} | ${`${localDir}`} + ${'baz'} | ${`${localDir}/baz`} + ${'/baz'} | ${`${localDir}/baz`} + ${'/foo/../bar'} | ${`${localDir}/bar`} `(`ensureLocalPath('$path', '$fullPath')`, ({ path, fullPath }) => { expect(ensureLocalPath(path)).toBe(fullPath); }); @@ -29,10 +30,11 @@ describe('util/fs/util', () => { }); test.each` - path | fullPath - ${''} | ${`${cacheDir}`} - ${'baz'} | ${`${cacheDir}/baz`} - ${'/baz'} | ${`${cacheDir}/baz`} + path | fullPath + ${''} | ${`${cacheDir}`} + ${'baz'} | ${`${cacheDir}/baz`} + ${'/baz'} | ${`${cacheDir}/baz`} + ${'/foo/../bar'} | ${`${cacheDir}/bar`} `(`ensureCachePath('$path', '$fullPath')`, ({ path, fullPath }) => { expect(ensureCachePath(path)).toBe(fullPath); }); From 246696552503b3bd4c7fbfd0c961a8aaf6f37a54 Mon Sep 17 00:00:00 2001 From: Sergei Zharinov Date: Mon, 11 Jul 2022 21:54:20 +0300 Subject: [PATCH 07/12] Handle already resolved paths --- .../manager/npm/post-update/yarn.spec.ts | 5 ++--- lib/util/fs/util.spec.ts | 20 +++++++++---------- lib/util/fs/util.ts | 6 +++++- 3 files changed, 17 insertions(+), 14 deletions(-) diff --git a/lib/modules/manager/npm/post-update/yarn.spec.ts b/lib/modules/manager/npm/post-update/yarn.spec.ts index daca86267c79db..6230d4c19b7f34 100644 --- a/lib/modules/manager/npm/post-update/yarn.spec.ts +++ b/lib/modules/manager/npm/post-update/yarn.spec.ts @@ -301,9 +301,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(); } diff --git a/lib/util/fs/util.spec.ts b/lib/util/fs/util.spec.ts index b552320f293ed8..ae8c7d4c5b9b70 100644 --- a/lib/util/fs/util.spec.ts +++ b/lib/util/fs/util.spec.ts @@ -11,11 +11,10 @@ describe('util/fs/util', () => { }); test.each` - path | fullPath - ${''} | ${`${localDir}`} - ${'baz'} | ${`${localDir}/baz`} - ${'/baz'} | ${`${localDir}/baz`} - ${'/foo/../bar'} | ${`${localDir}/bar`} + path | fullPath + ${''} | ${`${localDir}`} + ${'baz'} | ${`${localDir}/baz`} + ${'/baz'} | ${`${localDir}/baz`} `(`ensureLocalPath('$path', '$fullPath')`, ({ path, fullPath }) => { expect(ensureLocalPath(path)).toBe(fullPath); }); @@ -24,17 +23,17 @@ describe('util/fs/util', () => { path ${'..'} ${'../etc/passwd'} + ${'/foo/../bar'} ${'/foo/../../etc/passwd'} `(`ensureLocalPath('$path', '${localDir}') - throws`, ({ path }) => { expect(() => ensureLocalPath(path)).toThrow(FILE_ACCESS_VIOLATION_ERROR); }); test.each` - path | fullPath - ${''} | ${`${cacheDir}`} - ${'baz'} | ${`${cacheDir}/baz`} - ${'/baz'} | ${`${cacheDir}/baz`} - ${'/foo/../bar'} | ${`${cacheDir}/bar`} + path | fullPath + ${''} | ${`${cacheDir}`} + ${'baz'} | ${`${cacheDir}/baz`} + ${'/baz'} | ${`${cacheDir}/baz`} `(`ensureCachePath('$path', '$fullPath')`, ({ path, fullPath }) => { expect(ensureCachePath(path)).toBe(fullPath); }); @@ -43,6 +42,7 @@ describe('util/fs/util', () => { path ${'..'} ${'../etc/passwd'} + ${'/bar/../foo'} ${'/bar/../../etc/passwd'} `(`ensureCachePath('$path', '${cacheDir}') - throws`, ({ path }) => { expect(() => ensureCachePath(path)).toThrow(FILE_ACCESS_VIOLATION_ERROR); diff --git a/lib/util/fs/util.ts b/lib/util/fs/util.ts index 721efbab69274d..fdb703d6ed49f9 100644 --- a/lib/util/fs/util.ts +++ b/lib/util/fs/util.ts @@ -4,7 +4,11 @@ import { assertBaseDir } from './access'; function ensurePath(path: string, key: 'localDir' | 'cacheDir'): string { const baseDir = upath.resolve(GlobalConfig.get(key)!); - const fullPath = upath.resolve(upath.join(baseDir, path)); + let fullPath = path; + if (fullPath.startsWith(baseDir)) { + fullPath = fullPath.replace(baseDir, ''); + } + fullPath = upath.resolve(upath.join(baseDir, fullPath)); assertBaseDir(fullPath, baseDir); return fullPath; } From 74839202a86f34fbca760c2c938eadda105feafd Mon Sep 17 00:00:00 2001 From: Sergei Zharinov Date: Tue, 12 Jul 2022 11:04:35 +0300 Subject: [PATCH 08/12] Simplify --- lib/modules/manager/batect/extract.spec.ts | 2 -- lib/modules/manager/cocoapods/extract.spec.ts | 2 -- lib/modules/manager/flux/extract.spec.ts | 2 -- lib/modules/manager/gitlabci/extract.spec.ts | 2 -- lib/modules/manager/mix/extract.spec.ts | 2 -- lib/util/fs/access.ts | 13 ------------- lib/util/fs/util.ts | 13 ++++++++++++- 7 files changed, 12 insertions(+), 24 deletions(-) delete mode 100644 lib/util/fs/access.ts diff --git a/lib/modules/manager/batect/extract.spec.ts b/lib/modules/manager/batect/extract.spec.ts index 891bbe4025dd27..e36c85ece5e344 100644 --- a/lib/modules/manager/batect/extract.spec.ts +++ b/lib/modules/manager/batect/extract.spec.ts @@ -7,8 +7,6 @@ import { getDep } from '../dockerfile/extract'; import type { ExtractConfig, PackageDependency } from '../types'; import { extractAllPackageFiles } from '.'; -jest.mock('../../../util/fs/access'); - const fixturesDir = 'lib/modules/manager/batect/__fixtures__'; function createDockerDependency(tag: string): PackageDependency { diff --git a/lib/modules/manager/cocoapods/extract.spec.ts b/lib/modules/manager/cocoapods/extract.spec.ts index d6f61884d4ee7b..ff7b06b5c2f553 100644 --- a/lib/modules/manager/cocoapods/extract.spec.ts +++ b/lib/modules/manager/cocoapods/extract.spec.ts @@ -3,8 +3,6 @@ import { GlobalConfig } from '../../../config/global'; import type { RepoGlobalConfig } from '../../../config/types'; import { extractPackageFile } from '.'; -jest.mock('../../../util/fs/access'); - const simplePodfile = Fixtures.get('Podfile.simple'); const complexPodfile = Fixtures.get('Podfile.complex'); diff --git a/lib/modules/manager/flux/extract.spec.ts b/lib/modules/manager/flux/extract.spec.ts index c0f94a2f36afd3..331bd4c3ea23e2 100644 --- a/lib/modules/manager/flux/extract.spec.ts +++ b/lib/modules/manager/flux/extract.spec.ts @@ -4,8 +4,6 @@ import type { RepoGlobalConfig } from '../../../config/types'; import type { ExtractConfig } from '../types'; import { extractAllPackageFiles, extractPackageFile } from '.'; -jest.mock('../../../util/fs/access'); - const config: ExtractConfig = {}; const adminConfig: RepoGlobalConfig = { localDir: '' }; diff --git a/lib/modules/manager/gitlabci/extract.spec.ts b/lib/modules/manager/gitlabci/extract.spec.ts index 63806b01bd32b6..96a720b30eeae8 100644 --- a/lib/modules/manager/gitlabci/extract.spec.ts +++ b/lib/modules/manager/gitlabci/extract.spec.ts @@ -9,8 +9,6 @@ import { } from './extract'; import { extractAllPackageFiles, extractPackageFile } from '.'; -jest.mock('../../../util/fs/access'); - const config: ExtractConfig = {}; const adminConfig: RepoGlobalConfig = { localDir: '' }; diff --git a/lib/modules/manager/mix/extract.spec.ts b/lib/modules/manager/mix/extract.spec.ts index 8f27d815873dae..530b2267aa358a 100644 --- a/lib/modules/manager/mix/extract.spec.ts +++ b/lib/modules/manager/mix/extract.spec.ts @@ -2,8 +2,6 @@ import { Fixtures } from '../../../../test/fixtures'; import { GlobalConfig } from '../../../config/global'; import { extractPackageFile } from '.'; -jest.mock('../../../util/fs/access'); - describe('modules/manager/mix/extract', () => { beforeEach(() => { GlobalConfig.set({ localDir: '' }); diff --git a/lib/util/fs/access.ts b/lib/util/fs/access.ts deleted file mode 100644 index 91aac5bdb610a3..00000000000000 --- a/lib/util/fs/access.ts +++ /dev/null @@ -1,13 +0,0 @@ -import upath from 'upath'; -import { FILE_ACCESS_VIOLATION_ERROR } from '../../constants/error-messages'; -import { logger } from '../../logger'; - -export function assertBaseDir(path: string, baseDir: string): void { - if (!path.startsWith(upath.resolve(baseDir))) { - logger.warn( - { path, baseDir }, - 'Preventing access to file outside the base directory' - ); - throw new Error(FILE_ACCESS_VIOLATION_ERROR); - } -} diff --git a/lib/util/fs/util.ts b/lib/util/fs/util.ts index fdb703d6ed49f9..ad6f36d6e2b121 100644 --- a/lib/util/fs/util.ts +++ b/lib/util/fs/util.ts @@ -1,6 +1,17 @@ import upath from 'upath'; import { GlobalConfig } from '../../config/global'; -import { assertBaseDir } from './access'; +import { FILE_ACCESS_VIOLATION_ERROR } from '../../constants/error-messages'; +import { logger } from '../../logger'; + +export function assertBaseDir(path: string, baseDir: string): void { + if (!path.startsWith(upath.resolve(baseDir))) { + logger.warn( + { path, baseDir }, + 'Preventing access to file outside the base directory' + ); + throw new Error(FILE_ACCESS_VIOLATION_ERROR); + } +} function ensurePath(path: string, key: 'localDir' | 'cacheDir'): string { const baseDir = upath.resolve(GlobalConfig.get(key)!); From d42ef09c706e8397ec10742ec136f9e279f12464 Mon Sep 17 00:00:00 2001 From: Sergei Zharinov Date: Tue, 12 Jul 2022 11:06:06 +0300 Subject: [PATCH 09/12] remove mock --- lib/modules/manager/npm/post-update/yarn.spec.ts | 1 - 1 file changed, 1 deletion(-) diff --git a/lib/modules/manager/npm/post-update/yarn.spec.ts b/lib/modules/manager/npm/post-update/yarn.spec.ts index b4c0a6bb32c0e6..7ac9ac6f13c026 100644 --- a/lib/modules/manager/npm/post-update/yarn.spec.ts +++ b/lib/modules/manager/npm/post-update/yarn.spec.ts @@ -22,7 +22,6 @@ jest.mock('../../../../util/exec/common'); jest.mock('../../../../util/exec/env'); jest.mock('./node-version'); jest.mock('../../../datasource'); -jest.mock('../../../../util/fs/access'); delete process.env.NPM_CONFIG_CACHE; From 5d74dcb6644ea2881ca30f8f422753e673f47424 Mon Sep 17 00:00:00 2001 From: Sergei Zharinov Date: Tue, 12 Jul 2022 11:07:18 +0300 Subject: [PATCH 10/12] Don't export `assertBaseDir` --- lib/util/fs/util.ts | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/lib/util/fs/util.ts b/lib/util/fs/util.ts index ad6f36d6e2b121..f63226bf37176c 100644 --- a/lib/util/fs/util.ts +++ b/lib/util/fs/util.ts @@ -3,7 +3,7 @@ import { GlobalConfig } from '../../config/global'; import { FILE_ACCESS_VIOLATION_ERROR } from '../../constants/error-messages'; import { logger } from '../../logger'; -export function assertBaseDir(path: string, baseDir: string): void { +function assertBaseDir(path: string, baseDir: string): void { if (!path.startsWith(upath.resolve(baseDir))) { logger.warn( { path, baseDir }, From c1c449b2aae2c1d850d72f9783b240d2bf61b7ed Mon Sep 17 00:00:00 2001 From: Sergei Zharinov Date: Tue, 12 Jul 2022 11:50:47 +0300 Subject: [PATCH 11/12] PR review fixes --- lib/util/fs/index.spec.ts | 6 ++++-- lib/util/fs/index.ts | 9 ++------- 2 files changed, 6 insertions(+), 9 deletions(-) diff --git a/lib/util/fs/index.spec.ts b/lib/util/fs/index.spec.ts index e4751c24681c10..07b995aaf86ef1 100644 --- a/lib/util/fs/index.spec.ts +++ b/lib/util/fs/index.spec.ts @@ -59,7 +59,9 @@ describe('util/fs/index', () => { }); afterEach(async () => { - await localDirResult.cleanup(); + await localDirResult?.cleanup(); + await cacheDirResult?.cleanup(); + await tmpDirResult?.cleanup(); }); describe('getParentDir', () => { @@ -107,7 +109,7 @@ describe('util/fs/index', () => { it('reads string', async () => { await fs.outputFile(`${localDir}/file.txt`, 'foobar'); const res = await readLocalFile('file.txt', 'utf8'); - expect(typeof res).toBe('string'); + expect(res).toBe('foobar'); }); it('does not throw', async () => { diff --git a/lib/util/fs/index.ts b/lib/util/fs/index.ts index 94ea854ac63d09..c3cb9a7419d1cf 100644 --- a/lib/util/fs/index.ts +++ b/lib/util/fs/index.ts @@ -3,7 +3,6 @@ import util from 'util'; import is from '@sindresorhus/is'; import findUp from 'find-up'; import fs from 'fs-extra'; -import type { WriteFileOptions } from 'fs-extra'; import upath from 'upath'; import { GlobalConfig } from '../../config/global'; import { logger } from '../../logger'; @@ -235,13 +234,9 @@ export function readCacheFile( return encoding ? fs.readFile(fullPath, encoding) : fs.readFile(fullPath); } -export function outputCacheFile( - file: string, - data: unknown, - options?: WriteFileOptions | string -): Promise { +export function outputCacheFile(file: string, data: unknown): Promise { const filePath = ensureCachePath(file); - return fs.outputFile(filePath, data, options ?? {}); + return fs.outputFile(filePath, data); } export async function readSystemFile(fileName: string): Promise; From 92f1608cfe284075461fc7cd050e441ba3cf46d5 Mon Sep 17 00:00:00 2001 From: Sergei Zharinov Date: Tue, 12 Jul 2022 11:58:30 +0300 Subject: [PATCH 12/12] Fix test --- lib/util/fs/index.spec.ts | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/lib/util/fs/index.spec.ts b/lib/util/fs/index.spec.ts index 07b995aaf86ef1..71aa1257a76305 100644 --- a/lib/util/fs/index.spec.ts +++ b/lib/util/fs/index.spec.ts @@ -112,8 +112,8 @@ describe('util/fs/index', () => { expect(res).toBe('foobar'); }); - it('does not throw', async () => { - expect(await readLocalFile('')).toBeNull(); + it('returns null if file is not found', async () => { + expect(await readLocalFile('foobar')).toBeNull(); }); });