From 51da4556f7d9a35ec44a31692c9cb5bd6a014281 Mon Sep 17 00:00:00 2001 From: Andrei Matei Date: Wed, 16 Sep 2026 13:38:13 +0100 Subject: [PATCH] fix(ipa): infer read-only nested schemas --- .../IPA113SingletonHasUpdateMethod.test.js | 94 +++++++++++++++++++ ...32OperationMustBeAReadOnlyResource.test.js | 2 +- .../utils/resourceEvaluation.test.js | 88 +++++++++++++++++ tools/spectral/ipa/rulesets/IPA-106.yaml | 2 +- tools/spectral/ipa/rulesets/IPA-107.yaml | 2 +- tools/spectral/ipa/rulesets/IPA-108.yaml | 2 +- tools/spectral/ipa/rulesets/IPA-113.yaml | 4 +- tools/spectral/ipa/rulesets/IPA-132.yaml | 4 +- tools/spectral/ipa/rulesets/README.md | 14 +-- .../IPA132OperationMustBeAReadOnlyResource.js | 2 +- .../functions/utils/resourceEvaluation.js | 59 +++++++----- 11 files changed, 233 insertions(+), 40 deletions(-) diff --git a/tools/spectral/ipa/__tests__/IPA113SingletonHasUpdateMethod.test.js b/tools/spectral/ipa/__tests__/IPA113SingletonHasUpdateMethod.test.js index 703e702043..d4b2e33050 100644 --- a/tools/spectral/ipa/__tests__/IPA113SingletonHasUpdateMethod.test.js +++ b/tools/spectral/ipa/__tests__/IPA113SingletonHasUpdateMethod.test.js @@ -94,6 +94,100 @@ testRule('xgen-IPA-113-singleton-should-have-update-method', [ }, errors: [], }, + { + name: 'read-only singleton with referenced nested schema does not require update method', + document: { + paths: { + '/resource/{exampleId}/readOnlySingleton': { + get: { + responses: { + 200: { + content: { + 'application/json': { + schema: { + $ref: '#/components/schemas/ReadOnlySingleton', + }, + }, + }, + }, + }, + }, + }, + }, + components: { + schemas: { + ReadOnlySingleton: { + type: 'object', + properties: { + status: { type: 'string', readOnly: true }, + metadata: { + $ref: '#/components/schemas/ReadOnlyMetadata', + }, + }, + }, + ReadOnlyMetadata: { + type: 'object', + properties: { + createdAt: { type: 'string', readOnly: true }, + updatedAt: { type: 'string', readOnly: true }, + }, + }, + }, + }, + }, + errors: [], + }, + { + name: 'singleton with writable property in referenced nested schema requires update method', + document: { + paths: { + '/resource/{exampleId}/writableSingleton': { + get: { + responses: { + 200: { + content: { + 'application/json': { + schema: { + $ref: '#/components/schemas/WritableSingleton', + }, + }, + }, + }, + }, + }, + }, + }, + components: { + schemas: { + WritableSingleton: { + type: 'object', + properties: { + status: { type: 'string', readOnly: true }, + metadata: { + $ref: '#/components/schemas/WritableMetadata', + }, + }, + }, + WritableMetadata: { + type: 'object', + properties: { + createdAt: { type: 'string', readOnly: true }, + displayName: { type: 'string' }, + }, + }, + }, + }, + }, + errors: [ + { + code: 'xgen-IPA-113-singleton-should-have-update-method', + message: + 'Singleton resources should define the Update method. If this is not a singleton resource, please implement all CRUDL methods.', + path: ['paths', '/resource/{exampleId}/writableSingleton'], + severity: DiagnosticSeverity.Error, + }, + ], + }, { name: 'read-only singleton with List response', document: { diff --git a/tools/spectral/ipa/__tests__/IPA132OperationMustBeAReadOnlyResource.test.js b/tools/spectral/ipa/__tests__/IPA132OperationMustBeAReadOnlyResource.test.js index ce8bfbeaf1..2ef2278b94 100644 --- a/tools/spectral/ipa/__tests__/IPA132OperationMustBeAReadOnlyResource.test.js +++ b/tools/spectral/ipa/__tests__/IPA132OperationMustBeAReadOnlyResource.test.js @@ -2,7 +2,7 @@ import testRule from './__helpers__/testRule'; import { DiagnosticSeverity } from '@stoplight/types'; const READ_ONLY_SCHEMA_ERROR_MESSAGE = - 'The Operation resource must be read-only. All properties of the GET response schema must be marked as readOnly: true.'; + 'The Operation resource must be read-only. All properties of the GET response schema must be marked as readOnly: true or contain only read-only properties.'; const readOnlyGet = { responses: { diff --git a/tools/spectral/ipa/__tests__/utils/resourceEvaluation.test.js b/tools/spectral/ipa/__tests__/utils/resourceEvaluation.test.js index f92a809fb7..d9b04ed112 100644 --- a/tools/spectral/ipa/__tests__/utils/resourceEvaluation.test.js +++ b/tools/spectral/ipa/__tests__/utils/resourceEvaluation.test.js @@ -445,6 +445,82 @@ describe('tools/spectral/ipa/rulesets/functions/utils/resourceEvaluation.js', () }, expected: true, }, + { + description: 'schema with unmarked nested object containing only readOnly properties', + schema: { + type: 'object', + properties: { + metadata: { + type: 'object', + properties: { + createdBy: { type: 'string', readOnly: true }, + updatedBy: { type: 'string', readOnly: true }, + }, + }, + }, + }, + expected: true, + }, + { + description: 'schema with nested object containing a writable property', + schema: { + type: 'object', + properties: { + metadata: { + type: 'object', + properties: { + createdBy: { type: 'string', readOnly: true }, + displayName: { type: 'string' }, + }, + }, + }, + }, + expected: false, + }, + { + description: 'schema with nested array containing only readOnly properties', + schema: { + type: 'object', + properties: { + entries: { + type: 'array', + items: { + type: 'object', + properties: { + id: { type: 'string', readOnly: true }, + status: { type: 'string', readOnly: true }, + }, + }, + }, + }, + }, + expected: true, + }, + { + description: 'schema with nested allOf containing only readOnly properties', + schema: { + type: 'object', + properties: { + metadata: { + allOf: [ + { + type: 'object', + properties: { + createdBy: { type: 'string', readOnly: true }, + }, + }, + { + type: 'object', + properties: { + updatedBy: { type: 'string', readOnly: true }, + }, + }, + ], + }, + }, + }, + expected: true, + }, { description: 'schema with array items all readOnly', schema: { @@ -538,6 +614,18 @@ describe('tools/spectral/ipa/rulesets/functions/utils/resourceEvaluation.js', () expect(allPropertiesAreReadOnly(testCase.schema)).toEqual(testCase.expected); }); }); + + it('returns false for an unmarked circular schema', () => { + const schema = { + type: 'object', + properties: { + id: { type: 'string', readOnly: true }, + }, + }; + schema.properties.parent = schema; + + expect(allPropertiesAreReadOnly(schema)).toEqual(false); + }); }); describe('isReadOnlyResource', () => { diff --git a/tools/spectral/ipa/rulesets/IPA-106.yaml b/tools/spectral/ipa/rulesets/IPA-106.yaml index f3defb6fc2..aa8ca9581a 100644 --- a/tools/spectral/ipa/rulesets/IPA-106.yaml +++ b/tools/spectral/ipa/rulesets/IPA-106.yaml @@ -122,7 +122,7 @@ rules: ##### Implementation details Rule checks for the following conditions: - Applies to POST methods on resource collection paths - - Checks if the resource is a read-only resource (all properties in GET response have readOnly:true) + - Checks if the resource is a read-only resource (all properties in the GET response are marked as readOnly or contain only read-only properties) - If a resource does not have a standard GET method, it is not considered read-only (cannot determine the resource schema) - Fails if a Create method is defined on a read-only resource - Operation objects with `x-xgen-IPA-exception` for this rule are excluded from validation diff --git a/tools/spectral/ipa/rulesets/IPA-107.yaml b/tools/spectral/ipa/rulesets/IPA-107.yaml index 23f39556b2..104cad848a 100644 --- a/tools/spectral/ipa/rulesets/IPA-107.yaml +++ b/tools/spectral/ipa/rulesets/IPA-107.yaml @@ -122,7 +122,7 @@ rules: ##### Implementation details Rule checks for the following conditions: - Applies to PUT/PATCH methods on all resource paths - - Checks if the resource is a read-only resource (all properties in GET response have readOnly:true) + - Checks if the resource is a read-only resource (all properties in the GET response are marked as readOnly or contain only read-only properties) - If a resource does not have a standard GET method, it is not considered read-only (cannot determine the resource schema) - Fails if an Update method is defined on a read-only resource - Operation objects with `x-xgen-IPA-exception` for this rule are excluded from validation diff --git a/tools/spectral/ipa/rulesets/IPA-108.yaml b/tools/spectral/ipa/rulesets/IPA-108.yaml index 66c677e6e6..3d23c00a1e 100644 --- a/tools/spectral/ipa/rulesets/IPA-108.yaml +++ b/tools/spectral/ipa/rulesets/IPA-108.yaml @@ -62,7 +62,7 @@ rules: ##### Implementation details Rule checks for the following conditions: - Applies to DELETE methods on single resource paths and singleton resources - - Checks if the resource is a read-only resource (all properties in GET response have readOnly:true) + - Checks if the resource is a read-only resource (all properties in the GET response are marked as readOnly or contain only read-only properties) - If a resource does not have a standard GET method, it is not considered read-only (cannot determine the resource schema) - Fails if a Delete method is defined on a read-only resource - Operation objects with `x-xgen-IPA-exception` for this rule are excluded from validation diff --git a/tools/spectral/ipa/rulesets/IPA-113.yaml b/tools/spectral/ipa/rulesets/IPA-113.yaml index eaa966aaa7..a100d5fb2b 100644 --- a/tools/spectral/ipa/rulesets/IPA-113.yaml +++ b/tools/spectral/ipa/rulesets/IPA-113.yaml @@ -50,7 +50,7 @@ rules: ##### Implementation details Rule checks for the following conditions: - Applies only to singleton resources - - Excludes read-only singleton resources (where all properties in the GET response schema are marked as readOnly; for List responses, all properties in the items schema must be readOnly) + - Excludes read-only singleton resources (where all properties in the GET response schema are marked as readOnly or contain only read-only properties; for List responses, all properties in the items schema must be readOnly) - Checks that the resource has the PUT and/or PATCH methods defined message: '{{error}} https://mdb.link/mongodb-atlas-openapi-validation#xgen-IPA-113-singleton-should-have-update-method' severity: error @@ -142,7 +142,7 @@ rules: - Applies only to paths ending with :reset - Verifies that the parent singleton resource is not read-only - Uses existing isReadOnlyResource() helper function - - Fails if the singleton resource has all properties marked as readOnly: true + - Fails if all properties of the singleton resource are marked as readOnly or contain only read-only properties message: '{{error}} https://mdb.link/mongodb-atlas-openapi-validation#xgen-IPA-113-reset-method-not-on-readonly-singleton' severity: error given: '$.paths[*]' diff --git a/tools/spectral/ipa/rulesets/IPA-132.yaml b/tools/spectral/ipa/rulesets/IPA-132.yaml index 9261e1f93e..c7deaf3e90 100644 --- a/tools/spectral/ipa/rulesets/IPA-132.yaml +++ b/tools/spectral/ipa/rulesets/IPA-132.yaml @@ -52,7 +52,7 @@ rules: xgen-IPA-132-operation-must-be-a-read-only-resource: description: | Operations endpoints are read-only. They may only define the get method, and all properties - of the Operation resource must be readOnly. + of the Operation resource must be read-only. ##### Implementation details Rule checks for the following conditions: @@ -62,7 +62,7 @@ rules: - The path item must not define any HTTP method other than `get` - On the single Operation endpoint (`.../operations/{operationId}`), where the Get method is defined, all properties of every 2xx response schema of the `get` method must be - marked as `readOnly: true` + marked as `readOnly: true` or contain only read-only properties - Paths with `x-xgen-IPA-exception` for this rule are excluded from validation message: '{{error}} https://mdb.link/mongodb-atlas-openapi-validation#xgen-IPA-132-operation-must-be-a-read-only-resource' diff --git a/tools/spectral/ipa/rulesets/README.md b/tools/spectral/ipa/rulesets/README.md index 48aeb59ba0..c1aff88f3f 100644 --- a/tools/spectral/ipa/rulesets/README.md +++ b/tools/spectral/ipa/rulesets/README.md @@ -365,7 +365,7 @@ Read-only resources must not define the Create method. ##### Implementation details Rule checks for the following conditions: - Applies to POST methods on resource collection paths - - Checks if the resource is a read-only resource (all properties in GET response have readOnly:true) + - Checks if the resource is a read-only resource (all properties in the GET response are marked as readOnly or contain only read-only properties) - If a resource does not have a standard GET method, it is not considered read-only (cannot determine the resource schema) - Fails if a Create method is defined on a read-only resource - Operation objects with `x-xgen-IPA-exception` for this rule are excluded from validation @@ -484,7 +484,7 @@ Read-only resources must not define the Update method. ##### Implementation details Rule checks for the following conditions: - Applies to PUT/PATCH methods on all resource paths - - Checks if the resource is a read-only resource (all properties in GET response have readOnly:true) + - Checks if the resource is a read-only resource (all properties in the GET response are marked as readOnly or contain only read-only properties) - If a resource does not have a standard GET method, it is not considered read-only (cannot determine the resource schema) - Fails if an Update method is defined on a read-only resource - Operation objects with `x-xgen-IPA-exception` for this rule are excluded from validation @@ -577,7 +577,7 @@ Read-only resources must not define the Delete method. ##### Implementation details Rule checks for the following conditions: - Applies to DELETE methods on single resource paths and singleton resources - - Checks if the resource is a read-only resource (all properties in GET response have readOnly:true) + - Checks if the resource is a read-only resource (all properties in the GET response are marked as readOnly or contain only read-only properties) - If a resource does not have a standard GET method, it is not considered read-only (cannot determine the resource schema) - Fails if a Delete method is defined on a read-only resource - Operation objects with `x-xgen-IPA-exception` for this rule are excluded from validation @@ -891,7 +891,7 @@ Singleton resources should define the Update method. Validation for the presence ##### Implementation details Rule checks for the following conditions: - Applies only to singleton resources - - Excludes read-only singleton resources (where all properties in the GET response schema are marked as readOnly; for List responses, all properties in the items schema must be readOnly) + - Excludes read-only singleton resources (where all properties in the GET response schema are marked as readOnly or contain only read-only properties; for List responses, all properties in the items schema must be readOnly) - Checks that the resource has the PUT and/or PATCH methods defined #### xgen-IPA-113-reset-method-must-use-POST @@ -964,7 +964,7 @@ Rule checks for the following conditions: - Applies only to paths ending with :reset - Verifies that the parent singleton resource is not read-only - Uses existing isReadOnlyResource() helper function - - Fails if the singleton resource has all properties marked as readOnly: true + - Fails if all properties of the singleton resource are marked as readOnly or contain only read-only properties #### xgen-IPA-113-reset-method-valid-operation-id @@ -1469,7 +1469,7 @@ Rule checks for the following conditions: ![warn](https://img.shields.io/badge/warning-yellow) Operations endpoints are read-only. They may only define the get method, and all properties -of the Operation resource must be readOnly. +of the Operation resource must be read-only. ##### Implementation details Rule checks for the following conditions: @@ -1479,7 +1479,7 @@ Rule checks for the following conditions: - The path item must not define any HTTP method other than `get` - On the single Operation endpoint (`.../operations/{operationId}`), where the Get method is defined, all properties of every 2xx response schema of the `get` method must be - marked as `readOnly: true` + marked as `readOnly: true` or contain only read-only properties - Paths with `x-xgen-IPA-exception` for this rule are excluded from validation #### xgen-IPA-132-operations-endpoint-must-be-a-leaf-resource diff --git a/tools/spectral/ipa/rulesets/functions/IPA132OperationMustBeAReadOnlyResource.js b/tools/spectral/ipa/rulesets/functions/IPA132OperationMustBeAReadOnlyResource.js index 5a7d789a30..fa64d8d093 100644 --- a/tools/spectral/ipa/rulesets/functions/IPA132OperationMustBeAReadOnlyResource.js +++ b/tools/spectral/ipa/rulesets/functions/IPA132OperationMustBeAReadOnlyResource.js @@ -5,7 +5,7 @@ import { isOperationsPath, isSingleOperationPath } from './utils/longRunningOper const VALID_METHOD = 'get'; const HTTP_METHODS = ['get', 'put', 'post', 'delete', 'options', 'head', 'patch', 'trace']; const READ_ONLY_SCHEMA_ERROR_MESSAGE = - 'The Operation resource must be read-only. All properties of the GET response schema must be marked as readOnly: true.'; + 'The Operation resource must be read-only. All properties of the GET response schema must be marked as readOnly: true or contain only read-only properties.'; /** * Checks that an Operations endpoint defined by IPA-132 is a read-only resource: its path items diff --git a/tools/spectral/ipa/rulesets/functions/utils/resourceEvaluation.js b/tools/spectral/ipa/rulesets/functions/utils/resourceEvaluation.js index 607c76bf91..8bf56f08d2 100644 --- a/tools/spectral/ipa/rulesets/functions/utils/resourceEvaluation.js +++ b/tools/spectral/ipa/rulesets/functions/utils/resourceEvaluation.js @@ -242,51 +242,62 @@ export function removePrefix(path) { } /** - * Checks if all properties in a schema have readOnly: true. + * Checks if all properties in a schema are read-only. * * @param {Object} schema - The schema to check + * @param {Set} visiting - Schemas in the current traversal path * @returns {boolean} true if all properties are readOnly, false otherwise */ -export function allPropertiesAreReadOnly(schema) { +export function allPropertiesAreReadOnly(schema, visiting = new Set()) { if (!schema || typeof schema !== 'object') { return false; } - if (schema.properties) { - if (schema.properties.results && schema.properties.results.type === 'array' && schema.properties.results.items) { - return allPropertiesAreReadOnly(schema.properties.results.items); - } + if (visiting.has(schema)) { + return false; + } + visiting.add(schema); - for (const [, propSchema] of Object.entries(schema.properties)) { - if (propSchema.readOnly !== true) { - return false; + try { + if (schema.properties) { + if (schema.properties.results && schema.properties.results.type === 'array' && schema.properties.results.items) { + return isSchemaReadOnly(schema.properties.results.items, visiting); } + + const properties = Object.values(schema.properties); + return properties.length > 0 && properties.every((property) => isSchemaReadOnly(property, visiting)); } - return Object.keys(schema.properties).length > 0; - } - if (schema.items) { - return allPropertiesAreReadOnly(schema.items); - } + if (schema.items) { + return isSchemaReadOnly(schema.items, visiting); + } - if (Array.isArray(schema.allOf)) { - return schema.allOf.every((subSchema) => allPropertiesAreReadOnly(subSchema)); - } + if (Array.isArray(schema.allOf)) { + return schema.allOf.every((subSchema) => isSchemaReadOnly(subSchema, visiting)); + } - if (Array.isArray(schema.anyOf)) { - return schema.anyOf.some((subSchema) => allPropertiesAreReadOnly(subSchema)); - } + if (Array.isArray(schema.anyOf)) { + return schema.anyOf.some((subSchema) => isSchemaReadOnly(subSchema, visiting)); + } - if (Array.isArray(schema.oneOf)) { - return schema.oneOf.some((subSchema) => allPropertiesAreReadOnly(subSchema)); + if (Array.isArray(schema.oneOf)) { + return schema.oneOf.some((subSchema) => isSchemaReadOnly(subSchema, visiting)); + } + + return false; + } finally { + visiting.delete(schema); } +} - return false; +function isSchemaReadOnly(schema, visiting) { + return schema?.readOnly === true || allPropertiesAreReadOnly(schema, visiting); } /** * Checks if a resource is a read-only resource. - * A read-only resource has all properties in its GET response schema marked as readOnly: true. + * A read-only resource has all properties in its GET response schema marked as readOnly: true + * or composed only of read-only properties. * * @param {Object} resourcePathItems - All path items for the resource to be evaluated * @returns {boolean} true if the resource is read-only, false otherwise