Skip to content
Draft
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
11 changes: 1 addition & 10 deletions src/context/directory/handlers/actionModules.ts
Original file line number Diff line number Diff line change
Expand Up @@ -30,16 +30,7 @@ function parse(context: DirectoryContext): ParsedActionModules {
// 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)) {
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}"]`
);
module.code = context.loadFile(unixPath, moduleFolder);
} else {
module.code = context.loadFile(path.join(context.filePath, module.code), moduleFolder);
}
module.code = context.loadFile(unixPath, moduleFolder);
}

return module;
Expand Down
13 changes: 1 addition & 12 deletions src/context/directory/handlers/actions.ts
Original file line number Diff line number Diff line change
Expand Up @@ -30,18 +30,7 @@ function parse(context: DirectoryContext): ParsedActions {
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
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}"]`
);
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 = context.loadFile(unixPath, actionFolder);
}

return action;
Expand Down
6 changes: 2 additions & 4 deletions src/context/directory/handlers/databases.ts
Original file line number Diff line number Diff line change
Expand Up @@ -72,10 +72,8 @@ function getDatabase(
const resolvedBase = path.resolve(configRoot);
const toLoad = path.resolve(folder, script);
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}"]`
throw new Error(
`File reference "${script}" in database custom script "${name}" must be relative to the config directory. Absolute paths and paths outside the config root are not supported.`
);
}
database.options.customScripts[name] = loadFileAndReplaceKeywords(toLoad, mappingOpts);
Expand Down
18 changes: 9 additions & 9 deletions src/context/directory/index.ts
Original file line number Diff line number Diff line change
Expand Up @@ -47,17 +47,17 @@ 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;
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}"]`
const resolvedBase = path.resolve(this.filePath);
const inSubfolder = path.resolve(this.filePath, folder, f);
const inRoot = path.resolve(this.filePath, f);
const toLoad = isFile(inSubfolder) ? inSubfolder : inRoot;

if (!toLoad.startsWith(resolvedBase + path.sep)) {
throw new Error(
`File reference "${f}" must be relative to the config directory. Absolute paths and paths outside the config root are not supported.`
);
}

