diff --git a/src/ComponentsService.js b/src/ComponentsService.js index bf5b094..5779e59 100644 --- a/src/ComponentsService.js +++ b/src/ComponentsService.js @@ -1,19 +1,25 @@ 'use strict'; const { resolve } = require('path'); -const { isEmpty, path } = require('ramda'); +const { isEmpty } = require('ramda'); const { Graph, alg } = require('@dagrejs/graphlib'); const traverse = require('traverse'); const pLimit = require('./utils/p-limit'); const ServerlessError = require('./serverless-error'); +const { + createRegistry, + getOwnByPath, + hasOwn, + isReservedComponentId, +} = require('./utils/safe-object'); const utils = require('./utils'); const { loadComponent } = require('./load'); const colors = require('./cli/colors'); const ServerlessFramework = require('../components/framework'); -const INTERNAL_COMPONENTS = { +const INTERNAL_COMPONENTS = Object.assign(createRegistry(), { 'serverless-framework': resolve(__dirname, '../components/framework'), -}; +}); const formatError = (e) => { let formattedError = e instanceof Error ? e.message : e; @@ -34,7 +40,7 @@ const resolveObject = (object, context, method) => { let newValue = value; for (const match of matches) { const referencedPropertyPath = match.substring(2, match.length - 1).split('.'); - const referencedPropertyValue = path(referencedPropertyPath, context); + const referencedPropertyValue = getOwnByPath(context, referencedPropertyPath); if (referencedPropertyValue === undefined) { let errMsg = `The variable "${match}" cannot be resolved: the referenced output does not exist.`; @@ -80,9 +86,16 @@ const validateGraph = (graph) => { }; const getAllComponents = async (obj = {}, root = process.cwd()) => { - const allComponents = {}; + const allComponents = createRegistry(); for (const [key, val] of Object.entries(obj.services)) { + if (isReservedComponentId(key)) { + throw new ServerlessError( + `Service alias "${key}" is reserved and cannot be used.`, + 'INVALID_SERVICE_ALIAS' + ); + } + // By default assume `serverless-framework` component if (!val.component) { val.component = 'serverless-framework'; @@ -101,7 +114,7 @@ const getAllComponents = async (obj = {}, root = process.cwd()) => { path: localComponentPath, inputs: val, }; - } else if (val.component in INTERNAL_COMPONENTS) { + } else if (hasOwn(INTERNAL_COMPONENTS, val.component)) { // Internal component allComponents[key] = { path: INTERNAL_COMPONENTS[val.component], @@ -157,7 +170,7 @@ const setDependencies = (allComponents) => { for (const match of matches) { const referencedComponent = match.substring(2, match.length - 1).split('.')[0]; - if (!allComponents[referencedComponent]) { + if (!hasOwn(allComponents, referencedComponent)) { throw new ServerlessError( `The service "${referencedComponent}" does not exist. It is referenced by "${alias}" in expression "${match}".`, 'REFERENCED_COMPONENT_DOES_NOT_EXIST' @@ -173,7 +186,7 @@ const setDependencies = (allComponents) => { if (typeof allComponents[alias].inputs.dependsOn === 'string') { const explicitDependency = allComponents[alias].inputs.dependsOn; - if (!allComponents[explicitDependency]) { + if (!hasOwn(allComponents, explicitDependency)) { throw new ServerlessError( `The service "${explicitDependency}" referenced in "dependsOn" of "${alias}" does not exist`, 'REFERENCED_COMPONENT_DOES_NOT_EXIST' @@ -183,7 +196,7 @@ const setDependencies = (allComponents) => { } else { const explicitDependencies = allComponents[alias].inputs.dependsOn || []; for (const explicitDependency of explicitDependencies) { - if (!allComponents[explicitDependency]) { + if (!hasOwn(allComponents, explicitDependency)) { throw new ServerlessError( `The service "${explicitDependency}" referenced in "dependsOn" of "${alias}" does not exist`, 'REFERENCED_COMPONENT_DOES_NOT_EXIST' @@ -382,6 +395,13 @@ class ComponentsService { } async invokeComponentCommand(componentName, command, options) { + if (isReservedComponentId(componentName)) { + throw new ServerlessError( + `Service alias "${componentName}" is reserved and cannot be used.`, + 'INVALID_SERVICE_ALIAS' + ); + } + // We can have commands that do not have to call commands directly on the component, // but are global commands that can accept the componentName parameter // to filter out data @@ -394,6 +414,7 @@ class ComponentsService { const component = this.allComponents && + hasOwn(this.allComponents, componentName) && this.allComponents[componentName] && this.allComponents[componentName].instance; if (component === undefined) { @@ -402,32 +423,39 @@ class ComponentsService { this.context.logVerbose(`Invoking "${command}" on service "${componentName}"`); const isDefaultCommand = ['deploy', 'remove', 'logs', 'info', 'package'].includes(command); + const hasCustomCommand = + component && component.commands && hasOwn(component.commands, command); if (isDefaultCommand) { // Default command defined for all components (deploy, logs, dev, etc.) - if (!component || !component[command]) { + if (!component || typeof component[command] !== 'function') { throw new ServerlessError( `No method "${command}" on service "${componentName}"`, 'COMPONENT_COMMAND_NOT_FOUND' ); } handler = (opts) => component[command](opts); - } else if ( - (!component || !component.commands || !component.commands[command]) && - component instanceof ServerlessFramework - ) { + } else if (!hasCustomCommand && component instanceof ServerlessFramework) { // Workaround to invoke all custom Framework commands // TODO: Support options and validation handler = (opts) => component.command(command, opts); } else { // Custom command: the handler is defined in the component's `commands` property - if (!component || !component.commands || !component.commands[command]) { + if (!hasCustomCommand) { throw new ServerlessError( `No command "${command}" on service ${componentName}`, 'COMPONENT_COMMAND_NOT_FOUND' ); } const commandHandler = component.commands[command].handler; + + if (typeof commandHandler !== 'function') { + throw new ServerlessError( + `No command "${command}" on service ${componentName}`, + 'COMPONENT_COMMAND_NOT_FOUND' + ); + } + handler = (opts) => commandHandler.call(component, opts); } } diff --git a/src/Context.js b/src/Context.js index ffc4fd1..8db532d 100644 --- a/src/Context.js +++ b/src/Context.js @@ -12,6 +12,7 @@ const formatOutput = require('./cli/format-output'); const Progresses = require('./cli/Progresses'); const { stderrCliColors } = require('./cli/colors'); const ServerlessError = require('./serverless-error'); +const { createRegistry } = require('./utils/safe-object'); class Context { constructor(config) { @@ -21,7 +22,7 @@ class Context { /** @type {string} */ this.stage = config.stage; this.id = undefined; - this.componentCommandsOutcomes = {}; + this.componentCommandsOutcomes = createRegistry(); this.hasEnabledVerboseInteractively = false; this.progresses = new Progresses(this.output); diff --git a/src/cli/Progresses.js b/src/cli/Progresses.js index c0acc1e..9e7c25e 100644 --- a/src/cli/Progresses.js +++ b/src/cli/Progresses.js @@ -11,6 +11,7 @@ const { stderrCliColors: colors } = require('./colors'); const symbols = require('./symbols'); const isUnicodeSupported = require('is-unicode-supported'); const { stripVTControlCharacters: stripAnsi } = require('node:util'); +const { createRegistry, hasOwn } = require('../utils/safe-object'); const dots = { frames: ['⠋', '⠙', '⠹', '⠸', '⠼', '⠴', '⠦', '⠧', '⠇', '⠏'], @@ -40,7 +41,7 @@ class Progresses { spinner: isUnicodeSupported() ? dots : dashes, }; /** @type {Record} */ - this.progresses = {}; + this.progresses = createRegistry(); this.isCursorHidden = false; this.currentInterval = null; this.output = output; @@ -86,14 +87,14 @@ class Progresses { * @param {string} name */ exists(name) { - return this.progresses[name]; + return hasOwn(this.progresses, name) ? this.progresses[name] : undefined; } /** * @param {string} name */ isWaiting(name) { - return this.progresses[name] && this.progresses[name].status === 'waiting'; + return hasOwn(this.progresses, name) && this.progresses[name].status === 'waiting'; } /** @@ -101,7 +102,7 @@ class Progresses { * @param {string} text */ update(name, text) { - if (!this.progresses[name]) throw Error(`No progress with name ${name}`); + if (!hasOwn(this.progresses, name)) throw Error(`No progress with name ${name}`); this.progresses[name].text = text; this.updateSpinnerState(name); } @@ -111,7 +112,7 @@ class Progresses { * @param {string} [text] */ success(name, text) { - if (!this.progresses[name]) throw Error(`No progress with name ${name}`); + if (!hasOwn(this.progresses, name)) throw Error(`No progress with name ${name}`); this.progresses[name].status = 'success'; if (text) { this.progresses[name].text = text; @@ -127,7 +128,7 @@ class Progresses { error(name, error) { const errorMessage = error instanceof Error ? error.message : error; - if (!this.progresses[name]) throw Error(`No progress with name ${name}`); + if (!hasOwn(this.progresses, name)) throw Error(`No progress with name ${name}`); this.progresses[name].status = 'error'; this.progresses[name].text = 'error'; this.progresses[name].endTime = Date.now(); @@ -143,7 +144,7 @@ class Progresses { * @param {string} name */ skipped(name) { - if (!this.progresses[name]) throw Error(`No progress with name ${name}`); + if (!hasOwn(this.progresses, name)) throw Error(`No progress with name ${name}`); this.progresses[name].status = 'skipped'; this.progresses[name].text = 'skipped'; this.progresses[name].endTime = Date.now(); @@ -167,12 +168,12 @@ class Progresses { this.isCursorHidden = false; cliCursor.show(); } - this.progresses = {}; + this.progresses = createRegistry(); } updateSpinnerState(progressName) { // Log the contents of the progress that initiated state update to verbose - if (this.progresses[progressName]) { + if (hasOwn(this.progresses, progressName)) { const progress = this.progresses[progressName]; let logMessage = progress.text; if (progress.content) { diff --git a/src/configuration/validate.js b/src/configuration/validate.js index a1f7757..1b80758 100644 --- a/src/configuration/validate.js +++ b/src/configuration/validate.js @@ -4,10 +4,11 @@ const path = require('path'); const ServerlessError = require('../serverless-error'); const isObject = require('type/object/is'); const { default: Ajv } = require('ajv'); +const { RESERVED_COMPONENT_IDS, hasOwn, isReservedComponentId } = require('../utils/safe-object'); function validateConfiguration(configuration, configurationPath) { const configurationFilename = path.basename(configurationPath); - if (typeof configuration !== 'object') { + if (configuration == null || typeof configuration !== 'object') { throw new ServerlessError( `Resolved "${configurationFilename}" does not contain valid Compose configuration.\n` + 'Read about Serverless Framework Compose in the documentation: https://slss.io/docs-compose', @@ -15,7 +16,7 @@ function validateConfiguration(configuration, configurationPath) { ); } - if (!isObject(configuration.services)) { + if (!hasOwn(configuration, 'services') || !isObject(configuration.services)) { throw new ServerlessError( `Invalid configuration: "${configurationFilename}" must contain "services" property.\n` + 'Read about Serverless Framework Compose configuration in the documentation: https://slss.io/docs-compose', @@ -24,6 +25,13 @@ function validateConfiguration(configuration, configurationPath) { } Object.entries(configuration.services).forEach(([key, value]) => { + if (isReservedComponentId(key)) { + throw new ServerlessError( + `Invalid configuration: definition of "${key}" service uses a reserved alias. ` + + `Reserved aliases: ${Array.from(RESERVED_COMPONENT_IDS).join(', ')}.`, + 'INVALID_SERVICE_ALIAS' + ); + } if (!isObject(value)) { throw new ServerlessError( `Invalid configuration: definition of "${key}" service must be an object.\n` + @@ -47,7 +55,7 @@ function validateConfiguration(configuration, configurationPath) { 'custom', ]; frameworkConfigKeys.forEach((key) => { - if (key in configuration) { + if (hasOwn(configuration, key)) { throw new ServerlessError( `Invalid property "${key}" in "${configurationFilename}".\n` + 'This is a Serverless Framework option (serverless.yml) that is not supported in serverless-compose.yml.\n' + diff --git a/src/state/BaseStateStorage.js b/src/state/BaseStateStorage.js index 23c997b..4cfe6a2 100644 --- a/src/state/BaseStateStorage.js +++ b/src/state/BaseStateStorage.js @@ -1,5 +1,17 @@ 'use strict'; +const ServerlessError = require('../serverless-error'); +const { createRegistry, hasOwn, isReservedComponentId } = require('../utils/safe-object'); + +const ensureValidComponentId = (componentId) => { + if (!isReservedComponentId(componentId)) return; + + throw new ServerlessError( + `Component ID "${componentId}" is reserved and cannot be persisted.`, + 'INVALID_COMPONENT_ID' + ); +}; + class BaseStateStorage { async readServiceState(defaultState) { await this.readState(); @@ -15,19 +27,25 @@ class BaseStateStorage { async readComponentState(componentId) { await this.readState(); - return ( - (this.state.components && - this.state.components[componentId] && - this.state.components[componentId].state) || - {} - ); + if (isReservedComponentId(componentId)) { + return {}; + } + + if (!this.state.components || !hasOwn(this.state.components, componentId)) { + return {}; + } + + return this.state.components[componentId].state || {}; } async writeComponentState(componentId, componentState) { + ensureValidComponentId(componentId); await this.readState(); - this.state.components = this.state.components || {}; - this.state.components[componentId] = this.state.components[componentId] || {}; + this.state.components = this.state.components || createRegistry(); + if (!hasOwn(this.state.components, componentId)) { + this.state.components[componentId] = {}; + } this.state.components[componentId].state = componentState; await this.writeState(); @@ -37,11 +55,12 @@ class BaseStateStorage { await this.readState(); if (!this.state || !this.state.components) { - return {}; + return createRegistry(); } - const outputs = {}; + const outputs = createRegistry(); for (const [id, data] of Object.entries(this.state.components)) { + if (isReservedComponentId(id)) continue; outputs[id] = data.outputs || {}; } return outputs; @@ -50,18 +69,24 @@ class BaseStateStorage { async readComponentOutputs(componentId) { await this.readState(); - return ( - (this.state.components && - this.state.components[componentId] && - this.state.components[componentId].outputs) || - {} - ); + if (isReservedComponentId(componentId)) { + return {}; + } + + if (!this.state.components || !hasOwn(this.state.components, componentId)) { + return {}; + } + + return this.state.components[componentId].outputs || {}; } async writeComponentOutputs(componentId, componentOutputs) { + ensureValidComponentId(componentId); await this.readState(); - this.state.components = this.state.components || {}; - this.state.components[componentId] = this.state.components[componentId] || {}; + this.state.components = this.state.components || createRegistry(); + if (!hasOwn(this.state.components, componentId)) { + this.state.components[componentId] = {}; + } this.state.components[componentId].outputs = componentOutputs; await this.writeState(); diff --git a/src/state/LocalStateStorage.js b/src/state/LocalStateStorage.js index 6800f35..9971aaa 100644 --- a/src/state/LocalStateStorage.js +++ b/src/state/LocalStateStorage.js @@ -4,6 +4,7 @@ const BaseStateStorage = require('./BaseStateStorage'); const utils = require('../utils/fs'); const path = require('path'); const fsp = require('fs').promises; +const normalizeState = require('./normalize-state'); class LocalStateStorage extends BaseStateStorage { constructor(root, stage) { @@ -19,9 +20,9 @@ class LocalStateStorage extends BaseStateStorage { if (this.state === undefined) { const stateFilePath = path.join(this.stateRoot, `state.${this.stage}.json`); if (await utils.fileExists(stateFilePath)) { - this.state = await utils.readFile(stateFilePath); + this.state = normalizeState(await utils.readFile(stateFilePath)); } else { - this.state = {}; + this.state = normalizeState({}); } } return this.state; diff --git a/src/state/S3StateStorage.js b/src/state/S3StateStorage.js index 6948c49..ee83eef 100644 --- a/src/state/S3StateStorage.js +++ b/src/state/S3StateStorage.js @@ -5,6 +5,7 @@ const pLimit = require('../utils/p-limit'); const streamToString = require('../utils/stream-to-string'); const ServerlessError = require('../serverless-error'); const BaseStateStorage = require('./BaseStateStorage'); +const normalizeState = require('./normalize-state'); class S3StateStorage extends BaseStateStorage { constructor(config = {}) { @@ -33,10 +34,10 @@ class S3StateStorage extends BaseStateStorage { Key: this.stateKey, }); const readState = await streamToString(stateObjectFromS3.Body); - this.state = JSON.parse(readState); + this.state = normalizeState(JSON.parse(readState)); } catch (e) { if (e.Code === 'NoSuchKey') { - this.state = {}; + this.state = normalizeState({}); } else { throw new ServerlessError( `Could not read state from remote S3 bucket: ${e.message}`, diff --git a/src/state/normalize-state.js b/src/state/normalize-state.js new file mode 100644 index 0000000..6d4a4df --- /dev/null +++ b/src/state/normalize-state.js @@ -0,0 +1,38 @@ +'use strict'; + +const { + createRegistry, + hasOwn, + isReservedComponentId, + safeShallowAssign, +} = require('../utils/safe-object'); + +const isObjectLike = (value) => value != null && typeof value === 'object' && !Array.isArray(value); + +const normalizeComponentRecord = (componentRecord) => { + if (!isObjectLike(componentRecord)) return {}; + return safeShallowAssign({}, componentRecord); +}; + +module.exports = (state) => { + if (!isObjectLike(state)) return {}; + + const normalizedState = safeShallowAssign({}, state); + const rawComponents = + hasOwn(state, 'components') && isObjectLike(state.components) ? state.components : null; + + if (rawComponents) { + const normalizedComponents = createRegistry(); + + for (const [componentId, componentRecord] of Object.entries(rawComponents)) { + if (isReservedComponentId(componentId)) continue; + normalizedComponents[componentId] = normalizeComponentRecord(componentRecord); + } + + normalizedState.components = normalizedComponents; + } else if (hasOwn(state, 'components')) { + normalizedState.components = createRegistry(); + } + + return normalizedState; +}; diff --git a/src/utils/safe-object.js b/src/utils/safe-object.js new file mode 100644 index 0000000..6298125 --- /dev/null +++ b/src/utils/safe-object.js @@ -0,0 +1,62 @@ +'use strict'; + +const RESERVED_COMPONENT_IDS = new Set(['__proto__', 'constructor', 'prototype']); +const hasOwnProperty = Object.prototype.hasOwnProperty; + +const hasOwn = (object, key) => object != null && hasOwnProperty.call(object, key); + +const createRegistry = () => Object.create(null); + +const isReservedComponentId = (componentId) => RESERVED_COMPONENT_IDS.has(String(componentId)); + +const safeSet = (target, key, value) => { + if (isReservedComponentId(key)) { + Object.defineProperty(target, key, { + value, + writable: true, + enumerable: true, + configurable: true, + }); + } else { + target[key] = value; + } + + return target; +}; + +const safeShallowAssign = (target, ...sources) => { + for (const source of sources) { + if (source == null) continue; + + for (const [key, value] of Object.entries(source)) { + safeSet(target, key, value); + } + } + + return target; +}; + +const getOwnByPath = (source, path) => { + const segments = Array.isArray(path) + ? path.map((segment) => String(segment)) + : String(path).split('.').filter(Boolean); + + let current = source; + + for (const segment of segments) { + if (current == null || !hasOwn(current, segment)) return undefined; + current = current[segment]; + } + + return current; +}; + +module.exports = { + RESERVED_COMPONENT_IDS, + createRegistry, + getOwnByPath, + hasOwn, + isReservedComponentId, + safeSet, + safeShallowAssign, +}; diff --git a/test/unit/src/Context.test.js b/test/unit/src/Context.test.js index 782a5f9..b26dab0 100644 --- a/test/unit/src/Context.test.js +++ b/test/unit/src/Context.test.js @@ -4,6 +4,7 @@ const expect = require('chai').expect; const { stripVTControlCharacters: stripAnsi } = require('node:util'); const Context = require('../../../src/Context'); +const { createRegistry } = require('../../../src/utils/safe-object'); const readStream = require('../read-stream'); describe('test/unit/src/Context.test.js', () => { @@ -31,4 +32,14 @@ describe('test/unit/src/Context.test.js', () => { expect(stripAnsi(await readStream(context.output.stdout))).to.equal('value: 1\n'); }); + + it('renders null-prototype output maps', async () => { + const context = createContext(); + const outputs = createRegistry(); + outputs.value = 1; + + context.renderOutputs(outputs); + + expect(stripAnsi(await readStream(context.output.stdout))).to.equal('value: 1\n'); + }); }); diff --git a/test/unit/src/cli/Progresses.test.js b/test/unit/src/cli/Progresses.test.js index 0035f04..245296e 100644 --- a/test/unit/src/cli/Progresses.test.js +++ b/test/unit/src/cli/Progresses.test.js @@ -1,6 +1,7 @@ 'use strict'; const expect = require('chai').expect; +const sinon = require('sinon'); const Progresses = require('../../../../src/cli/Progresses'); @@ -32,4 +33,22 @@ describe('test/unit/src/cli/Progresses.test.js', () => { 'two\nthree\nfour' ); }); + + it('tracks progresses in a null-prototype registry', () => { + const progresses = Object.create(Progresses.prototype); + progresses.output = { + interactiveStderr: null, + verbose: sinon.spy(), + }; + progresses.progresses = Object.create(null); + progresses.updateSpinnerState = sinon.stub(); + + progresses.add('service'); + progresses.start('service', 'deploying'); + progresses.success('service', 'done'); + + expect(Object.getPrototypeOf(progresses.progresses)).to.equal(null); + expect(progresses.exists('service')).to.deep.include({ status: 'success', text: 'done' }); + expect(progresses.exists('constructor')).to.equal(undefined); + }); }); diff --git a/test/unit/src/components-service.test.js b/test/unit/src/components-service.test.js index ffc0970..d42de89 100644 --- a/test/unit/src/components-service.test.js +++ b/test/unit/src/components-service.test.js @@ -61,6 +61,7 @@ describe('test/unit/src/components-service.test.js', () => { }); it('has properly resolved components', () => { + expect(Object.getPrototypeOf(componentsService.allComponents)).to.equal(null); expect(componentsService.allComponents).to.deep.equal({ anotherservice: { dependencies: ['consumer'], @@ -93,6 +94,33 @@ describe('test/unit/src/components-service.test.js', () => { }); }); + it('does not treat inherited object keys as internal components', async () => { + const context = new Context({ + root: process.cwd(), + stage: 'dev', + disableIO: true, + configuration: {}, + }); + await context.init(); + + const localComponentsService = new ComponentsService( + context, + { + services: { + resources: { + component: 'constructor', + path: 'resources', + }, + }, + }, + {} + ); + + await localComponentsService.init(); + + expect(localComponentsService.allComponents.resources.path).to.equal('constructor'); + }); + it('has properly resolved components graph', () => { expect(componentsService.componentsGraph.nodeCount()).to.equal(3); expect(componentsService.componentsGraph.edgeCount()).to.equal(2); @@ -166,6 +194,159 @@ describe('test/unit/src/components-service.test.js', () => { ); }); + it('rejects inherited dependency names that are not real services', async () => { + const configuration = { + services: { + foundation: { + component: '@foo/foundation', + path: 'foundation', + dependsOn: 'constructor', + }, + }, + }; + const context = new Context({ + root: process.cwd(), + stage: 'dev', + disableIO: true, + configuration: {}, + }); + await context.init(); + + const localComponentsService = new ComponentsService(context, configuration, {}); + + await expect(localComponentsService.init()).to.eventually.be.rejected.and.have.property( + 'code', + 'REFERENCED_COMPONENT_DOES_NOT_EXIST' + ); + }); + + it('rejects reserved service aliases during initialization', async () => { + const context = new Context({ + root: process.cwd(), + stage: 'dev', + disableIO: true, + configuration: {}, + }); + await context.init(); + + const localComponentsService = new ComponentsService( + context, + { + services: { + constructor: { + path: 'resources', + }, + }, + }, + {} + ); + + await expect(localComponentsService.init()).to.eventually.be.rejected.and.have.property( + 'code', + 'INVALID_SERVICE_ALIAS' + ); + }); + + it('does not resolve inherited output paths from prototypes', async () => { + const loadComponent = sinon.stub().callsFake(async ({ alias, inputs }) => ({ + inputs, + async deploy() { + return undefined; + }, + commands: {}, + alias, + })); + const ComponentsServiceWithStubbedLoad = proxyquire('../../../src/ComponentsService', { + './load': { loadComponent }, + }); + const context = new Context({ + root: process.cwd(), + stage: 'dev', + disableIO: true, + configuration: {}, + }); + await context.init(); + context.stateStorage.readComponentsOutputs = async () => ({ + foundation: { + endpoint: 'https://example.com', + }, + }); + + const configuration = { + services: { + foundation: { + component: '@foo/foundation', + path: 'foundation', + }, + api: { + component: '@foo/api', + path: 'api', + dependsOn: 'foundation', + params: { + inherited: '${foundation.constructor.name}', + }, + }, + }, + }; + + const localComponentsService = new ComponentsServiceWithStubbedLoad(context, configuration, {}); + await localComponentsService.init(); + + await expect(localComponentsService.deploy()).to.eventually.be.rejected.and.have.property( + 'code', + 'REFERENCED_OUTPUT_DOES_NOT_EXIST' + ); + }); + + it('resolves own array output properties like length', async () => { + const loadComponent = sinon.stub().callsFake(async ({ alias, inputs }) => ({ + inputs, + async deploy() { + return undefined; + }, + commands: {}, + alias, + })); + const ComponentsServiceWithStubbedLoad = proxyquire('../../../src/ComponentsService', { + './load': { loadComponent }, + }); + const context = new Context({ + root: process.cwd(), + stage: 'dev', + disableIO: true, + configuration: {}, + }); + await context.init(); + context.stateStorage.readComponentsOutputs = async () => ({ + foundation: { + items: ['one', 'two', 'three'], + }, + }); + + const configuration = { + services: { + foundation: { + component: '@foo/foundation', + path: 'foundation', + }, + api: { + component: '@foo/api', + path: 'api', + dependsOn: 'foundation', + params: { + count: '${foundation.items.length}', + }, + }, + }, + }; + + const localComponentsService = new ComponentsServiceWithStubbedLoad(context, configuration, {}); + await localComponentsService.init(); + await localComponentsService.deploy(); + + expect(loadComponent.secondCall.args[0].inputs.params.count).to.equal(3); + }); + it('deploys dependencies before dependents', async () => { const order = []; const loadComponent = sinon.stub().callsFake(async ({ alias }) => ({ @@ -528,4 +709,171 @@ describe('test/unit/src/components-service.test.js', () => { ['somethingelse: 123', ''].join('\n') ); }); + + it('rejects reserved component aliases in direct command invocation', async () => { + const context = new Context({ + root: process.cwd(), + stage: 'dev', + disableIO: true, + configuration: {}, + }); + await context.init(); + const localComponentsService = new ComponentsService(context, { services: {} }, {}); + + await expect( + localComponentsService.invokeComponentCommand('__proto__', 'outputs', {}) + ).to.eventually.be.rejected.and.have.property('code', 'INVALID_SERVICE_ALIAS'); + }); + + it('rejects inherited Object prototype command names', async () => { + const loadComponent = sinon.stub().resolves({ + commands: {}, + }); + const ComponentsServiceWithStubbedLoad = proxyquire('../../../src/ComponentsService', { + './load': { loadComponent }, + }); + const context = new Context({ + root: process.cwd(), + stage: 'dev', + disableIO: true, + configuration: {}, + }); + await context.init(); + + const localComponentsService = new ComponentsServiceWithStubbedLoad( + context, + { + services: { + foundation: { + component: '@foo/foundation', + path: 'foundation', + }, + }, + }, + {} + ); + await localComponentsService.init(); + + await expect( + localComponentsService.invokeComponentCommand('foundation', 'toString', {}) + ).to.eventually.be.rejected.and.have.property('code', 'COMPONENT_COMMAND_NOT_FOUND'); + }); + + it('does not invoke inherited custom commands', async () => { + const inheritedHandler = sinon.spy(); + const loadComponent = sinon.stub().resolves({ + commands: Object.create({ + inherited: { + handler: inheritedHandler, + }, + }), + }); + const ComponentsServiceWithStubbedLoad = proxyquire('../../../src/ComponentsService', { + './load': { loadComponent }, + }); + const context = new Context({ + root: process.cwd(), + stage: 'dev', + disableIO: true, + configuration: {}, + }); + await context.init(); + + const localComponentsService = new ComponentsServiceWithStubbedLoad( + context, + { + services: { + foundation: { + component: '@foo/foundation', + path: 'foundation', + }, + }, + }, + {} + ); + await localComponentsService.init(); + + await expect( + localComponentsService.invokeComponentCommand('foundation', 'inherited', {}) + ).to.eventually.be.rejected.and.have.property('code', 'COMPONENT_COMMAND_NOT_FOUND'); + expect(inheritedHandler.called).to.equal(false); + }); + + it('rejects custom commands without callable handlers', async () => { + const loadComponent = sinon.stub().resolves({ + commands: { + broken: { + handler: 'not-a-function', + }, + }, + }); + const ComponentsServiceWithStubbedLoad = proxyquire('../../../src/ComponentsService', { + './load': { loadComponent }, + }); + const context = new Context({ + root: process.cwd(), + stage: 'dev', + disableIO: true, + configuration: {}, + }); + await context.init(); + + const localComponentsService = new ComponentsServiceWithStubbedLoad( + context, + { + services: { + foundation: { + component: '@foo/foundation', + path: 'foundation', + }, + }, + }, + {} + ); + await localComponentsService.init(); + + await expect( + localComponentsService.invokeComponentCommand('foundation', 'broken', {}) + ).to.eventually.be.rejected.and.have.property('code', 'COMPONENT_COMMAND_NOT_FOUND'); + }); + + it('supports default commands implemented on component prototypes', async () => { + const deploy = sinon.stub().resolves(); + + class FakeComponent { + async deploy(options) { + return deploy(options); + } + } + + const loadComponent = sinon.stub().resolves(new FakeComponent()); + const ComponentsServiceWithStubbedLoad = proxyquire('../../../src/ComponentsService', { + './load': { loadComponent }, + }); + const context = new Context({ + root: process.cwd(), + stage: 'dev', + disableIO: true, + configuration: {}, + }); + await context.init(); + + const localComponentsService = new ComponentsServiceWithStubbedLoad( + context, + { + services: { + foundation: { + component: '@foo/foundation', + path: 'foundation', + }, + }, + }, + {} + ); + await localComponentsService.init(); + await localComponentsService.invokeComponentCommand('foundation', 'deploy', { force: true }); + + expect(deploy.calledOnceWithExactly({ force: true })).to.equal(true); + expect(context.componentCommandsOutcomes.foundation).to.equal('success'); + }); }); diff --git a/test/unit/src/configuration/read.test.js b/test/unit/src/configuration/read.test.js index 32897ac..a35eb58 100644 --- a/test/unit/src/configuration/read.test.js +++ b/test/unit/src/configuration/read.test.js @@ -131,4 +131,18 @@ describe('test/unit/src/configuration/read.test.js', () => { 'INVALID_COMPOSE_CONFIGURATION_STRUCTURE' ); }); + + it('preserves nested own unsafe keys as data when reading configuration', async () => { + configurationPath = 'serverless-compose-unsafe.json'; + await fsp.writeFile( + configurationPath, + '{"services":{"resources":{"path":"resources","params":{"__proto__":{"value":"ok"}}}}}' + ); + + const configuration = await readConfiguration(configurationPath); + + expect( + Object.getOwnPropertyDescriptor(configuration.services.resources.params, '__proto__').value + ).to.deep.equal({ value: 'ok' }); + }); }); diff --git a/test/unit/src/configuration/validate.test.js b/test/unit/src/configuration/validate.test.js index 44ff6d9..5ab25e1 100644 --- a/test/unit/src/configuration/validate.test.js +++ b/test/unit/src/configuration/validate.test.js @@ -15,6 +15,12 @@ describe('test/unit/src/configuration/validate.test.js', () => { .and.have.property('code', 'INVALID_NON_OBJECT_CONFIGURATION'); }); + it('rejects null config', () => { + expect(() => validateConfiguration(null, configurationPath)) + .to.throw() + .and.have.property('code', 'INVALID_NON_OBJECT_CONFIGURATION'); + }); + it('rejects non-object "services" in config', () => { expect(() => validateConfiguration({ services: 'string' }, configurationPath)) .to.throw() @@ -27,6 +33,12 @@ describe('test/unit/src/configuration/validate.test.js', () => { .and.have.property('code', 'INVALID_NON_OBJECT_SERVICES_CONFIGURATION'); }); + it('does not treat inherited services as valid configuration', () => { + expect(() => validateConfiguration(Object.create({ services: {} }), configurationPath)) + .to.throw() + .and.have.property('code', 'INVALID_NON_OBJECT_SERVICES_CONFIGURATION'); + }); + it('rejects non-object configuration of specific services', () => { expect(() => validateConfiguration( @@ -56,6 +68,13 @@ describe('test/unit/src/configuration/validate.test.js', () => { .and.have.property('code', 'INVALID_CONFIGURATION'); }); + it('ignores inherited Framework-specific properties', () => { + const configuration = Object.create({ provider: {} }); + configuration.services = {}; + + expect(() => validateConfiguration(configuration, configurationPath)).not.to.throw(); + }); + it('rejects configuration with unknown properties', () => { expect(() => validateConfiguration( @@ -82,6 +101,30 @@ describe('test/unit/src/configuration/validate.test.js', () => { ).not.to.throw(); }); + it('rejects reserved service aliases', () => { + const reservedConfigurations = [ + { + services: JSON.parse('{"__proto__":{"path":"resources"}}'), + }, + { + services: { + constructor: { path: 'resources' }, + }, + }, + { + services: { + prototype: { path: 'resources' }, + }, + }, + ]; + + reservedConfigurations.forEach((configuration) => { + expect(() => validateConfiguration(configuration, configurationPath)) + .to.throw() + .and.have.property('code', 'INVALID_SERVICE_ALIAS'); + }); + }); + it('rejects invalid component inputs', () => { // This test validates multiple scenarios of different error messages const schema = { diff --git a/test/unit/src/state/BaseStateStorage.test.js b/test/unit/src/state/BaseStateStorage.test.js new file mode 100644 index 0000000..82f59a2 --- /dev/null +++ b/test/unit/src/state/BaseStateStorage.test.js @@ -0,0 +1,83 @@ +'use strict'; + +const expect = require('chai').expect; + +const BaseStateStorage = require('../../../../src/state/BaseStateStorage'); + +class InMemoryStateStorage extends BaseStateStorage { + constructor(state = {}) { + super(); + this.state = state; + } + + async readState() { + return this.state; + } + + async writeState() { + return undefined; + } +} + +describe('test/unit/src/state/BaseStateStorage.test.js', () => { + it('writes component state and outputs into a null-prototype components registry', async () => { + const storage = new InMemoryStateStorage(); + + await storage.writeComponentState('service', { deployed: true }); + await storage.writeComponentOutputs('service', { endpoint: 'https://example.com' }); + + expect(Object.getPrototypeOf(storage.state.components)).to.equal(null); + expect(await storage.readComponentState('service')).to.deep.equal({ deployed: true }); + expect(await storage.readComponentOutputs('service')).to.deep.equal({ + endpoint: 'https://example.com', + }); + }); + + it('does not resolve inherited component ids on reads', async () => { + const storage = new InMemoryStateStorage({ components: Object.create(null) }); + + expect(await storage.readComponentState('constructor')).to.deep.equal({}); + expect(await storage.readComponentOutputs('constructor')).to.deep.equal({}); + }); + + it('rejects reserved component ids on writes', async () => { + const storage = new InMemoryStateStorage(); + + await expect( + storage.writeComponentState('__proto__', { deployed: true }) + ).to.eventually.be.rejected.and.have.property('code', 'INVALID_COMPONENT_ID'); + await expect( + storage.writeComponentOutputs('constructor', { endpoint: 'https://example.com' }) + ).to.eventually.be.rejected.and.have.property('code', 'INVALID_COMPONENT_ID'); + + expect(storage.state.components).to.equal(undefined); + }); + + it('aggregates outputs into a null-prototype registry', async () => { + const storage = new InMemoryStateStorage({ + components: Object.assign(Object.create(null), { + service: { + outputs: { ok: true }, + }, + }), + }); + + const outputs = await storage.readComponentsOutputs(); + + expect(Object.getPrototypeOf(outputs)).to.equal(null); + expect(outputs.service).to.deep.equal({ ok: true }); + }); + + it('skips reserved component ids when aggregating outputs from raw state', async () => { + const storage = new InMemoryStateStorage({ + components: JSON.parse( + '{"__proto__":{"outputs":{"hidden":true}},"service":{"outputs":{"ok":true}}}' + ), + }); + + const outputs = await storage.readComponentsOutputs(); + + expect(outputs).to.deep.equal(Object.assign(Object.create(null), { service: { ok: true } })); + expect(await storage.readComponentOutputs('__proto__')).to.deep.equal({}); + }); +}); diff --git a/test/unit/src/state/LocalStateStorage.test.js b/test/unit/src/state/LocalStateStorage.test.js new file mode 100644 index 0000000..4204a19 --- /dev/null +++ b/test/unit/src/state/LocalStateStorage.test.js @@ -0,0 +1,56 @@ +'use strict'; + +const os = require('os'); +const path = require('path'); +const fsp = require('fs').promises; +const fse = require('fs-extra'); +const expect = require('chai').expect; + +const LocalStateStorage = require('../../../../src/state/LocalStateStorage'); + +describe('test/unit/src/state/LocalStateStorage.test.js', () => { + let rootDir; + + beforeEach(async () => { + rootDir = await fsp.mkdtemp(path.join(os.tmpdir(), 'compose-local-state-')); + await fse.ensureDir(path.join(rootDir, '.serverless')); + }); + + afterEach(async () => { + if (rootDir) { + await fse.remove(rootDir); + } + }); + + it('normalizes reserved top-level component ids while preserving nested payload unsafe keys', async () => { + const statePath = path.join(rootDir, '.serverless', 'state.dev.json'); + await fsp.writeFile( + statePath, + JSON.stringify({ + components: { + __proto__: { + outputs: { hidden: true }, + }, + constructor: { + outputs: { hidden: true }, + }, + prototype: { + outputs: { hidden: true }, + }, + service: { + outputs: JSON.parse('{"__proto__":{"value":"ok"}}'), + }, + }, + }) + ); + + const storage = new LocalStateStorage(rootDir, 'dev'); + const state = await storage.readState(); + + expect(Object.getPrototypeOf(state.components)).to.equal(null); + expect(Object.keys(state.components)).to.deep.equal(['service']); + expect( + Object.getOwnPropertyDescriptor(state.components.service.outputs, '__proto__').value + ).to.deep.equal({ value: 'ok' }); + }); +}); diff --git a/test/unit/src/state/S3StateStorage.test.js b/test/unit/src/state/S3StateStorage.test.js index c2767c7..61286f2 100644 --- a/test/unit/src/state/S3StateStorage.test.js +++ b/test/unit/src/state/S3StateStorage.test.js @@ -36,12 +36,49 @@ describe('test/unit/src/state/S3StateStorage.test.js', () => { s3StateStorage.s3Client = mockedS3Client; const result = await s3StateStorage.readState(); expect(result).to.deep.equal({ components: { resources: { state: {} } } }); + expect(Object.getPrototypeOf(result.components)).to.equal(null); expect(mockedS3Client.getObject).to.have.been.calledOnceWithExactly({ Bucket: bucketName, Key: stateKey, }); }); + it('normalizes reserved component ids from remote state while preserving nested payload data', async () => { + const s3StateStorage = new S3StateStorage({ bucketName, stateKey }); + const mockedS3Client = { + getObject: sinon.stub().resolves({ + Body: stream.Readable.from([ + Buffer.from( + JSON.stringify({ + components: { + __proto__: { + outputs: { hidden: true }, + }, + constructor: { + outputs: { hidden: true }, + }, + prototype: { + outputs: { hidden: true }, + }, + resources: { + outputs: JSON.parse('{"__proto__":{"value":"ok"}}'), + }, + }, + }) + ), + ]), + }), + }; + s3StateStorage.s3Client = mockedS3Client; + + const result = await s3StateStorage.readState(); + + expect(Object.keys(result.components)).to.deep.equal(['resources']); + expect( + Object.getOwnPropertyDescriptor(result.components.resources.outputs, '__proto__').value + ).to.deep.equal({ value: 'ok' }); + }); + it('gracefully handles situation where state file in S3 is not present', async () => { const s3StateStorage = new S3StateStorage({ bucketName, stateKey }); const getError = new Error(); diff --git a/test/unit/src/state/normalize-state.test.js b/test/unit/src/state/normalize-state.test.js new file mode 100644 index 0000000..6267270 --- /dev/null +++ b/test/unit/src/state/normalize-state.test.js @@ -0,0 +1,36 @@ +'use strict'; + +const expect = require('chai').expect; + +const normalizeState = require('../../../../src/state/normalize-state'); + +describe('test/unit/src/state/normalize-state.test.js', () => { + it('normalizes non-object state to an empty object', () => { + expect(normalizeState(null)).to.deep.equal({}); + expect(normalizeState(42)).to.deep.equal({}); + }); + + it('drops reserved top-level component ids while preserving nested unsafe payload keys', () => { + const normalized = normalizeState({ + components: JSON.parse( + '{"__proto__":{"outputs":{"hidden":true}},"constructor":{"state":{"hidden":true}},"prototype":{"outputs":{"hidden":true}},"service":{"outputs":{"__proto__":{"value":"ok"}}}}' + ), + }); + + expect(Object.getPrototypeOf(normalized.components)).to.equal(null); + expect(Object.keys(normalized.components)).to.deep.equal(['service']); + expect( + Object.getOwnPropertyDescriptor(normalized.components.service.outputs, '__proto__').value + ).to.deep.equal({ value: 'ok' }); + }); + + it('normalizes non-object component records to empty objects', () => { + const normalized = normalizeState({ + components: { + service: 'invalid', + }, + }); + + expect(normalized.components.service).to.deep.equal({}); + }); +}); diff --git a/test/unit/src/utils/safe-object.test.js b/test/unit/src/utils/safe-object.test.js new file mode 100644 index 0000000..cc71d93 --- /dev/null +++ b/test/unit/src/utils/safe-object.test.js @@ -0,0 +1,88 @@ +'use strict'; + +const expect = require('chai').expect; + +const { + createRegistry, + getOwnByPath, + hasOwn, + isReservedComponentId, + safeSet, + safeShallowAssign, +} = require('../../../../src/utils/safe-object'); + +describe('test/unit/src/utils/safe-object.test.js', () => { + afterEach(() => { + delete Object.prototype.polluted; + }); + + it('creates null-prototype registries', () => { + expect(Object.getPrototypeOf(createRegistry())).to.equal(null); + }); + + it('detects reserved component ids', () => { + expect(isReservedComponentId('__proto__')).to.equal(true); + expect(isReservedComponentId('constructor')).to.equal(true); + expect(isReservedComponentId('prototype')).to.equal(true); + expect(isReservedComponentId('service')).to.equal(false); + }); + + it('distinguishes own and inherited properties with hasOwn', () => { + expect(hasOwn(null, 'value')).to.equal(false); + expect(hasOwn(undefined, 'value')).to.equal(false); + expect(hasOwn({}, 'toString')).to.equal(false); + expect(hasOwn({ toString: 'own' }, 'toString')).to.equal(true); + expect(hasOwn([1, 2], 'length')).to.equal(true); + expect(hasOwn('ab', 'length')).to.equal(true); + }); + + it('preserves unsafe keys as own data when setting and assigning values', () => { + const target = {}; + + safeSet(target, '__proto__', { polluted: 'no' }); + safeSet(target, 'constructor', 'ctor'); + safeSet(target, 'prototype', 'proto'); + + expect(Object.getPrototypeOf(target)).to.equal(Object.prototype); + expect(Object.getOwnPropertyDescriptor(target, '__proto__').value).to.deep.equal({ + polluted: 'no', + }); + expect(Object.getOwnPropertyDescriptor(target, 'constructor').value).to.equal('ctor'); + expect(Object.getOwnPropertyDescriptor(target, 'prototype').value).to.equal('proto'); + expect({}.polluted).to.equal(undefined); + + const source = safeShallowAssign( + {}, + { + nested: safeShallowAssign({}, JSON.parse('{"__proto__":{"value":"ok"}}')), + } + ); + + expect(getOwnByPath(source, ['nested', '__proto__', 'value'])).to.equal('ok'); + }); + + it('ignores nullish sources when shallow assigning', () => { + const target = safeShallowAssign({}, null, { value: 'first' }, undefined, { value: 'second' }); + + expect(target).to.deep.equal({ value: 'second' }); + }); + + it('reads only own properties while supporting array and string own properties', () => { + const source = safeShallowAssign( + {}, + { + list: ['one', 'two'], + nested: safeShallowAssign({}, JSON.parse('{"__proto__":{"value":"ok"}}')), + text: 'ab', + } + ); + + expect(getOwnByPath(source, 'list.length')).to.equal(2); + expect(getOwnByPath(source, ['list', '1'])).to.equal('two'); + expect(getOwnByPath(source, ['text', 'length'])).to.equal(2); + expect(getOwnByPath(source, ['nested', '__proto__', 'value'])).to.equal('ok'); + expect(getOwnByPath({ nested: {} }, ['nested', 'constructor', 'name'])).to.equal(undefined); + expect(getOwnByPath({ nested: null }, 'nested.value')).to.equal(undefined); + expect(getOwnByPath({ nested: 5 }, 'nested.value')).to.equal(undefined); + }); +});