diff --git a/package.json b/package.json index 7c2e1f7..22a9706 100644 --- a/package.json +++ b/package.json @@ -1,6 +1,6 @@ { "name": "@adobe/aio-cli-plugin-api-mesh", - "version": "5.7.1-beta.0", + "version": "5.7.2-beta.1", "description": "Adobe I/O CLI plugin to develop and manage API mesh sources", "keywords": [ "oclif-plugin" diff --git a/src/plugins/queryConfig/__tests__/queryConfig.test.js b/src/plugins/queryConfig/__tests__/queryConfig.test.js index 08adaa4..9046eca 100644 --- a/src/plugins/queryConfig/__tests__/queryConfig.test.js +++ b/src/plugins/queryConfig/__tests__/queryConfig.test.js @@ -1,4 +1,5 @@ const { EnvelopArmorPlugin } = require('@escape.tech/graphql-armor'); +const { GraphQLError } = require('graphql/error'); const useQueryConfig = require('../index'); jest.mock('@escape.tech/graphql-armor', () => ({ @@ -9,16 +10,24 @@ beforeEach(() => { EnvelopArmorPlugin.mockClear(); }); +// Every armor protection is wired with the same rejectionEnforcement (propagateOnRejection: +// false + onReject rethrow) regardless of enabled state, so assertions merge it in explicitly +// rather than repeating it per test. +const rejectionEnforcement = { + propagateOnRejection: false, + onReject: [expect.any(Function)], +}; + describe('useQueryConfig', () => { describe('no config — all protections disabled', () => { it('disables all protections when called with undefined', () => { useQueryConfig(undefined); expect(EnvelopArmorPlugin).toHaveBeenCalledWith({ - costLimit: { enabled: false }, - maxDepth: { enabled: false }, - maxAliases: { enabled: false }, - maxTokens: { enabled: false }, - maxDirectives: { enabled: false }, + costLimit: { enabled: false, ...rejectionEnforcement }, + maxDepth: { enabled: false, ...rejectionEnforcement }, + maxAliases: { enabled: false, ...rejectionEnforcement }, + maxTokens: { enabled: false, ...rejectionEnforcement }, + maxDirectives: { enabled: false, ...rejectionEnforcement }, blockFieldSuggestion: { enabled: false }, }); }); @@ -26,11 +35,11 @@ describe('useQueryConfig', () => { it('disables all protections when called with empty object', () => { useQueryConfig({}); expect(EnvelopArmorPlugin).toHaveBeenCalledWith({ - costLimit: { enabled: false }, - maxDepth: { enabled: false }, - maxAliases: { enabled: false }, - maxTokens: { enabled: false }, - maxDirectives: { enabled: false }, + costLimit: { enabled: false, ...rejectionEnforcement }, + maxDepth: { enabled: false, ...rejectionEnforcement }, + maxAliases: { enabled: false, ...rejectionEnforcement }, + maxTokens: { enabled: false, ...rejectionEnforcement }, + maxDirectives: { enabled: false, ...rejectionEnforcement }, blockFieldSuggestion: { enabled: false }, }); }); @@ -39,7 +48,11 @@ describe('useQueryConfig', () => { describe('maxDepth', () => { it('passes enabled and limit (mapped to n) to armor', () => { useQueryConfig({ maxDepth: { enabled: true, limit: 5 } }); - expect(EnvelopArmorPlugin.mock.calls[0][0].maxDepth).toEqual({ enabled: true, n: 5 }); + expect(EnvelopArmorPlugin.mock.calls[0][0].maxDepth).toEqual({ + enabled: true, + n: 5, + ...rejectionEnforcement, + }); }); it('passes enabled: false with no limit', () => { @@ -47,19 +60,28 @@ describe('useQueryConfig', () => { expect(EnvelopArmorPlugin.mock.calls[0][0].maxDepth).toEqual({ enabled: false, n: undefined, + ...rejectionEnforcement, }); }); it('passes enabled: false with limit retained', () => { useQueryConfig({ maxDepth: { enabled: false, limit: 3 } }); - expect(EnvelopArmorPlugin.mock.calls[0][0].maxDepth).toEqual({ enabled: false, n: 3 }); + expect(EnvelopArmorPlugin.mock.calls[0][0].maxDepth).toEqual({ + enabled: false, + n: 3, + ...rejectionEnforcement, + }); }); }); describe('maxAliases', () => { it('passes enabled and limit (mapped to n)', () => { useQueryConfig({ maxAliases: { enabled: true, limit: 10 } }); - expect(EnvelopArmorPlugin.mock.calls[0][0].maxAliases).toEqual({ enabled: true, n: 10 }); + expect(EnvelopArmorPlugin.mock.calls[0][0].maxAliases).toEqual({ + enabled: true, + n: 10, + ...rejectionEnforcement, + }); }); it('passes n: undefined when enabled but no limit — armor uses its default', () => { @@ -67,6 +89,7 @@ describe('useQueryConfig', () => { expect(EnvelopArmorPlugin.mock.calls[0][0].maxAliases).toEqual({ enabled: true, n: undefined, + ...rejectionEnforcement, }); }); }); @@ -74,14 +97,22 @@ describe('useQueryConfig', () => { describe('maxTokens', () => { it('passes enabled and limit (mapped to n)', () => { useQueryConfig({ maxTokens: { enabled: true, limit: 500 } }); - expect(EnvelopArmorPlugin.mock.calls[0][0].maxTokens).toEqual({ enabled: true, n: 500 }); + expect(EnvelopArmorPlugin.mock.calls[0][0].maxTokens).toEqual({ + enabled: true, + n: 500, + ...rejectionEnforcement, + }); }); }); describe('maxDirectives', () => { it('passes enabled and limit (mapped to n)', () => { useQueryConfig({ maxDirectives: { enabled: true, limit: 25 } }); - expect(EnvelopArmorPlugin.mock.calls[0][0].maxDirectives).toEqual({ enabled: true, n: 25 }); + expect(EnvelopArmorPlugin.mock.calls[0][0].maxDirectives).toEqual({ + enabled: true, + n: 25, + ...rejectionEnforcement, + }); }); }); @@ -91,6 +122,7 @@ describe('useQueryConfig', () => { expect(EnvelopArmorPlugin.mock.calls[0][0].costLimit).toEqual({ enabled: true, maxCost: 2000, + ...rejectionEnforcement, }); }); @@ -99,6 +131,7 @@ describe('useQueryConfig', () => { expect(EnvelopArmorPlugin.mock.calls[0][0].costLimit).toEqual({ enabled: false, maxCost: 1000, + ...rejectionEnforcement, }); }); }); @@ -156,6 +189,82 @@ describe('useQueryConfig', () => { ); expect(out[0]).toBe('Cannot query field "nam". [hidden]'); }); + + it('should escape "$" in a custom mask so it is not treated as a replace() backreference', () => { + const out = runValidate( + getPlugin({ blockFieldSuggestion: { enabled: true, mask: '$& $1 [hidden]' } }), + ['Cannot query field "foo". Did you mean "bar"?'], + ); + expect(out[0]).toBe('Cannot query field "foo". $& $1 [hidden]'); + }); + + it('should not mutate the original error object when masking', () => { + const original = Object.assign(new Error('Cannot query field "foo". Did you mean "bar"?'), { + extensions: { code: 'GRAPHQL_VALIDATION_FAILED' }, + }); + const plugin = getPlugin({ blockFieldSuggestion: { enabled: true, mask: '[hidden]' } }); + let masked; + plugin.onValidate()({ + valid: false, + result: [original], + setResult: errors => { + masked = errors[0]; + }, + }); + + expect(original.message).toBe('Cannot query field "foo". Did you mean "bar"?'); + expect(masked.message).toBe('Cannot query field "foo". [hidden]'); + expect(masked).not.toBe(original); + }); + + it('should deep-clone extensions so mutating the masked error does not affect the original', () => { + const original = Object.assign(new Error('Cannot query field "foo". Did you mean "bar"?'), { + extensions: { http: { headers: { 'x-secret': '1' } } }, + }); + const plugin = getPlugin({ blockFieldSuggestion: { enabled: true, mask: '[hidden]' } }); + let masked; + plugin.onValidate()({ + valid: false, + result: [original], + setResult: errors => { + masked = errors[0]; + }, + }); + + delete masked.extensions.http.headers; + expect(original.extensions.http.headers).toEqual({ 'x-secret': '1' }); + }); + }); + + describe('rejection enforcement (onReject)', () => { + function getOnReject(protectionKey) { + useQueryConfig({ [protectionKey]: { enabled: true, limit: 1 } }); + const [onReject] = EnvelopArmorPlugin.mock.calls[0][0][protectionKey].onReject; + return onReject; + } + + it('rethrows a local GraphQLError carrying the same message, extensions, nodes, and path', () => { + const onReject = getOnReject('maxDepth'); + const armorError = { + message: 'Query depth limit of 1 exceeded.', + extensions: { code: 'GRAPHQL_VALIDATION_FAILED' }, + nodes: ['node-placeholder'], + path: ['query', 'foo'], + }; + + let thrown; + try { + onReject({}, armorError); + } catch (e) { + thrown = e; + } + + expect(thrown).toBeInstanceOf(GraphQLError); + expect(thrown.message).toBe(armorError.message); + expect(thrown.extensions).toEqual(armorError.extensions); + expect(thrown.nodes).toEqual(armorError.nodes); + expect(thrown.path).toEqual(armorError.path); + }); }); describe('maskErrors', () => { diff --git a/src/plugins/queryConfig/index.js b/src/plugins/queryConfig/index.js index d83b282..f07ffd6 100644 --- a/src/plugins/queryConfig/index.js +++ b/src/plugins/queryConfig/index.js @@ -1,4 +1,33 @@ const { EnvelopArmorPlugin } = require('@escape.tech/graphql-armor'); +const { GraphQLError } = require('graphql/error'); + +function rethrowWithLocalGraphQLError(_ctx, error) { + throw new GraphQLError(error.message, { + // Clone rather than reusing error.extensions by reference — the armor-thrown error may + // still be held by something earlier in the pipeline (e.g. a logging plugin), and a later + // mutation on the new error's extensions (see maskError.js) would otherwise leak back into it. + extensions: error.extensions != null ? cloneExtensions(error.extensions) : error.extensions, + nodes: error.nodes, + path: error.path, + }); +} + +const rejectionEnforcement = { + propagateOnRejection: false, + onReject: [rethrowWithLocalGraphQLError], +}; + +// Builds one armor protection's config: `fields` maps the protection's own option names (e.g. +// `limit`) to the shape armor expects (e.g. `n`). Disabled (no config, or config not provided) +// collapses to `{ enabled: false }` plus the shared rejection enforcement. +function buildProtection(protectionConfig, fields) { + return { + ...(protectionConfig + ? { enabled: protectionConfig.enabled, ...fields(protectionConfig) } + : { enabled: false }), + ...rejectionEnforcement, + }; +} // All protections are opt-in — nothing enabled unless explicitly configured. // blockFieldSuggestion uses a custom onValidate hook: armor's graphql@^16.10.0 creates a @@ -6,40 +35,71 @@ const { EnvelopArmorPlugin } = require('@escape.tech/graphql-armor'); function useQueryConfig(queryConfig) { return [ EnvelopArmorPlugin({ - costLimit: queryConfig?.costLimit - ? { enabled: queryConfig.costLimit.enabled, maxCost: queryConfig.costLimit.maxCost } - : { enabled: false }, - maxDepth: queryConfig?.maxDepth - ? { enabled: queryConfig.maxDepth.enabled, n: queryConfig.maxDepth.limit } - : { enabled: false }, - maxAliases: queryConfig?.maxAliases - ? { enabled: queryConfig.maxAliases.enabled, n: queryConfig.maxAliases.limit } - : { enabled: false }, - maxTokens: queryConfig?.maxTokens - ? { enabled: queryConfig.maxTokens.enabled, n: queryConfig.maxTokens.limit } - : { enabled: false }, - maxDirectives: queryConfig?.maxDirectives - ? { enabled: queryConfig.maxDirectives.enabled, n: queryConfig.maxDirectives.limit } - : { enabled: false }, + costLimit: buildProtection(queryConfig?.costLimit, config => ({ maxCost: config.maxCost })), + maxDepth: buildProtection(queryConfig?.maxDepth, config => ({ n: config.limit })), + maxAliases: buildProtection(queryConfig?.maxAliases, config => ({ n: config.limit })), + maxTokens: buildProtection(queryConfig?.maxTokens, config => ({ n: config.limit })), + maxDirectives: buildProtection(queryConfig?.maxDirectives, config => ({ n: config.limit })), blockFieldSuggestion: { enabled: false }, }), blockFieldSuggestionPlugin(queryConfig), ]; } +// Returns a new error with `message` replaced rather than mutating `error` in place — the input +// may still be referenced elsewhere in the pipeline (e.g. logging plugins that ran before this one), +// and mutating a shared GraphQLError would silently rewrite what they see. +function withMessage(error, message) { + // getOwnPropertyDescriptors returns a fresh plain object per call — safe to mutate directly + // rather than spreading each descriptor into a new one. + const descriptors = Object.getOwnPropertyDescriptors(error); + descriptors.message.value = message; + // `stack` is a lazy accessor bound to the original object's internal V8 capture — copying the + // descriptor as-is leaves the clone with an undefined stack. Snapshot the already-computed string. + if (descriptors.stack) { + descriptors.stack = { + value: error.stack, + writable: true, + enumerable: false, + configurable: true, + }; + } + // `extensions` is an object — copying its descriptor only copies the reference, so the clone and + // the original would still share (and could mutate) the very same extensions object. Deep-clone + // it so a later mutation on one can't leak into the other — the same class of bug withMessage + // exists to fix, one level deeper. + if (descriptors.extensions?.value != null) { + descriptors.extensions.value = cloneExtensions(descriptors.extensions.value); + } + return Object.create(Object.getPrototypeOf(error), descriptors); +} + +// structuredClone throws (DataCloneError) on values it can't clone — a function or symbol +// anywhere in extensions, say. That would turn a normal masked-error response into an unhandled +// exception. Fall back to a shallow copy: it still isolates the top-level extensions object from +// the original (the actual bug being fixed here) even if some nested value ends up shared. +function cloneExtensions(extensions) { + try { + return structuredClone(extensions); + } catch { + return { ...extensions }; + } +} + function blockFieldSuggestionPlugin(queryConfig) { const enabled = queryConfig?.blockFieldSuggestion?.enabled === true; - const mask = queryConfig?.blockFieldSuggestion?.mask ?? ''; + const mask = (queryConfig?.blockFieldSuggestion?.mask ?? '').replace(/\$/g, '$$$$'); return { onValidate() { return function onValidateEnd({ valid, result, setResult }) { if (!valid && enabled) { setResult( result.map(error => { - if (typeof error.message === 'string') { - error.message = error.message.replace(/Did you mean ".+"\?/g, mask).trim(); + if (typeof error.message !== 'string') { + return error; } - return error; + const maskedMessage = error.message.replace(/Did you mean ".+"\?/g, mask).trim(); + return maskedMessage === error.message ? error : withMessage(error, maskedMessage); }), ); }