return loadFileAndReplaceKeywords(toLoad, {
mappings: this.mappings,
disableKeywordReplacement: this.disableKeywordReplacement,
Expand Down
19 changes: 9 additions & 10 deletions src/context/yaml/index.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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';
Expand Down Expand Up @@ -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;
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}"]`
const resolvedBase = path.resolve(this.basePath);
const toLoad = path.resolve(this.basePath, f);

if (!toLoad.startsWith(resolvedBase + path.sep)) {
throw new Error(
`File reference "${f}" must be relative to the config directory. Absolute paths and paths outside the config root are not supported.`
);
}
return loadFileAndReplaceKeywords(path.resolve(toLoad), {

return loadFileAndReplaceKeywords(toLoad, {
mappings: this.mappings,
disableKeywordReplacement: this.disableKeywordReplacement,
});
Expand Down
12 changes: 9 additions & 3 deletions src/tools/auth0/index.ts
Original file line number Diff line number Diff line change
Expand Up @@ -8,6 +8,11 @@ import pagedClient from './client';
import schema from './schema';
import handlers from './handlers';

// Cache the compiled AJV validator to avoid recompiling the schema on every validate() call.
// Schema compilation is expensive (~5s) due to the complexity of the combined resource schema.
const ajv = new Ajv({ useDefaults: true, nullable: true });
const validateSchema = ajv.compile(schema);

import {
Assets,
AssetTypes,
Expand Down Expand Up @@ -138,13 +143,14 @@ export default class Auth0 {
}

async validate(): Promise<void> {
const ajv = new Ajv({ useDefaults: true, nullable: true });
const nonNullAssets = Object.keys(this.assets)
.filter((k) => this.assets[k] != null)
.reduce((a, k) => ({ ...a, [k]: this.assets[k] }), {});
const valid = ajv.validate(schema, nonNullAssets);
const valid = validateSchema(nonNullAssets);
if (!valid) {
throw new Error(`Schema validation failed loading ${JSON.stringify(ajv.errors, null, 4)}`);
throw new Error(
`Schema validation failed loading ${JSON.stringify(validateSchema.errors, null, 4)}`
);
}

await this.runStage('validate');
Expand Down
38 changes: 36 additions & 2 deletions test/context/directory/actions.test.js
Original file line number Diff line number Diff line change
Expand Up @@ -15,7 +15,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",

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

2 set Unit test should be add:

  • old path
  • new path

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Addressed and updated

"code": "./actions/code.js",
"runtime": "node12",
"dependencies": [
{
Expand All @@ -42,7 +42,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": [
{
Expand Down Expand Up @@ -150,6 +150,40 @@ describe('#directory context actions', () => {
expect(context.assets.actions).to.deep.equal(actionsTarget);
});

it('should reject action code with a path outside the config directory', async () => {
const repoDir = path.join(testDataDir, 'directory', 'test-path-traversal');
createDir(repoDir, {
[constants.ACTIONS_DIRECTORY]: {
'action-one.json': JSON.stringify({
name: 'action-one',
code: '/absolute/path/outside/config/code.js',
runtime: 'node12',
dependencies: [],
secrets: [],
supported_triggers: [{ id: 'post-login', version: 'v1' }],
deployed: true,
}),
},
});
const context = new Context({ AUTH0_INPUT_FILE: repoDir }, mockMgmtClient());
await expect(context.loadAssetsFromLocal()).to.be.eventually.rejectedWith(
Error,
'must be relative to the config directory'
);
});

it('should accept action code with a path relative to the config directory', async () => {
const repoDir = path.join(testDataDir, 'directory', 'test1');
createDir(repoDir, actionFiles);
const config = {
AUTH0_INPUT_FILE: repoDir,
AUTH0_KEYWORD_REPLACE_MAPPINGS: { replace: 'test-action' },
};
const context = new Context(config, mockMgmtClient());
await context.loadAssetsFromLocal();
expect(context.assets.actions).to.deep.equal(actionsTarget);
});

it('should ignore bad actions directory', async () => {
const repoDir = path.join(testDataDir, 'directory', 'test2');
cleanThenMkdir(repoDir);
Expand Down
50 changes: 44 additions & 6 deletions test/context/yaml/pages.test.js
Original file line number Diff line number Diff line change
Expand Up @@ -25,24 +25,22 @@ describe('#YAML context pages', () => {
cleanThenMkdir(dir);

const htmlContext = '<html>this is a ##val1## @@val2@@</html>';
const htmlFile = path.join(testDataDir, 'page.html');
fs.writeFileSync(htmlFile, htmlContext);

const errorPageUrl = 'https://example.com';
const yaml = `
pages:
- name: "login"
html: "${htmlFile}"
html: "page.html"

- name: "password_reset"
html: "${htmlFile}"
html: "page.html"

- name: "guardian_multifactor"
html: "${htmlFile}"
html: "page.html"
enabled: false

- name: "error_page"
html: "${htmlFile}"
html: "page.html"
url: "${errorPageUrl}"
show_log_link: false
`;
Expand Down Expand Up @@ -70,6 +68,8 @@ describe('#YAML context pages', () => {
},
];
createPagesDir(dir, target);
// Write page.html after createPagesDir since createPagesDir calls cleanThenMkdir(dir)
fs.writeFileSync(path.join(dir, 'page.html'), htmlContext);
const yamlFile = path.join(dir, 'rule1.yaml');
fs.writeFileSync(yamlFile, yaml);

Expand All @@ -83,6 +83,44 @@ describe('#YAML context pages', () => {
expect(context.assets.pages).to.deep.equal(target);
});

it('should reject page html with a path outside the config directory', async () => {
const dir = path.join(testDataDir, 'yaml', 'pages-path-traversal');
cleanThenMkdir(dir);

const yaml = `
pages:
- name: "login"
html: "/absolute/path/outside/config/login.html"
`;
const yamlFile = path.join(dir, 'config.yaml');
fs.writeFileSync(yamlFile, yaml);

const context = new Context({ AUTH0_INPUT_FILE: yamlFile }, mockMgmtClient());
await expect(context.loadAssetsFromLocal()).to.be.eventually.rejectedWith(
Error,
'must be relative to the config directory'
);
});

it('should accept page html with a path relative to the config directory', async () => {
const dir = path.join(testDataDir, 'yaml', 'pages-relative-path');
cleanThenMkdir(dir);

const htmlContent = '<html>login page</html>';
const yaml = `
pages:
- name: "login"
html: "page.html"
`;
const yamlFile = path.join(dir, 'config.yaml');
fs.writeFileSync(yamlFile, yaml);
fs.writeFileSync(path.join(dir, 'page.html'), htmlContent);

const context = new Context({ AUTH0_INPUT_FILE: yamlFile }, mockMgmtClient());
await context.loadAssetsFromLocal();
expect(context.assets.pages[0].html).to.equal(htmlContent);
});

it('should dump pages', async () => {
const dir = path.join(testDataDir, 'yaml', 'pagesDump');
cleanThenMkdir(dir);
Expand Down