From c5667f4902581c2450b70baaa92ff9e362ec2b91 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Jakub=20Ro=C5=BCek?= Date: Mon, 20 Jan 2020 00:13:05 +0100 Subject: [PATCH 1/3] feat: expose available functions to custom functions --- docs/guides/custom-functions.md | 2 + .../custom-functions-oas-ruleset.json | 13 ++++++ .../__fixtures__/functions/validateBar.js | 3 ++ src/__tests__/linter.jest.test.ts | 12 ++++++ src/functions/index.ts | 42 +++++++++++++------ src/rulesets/__tests__/evaluators.test.ts | 17 +++++++- src/rulesets/evaluators.ts | 8 ++++ src/spectral.ts | 13 ++++-- src/types/function.ts | 5 +++ 9 files changed, 97 insertions(+), 18 deletions(-) create mode 100644 src/__tests__/__fixtures__/functions/validateBar.js diff --git a/docs/guides/custom-functions.md b/docs/guides/custom-functions.md index 72143927f..7da9b2b2a 100644 --- a/docs/guides/custom-functions.md +++ b/docs/guides/custom-functions.md @@ -2,6 +2,8 @@ If the built-in functions are not enough for your [custom ruleset](../getting-started/rulesets.md), Spectral allows you to write and use your own custom functions. +Please, do keep in mind that for the time being, the code is **not** executed in a sandboxed environment, so be very careful when including external rulesets. + A custom function might be any JavaScript function compliant with [IFunction](https://github.com/stoplightio/spectral/blob/90a0864863fa232bf367a26dace61fd9f93198db/src/types/function.ts#L3#L8) type. ```ts diff --git a/src/__tests__/__fixtures__/custom-functions-oas-ruleset.json b/src/__tests__/__fixtures__/custom-functions-oas-ruleset.json index 6aa600903..b1ccf693d 100644 --- a/src/__tests__/__fixtures__/custom-functions-oas-ruleset.json +++ b/src/__tests__/__fixtures__/custom-functions-oas-ruleset.json @@ -2,6 +2,7 @@ "extends": ["spectral:oas"], "functions": [ "truthy", + "validateBar", "deepHasIn", [ "hasIn", @@ -60,6 +61,18 @@ "path": "/bar.get" } } + }, + "validate-bar": { + "message": "`bar` property should be a string", + "given": "$", + "then": { + "function": "validateBar", + "functionOptions": { + "schema": { + "type": "string" + } + } + } } } } diff --git a/src/__tests__/__fixtures__/functions/validateBar.js b/src/__tests__/__fixtures__/functions/validateBar.js new file mode 100644 index 000000000..32a975801 --- /dev/null +++ b/src/__tests__/__fixtures__/functions/validateBar.js @@ -0,0 +1,3 @@ +module.exports = function(targetVal, opts, paths, otherValues) { + return 'bar' in targetVal ? this.functions.schema(targetVal.bar, opts, paths, otherValues) : void 0; +}; diff --git a/src/__tests__/linter.jest.test.ts b/src/__tests__/linter.jest.test.ts index 2c506f03d..232c4effd 100644 --- a/src/__tests__/linter.jest.test.ts +++ b/src/__tests__/linter.jest.test.ts @@ -96,6 +96,18 @@ describe('Linter', () => { ]); }); + it('should expose all available functions to custom functions', async () => { + await spectral.loadRuleset(customDirectoryFunctionsRuleset); + expect(await spectral.run({ bar: 2 })).toEqual( + expect.arrayContaining([ + expect.objectContaining({ + code: 'validate-bar', + message: '`bar` property should be a string', + }), + ]), + ); + }); + it('should report resolving errors for correct files', async () => { spectral = new Spectral({ resolver: httpAndFileResolver }); diff --git a/src/functions/index.ts b/src/functions/index.ts index 79c4e72a7..f82bd0c0d 100644 --- a/src/functions/index.ts +++ b/src/functions/index.ts @@ -1,15 +1,31 @@ +import { alphabetical } from './alphabetical'; +import { casing } from './casing'; +import { enumeration } from './enumeration'; +import { falsy } from './falsy'; +import { length } from './length'; +import { pattern } from './pattern'; +import { schema } from './schema'; +import { schemaPath } from './schema-path'; +import { truthy } from './truthy'; +import { typedEnum } from './typedEnum'; +import { undefined } from './undefined'; +import { unreferencedReusableObject } from './unreferencedReusableObject'; +import { xor } from './xor'; + export const functions = { - alphabetical: require('./alphabetical').alphabetical, - casing: require('./casing').casing, - enumeration: require('./enumeration').enumeration, - length: require('./length').length, - pattern: require('./pattern').pattern, - falsy: require('./falsy').falsy, - schema: require('./schema').schema, - schemaPath: require('./schema-path').schemaPath, - truthy: require('./truthy').truthy, - undefined: require('./undefined').undefined, - xor: require('./xor').xor, - unreferencedReusableObject: require('./unreferencedReusableObject').unreferencedReusableObject, - typedEnum: require('./typedEnum').typedEnum, + alphabetical, + casing, + enumeration, + length, + pattern, + falsy, + schema, + schemaPath, + truthy, + undefined, + xor, + unreferencedReusableObject, + typedEnum, }; + +export type DefaultFunctions = typeof functions; diff --git a/src/rulesets/__tests__/evaluators.test.ts b/src/rulesets/__tests__/evaluators.test.ts index 92551967e..32bde66df 100644 --- a/src/rulesets/__tests__/evaluators.test.ts +++ b/src/rulesets/__tests__/evaluators.test.ts @@ -1,4 +1,4 @@ -import { evaluateExport } from '../evaluators'; +import { evaluateExport, setFunctionContext } from '../evaluators'; describe('Code evaluators', () => { describe('Export evaluator', () => { @@ -60,4 +60,19 @@ describe('Code evaluators', () => { expect(() => evaluateExport(`this.returnExports = {}`, null)).toThrow(); }); }); + + describe('setFunctionContext', () => { + it('binds context to given function', () => { + const context = { a: true }; + const fn = setFunctionContext(context, jest.fn().mockReturnThis()); + expect(fn()).toStrictEqual({ a: true }); + }); + + it('deep-copies provided context', () => { + const context = { a: true }; + const fn = setFunctionContext(context, jest.fn().mockReturnThis()); + const fn2 = setFunctionContext(context, jest.fn().mockReturnThis()); + expect(fn()).not.toBe(fn2()); + }); + }); }); diff --git a/src/rulesets/evaluators.ts b/src/rulesets/evaluators.ts index 37fb183ae..c31ddbc07 100644 --- a/src/rulesets/evaluators.ts +++ b/src/rulesets/evaluators.ts @@ -147,5 +147,13 @@ export const compileExportedFunction = ( value: name, }); + Object.freeze(fn); return fn; }; + +export function setFunctionContext(context: unknown, fn: Function) { + return Function.prototype.bind.call( + fn, + Object.freeze(Object.defineProperties({}, Object.getOwnPropertyDescriptors(context))), + ); +} diff --git a/src/spectral.ts b/src/spectral.ts index 2bd8b7e92..4d1a69d66 100644 --- a/src/spectral.ts +++ b/src/spectral.ts @@ -7,10 +7,10 @@ import { memoize, merge } from 'lodash'; import { STATIC_ASSETS } from './assets'; import { Document, IDocument, IParsedResult, isParsedResult, ParsedDocument } from './document'; import { DocumentInventory } from './documentInventory'; -import { functions as defaultFunctions } from './functions'; +import { DefaultFunctions, functions as defaultFunctions } from './functions'; import * as Parsers from './parsers'; import { readRuleset } from './rulesets'; -import { compileExportedFunction } from './rulesets/evaluators'; +import { compileExportedFunction, setFunctionContext } from './rulesets/evaluators'; import { mergeExceptions } from './rulesets/mergers/exceptions'; import { IRulesetReadOptions } from './rulesets/reader'; import { DEFAULT_SEVERITY_LEVEL, getDiagnosticSeverity } from './rulesets/severity'; @@ -19,6 +19,7 @@ import { FormatLookup, FunctionCollection, IConstructorOpts, + IFunctionContext, IResolver, IRuleResult, IRunOpts, @@ -39,7 +40,7 @@ export * from './types'; export class Spectral { private readonly _resolver: IResolver; - public functions: FunctionCollection = { ...defaultFunctions }; + public functions: FunctionCollection & DefaultFunctions = { ...defaultFunctions }; public rules: RunRuleCollection = {}; public exceptions: RulesetExceptionCollection = {}; public formats: RegisteredFormats; @@ -168,7 +169,11 @@ export class Spectral { return fns; } - fns[key] = compileExportedFunction(code, name, source, schema); + const context: IFunctionContext = { + functions: this.functions, + }; + + fns[key] = setFunctionContext(context, compileExportedFunction(code, name, source, schema)); return fns; }, { diff --git a/src/types/function.ts b/src/types/function.ts index 9d4e3a8ca..d8c71a508 100644 --- a/src/types/function.ts +++ b/src/types/function.ts @@ -1,5 +1,10 @@ import { JsonPath } from '@stoplight/types'; import { DocumentInventory } from '../documentInventory'; +import { DefaultFunctions } from '../functions'; + +export interface IFunctionContext { + functions: DefaultFunctions; +} export type IFunction = ( targetValue: any, From 1d6f61cdd157bf74a33fb030b0bb8cb66e6496c1 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Jakub=20Ro=C5=BCek?= Date: Tue, 21 Jan 2020 22:15:42 +0100 Subject: [PATCH 2/3] chore: rename default functions to core functions --- src/functions/index.ts | 2 +- src/spectral.ts | 8 ++++---- src/types/function.ts | 4 ++-- 3 files changed, 7 insertions(+), 7 deletions(-) diff --git a/src/functions/index.ts b/src/functions/index.ts index f82bd0c0d..8ae274d78 100644 --- a/src/functions/index.ts +++ b/src/functions/index.ts @@ -28,4 +28,4 @@ export const functions = { typedEnum, }; -export type DefaultFunctions = typeof functions; +export type CoreFunctions = typeof functions; diff --git a/src/spectral.ts b/src/spectral.ts index 4d1a69d66..4fb6f6811 100644 --- a/src/spectral.ts +++ b/src/spectral.ts @@ -7,7 +7,7 @@ import { memoize, merge } from 'lodash'; import { STATIC_ASSETS } from './assets'; import { Document, IDocument, IParsedResult, isParsedResult, ParsedDocument } from './document'; import { DocumentInventory } from './documentInventory'; -import { DefaultFunctions, functions as defaultFunctions } from './functions'; +import { CoreFunctions, functions as coreFunctions } from './functions'; import * as Parsers from './parsers'; import { readRuleset } from './rulesets'; import { compileExportedFunction, setFunctionContext } from './rulesets/evaluators'; @@ -40,7 +40,7 @@ export * from './types'; export class Spectral { private readonly _resolver: IResolver; - public functions: FunctionCollection & DefaultFunctions = { ...defaultFunctions }; + public functions: FunctionCollection & CoreFunctions = { ...coreFunctions }; public rules: RunRuleCollection = {}; public exceptions: RulesetExceptionCollection = {}; public formats: RegisteredFormats; @@ -111,7 +111,7 @@ export class Spectral { public setFunctions(functions: FunctionCollection) { empty(this.functions); - Object.assign(this.functions, { ...defaultFunctions, ...functions }); + Object.assign(this.functions, { ...coreFunctions, ...functions }); } public setRules(rules: RuleCollection) { @@ -177,7 +177,7 @@ export class Spectral { return fns; }, { - ...defaultFunctions, + ...coreFunctions, }, ), ); diff --git a/src/types/function.ts b/src/types/function.ts index d8c71a508..356167df6 100644 --- a/src/types/function.ts +++ b/src/types/function.ts @@ -1,9 +1,9 @@ import { JsonPath } from '@stoplight/types'; import { DocumentInventory } from '../documentInventory'; -import { DefaultFunctions } from '../functions'; +import { CoreFunctions } from '../functions'; export interface IFunctionContext { - functions: DefaultFunctions; + functions: CoreFunctions; } export type IFunction = ( From 8c09d18176efded9114ef9f71797b7b73f776559 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Jakub=20Ro=C5=BCek?= Date: Sun, 29 Mar 2020 17:18:40 +0200 Subject: [PATCH 3/3] docs: list possible dangers --- docs/guides/custom-functions.md | 14 ++++++++++++++ 1 file changed, 14 insertions(+) diff --git a/docs/guides/custom-functions.md b/docs/guides/custom-functions.md index 7da9b2b2a..e91ea4fba 100644 --- a/docs/guides/custom-functions.md +++ b/docs/guides/custom-functions.md @@ -3,6 +3,20 @@ If the built-in functions are not enough for your [custom ruleset](../getting-started/rulesets.md), Spectral allows you to write and use your own custom functions. Please, do keep in mind that for the time being, the code is **not** executed in a sandboxed environment, so be very careful when including external rulesets. +This indicates that almost any arbitrary code can be executed. +Potential risks include: +- data / credentials infiltration, +- data tampering, +- running cpu-intensive tasks, i.e. crypto-mining. + +While the risk is relatively low, you should be careful about including **external rulesets** you are not in charge of, in particular these that leverage custom functions. +You are strongly encouraged to review the custom functions a given ruleset provides. +What you should hunt for is: +- obfuscated code, +- calls untrusted external library, +- places where remote code is executed. + +If you notice any weirdness, consider forking the ruleset and removal of any evil-looking code. A custom function might be any JavaScript function compliant with [IFunction](https://github.com/stoplightio/spectral/blob/90a0864863fa232bf367a26dace61fd9f93198db/src/types/function.ts#L3#L8) type.