diff --git a/src/context/directory/handlers/actionModules.ts b/src/context/directory/handlers/actionModules.ts index eb90d69a1..f88660982 100644 --- a/src/context/directory/handlers/actionModules.ts +++ b/src/context/directory/handlers/actionModules.ts @@ -1,6 +1,6 @@ import path from 'path'; import fs from 'fs-extra'; -import { constants } from '../../../tools'; +import { constants, loadFileAndReplaceKeywords } from '../../../tools'; import { getFiles, existsMustBeDir, loadJSON, sanitize, dumpJSON } from '../../../utils'; import log from '../../../logger'; @@ -24,22 +24,21 @@ function parse(context: DirectoryContext): ParsedActionModules { disableKeywordReplacement: context.disableKeywordReplacement, }), }; - const moduleFolder = path.join(constants.ACTION_MODULES_DIRECTORY, `${module.name}`); - if (module.code) { - // The `module.code` can be a file path. It needs to be loaded. - // It can be a relative path, so we need to handle both cases. - const unixPath = module.code.replace(/[\\/]+/g, '/').replace(/^([a-zA-Z]+:|\.\/)/, ''); - if (fs.existsSync(unixPath)) { + const normalizedCode = module.code.replace(/\\/g, '/'); + const configRoot = path.resolve(context.filePath); + const resolvedPath = path.resolve(context.filePath, normalizedCode); + if (!resolvedPath.startsWith(configRoot + path.sep)) { log.warn( - `Support for absolute paths and paths outside the config root will be deprecated in a future version to improve the security of the tool. ` + - `Please update your configuration to use paths relative to the config directory. ` + - `Current absolute path used: ["${module.code}"]` + `Path "${module.code}" resolves to "${resolvedPath}" which is outside the config directory "${configRoot}". ` + + `This will be blocked as an error in the next major release. ` + + `Move the file inside your config directory.` ); - module.code = context.loadFile(unixPath, moduleFolder); - } else { - module.code = context.loadFile(path.join(context.filePath, module.code), moduleFolder); } + module.code = loadFileAndReplaceKeywords(resolvedPath, { + mappings: context.mappings, + disableKeywordReplacement: context.disableKeywordReplacement, + }); } return module; diff --git a/src/context/directory/handlers/actions.ts b/src/context/directory/handlers/actions.ts index 25df66cec..a8b49f9a1 100644 --- a/src/context/directory/handlers/actions.ts +++ b/src/context/directory/handlers/actions.ts @@ -1,7 +1,7 @@ /* eslint-disable consistent-return */ import path from 'path'; import fs from 'fs-extra'; -import { constants } from '../../../tools'; +import { constants, loadFileAndReplaceKeywords } from '../../../tools'; import { getFiles, existsMustBeDir, loadJSON, sanitize, dumpJSON } from '../../../utils'; import log from '../../../logger'; @@ -25,23 +25,21 @@ function parse(context: DirectoryContext): ParsedActions { disableKeywordReplacement: context.disableKeywordReplacement, }), }; - const actionFolder = path.join(constants.ACTIONS_DIRECTORY, `${action.name}`); - if (action.code) { - // Convert `action.code` path to Unix-style path by replacing backslashes and multiple slashes with a single forward slash, and remove leading drive letters or './'. - const unixPath = action.code.replace(/[\\/]+/g, '/').replace(/^([a-zA-Z]+:|\.\/)/, ''); - if (fs.existsSync(unixPath)) { - // If the Unix-style path exists, load the file from that path + const normalizedCode = action.code.replace(/\\/g, '/'); + const configRoot = path.resolve(context.filePath); + const resolvedPath = path.resolve(context.filePath, normalizedCode); + if (!resolvedPath.startsWith(configRoot + path.sep)) { log.warn( - `Support for absolute paths and paths outside the config root will be deprecated in a future version to improve the security of the tool. ` + - `Please update your configuration to use paths relative to the config directory. ` + - `Current absolute path used: ["${action.code}"]` + `Path "${action.code}" resolves to "${resolvedPath}" which is outside the config directory "${configRoot}". ` + + `This will be blocked as an error in the next major release. ` + + `Move the file inside your config directory.` ); - action.code = context.loadFile(unixPath, actionFolder); - } else { - // Otherwise, load the file from the context's file path - action.code = context.loadFile(path.join(context.filePath, action.code), actionFolder); } + action.code = loadFileAndReplaceKeywords(resolvedPath, { + mappings: context.mappings, + disableKeywordReplacement: context.disableKeywordReplacement, + }); } return action; diff --git a/src/context/directory/handlers/databases.ts b/src/context/directory/handlers/databases.ts index 99ffbb1ee..85ebc834b 100644 --- a/src/context/directory/handlers/databases.ts +++ b/src/context/directory/handlers/databases.ts @@ -70,12 +70,12 @@ function getDatabase( log.warn('Skipping invalid database configuration: ' + name); } else { const resolvedBase = path.resolve(configRoot); - const toLoad = path.resolve(folder, script); + const toLoad = path.resolve(folder, script.replace(/\\/g, '/')); if (!toLoad.startsWith(resolvedBase + path.sep)) { log.warn( - `Support for absolute paths and paths outside the config root will be deprecated in a future version to improve the security of the tool. ` + - `Please update your configuration to use paths relative to the config directory. ` + - `Current absolute path used: ["${script}"]` + `Path "${script}" resolves to "${toLoad}" which is outside the config directory "${resolvedBase}". ` + + `This will be blocked as an error in the next major release. ` + + `Move the file inside your config directory.` ); } database.options.customScripts[name] = loadFileAndReplaceKeywords(toLoad, mappingOpts); diff --git a/src/context/directory/handlers/hooks.ts b/src/context/directory/handlers/hooks.ts index 82e2db64b..f7d27126b 100644 --- a/src/context/directory/handlers/hooks.ts +++ b/src/context/directory/handlers/hooks.ts @@ -1,6 +1,6 @@ import path from 'path'; import fs from 'fs-extra'; -import { constants } from '../../../tools'; +import { constants, loadFileAndReplaceKeywords } from '../../../tools'; import { getFiles, existsMustBeDir, dumpJSON, loadJSON, sanitize } from '../../../utils'; import log from '../../../logger'; @@ -24,7 +24,24 @@ function parse(context: DirectoryContext): ParsedHooks { }), }; if (hook.script) { - hook.script = context.loadFile(hook.script, constants.HOOKS_DIRECTORY); + const normalizedScript = hook.script.replace(/\\/g, '/'); + const configRoot = path.resolve(context.filePath); + const resolvedPath = path.resolve( + context.filePath, + constants.HOOKS_DIRECTORY, + normalizedScript + ); + if (!resolvedPath.startsWith(configRoot + path.sep)) { + log.warn( + `Path "${hook.script}" resolves to "${resolvedPath}" which is outside the config directory "${configRoot}". ` + + `This will be blocked as an error in the next major release. ` + + `Move the file inside your config directory.` + ); + } + hook.script = loadFileAndReplaceKeywords(resolvedPath, { + mappings: context.mappings, + disableKeywordReplacement: context.disableKeywordReplacement, + }); } hook.name = hook.name.toLowerCase().replace(/\s/g, '-'); diff --git a/src/context/directory/handlers/rules.ts b/src/context/directory/handlers/rules.ts index 716b24507..2631c028b 100644 --- a/src/context/directory/handlers/rules.ts +++ b/src/context/directory/handlers/rules.ts @@ -1,6 +1,6 @@ import path from 'path'; import fs from 'fs-extra'; -import { constants } from '../../../tools'; +import { constants, loadFileAndReplaceKeywords } from '../../../tools'; import log from '../../../logger'; import { getFiles, existsMustBeDir, dumpJSON, loadJSON, sanitize } from '../../../utils'; @@ -25,7 +25,24 @@ function parse(context: DirectoryContext): ParsedRules { }), }; if (rule.script) { - rule.script = context.loadFile(rule.script, constants.RULES_DIRECTORY); + const normalizedScript = rule.script.replace(/\\/g, '/'); + const configRoot = path.resolve(context.filePath); + const resolvedPath = path.resolve( + context.filePath, + constants.RULES_DIRECTORY, + normalizedScript + ); + if (!resolvedPath.startsWith(configRoot + path.sep)) { + log.warn( + `Path "${rule.script}" resolves to "${resolvedPath}" which is outside the config directory "${configRoot}". ` + + `This will be blocked as an error in the next major release. ` + + `Move the file inside your config directory.` + ); + } + rule.script = loadFileAndReplaceKeywords(resolvedPath, { + mappings: context.mappings, + disableKeywordReplacement: context.disableKeywordReplacement, + }); } return rule; }); diff --git a/src/context/directory/index.ts b/src/context/directory/index.ts index bc5d211c7..3411c7f8f 100644 --- a/src/context/directory/index.ts +++ b/src/context/directory/index.ts @@ -6,7 +6,7 @@ import pagedClient from '../../tools/auth0/client'; import cleanAssets from '../../readonly'; import log from '../../logger'; import handlers, { DirectoryHandler } from './handlers'; -import { isDirectory, isFile, stripIdentifiers, toConfigFn } from '../../utils'; +import { isDirectory, stripIdentifiers, toConfigFn } from '../../utils'; import { Assets, Auth0APIClient, Config, AssetTypes } from '../../types'; import { filterOnlyIncludedResourceTypes } from '..'; import { preserveKeywords } from '../../keywordPreservation'; @@ -47,15 +47,14 @@ export default class DirectoryContext { } loadFile(f: string, folder: string) { - const basePath = path.join(this.filePath, folder); - let toLoad = path.join(basePath, f); - if (!isFile(toLoad)) { - // try load not relative to yaml file - toLoad = f; + const configRoot = path.resolve(this.filePath); + const basePath = path.resolve(this.filePath, folder); + const toLoad = path.resolve(basePath, f.replace(/\\/g, '/')); + if (!toLoad.startsWith(configRoot + path.sep)) { log.warn( - `Support for absolute paths and paths outside the config root will be deprecated in a future version to improve the security of the tool. ` + - `Please update your configuration to use paths relative to the config directory. ` + - `Current absolute path used: ["${f}"]` + `Path "${f}" resolves to "${toLoad}" which is outside the config directory "${configRoot}". ` + + `This will be blocked as an error in the next major release. ` + + `Move the file inside your config directory.` ); } return loadFileAndReplaceKeywords(toLoad, { diff --git a/src/context/yaml/index.ts b/src/context/yaml/index.ts index 5a965cd9f..7b25a8f71 100644 --- a/src/context/yaml/index.ts +++ b/src/context/yaml/index.ts @@ -11,7 +11,7 @@ import { import pagedClient from '../../tools/auth0/client'; import log from '../../logger'; -import { isFile, toConfigFn, stripIdentifiers, formatResults, recordsSorter } from '../../utils'; +import { toConfigFn, stripIdentifiers, formatResults, recordsSorter } from '../../utils'; import handlers, { YAMLHandler } from './handlers'; import cleanAssets from '../../readonly'; import { Assets, Config, Auth0APIClient, AssetTypes, KeywordMappings } from '../../types'; @@ -58,17 +58,16 @@ export default class YAMLContext { } loadFile(f) { - let toLoad = path.join(this.basePath, f); - if (!isFile(toLoad)) { - // try load not relative to yaml file - toLoad = f; + const configRoot = path.resolve(this.basePath); + const toLoad = path.resolve(this.basePath, f.replace(/\\/g, '/')); + if (!toLoad.startsWith(configRoot + path.sep)) { log.warn( - `Support for absolute paths and paths outside the config root will be deprecated in a future version to improve the security of the tool. ` + - `Please update your configuration to use paths relative to the config directory. ` + - `Current absolute path used: ["${f}"]` + `Path "${f}" resolves to "${toLoad}" which is outside the config directory "${configRoot}". ` + + `This will be blocked as an error in the next major release. ` + + `Move the file inside your config directory.` ); } - return loadFileAndReplaceKeywords(path.resolve(toLoad), { + return loadFileAndReplaceKeywords(toLoad, { mappings: this.mappings, disableKeywordReplacement: this.disableKeywordReplacement, }); diff --git a/test/context/directory/actionModules.test.ts b/test/context/directory/actionModules.test.ts index 73dd98de4..f4c95bf3b 100644 --- a/test/context/directory/actionModules.test.ts +++ b/test/context/directory/actionModules.test.ts @@ -1,7 +1,9 @@ import path from 'path'; import fs from 'fs-extra'; +import sinon from 'sinon'; import { expect } from 'chai'; import { constants } from '../../../src/tools'; +import log from '../../../src/logger'; import Context from '../../../src/context/directory'; import handler from '../../../src/context/directory/handlers/actionModules'; @@ -94,6 +96,36 @@ describe('#directory context actionModules', () => { .and.have.property('message', errorMessage); }); + it('should warn when module code path resolves outside the config directory', async () => { + const repoDir = path.join(testDataDir, 'directory', 'actionModules-traversal-warn'); + const outsideFile = path.join(testDataDir, 'directory', 'outside-module-code.js'); + fs.ensureDirSync(path.join(repoDir, constants.ACTION_MODULES_DIRECTORY)); + fs.writeFileSync(outsideFile, 'module.exports = {};'); + createDir(repoDir, { + [constants.ACTION_MODULES_DIRECTORY]: { + 'module-one.json': JSON.stringify({ + name: 'module-one', + code: '../outside-module-code.js', + dependencies: [], + secrets: [], + }), + }, + }); + const context = new Context({ AUTH0_INPUT_FILE: repoDir }, mockMgmtClient()); + if (log.warn.restore) log.warn.restore(); + const warnSpy = sinon.spy(log, 'warn'); + try { + await context.loadAssetsFromLocal(); + const deprecationWarned = warnSpy.args.some(([msg]) => + msg.includes('will be blocked as an error') + ); + expect(deprecationWarned).to.be.true; + } finally { + warnSpy.restore(); + fs.removeSync(outsideFile); + } + }); + it('should dump action modules', async () => { const moduleName = 'module-one'; const dir = path.join(testDataDir, 'directory', 'actionModules4'); diff --git a/test/context/directory/actions.test.js b/test/context/directory/actions.test.js index 976d693af..645a601f3 100644 --- a/test/context/directory/actions.test.js +++ b/test/context/directory/actions.test.js @@ -1,8 +1,10 @@ import path from 'path'; import fs from 'fs-extra'; +import sinon from 'sinon'; import { expect } from 'chai'; import { constants } from '../../../src/tools'; +import log from '../../../src/logger'; import Context from '../../../src/context/directory'; import handler from '../../../src/context/directory/handlers/actions'; @@ -15,7 +17,7 @@ const actionFiles = { '/** @type {PostLoginAction} */ module.exports = async (event, context) => { console.log(@@replace@@); return {}; };', 'action-one.json': `{ "name": "action-one", - "code": "./local/testData/directory/test1/actions/code.js", + "code": "./actions/code.js", "runtime": "node12", "dependencies": [ { @@ -42,7 +44,7 @@ const actionFilesWin32 = { '/** @type {PostLoginAction} */ module.exports = async (event, context) => { console.log(@@replace@@); return {}; };', 'action-one.json': `{ "name": "action-one", - "code": "local\\\\testData\\\\directory\\\\test1\\\\actions\\\\code.js", + "code": "actions\\\\code.js", "runtime": "node12", "dependencies": [ { @@ -351,6 +353,75 @@ describe('#directory context actions', () => { expect(context.assets.actions).to.deep.equal(target); }); + it('should not warn when action code path is relative and inside the config root', async () => { + const repoDir = path.join(testDataDir, 'directory', 'test-no-warn'); + const files = { + [constants.ACTIONS_DIRECTORY]: { + 'code.js': 'module.exports = () => {};', + 'action-one.json': `{ + "name": "action-one", + "code": "./actions/code.js", + "runtime": "node18", + "dependencies": [], + "secrets": [], + "status": "built", + "supported_triggers": [{ "id": "post-login", "version": "v3" }], + "deployed": true + }`, + }, + }; + createDir(repoDir, files); + const config = { AUTH0_INPUT_FILE: repoDir }; + const context = new Context(config, mockMgmtClient()); + if (log.warn.restore) log.warn.restore(); + const warnSpy = sinon.spy(log, 'warn'); + try { + await context.loadAssetsFromLocal(); + const deprecationWarned = warnSpy.args.some(([msg]) => + msg.includes('will be blocked as an error') + ); + expect(deprecationWarned).to.be.false; + } finally { + warnSpy.restore(); + } + }); + + it('should warn when action code path resolves outside the config root', async () => { + const repoDir = path.join(testDataDir, 'directory', 'test-traversal-warn'); + const outsideFile = path.join(testDataDir, 'directory', 'outside-action-code.js'); + fs.ensureDirSync(path.join(repoDir, constants.ACTIONS_DIRECTORY)); + fs.writeFileSync(outsideFile, 'module.exports = () => {};'); + const files = { + [constants.ACTIONS_DIRECTORY]: { + 'action-one.json': `{ + "name": "action-one", + "code": "../outside-action-code.js", + "runtime": "node18", + "dependencies": [], + "secrets": [], + "status": "built", + "supported_triggers": [{ "id": "post-login", "version": "v3" }], + "deployed": true + }`, + }, + }; + createDir(repoDir, files); + const config = { AUTH0_INPUT_FILE: repoDir }; + const context = new Context(config, mockMgmtClient()); + if (log.warn.restore) log.warn.restore(); + const warnSpy = sinon.spy(log, 'warn'); + try { + await context.loadAssetsFromLocal(); + const deprecationWarned = warnSpy.args.some(([msg]) => + msg.includes('will be blocked as an error') + ); + expect(deprecationWarned).to.be.true; + } finally { + warnSpy.restore(); + fs.removeSync(outsideFile); + } + }); + it('should dump actions with modules', async () => { const actionName = 'action-with-modules'; const dir = path.join(testDataDir, 'directory', 'test-action-modules'); diff --git a/test/context/directory/databases.test.js b/test/context/directory/databases.test.js index 4d7670728..fbe82f0ef 100644 --- a/test/context/directory/databases.test.js +++ b/test/context/directory/databases.test.js @@ -1,7 +1,9 @@ import path from 'path'; import fs from 'fs-extra'; +import sinon from 'sinon'; import { expect } from 'chai'; import { constants } from '../../../src/tools'; +import log from '../../../src/logger'; import Context from '../../../src/context/directory'; import handler from '../../../src/context/directory/handlers/databases'; @@ -165,6 +167,41 @@ describe('#directory context databases', () => { ]); }); + it('should warn when customScript path resolves outside the config directory', async () => { + // Scripts are resolved from the connection subfolder (database-connections/users/), + // so ../../../ is needed to escape the config root. + const repoDir = path.join(testDataDir, 'directory', 'databases-traversal-warn'); + const outsideFile = path.join(testDataDir, 'directory', 'outside-login.js'); + cleanThenMkdir(repoDir); + fs.writeFileSync(outsideFile, 'function login() {}'); + createDir(path.join(repoDir, constants.DATABASE_CONNECTIONS_DIRECTORY), { + users: { + 'database.json': JSON.stringify({ + name: 'users', + options: { + enabledDatabaseCustomization: true, + customScripts: { + login: '../../../outside-login.js', + }, + }, + }), + }, + }); + const context = new Context({ AUTH0_INPUT_FILE: repoDir }, mockMgmtClient()); + if (log.warn.restore) log.warn.restore(); + const warnSpy = sinon.spy(log, 'warn'); + try { + await context.loadAssetsFromLocal(); + const deprecationWarned = warnSpy.args.some(([msg]) => + msg.includes('will be blocked as an error') + ); + expect(deprecationWarned).to.be.true; + } finally { + warnSpy.restore(); + fs.removeSync(outsideFile); + } + }); + const dbDumpDir = path.join(testDataDir, 'directory', 'databasesDump'); it('should dump normal databases', async () => { diff --git a/test/context/directory/hooks.test.js b/test/context/directory/hooks.test.js index 92c98b3de..a4a9676ac 100644 --- a/test/context/directory/hooks.test.js +++ b/test/context/directory/hooks.test.js @@ -1,7 +1,9 @@ import path from 'path'; import fs from 'fs-extra'; +import sinon from 'sinon'; import { expect } from 'chai'; import { constants } from '../../../src/tools'; +import log from '../../../src/logger'; import Context from '../../../src/context/directory'; import handler from '../../../src/context/directory/handlers/hooks'; @@ -75,6 +77,32 @@ describe('#directory context hooks', () => { .and.have.property('message', errorMessage); }); + it('should warn when hook script path resolves outside the config root', async () => { + const repoDir = path.join(testDataDir, 'directory', 'hooks-traversal-warn'); + const outsideFile = path.join(testDataDir, 'directory', 'outside-hook.js'); + fs.ensureDirSync(path.join(repoDir, constants.HOOKS_DIRECTORY)); + fs.writeFileSync(outsideFile, 'function outside() {}'); + const traversalHooks = { + 'some-hook.json': + '{ "name": "Some Hook", "enabled": true, "script": "../../outside-hook.js", "triggerId": "credentials-exchange" }', + }; + createDir(repoDir, { [constants.HOOKS_DIRECTORY]: traversalHooks }); + const config = { AUTH0_INPUT_FILE: repoDir }; + const context = new Context(config, mockMgmtClient()); + if (log.warn.restore) log.warn.restore(); + const warnSpy = sinon.spy(log, 'warn'); + try { + await context.loadAssetsFromLocal(); + const deprecationWarned = warnSpy.args.some(([msg]) => + msg.includes('will be blocked as an error') + ); + expect(deprecationWarned).to.be.true; + } finally { + warnSpy.restore(); + fs.removeSync(outsideFile); + } + }); + it('should dump hooks', async () => { const dir = path.join(testDataDir, 'yaml', 'hooksDump'); cleanThenMkdir(dir); diff --git a/test/context/directory/rules.test.js b/test/context/directory/rules.test.js index 372bec565..b55dfeedf 100644 --- a/test/context/directory/rules.test.js +++ b/test/context/directory/rules.test.js @@ -1,7 +1,9 @@ import path from 'path'; import fs from 'fs-extra'; +import sinon from 'sinon'; import { expect } from 'chai'; import { constants } from '../../../src/tools'; +import log from '../../../src/logger'; import Context from '../../../src/context/directory'; import handler from '../../../src/context/directory/handlers/rules'; @@ -128,6 +130,31 @@ describe('#directory context rules', () => { ); }); + it('should warn when rule script path resolves outside the config root', async () => { + const repoDir = path.join(testDataDir, 'directory', 'rules-traversal-warn'); + const outsideFile = path.join(testDataDir, 'directory', 'outside-rule.js'); + fs.ensureDirSync(path.join(repoDir, constants.RULES_DIRECTORY)); + fs.writeFileSync(outsideFile, 'function outside() {}'); + const traversalRules = { + 'somerule.json': '{ "name": "somerule", "enabled": true, "script": "../../outside-rule.js" }', + }; + createDir(repoDir, { [constants.RULES_DIRECTORY]: traversalRules }); + const config = { AUTH0_INPUT_FILE: repoDir }; + const context = new Context(config, mockMgmtClient()); + if (log.warn.restore) log.warn.restore(); + const warnSpy = sinon.spy(log, 'warn'); + try { + await context.loadAssetsFromLocal(); + const deprecationWarned = warnSpy.args.some(([msg]) => + msg.includes('will be blocked as an error') + ); + expect(deprecationWarned).to.be.true; + } finally { + warnSpy.restore(); + fs.removeSync(outsideFile); + } + }); + it('should not dump excluded rules', async () => { const dir = path.join(testDataDir, 'directory', 'rulesDumpExclude'); cleanThenMkdir(dir);