fix: Fixed armor plugin rejection handling issue

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"

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,28 +10,36 @@ 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 },
 			});
 		});
 
 		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,49 +48,71 @@ 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', () => {
 			useQueryConfig({ maxDepth: { enabled: false } });
 			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', () => {
 			useQueryConfig({ maxAliases: { enabled: true } });
 			expect(EnvelopArmorPlugin.mock.calls[0][0].maxAliases).toEqual({
 				enabled: true,
 				n: undefined,
+				...rejectionEnforcement,
 			});
 		});
 	});
 
 	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', () => {

src/plugins/queryConfig/index.js

@@ -1,45 +1,105 @@
 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
 // duplicate class under nohoist, breaking its instanceof GraphQLError check.
 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);
 						}),
 					);
 				}