diff --git a/tools/spectral/ipa/__tests__/createMethodRequestHasNoReadonlyFields.test.js b/tools/spectral/ipa/__tests__/createMethodRequestHasNoReadonlyFields.test.js new file mode 100644 index 0000000000..657e1937d1 --- /dev/null +++ b/tools/spectral/ipa/__tests__/createMethodRequestHasNoReadonlyFields.test.js @@ -0,0 +1,261 @@ +import testRule from './__helpers__/testRule'; +import { DiagnosticSeverity } from '@stoplight/types'; + +const componentSchemas = { + schemas: { + SchemaWithReadOnly: { + type: 'object', + properties: { + id: { + type: 'string', + readOnly: true, + }, + name: { + type: 'string', + }, + }, + }, + SchemaWithoutReadOnly: { + type: 'object', + properties: { + name: { + type: 'string', + }, + description: { + type: 'string', + }, + }, + }, + NestedSchemaWithReadOnly: { + type: 'object', + properties: { + user: { + type: 'object', + properties: { + userId: { + type: 'string', + readOnly: true, + }, + username: { + type: 'string', + }, + }, + }, + }, + }, + ArraySchemaWithReadOnly: { + type: 'object', + properties: { + items: { + type: 'array', + items: { + type: 'object', + properties: { + itemId: { + type: 'string', + readOnly: true, + }, + itemName: { + type: 'string', + }, + }, + }, + }, + }, + }, + }, +}; + +testRule('xgen-IPA-106-create-method-request-has-no-readonly-fields', [ + { + name: 'valid methods - no readOnly fields', + document: { + components: componentSchemas, + paths: { + '/valid-resource': { + post: { + requestBody: { + content: { + 'application/vnd.atlas.2023-01-01+json': { + schema: { + $ref: '#/components/schemas/SchemaWithoutReadOnly', + }, + }, + 'application/vnd.atlas.2024-01-01+json': { + schema: { + type: 'string', + }, + }, + }, + }, + }, + }, + }, + }, + errors: [], + }, + { + name: 'valid methods - custom method can have readOnly fields', + document: { + components: componentSchemas, + paths: { + '/resource:customAction': { + post: { + requestBody: { + content: { + 'application/vnd.atlas.2023-01-01+json': { + schema: { + $ref: '#/components/schemas/SchemaWithReadOnly', + }, + }, + }, + }, + }, + }, + }, + }, + errors: [], + }, + { + name: 'invalid methods - direct readOnly field', + document: { + components: componentSchemas, + paths: { + '/invalid-resource': { + post: { + requestBody: { + content: { + 'application/vnd.atlas.2023-01-01+json': { + schema: { + $ref: '#/components/schemas/SchemaWithReadOnly', + }, + }, + 'application/vnd.atlas.2024-01-01+json': { + schema: { + type: 'string', + readOnly: true, + }, + }, + }, + }, + }, + }, + }, + }, + errors: [ + { + code: 'xgen-IPA-106-create-method-request-has-no-readonly-fields', + message: + 'The Create method request object must not include input fields (readOnly properties). Found readOnly property at: id. http://go/ipa/106', + path: ['paths', '/invalid-resource', 'post', 'requestBody', 'content', 'application/vnd.atlas.2023-01-01+json'], + severity: DiagnosticSeverity.Warning, + }, + { + code: 'xgen-IPA-106-create-method-request-has-no-readonly-fields', + message: + 'The Create method request object must not include input fields (readOnly properties). Found readOnly property at one of the inline schemas. http://go/ipa/106', + path: ['paths', '/invalid-resource', 'post', 'requestBody', 'content', 'application/vnd.atlas.2024-01-01+json'], + severity: DiagnosticSeverity.Warning, + }, + ], + }, + { + name: 'invalid methods - nested readOnly field', + document: { + components: componentSchemas, + paths: { + '/nested-invalid-resource': { + post: { + requestBody: { + content: { + 'application/vnd.atlas.2023-01-01+json': { + schema: { + $ref: '#/components/schemas/NestedSchemaWithReadOnly', + }, + }, + }, + }, + }, + }, + }, + }, + errors: [ + { + code: 'xgen-IPA-106-create-method-request-has-no-readonly-fields', + message: + 'The Create method request object must not include input fields (readOnly properties). Found readOnly property at: user.userId. http://go/ipa/106', + path: [ + 'paths', + '/nested-invalid-resource', + 'post', + 'requestBody', + 'content', + 'application/vnd.atlas.2023-01-01+json', + ], + severity: DiagnosticSeverity.Warning, + }, + ], + }, + { + name: 'invalid methods - array with readOnly field', + document: { + components: componentSchemas, + paths: { + '/array-invalid-resource': { + post: { + requestBody: { + content: { + 'application/vnd.atlas.2023-01-01+json': { + schema: { + $ref: '#/components/schemas/ArraySchemaWithReadOnly', + }, + }, + }, + }, + }, + }, + }, + }, + errors: [ + { + code: 'xgen-IPA-106-create-method-request-has-no-readonly-fields', + message: + 'The Create method request object must not include input fields (readOnly properties). Found readOnly property at: items.items.itemId. http://go/ipa/106', + path: [ + 'paths', + '/array-invalid-resource', + 'post', + 'requestBody', + 'content', + 'application/vnd.atlas.2023-01-01+json', + ], + severity: DiagnosticSeverity.Warning, + }, + ], + }, + { + name: 'methods with exceptions', + document: { + components: componentSchemas, + paths: { + '/excepted-resource': { + post: { + requestBody: { + content: { + 'application/vnd.atlas.2023-01-01+json': { + schema: { + $ref: '#/components/schemas/SchemaWithReadOnly', + }, + 'x-xgen-IPA-exception': { + 'xgen-IPA-106-create-method-request-has-no-readonly-fields': 'Reason', + }, + }, + }, + }, + }, + }, + }, + }, + errors: [], + }, +]); diff --git a/tools/spectral/ipa/__tests__/getMethodResponseHasNoInputFields.test.js b/tools/spectral/ipa/__tests__/getMethodResponseHasNoInputFields.test.js index bd6afda00a..9cd8dc171f 100644 --- a/tools/spectral/ipa/__tests__/getMethodResponseHasNoInputFields.test.js +++ b/tools/spectral/ipa/__tests__/getMethodResponseHasNoInputFields.test.js @@ -131,7 +131,7 @@ testRule('xgen-IPA-104-get-method-response-has-no-input-fields', [ { code: 'xgen-IPA-104-get-method-response-has-no-input-fields', message: - 'The get method response object must not include output fields (writeOnly properties). http://go/ipa/104', + 'The get method response object must not include output fields (writeOnly properties). Found writeOnly property at: name. http://go/ipa/104', path: [ 'paths', '/resource/{id}', @@ -146,7 +146,7 @@ testRule('xgen-IPA-104-get-method-response-has-no-input-fields', [ { code: 'xgen-IPA-104-get-method-response-has-no-input-fields', message: - 'The get method response object must not include output fields (writeOnly properties). http://go/ipa/104', + 'The get method response object must not include output fields (writeOnly properties). Found writeOnly property at: name. http://go/ipa/104', path: [ 'paths', '/resource/{id}/singleton', diff --git a/tools/spectral/ipa/__tests__/utils/schemaUtils.test.js b/tools/spectral/ipa/__tests__/utils/schemaUtils.test.js new file mode 100644 index 0000000000..d46714e27b --- /dev/null +++ b/tools/spectral/ipa/__tests__/utils/schemaUtils.test.js @@ -0,0 +1,247 @@ +// schemaUtils.test.js +import { findPropertiesByAttribute } from '../../rulesets/functions/utils/schemaUtils'; +import { describe, expect, it } from '@jest/globals'; + +describe('findPropertiesByAttribute', () => { + const mockPath = ['paths', '/resources', 'get', 'responses', '200', 'content', 'application/json']; + const errorMessage = 'Test error message'; + + it('handles primitive values', () => { + expect(findPropertiesByAttribute(null, 'readOnly', mockPath, [], errorMessage)).toEqual([]); + expect(findPropertiesByAttribute(undefined, 'readOnly', mockPath, [], errorMessage)).toEqual([]); + expect(findPropertiesByAttribute('string', 'readOnly', mockPath, [], errorMessage)).toEqual([]); + expect(findPropertiesByAttribute(123, 'readOnly', mockPath, [], errorMessage)).toEqual([]); + expect(findPropertiesByAttribute(true, 'readOnly', mockPath, [], errorMessage)).toEqual([]); + }); + + it('detects direct attribute match', () => { + const schema = { + type: 'string', + readOnly: true, + }; + + const errors = findPropertiesByAttribute(schema, 'readOnly', mockPath, [], errorMessage); + + expect(errors).toHaveLength(1); + expect(errors[0]).toEqual({ + path: mockPath, + message: `${errorMessage} Found readOnly property at one of the inline schemas.`, + }); + }); + + it('detects properties with the specified attribute', () => { + const schema = { + type: 'object', + properties: { + id: { + type: 'string', + readOnly: true, + }, + name: { + type: 'string', + }, + password: { + type: 'string', + writeOnly: true, + }, + }, + }; + + // Testing readOnly detection + let errors = findPropertiesByAttribute(schema, 'readOnly', mockPath, [], errorMessage); + expect(errors).toHaveLength(1); + expect(errors[0].message).toContain('Found readOnly property at: id.'); + + // Testing writeOnly detection + errors = findPropertiesByAttribute(schema, 'writeOnly', mockPath, [], errorMessage); + expect(errors).toHaveLength(1); + expect(errors[0].message).toContain('Found writeOnly property at: password.'); + }); + + it('detects nested properties with the specified attribute', () => { + const schema = { + type: 'object', + properties: { + user: { + type: 'object', + properties: { + id: { + type: 'string', + readOnly: true, + }, + credentials: { + type: 'object', + properties: { + password: { + type: 'string', + writeOnly: true, + }, + }, + }, + }, + }, + }, + }; + + // Testing deep readOnly detection + let errors = findPropertiesByAttribute(schema, 'readOnly', mockPath, [], errorMessage); + expect(errors).toHaveLength(1); + expect(errors[0].message).toContain('Found readOnly property at: user.id.'); + + // Testing deep writeOnly detection + errors = findPropertiesByAttribute(schema, 'writeOnly', mockPath, [], errorMessage); + expect(errors).toHaveLength(1); + expect(errors[0].message).toContain('Found writeOnly property at: user.credentials.password.'); + }); + + it('detects properties in array items', () => { + const schema = { + type: 'object', + properties: { + items: { + type: 'array', + items: { + type: 'object', + properties: { + id: { + type: 'string', + readOnly: true, + }, + secret: { + type: 'string', + writeOnly: true, + }, + }, + }, + }, + }, + }; + + // Testing readOnly in array items + let errors = findPropertiesByAttribute(schema, 'readOnly', mockPath, [], errorMessage); + expect(errors).toHaveLength(1); + expect(errors[0].message).toContain('Found readOnly property at: items.items.id.'); + + // Testing writeOnly in array items + errors = findPropertiesByAttribute(schema, 'writeOnly', mockPath, [], errorMessage); + expect(errors).toHaveLength(1); + expect(errors[0].message).toContain('Found writeOnly property at: items.items.secret.'); + }); + + it('detects properties in schema combiners', () => { + const schema = { + allOf: [ + { + type: 'object', + properties: { + id: { + type: 'string', + readOnly: true, + }, + }, + }, + ], + anyOf: [ + { + type: 'object', + properties: { + key: { + type: 'string', + writeOnly: true, + }, + }, + }, + ], + oneOf: [ + { + type: 'object', + }, + { + type: 'object', + properties: { + token: { + type: 'string', + readOnly: true, + }, + }, + }, + ], + }; + + // Testing readOnly in combiners + let errors = findPropertiesByAttribute(schema, 'readOnly', mockPath, [], errorMessage); + expect(errors).toHaveLength(2); + expect(errors[0].message).toContain('Found readOnly property at: allOf.0.id.'); + expect(errors[1].message).toContain('Found readOnly property at: oneOf.1.token.'); + + // Testing writeOnly in combiners + errors = findPropertiesByAttribute(schema, 'writeOnly', mockPath, [], errorMessage); + expect(errors).toHaveLength(1); + expect(errors[0].message).toContain('Found writeOnly property at: anyOf.0.key.'); + }); + + it('correctly accumulates multiple errors', () => { + const schema = { + type: 'object', + properties: { + id: { + type: 'string', + readOnly: true, + }, + nested: { + type: 'object', + properties: { + innerId: { + type: 'string', + readOnly: true, + }, + }, + }, + items: { + type: 'array', + items: { + readOnly: true, + }, + }, + }, + }; + + const errors = findPropertiesByAttribute(schema, 'readOnly', mockPath, [], errorMessage); + + expect(errors).toHaveLength(3); + expect(errors[0].message).toContain('Found readOnly property at: id.'); + expect(errors[1].message).toContain('Found readOnly property at: nested.innerId.'); + expect(errors[2].message).toContain('Found readOnly property at: items.items.'); + }); + + it('handles empty objects', () => { + const schema = {}; + const errors = findPropertiesByAttribute(schema, 'readOnly', mockPath, [], errorMessage); + expect(errors).toHaveLength(0); + }); + + it('handles schemas with no matching attributes', () => { + const schema = { + type: 'object', + properties: { + id: { type: 'string' }, + name: { type: 'string' }, + nested: { + type: 'object', + properties: { + value: { type: 'number' }, + }, + }, + items: { + type: 'array', + items: { + type: 'string', + }, + }, + }, + }; + + const errors = findPropertiesByAttribute(schema, 'readOnly', mockPath, [], errorMessage); + expect(errors).toHaveLength(0); + }); +}); diff --git a/tools/spectral/ipa/rulesets/IPA-106.yaml b/tools/spectral/ipa/rulesets/IPA-106.yaml index 97c8b0beef..d6fcf29c03 100644 --- a/tools/spectral/ipa/rulesets/IPA-106.yaml +++ b/tools/spectral/ipa/rulesets/IPA-106.yaml @@ -5,6 +5,7 @@ functions: - createMethodRequestBodyIsRequestSuffixedObject - createMethodShouldNotHaveQueryParameters - createMethodRequestBodyIsGetResponse + - createMethodRequestHasNoReadonlyFields rules: xgen-IPA-106-create-method-request-body-is-request-suffixed-object: @@ -34,3 +35,11 @@ rules: function: 'createMethodRequestBodyIsGetResponse' functionOptions: ignoredValues: ['readOnly', 'writeOnly'] + xgen-IPA-106-create-method-request-has-no-readonly-fields: + description: 'Create method Request object must not include fields with readOnly:true. http://go/ipa/106' + message: '{{error}} http://go/ipa/106' + severity: warn + given: '$.paths[*].post.requestBody.content' + then: + field: '@key' + function: 'createMethodRequestHasNoReadonlyFields' diff --git a/tools/spectral/ipa/rulesets/README.md b/tools/spectral/ipa/rulesets/README.md index 8cb53308eb..cdb830ceed 100644 --- a/tools/spectral/ipa/rulesets/README.md +++ b/tools/spectral/ipa/rulesets/README.md @@ -60,6 +60,7 @@ For rule definitions, see [IPA-106.yaml](https://github.com/mongodb/openapi/blob | xgen-IPA-106-create-method-request-body-is-get-method-response | Request body content of the Create method and response content of the Get method should refer to the same resource. readOnly/writeOnly properties will be ignored. http://go/ipa/106 | warn | +| xgen-IPA-106-create-method-request-has-no-readonly-fields | Create method Request object must not include fields with readOnly:true. http://go/ipa/106 | warn | ### IPA-108 diff --git a/tools/spectral/ipa/rulesets/functions/createMethodRequestHasNoReadonlyFields.js b/tools/spectral/ipa/rulesets/functions/createMethodRequestHasNoReadonlyFields.js new file mode 100644 index 0000000000..6bcdfd541e --- /dev/null +++ b/tools/spectral/ipa/rulesets/functions/createMethodRequestHasNoReadonlyFields.js @@ -0,0 +1,40 @@ +import { isCustomMethodIdentifier } from './utils/resourceEvaluation.js'; +import { resolveObject } from './utils/componentUtils.js'; +import { hasException } from './utils/exceptions.js'; +import { collectAdoption, collectAndReturnViolation, collectException } from './utils/collectionUtils.js'; +import { findPropertiesByAttribute } from './utils/schemaUtils.js'; + +const RULE_NAME = 'xgen-IPA-106-create-method-request-has-no-readonly-fields'; +const ERROR_MESSAGE = 'The Create method request object must not include input fields (readOnly properties).'; + +export default (input, _, { path, documentInventory }) => { + const resourcePath = path[1]; + const oas = documentInventory.resolved; + let mediaType = input; + + if (isCustomMethodIdentifier(resourcePath) || !mediaType.endsWith('json')) { + return; + } + + const requestContentPerMediaType = resolveObject(oas, path); + if (!requestContentPerMediaType || !requestContentPerMediaType.schema) { + return; + } + + if (hasException(requestContentPerMediaType, RULE_NAME)) { + collectException(requestContentPerMediaType, RULE_NAME, path); + return; + } + + const errors = checkViolationsAndReturnErrors(requestContentPerMediaType, path); + + if (errors.length !== 0) { + return collectAndReturnViolation(path, RULE_NAME, errors); + } + + collectAdoption(path, RULE_NAME); +}; + +function checkViolationsAndReturnErrors(contentPerMediaType, path) { + return findPropertiesByAttribute(contentPerMediaType.schema, 'readOnly', path, [], ERROR_MESSAGE); +} diff --git a/tools/spectral/ipa/rulesets/functions/getMethodResponseHasNoInputFields.js b/tools/spectral/ipa/rulesets/functions/getMethodResponseHasNoInputFields.js index c8236053ab..5fe7e5a012 100644 --- a/tools/spectral/ipa/rulesets/functions/getMethodResponseHasNoInputFields.js +++ b/tools/spectral/ipa/rulesets/functions/getMethodResponseHasNoInputFields.js @@ -7,6 +7,7 @@ import { isSingletonResource, } from './utils/resourceEvaluation.js'; import { resolveObject } from './utils/componentUtils.js'; +import { findPropertiesByAttribute } from './utils/schemaUtils.js'; const RULE_NAME = 'xgen-IPA-104-get-method-response-has-no-input-fields'; const ERROR_MESSAGE = 'The get method response object must not include output fields (writeOnly properties).'; @@ -43,14 +44,5 @@ export default (input, _, { path, documentInventory }) => { }; function checkViolationsAndReturnErrors(contentPerMediaType, path) { - const schema = contentPerMediaType.schema; - const properties = schema.properties; - if (properties) { - for (const [value] of Object.entries(properties)) { - if (properties[value].writeOnly) { - return [{ path, message: ERROR_MESSAGE }]; - } - } - } - return []; + return findPropertiesByAttribute(contentPerMediaType.schema, 'writeOnly', path, [], ERROR_MESSAGE); } diff --git a/tools/spectral/ipa/rulesets/functions/utils/schemaUtils.js b/tools/spectral/ipa/rulesets/functions/utils/schemaUtils.js index 18114a8481..11648b38f1 100644 --- a/tools/spectral/ipa/rulesets/functions/utils/schemaUtils.js +++ b/tools/spectral/ipa/rulesets/functions/utils/schemaUtils.js @@ -26,3 +26,55 @@ export function getSchemaPathFromEnumPath(path) { } return path.slice(0, enumIndex); } + +/** + * Recursively searches a schema to find properties with a specific attribute + * + * @param {Object} schema - The schema to check + * @param {string} attributeName - The attribute to check for (e.g. 'readOnly', 'writeOnly') + * @param {Array} path - The path to the current schema in the document + * @param {Array} errors - Accumulator for errors found + * @param {string} errorMessage - The base error message to use + * @param {Array} propPath - The current property path (for error messages) + * @returns {Array} The accumulated errors + */ +export function findPropertiesByAttribute(schema, attributeName, path, errors = [], errorMessage, propPath = []) { + if (!schema || typeof schema !== 'object') { + return errors; + } + + // Check if this schema has the attribute set to true + if (schema[attributeName] === true) { + errors.push({ + path, + message: + propPath.length > 0 + ? `${errorMessage} Found ${attributeName} property at: ${propPath.join('.')}.` + : `${errorMessage} Found ${attributeName} property at one of the inline schemas.`, + }); + return errors; + } + + // Check properties in object schemas + if (schema.properties) { + for (const [propName, propSchema] of Object.entries(schema.properties)) { + findPropertiesByAttribute(propSchema, attributeName, path, errors, errorMessage, [...propPath, propName]); + } + } + + // Check items in array schemas + if (schema.items) { + findPropertiesByAttribute(schema.items, attributeName, path, errors, errorMessage, [...propPath, 'items']); + } + + // Check allOf, anyOf, oneOf schemas + ['allOf', 'anyOf', 'oneOf'].forEach((combiner) => { + if (Array.isArray(schema[combiner])) { + schema[combiner].forEach((subSchema, index) => { + findPropertiesByAttribute(subSchema, attributeName, path, errors, errorMessage, [...propPath, combiner, index]); + }); + } + }); + + return errors; +}