From 51da4556f7d9a35ec44a31692c9cb5bd6a014281 Mon Sep 17 00:00:00 2001 From: Andrei Matei Date: Wed, 16 Sep 2026 13:38:13 +0100 Subject: [PATCH 1/6] 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 From 637b239073d557f6f8769e6f724d3bcafd7f4a0d Mon Sep 17 00:00:00 2001 From: Andrei Matei Date: Wed, 23 Sep 2026 14:43:55 +0100 Subject: [PATCH 2/6] fix(ipa): check all read-only schema composition branches --- .../IPA113SingletonHasUpdateMethod.test.js | 63 ++++++++++++++++ .../utils/resourceEvaluation.test.js | 72 +++++++++++++++++++ .../functions/utils/resourceEvaluation.js | 27 +++---- 3 files changed, 149 insertions(+), 13 deletions(-) diff --git a/tools/spectral/ipa/__tests__/IPA113SingletonHasUpdateMethod.test.js b/tools/spectral/ipa/__tests__/IPA113SingletonHasUpdateMethod.test.js index d4b2e33050..f526aaa620 100644 --- a/tools/spectral/ipa/__tests__/IPA113SingletonHasUpdateMethod.test.js +++ b/tools/spectral/ipa/__tests__/IPA113SingletonHasUpdateMethod.test.js @@ -188,6 +188,69 @@ testRule('xgen-IPA-113-singleton-should-have-update-method', [ }, ], }, + ...['allOf', 'anyOf', 'oneOf'].flatMap((composition) => + [ + { description: 'read-only branches', withProperties: false, writable: false }, + { description: 'a writable branch', withProperties: false, writable: true }, + { description: 'read-only branches alongside properties', withProperties: true, writable: false }, + { description: 'a writable branch alongside read-only properties', withProperties: true, writable: true }, + ].map(({ description, withProperties, writable }) => ({ + name: `singleton with referenced nested ${composition} containing ${description}`, + document: { + paths: { + '/resource/{exampleId}/singleton': { + get: { + responses: { + 200: { + content: { + 'application/json': { + schema: { $ref: '#/components/schemas/Singleton' }, + }, + }, + }, + }, + }, + }, + }, + components: { + schemas: { + Singleton: { + type: 'object', + properties: { + metadata: { $ref: '#/components/schemas/Metadata' }, + }, + }, + Metadata: { + ...(withProperties ? { properties: { id: { type: 'string', readOnly: true } } } : {}), + [composition]: [ + { $ref: '#/components/schemas/ReadOnlyMetadataPart' }, + { $ref: '#/components/schemas/OtherMetadataPart' }, + ], + }, + ReadOnlyMetadataPart: { + type: 'object', + properties: { createdAt: { type: 'string', readOnly: true } }, + }, + OtherMetadataPart: { + type: 'object', + properties: { displayName: { type: 'string', readOnly: !writable } }, + }, + }, + }, + }, + errors: writable + ? [ + { + 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}/singleton'], + severity: DiagnosticSeverity.Error, + }, + ] + : [], + })) + ), { name: 'read-only singleton with List response', document: { diff --git a/tools/spectral/ipa/__tests__/utils/resourceEvaluation.test.js b/tools/spectral/ipa/__tests__/utils/resourceEvaluation.test.js index d9b04ed112..d20915e212 100644 --- a/tools/spectral/ipa/__tests__/utils/resourceEvaluation.test.js +++ b/tools/spectral/ipa/__tests__/utils/resourceEvaluation.test.js @@ -615,6 +615,78 @@ describe('tools/spectral/ipa/rulesets/functions/utils/resourceEvaluation.js', () }); }); + describe.each(['allOf', 'anyOf', 'oneOf'])('%s compositions', (composition) => { + const readOnlySchema = { + type: 'object', + properties: { id: { type: 'string', readOnly: true } }, + }; + const writableSchema = { + type: 'object', + properties: { displayName: { type: 'string' } }, + }; + + it.each([ + { + description: 'all branches are read-only, including a shared schema', + schema: { [composition]: [readOnlySchema, readOnlySchema] }, + expected: true, + }, + { + description: 'a writable branch follows a read-only branch', + schema: { [composition]: [readOnlySchema, writableSchema] }, + expected: false, + }, + { + description: 'a writable branch precedes a read-only branch', + schema: { [composition]: [writableSchema, readOnlySchema] }, + expected: false, + }, + { + description: 'read-only properties coexist with read-only branches', + schema: { ...readOnlySchema, [composition]: [readOnlySchema] }, + expected: true, + }, + { + description: 'read-only properties coexist with a writable branch', + schema: { ...readOnlySchema, [composition]: [writableSchema] }, + expected: false, + }, + { + description: 'writable properties coexist with a read-only branch', + schema: { ...writableSchema, [composition]: [readOnlySchema] }, + expected: false, + }, + { + description: 'read-only array items coexist with a writable branch', + schema: { type: 'array', items: readOnlySchema, [composition]: [{ type: 'array', items: writableSchema }] }, + expected: false, + }, + { + description: 'the composition is empty', + schema: { [composition]: [] }, + expected: false, + }, + ])('returns $expected when $description', ({ schema, expected }) => { + expect(allPropertiesAreReadOnly({ type: 'object', properties: { metadata: schema } })).toEqual(expected); + }); + + it('returns false for a circular branch after a read-only branch', () => { + const schema = { [composition]: [readOnlySchema] }; + schema[composition].push(schema); + + expect(allPropertiesAreReadOnly(schema)).toEqual(false); + }); + }); + + it('checks sibling composition keywords even when allOf is read-only', () => { + const schema = { + allOf: [{ type: 'object', properties: { id: { type: 'string', readOnly: true } } }], + anyOf: [{ type: 'object', properties: { displayName: { type: 'string' } } }], + }; + + expect(allPropertiesAreReadOnly(schema)).toEqual(false); + }); + it('returns false for an unmarked circular schema', () => { const schema = { type: 'object', diff --git a/tools/spectral/ipa/rulesets/functions/utils/resourceEvaluation.js b/tools/spectral/ipa/rulesets/functions/utils/resourceEvaluation.js index 8bf56f08d2..ad8fbebe2f 100644 --- a/tools/spectral/ipa/rulesets/functions/utils/resourceEvaluation.js +++ b/tools/spectral/ipa/rulesets/functions/utils/resourceEvaluation.js @@ -259,6 +259,19 @@ export function allPropertiesAreReadOnly(schema, visiting = new Set()) { visiting.add(schema); try { + // Compositions can coexist with properties or items; none may hide writable fields in the others. + let hasReadOnlyComposition = false; + for (const composition of ['allOf', 'anyOf', 'oneOf']) { + const branches = schema[composition]; + if (!Array.isArray(branches)) { + continue; + } + if (branches.length === 0 || !branches.every((subSchema) => isSchemaReadOnly(subSchema, visiting))) { + return false; + } + hasReadOnlyComposition = true; + } + if (schema.properties) { if (schema.properties.results && schema.properties.results.type === 'array' && schema.properties.results.items) { return isSchemaReadOnly(schema.properties.results.items, visiting); @@ -272,19 +285,7 @@ export function allPropertiesAreReadOnly(schema, visiting = new Set()) { return isSchemaReadOnly(schema.items, visiting); } - if (Array.isArray(schema.allOf)) { - return schema.allOf.every((subSchema) => isSchemaReadOnly(subSchema, visiting)); - } - - if (Array.isArray(schema.anyOf)) { - return schema.anyOf.some((subSchema) => isSchemaReadOnly(subSchema, visiting)); - } - - if (Array.isArray(schema.oneOf)) { - return schema.oneOf.some((subSchema) => isSchemaReadOnly(subSchema, visiting)); - } - - return false; + return hasReadOnlyComposition; } finally { visiting.delete(schema); } From 428806fb68e5e1ba39d13883bc764e43108dc3fd Mon Sep 17 00:00:00 2001 From: Andrei Matei Date: Thu, 24 Sep 2026 12:16:45 +0100 Subject: [PATCH 3/6] fix(ipa): keep read-only schema evaluation generic --- .../ipa/__tests__/utils/resourceEvaluation.test.js | 11 +++++------ .../rulesets/functions/utils/resourceEvaluation.js | 4 ---- 2 files changed, 5 insertions(+), 10 deletions(-) diff --git a/tools/spectral/ipa/__tests__/utils/resourceEvaluation.test.js b/tools/spectral/ipa/__tests__/utils/resourceEvaluation.test.js index d20915e212..17503738f1 100644 --- a/tools/spectral/ipa/__tests__/utils/resourceEvaluation.test.js +++ b/tools/spectral/ipa/__tests__/utils/resourceEvaluation.test.js @@ -566,28 +566,27 @@ describe('tools/spectral/ipa/rulesets/functions/utils/resourceEvaluation.js', () expected: false, }, { - description: 'singleton List response with all readOnly items', + description: 'schema with readOnly array property containing writable item schema', schema: { type: 'object', properties: { - results: { + auditEvents: { type: 'array', + readOnly: true, items: { type: 'object', properties: { id: { type: 'string', readOnly: true }, - name: { type: 'string', readOnly: true }, - status: { type: 'string', readOnly: true }, + message: { type: 'string' }, }, }, }, - totalCount: { type: 'integer' }, }, }, expected: true, }, { - description: 'singleton List response with some non-readOnly items', + description: 'list response shape with writable metadata is not treated specially', schema: { type: 'object', properties: { diff --git a/tools/spectral/ipa/rulesets/functions/utils/resourceEvaluation.js b/tools/spectral/ipa/rulesets/functions/utils/resourceEvaluation.js index ad8fbebe2f..57db4d2cf2 100644 --- a/tools/spectral/ipa/rulesets/functions/utils/resourceEvaluation.js +++ b/tools/spectral/ipa/rulesets/functions/utils/resourceEvaluation.js @@ -273,10 +273,6 @@ export function allPropertiesAreReadOnly(schema, visiting = new Set()) { } 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)); } From a52373e95901a9d4e3c8032636c32b89ea8036cd Mon Sep 17 00:00:00 2001 From: Andrei Matei Date: Thu, 24 Sep 2026 14:36:27 +0100 Subject: [PATCH 4/6] fix(ipa): ignore list responses for read-only resources --- ...3ResetMethodNotOnReadonlySingleton.test.js | 41 +++++++++++++++++++ .../IPA113SingletonHasUpdateMethod.test.js | 16 ++++++-- .../utils/resourceEvaluation.test.js | 27 ++++++++++++ .../functions/utils/resourceEvaluation.js | 16 ++++++++ 4 files changed, 97 insertions(+), 3 deletions(-) diff --git a/tools/spectral/ipa/__tests__/IPA113ResetMethodNotOnReadonlySingleton.test.js b/tools/spectral/ipa/__tests__/IPA113ResetMethodNotOnReadonlySingleton.test.js index 49a40c6c32..e0414069c9 100644 --- a/tools/spectral/ipa/__tests__/IPA113ResetMethodNotOnReadonlySingleton.test.js +++ b/tools/spectral/ipa/__tests__/IPA113ResetMethodNotOnReadonlySingleton.test.js @@ -32,6 +32,47 @@ testRule('xgen-IPA-113-reset-method-not-on-readonly-singleton', [ }, errors: [], }, + { + name: 'valid reset when base path returns list response shape', + document: { + paths: { + '/resource/{exampleId}/listSingleton': { + get: { + responses: { + 200: { + content: { + 'application/json': { + schema: { + type: 'object', + properties: { + results: { + type: 'array', + readOnly: true, + items: { + type: 'object', + properties: { + id: { type: 'string', readOnly: true }, + status: { type: 'string', readOnly: true }, + }, + }, + }, + totalCount: { type: 'integer', readOnly: true }, + }, + required: ['results'], + }, + }, + }, + }, + }, + }, + }, + '/resource/{exampleId}/listSingleton:reset': { + post: {}, + }, + }, + }, + errors: [], + }, { name: 'invalid reset on read-only singleton', document: { diff --git a/tools/spectral/ipa/__tests__/IPA113SingletonHasUpdateMethod.test.js b/tools/spectral/ipa/__tests__/IPA113SingletonHasUpdateMethod.test.js index f526aaa620..9ce598006f 100644 --- a/tools/spectral/ipa/__tests__/IPA113SingletonHasUpdateMethod.test.js +++ b/tools/spectral/ipa/__tests__/IPA113SingletonHasUpdateMethod.test.js @@ -252,7 +252,7 @@ testRule('xgen-IPA-113-singleton-should-have-update-method', [ })) ), { - name: 'read-only singleton with List response', + name: 'list response shape does not prove singleton is read-only', document: { paths: { '/resource/{exampleId}/readOnlyListSingleton': { @@ -266,6 +266,7 @@ testRule('xgen-IPA-113-singleton-should-have-update-method', [ properties: { results: { type: 'array', + readOnly: true, items: { type: 'object', properties: { @@ -275,8 +276,9 @@ testRule('xgen-IPA-113-singleton-should-have-update-method', [ }, }, }, - totalCount: { type: 'integer' }, + totalCount: { type: 'integer', readOnly: true }, }, + required: ['results'], }, }, }, @@ -286,7 +288,15 @@ testRule('xgen-IPA-113-singleton-should-have-update-method', [ }, }, }, - errors: [], + 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}/readOnlyListSingleton'], + severity: DiagnosticSeverity.Error, + }, + ], }, { name: 'writable singleton with List response', diff --git a/tools/spectral/ipa/__tests__/utils/resourceEvaluation.test.js b/tools/spectral/ipa/__tests__/utils/resourceEvaluation.test.js index 17503738f1..ba86bae144 100644 --- a/tools/spectral/ipa/__tests__/utils/resourceEvaluation.test.js +++ b/tools/spectral/ipa/__tests__/utils/resourceEvaluation.test.js @@ -726,6 +726,33 @@ describe('tools/spectral/ipa/rulesets/functions/utils/resourceEvaluation.js', () resourcePathItems: readOnlySingleton, expected: true, }, + { + description: 'singleton with list response shape', + resourcePathItems: { + '/resource/{id}/listSingleton': { + get: { + responses: { + 200: { + content: { + 'application/json': { + schema: { + type: 'object', + properties: { + links: { type: 'array', readOnly: true, items: { type: 'object' } }, + results: { type: 'array', readOnly: true, items: { type: 'object' } }, + totalCount: { type: 'integer', readOnly: true }, + }, + required: ['results'], + }, + }, + }, + }, + }, + }, + }, + }, + expected: false, + }, { description: 'resource with xgen-IPA-104-resource-has-GET exception', resourcePathItems: { diff --git a/tools/spectral/ipa/rulesets/functions/utils/resourceEvaluation.js b/tools/spectral/ipa/rulesets/functions/utils/resourceEvaluation.js index 57db4d2cf2..0024fe6486 100644 --- a/tools/spectral/ipa/rulesets/functions/utils/resourceEvaluation.js +++ b/tools/spectral/ipa/rulesets/functions/utils/resourceEvaluation.js @@ -359,6 +359,12 @@ export function isReadOnlyResource(resourcePathItems) { continue; } + // List responses are containers around resources, not resource schemas themselves. + // They cannot prove that a singleton or resource is read-only. + if (isListResponseSchema(mediaTypeObj.schema)) { + return false; + } + if (!allPropertiesAreReadOnly(mediaTypeObj.schema)) { return false; } @@ -366,3 +372,13 @@ export function isReadOnlyResource(resourcePathItems) { return true; } + +function isListResponseSchema(schema) { + const properties = schema?.properties; + const results = properties?.results; + return ( + results?.type === 'array' && + Boolean(results.items) && + Boolean(properties.links || properties.totalCount || schema.required?.includes('results')) + ); +} From 538691f2ce1b92f040acbca167fee38429989188 Mon Sep 17 00:00:00 2001 From: Andrei Matei Date: Mon, 28 Sep 2026 13:11:47 +0100 Subject: [PATCH 5/6] fix(ipa): handle read-only dictionaries and composed schemas --- .../__tests__/ReadOnlySchemaTraversal.test.js | 95 +++++++ .../__helpers__/readOnlySchemaCases.js | 256 ++++++++++++++++++ .../utils/resourceEvaluation.test.js | 58 ++++ tools/spectral/ipa/rulesets/IPA-113.yaml | 9 +- tools/spectral/ipa/rulesets/README.md | 9 +- .../functions/utils/resourceEvaluation.js | 112 ++++++-- 6 files changed, 514 insertions(+), 25 deletions(-) create mode 100644 tools/spectral/ipa/__tests__/ReadOnlySchemaTraversal.test.js create mode 100644 tools/spectral/ipa/__tests__/__helpers__/readOnlySchemaCases.js diff --git a/tools/spectral/ipa/__tests__/ReadOnlySchemaTraversal.test.js b/tools/spectral/ipa/__tests__/ReadOnlySchemaTraversal.test.js new file mode 100644 index 0000000000..470fb8656a --- /dev/null +++ b/tools/spectral/ipa/__tests__/ReadOnlySchemaTraversal.test.js @@ -0,0 +1,95 @@ +import { DiagnosticSeverity } from '@stoplight/types'; +import testRule from './__helpers__/testRule'; +import { documentForResponseSchema, listResponseCases, readOnlySchemaCases } from './__helpers__/readOnlySchemaCases'; + +const singletonPath = '/resource/{exampleId}/singleton'; +const resourceCases = [ + ...readOnlySchemaCases, + ...listResponseCases, + ...['allOf', 'anyOf', 'oneOf'].map((composition) => ({ + description: `a referenced list response wrapped in ${composition}`, + schema: { [composition]: [{ $ref: '#/components/schemas/ListResponse' }] }, + expected: false, + })), +]; + +const singletonRule = 'xgen-IPA-113-singleton-should-have-update-method'; +testRule( + singletonRule, + resourceCases.map(({ description, schema, expected }) => ({ + name: `singleton with ${description}`, + document: documentForResponseSchema(schema, singletonPath), + errors: expected + ? [] + : [ + { + code: singletonRule, + message: 'Singleton resources should define the Update method.', + path: ['paths', singletonPath], + severity: DiagnosticSeverity.Error, + }, + ], + })) +); + +const updateRule = 'xgen-IPA-107-readonly-resource-should-not-have-update-method'; +testRule( + updateRule, + resourceCases.map(({ description, schema, expected }) => ({ + name: `PATCH on a singleton with ${description}`, + document: documentForResponseSchema(schema, singletonPath, { patch: {} }), + errors: expected + ? [ + { + code: updateRule, + message: 'Read-only resources must not define the Update method.', + path: ['paths', singletonPath, 'patch'], + severity: DiagnosticSeverity.Error, + }, + ] + : [], + })) +); + +const resetRule = 'xgen-IPA-113-reset-method-not-on-readonly-singleton'; +testRule( + resetRule, + resourceCases.map(({ description, schema, expected }) => { + const document = documentForResponseSchema(schema, singletonPath); + document.paths[`${singletonPath}:reset`] = { post: {} }; + return { + name: `reset on a singleton with ${description}`, + document, + errors: expected + ? [ + { + code: resetRule, + message: 'Read-only singleton resources must not define a :reset custom method.', + path: ['paths', `${singletonPath}:reset`], + severity: DiagnosticSeverity.Error, + }, + ] + : [], + }; + }) +); + +const operationRule = 'xgen-IPA-132-operation-must-be-a-read-only-resource'; +const operationPath = '/resource/{exampleId}/operations/{operationId}'; +testRule( + operationRule, + readOnlySchemaCases.map(({ description, schema, expected }) => ({ + name: `Operation response with ${description}`, + document: documentForResponseSchema(schema, operationPath), + errors: expected + ? [] + : [ + { + code: operationRule, + message: 'The Operation resource must be read-only.', + path: ['paths', operationPath, 'get'], + severity: DiagnosticSeverity.Warning, + }, + ], + })) +); diff --git a/tools/spectral/ipa/__tests__/__helpers__/readOnlySchemaCases.js b/tools/spectral/ipa/__tests__/__helpers__/readOnlySchemaCases.js new file mode 100644 index 0000000000..f1fc8ea940 --- /dev/null +++ b/tools/spectral/ipa/__tests__/__helpers__/readOnlySchemaCases.js @@ -0,0 +1,256 @@ +const readOnlyObject = { + type: 'object', + properties: { id: { type: 'string', readOnly: true } }, +}; + +export const readOnlySchemaCases = [ + { + description: 'an empty schema without a read-only annotation', + schema: {}, + expected: false, + }, + { + description: 'an open empty object', + schema: { type: 'object', properties: {} }, + expected: false, + }, + ...[undefined, {}].map((properties) => ({ + description: `a closed empty object with ${properties ? 'empty' : 'omitted'} properties`, + schema: { type: 'object', ...(properties ? { properties } : {}), additionalProperties: false }, + expected: true, + })), + { + description: 'an explicitly read-only empty object', + schema: { type: 'object', readOnly: true }, + expected: true, + }, + ...[true, {}, { type: 'string' }].flatMap((additionalProperties) => [ + { + description: `read-only named fields with writable additionalProperties ${JSON.stringify(additionalProperties)}`, + schema: { ...readOnlyObject, additionalProperties }, + expected: false, + }, + { + description: `an explicitly read-only object with additionalProperties ${JSON.stringify(additionalProperties)}`, + schema: { ...readOnlyObject, readOnly: true, additionalProperties }, + expected: true, + }, + ]), + { + description: 'an explicitly read-only object with writable named and additional fields', + schema: { + type: 'object', + readOnly: true, + properties: { name: { type: 'string' } }, + additionalProperties: { type: 'string' }, + }, + expected: true, + }, + { + description: 'a nested object with read-only named fields and writable dictionary entries', + schema: { + type: 'object', + properties: { + metadata: { ...readOnlyObject, additionalProperties: { type: 'string' } }, + }, + }, + expected: false, + }, + { + description: 'a dictionary of explicitly read-only values', + schema: { type: 'object', additionalProperties: { type: 'string', readOnly: true } }, + expected: true, + }, + { + description: 'a dictionary of objects containing only read-only fields', + schema: { type: 'object', additionalProperties: readOnlyObject }, + expected: true, + }, + { + description: 'a dictionary of objects containing writable fields', + schema: { + type: 'object', + additionalProperties: { + type: 'object', + properties: { id: { type: 'string', readOnly: true }, name: { type: 'string' } }, + }, + }, + expected: false, + }, + { + description: 'a dictionary of open empty objects', + schema: { type: 'object', additionalProperties: { type: 'object' } }, + expected: false, + }, + { + description: 'read-only named fields alongside read-only dictionary entries', + schema: { ...readOnlyObject, additionalProperties: readOnlyObject }, + expected: true, + }, + { + description: 'writable named fields alongside read-only dictionary entries', + schema: { + type: 'object', + properties: { name: { type: 'string' } }, + additionalProperties: readOnlyObject, + }, + expected: false, + }, + { + description: 'an array without an items schema', + schema: { type: 'array' }, + expected: false, + }, + { + description: 'an array with only object-specific constraints', + schema: { type: 'array', properties: readOnlyObject.properties, additionalProperties: false }, + expected: false, + }, + { + description: 'an object with only an array-specific constraint', + schema: { type: 'object', items: readOnlyObject }, + expected: false, + }, + { + description: 'an enum without field definitions', + schema: { enum: ['ONE', 'TWO'] }, + expected: false, + }, + ...['allOf', 'anyOf', 'oneOf'].flatMap((composition) => [ + { + description: `read-only fields with a required-only ${composition} branch`, + schema: { ...readOnlyObject, [composition]: [{ required: ['id'] }] }, + expected: true, + }, + { + description: `read-only fields with an object-type-only ${composition} branch`, + schema: { ...readOnlyObject, [composition]: [{ type: 'object' }] }, + expected: true, + }, + { + description: `read-only array items with an array-type-only ${composition} branch`, + schema: { type: 'array', items: readOnlyObject, [composition]: [{ type: 'array' }] }, + expected: true, + }, + { + description: `writable fields with a required-only ${composition} branch`, + schema: { type: 'object', properties: { id: { type: 'string' } }, [composition]: [{ required: ['id'] }] }, + expected: false, + }, + { + description: `a ${composition} containing only constraints`, + schema: { [composition]: [{ type: 'object', required: ['id'] }] }, + expected: false, + }, + { + description: `a ${composition} containing read-only fields and a constraint-only branch`, + schema: { [composition]: [readOnlyObject, { required: ['id'] }] }, + expected: composition === 'allOf', + }, + { + description: `a ${composition} containing read-only fields and an unconstrained branch`, + schema: { [composition]: [readOnlyObject, {}] }, + expected: composition === 'allOf', + }, + { + description: `a ${composition} containing array items and an array-type-only branch`, + schema: { [composition]: [{ items: readOnlyObject }, { type: 'array' }] }, + expected: composition === 'allOf', + }, + { + description: `a ${composition} containing read-only fields and a writable primitive`, + schema: { [composition]: [readOnlyObject, { type: 'string' }] }, + expected: false, + }, + { + description: `empty properties alongside read-only ${composition} branches`, + schema: { properties: {}, [composition]: [readOnlyObject] }, + expected: true, + }, + ]), +]; + +export const listResponseSchema = { + type: 'object', + properties: { + results: { + type: 'array', + readOnly: true, + items: { type: 'object', properties: { name: { type: 'string' } } }, + }, + totalCount: { type: 'integer', readOnly: true }, + }, +}; + +export const listResponseCases = [ + { description: 'an inline list response', schema: listResponseSchema, expected: false }, + ...['allOf', 'anyOf', 'oneOf'].map((composition) => ({ + description: `a list response wrapped in ${composition}`, + schema: { [composition]: [listResponseSchema] }, + expected: false, + })), + { + description: 'list fields split across allOf branches', + schema: { + allOf: [ + { type: 'object', properties: { results: listResponseSchema.properties.results } }, + { type: 'object', properties: { totalCount: listResponseSchema.properties.totalCount } }, + ], + }, + expected: false, + }, + { + description: 'list results and a required constraint split across allOf branches', + schema: { + allOf: [ + { type: 'object', properties: { results: listResponseSchema.properties.results } }, + { required: ['results'] }, + ], + }, + expected: false, + }, + ...['anyOf', 'oneOf'].flatMap((composition) => [ + { + description: `list metadata shared with a ${composition} results branch`, + schema: { + properties: { totalCount: listResponseSchema.properties.totalCount }, + [composition]: [{ properties: { results: listResponseSchema.properties.results } }, readOnlyObject], + }, + expected: false, + }, + { + description: `non-list fields in separate ${composition} alternatives`, + schema: { + [composition]: [ + { properties: { results: listResponseSchema.properties.results } }, + { properties: { totalCount: listResponseSchema.properties.totalCount } }, + ], + }, + expected: true, + }, + ]), + { + description: 'a resource containing a nested list rather than being a list itself', + schema: { type: 'object', properties: { history: listResponseSchema } }, + expected: true, + }, +]; + +export function documentForResponseSchema(schema, resourcePath, methods = {}) { + return { + paths: { + [resourcePath]: { + ...methods, + get: { + responses: { + 200: { + description: 'OK', + content: { 'application/json': { schema: { $ref: '#/components/schemas/Response' } } }, + }, + }, + }, + }, + }, + components: { schemas: { Response: schema, ListResponse: listResponseSchema } }, + }; +} diff --git a/tools/spectral/ipa/__tests__/utils/resourceEvaluation.test.js b/tools/spectral/ipa/__tests__/utils/resourceEvaluation.test.js index ba86bae144..fe00647fae 100644 --- a/tools/spectral/ipa/__tests__/utils/resourceEvaluation.test.js +++ b/tools/spectral/ipa/__tests__/utils/resourceEvaluation.test.js @@ -9,6 +9,7 @@ import { isSingletonResource, resourceBelongsToSingleParent, } from '../../rulesets/functions/utils/resourceEvaluation'; +import { listResponseCases, listResponseSchema, readOnlySchemaCases } from '../__helpers__/readOnlySchemaCases'; const resource = { '/resource': { @@ -396,6 +397,7 @@ describe('tools/spectral/ipa/rulesets/functions/utils/resourceEvaluation.js', () describe('allPropertiesAreReadOnly', () => { const testCases = [ + ...readOnlySchemaCases, { description: 'schema with all properties readOnly', schema: { @@ -697,9 +699,65 @@ describe('tools/spectral/ipa/rulesets/functions/utils/resourceEvaluation.js', () expect(allPropertiesAreReadOnly(schema)).toEqual(false); }); + + it('returns false for a circular dictionary value', () => { + const schema = { type: 'object' }; + schema.additionalProperties = schema; + + expect(allPropertiesAreReadOnly(schema)).toEqual(false); + }); + + it('honors an explicit read-only annotation on a circular dictionary', () => { + const schema = { type: 'object', readOnly: true }; + schema.additionalProperties = schema; + + expect(allPropertiesAreReadOnly(schema)).toEqual(true); + }); + + it('checks a shared schema independently in named and additional properties', () => { + const value = { type: 'object', properties: { id: { type: 'string', readOnly: true } } }; + const schema = { type: 'object', properties: { metadata: value }, additionalProperties: value }; + + expect(allPropertiesAreReadOnly(schema)).toEqual(true); + }); + + it('does not treat an unresolved reference as a neutral constraint', () => { + const schema = { + properties: { id: { type: 'string', readOnly: true } }, + allOf: [{ $ref: '#/components/schemas/Unknown' }], + }; + + expect(allPropertiesAreReadOnly(schema)).toEqual(false); + }); }); describe('isReadOnlyResource', () => { + function resourceForSchema(schema) { + return { + '/resource/{id}/singleton': { + get: { responses: { 200: { content: { 'application/json': { schema } } } } }, + }, + }; + } + + it.each(listResponseCases)('returns $expected for $description', ({ schema, expected }) => { + expect(isReadOnlyResource(resourceForSchema(schema))).toEqual(expected); + }); + + it('handles a cycle while detecting a list response in another branch', () => { + const schema = { readOnly: true, allOf: [] }; + schema.allOf.push(schema, listResponseSchema); + + expect(isReadOnlyResource(resourceForSchema(schema))).toEqual(false); + }); + + it('handles a read-only circular schema without list fields', () => { + const schema = { readOnly: true, allOf: [] }; + schema.allOf.push(schema); + + expect(isReadOnlyResource(resourceForSchema(schema))).toEqual(true); + }); + const testCases = [ { description: 'read-only resource', diff --git a/tools/spectral/ipa/rulesets/IPA-113.yaml b/tools/spectral/ipa/rulesets/IPA-113.yaml index a100d5fb2b..02771738fb 100644 --- a/tools/spectral/ipa/rulesets/IPA-113.yaml +++ b/tools/spectral/ipa/rulesets/IPA-113.yaml @@ -50,7 +50,13 @@ 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 or contain only read-only properties; for List responses, all properties in the items schema must be readOnly) + - Excludes read-only singleton resources (where the GET response schema is marked as readOnly or contains only read-only fields) + - An explicit `readOnly: true` annotation covers the whole object, including additional properties + - Otherwise, checks declared properties, array items, and explicit additionalProperties schemas recursively; writable dictionary entries prevent read-only inference + - Omitted additionalProperties does not override read-only declared fields, but an empty or constraint-only schema alone does not prove read-only status + - An explicitly closed empty object (`additionalProperties: false`) has no writable fields; use of that keyword is validated separately by IPA-118 + - Checks all composition branches; constraint-only branches may accompany read-only field definitions but cannot establish read-only status on their own + - List response envelopes, including those defined through composition, do not establish that a singleton is read-only - 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 @@ -143,6 +149,7 @@ rules: - Verifies that the parent singleton resource is not read-only - Uses existing isReadOnlyResource() helper function - Fails if all properties of the singleton resource are marked as readOnly or contain only read-only properties + - List response envelopes, including those defined through composition, do not establish that a singleton is read-only 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/README.md b/tools/spectral/ipa/rulesets/README.md index c1aff88f3f..eb682fc4a3 100644 --- a/tools/spectral/ipa/rulesets/README.md +++ b/tools/spectral/ipa/rulesets/README.md @@ -891,7 +891,13 @@ 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 or contain only read-only properties; for List responses, all properties in the items schema must be readOnly) + - Excludes read-only singleton resources (where the GET response schema is marked as readOnly or contains only read-only fields) + - An explicit `readOnly: true` annotation covers the whole object, including additional properties + - Otherwise, checks declared properties, array items, and explicit additionalProperties schemas recursively; writable dictionary entries prevent read-only inference + - Omitted additionalProperties does not override read-only declared fields, but an empty or constraint-only schema alone does not prove read-only status + - An explicitly closed empty object (`additionalProperties: false`) has no writable fields; use of that keyword is validated separately by IPA-118 + - Checks all composition branches; constraint-only branches may accompany read-only field definitions but cannot establish read-only status on their own + - List response envelopes, including those defined through composition, do not establish that a singleton is read-only - Checks that the resource has the PUT and/or PATCH methods defined #### xgen-IPA-113-reset-method-must-use-POST @@ -965,6 +971,7 @@ Rule checks for the following conditions: - Verifies that the parent singleton resource is not read-only - Uses existing isReadOnlyResource() helper function - Fails if all properties of the singleton resource are marked as readOnly or contain only read-only properties + - List response envelopes, including those defined through composition, do not establish that a singleton is read-only #### xgen-IPA-113-reset-method-valid-operation-id diff --git a/tools/spectral/ipa/rulesets/functions/utils/resourceEvaluation.js b/tools/spectral/ipa/rulesets/functions/utils/resourceEvaluation.js index 0024fe6486..34619b9f39 100644 --- a/tools/spectral/ipa/rulesets/functions/utils/resourceEvaluation.js +++ b/tools/spectral/ipa/rulesets/functions/utils/resourceEvaluation.js @@ -249,48 +249,78 @@ export function removePrefix(path) { * @returns {boolean} true if all properties are readOnly, false otherwise */ export function allPropertiesAreReadOnly(schema, visiting = new Set()) { + return getReadOnlyState(schema, visiting) === true; +} + +/** + * Returns true for read-only fields, false for writable/unknown fields, and undefined for + * constraints that do not define fields. A constraint-only schema cannot prove read-only + * status on its own, but can accompany field definitions in a composition. + */ +function getReadOnlyState(schema, visiting) { if (!schema || typeof schema !== 'object') { return false; } + if (schema.readOnly === true) { + return true; + } + if (visiting.has(schema)) { return false; } visiting.add(schema); try { - // Compositions can coexist with properties or items; none may hide writable fields in the others. - let hasReadOnlyComposition = false; - for (const composition of ['allOf', 'anyOf', 'oneOf']) { - const branches = schema[composition]; - if (!Array.isArray(branches)) { - continue; + if (schema.$ref || (schema.type && schema.type !== 'object' && schema.type !== 'array')) { + return false; + } + + let hasReadOnlyFields = false; + if (schema.properties && schema.type !== 'array') { + const properties = Object.values(schema.properties); + if (!properties.every((property) => allPropertiesAreReadOnly(property, visiting))) { + return false; } - if (branches.length === 0 || !branches.every((subSchema) => isSchemaReadOnly(subSchema, visiting))) { + hasReadOnlyFields = properties.length > 0; + } + + if (schema.items && schema.type !== 'object') { + if (!allPropertiesAreReadOnly(schema.items, visiting)) { return false; } - hasReadOnlyComposition = true; + hasReadOnlyFields = true; } - if (schema.properties) { - const properties = Object.values(schema.properties); - return properties.length > 0 && properties.every((property) => isSchemaReadOnly(property, visiting)); + // Infer from declared fields, retaining the convention that omitted additionalProperties + // does not override read-only named fields. Explicit dictionaries must be checked. + if (schema.additionalProperties !== undefined && schema.type !== 'array') { + if (schema.additionalProperties !== false && !allPropertiesAreReadOnly(schema.additionalProperties, visiting)) { + return false; + } + hasReadOnlyFields = true; } - if (schema.items) { - return isSchemaReadOnly(schema.items, visiting); + for (const composition of ['allOf', 'anyOf', 'oneOf']) { + const branches = schema[composition]; + if (!Array.isArray(branches)) { + continue; + } + const states = branches.map((subSchema) => getReadOnlyState(subSchema, visiting)); + if (states.length === 0 || states.includes(false)) { + return false; + } + // allOf shares field definitions between branches. Alternatives must each establish + // read-only fields unless a sibling definition already constrains the whole schema. + hasReadOnlyFields ||= composition === 'allOf' ? states.includes(true) : states.every((state) => state === true); } - return hasReadOnlyComposition; + return hasReadOnlyFields ? true : undefined; } finally { visiting.delete(schema); } } -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 @@ -374,11 +404,47 @@ export function isReadOnlyResource(resourcePathItems) { } function isListResponseSchema(schema) { + return getResponseShapes(schema, new Set()).has(RESULTS_ARRAY | PAGINATION_METADATA); +} + +const RESULTS_ARRAY = 1; +const PAGINATION_METADATA = 2; + +// Track possible response shapes so allOf can combine fields split across branches without +// combining fields from mutually exclusive anyOf/oneOf alternatives. There are only four shapes. +function getResponseShapes(schema, visiting) { + if (!schema || typeof schema !== 'object' || visiting.has(schema)) { + return new Set([0]); + } + const properties = schema?.properties; const results = properties?.results; - return ( - results?.type === 'array' && - Boolean(results.items) && - Boolean(properties.links || properties.totalCount || schema.required?.includes('results')) - ); + const resultsShape = results?.type === 'array' && results.items ? RESULTS_ARRAY : 0; + const metadataShape = + properties?.links || properties?.totalCount || schema.required?.includes('results') ? PAGINATION_METADATA : 0; + let shapes = new Set([resultsShape | metadataShape]); + + visiting.add(schema); + try { + for (const composition of ['allOf', 'anyOf', 'oneOf']) { + if (!Array.isArray(schema[composition]) || schema[composition].length === 0) { + continue; + } + const branches = schema[composition].map((branch) => getResponseShapes(branch, visiting)); + if (composition === 'allOf') { + for (const branch of branches) { + shapes = combineResponseShapes(shapes, branch); + } + } else { + shapes = combineResponseShapes(shapes, new Set(branches.flatMap((branch) => [...branch]))); + } + } + return shapes; + } finally { + visiting.delete(schema); + } +} + +function combineResponseShapes(left, right) { + return new Set([...left].flatMap((a) => [...right].map((b) => a | b))); } From bc8989208251bdb4eca1d0064c32636c6d43cd77 Mon Sep 17 00:00:00 2001 From: Andrei Matei Date: Mon, 28 Sep 2026 13:32:23 +0100 Subject: [PATCH 6/6] fix(ipa): scope read-only annotations to response fields --- .../__helpers__/readOnlySchemaCases.js | 44 ++++++++++++++++--- .../utils/resourceEvaluation.test.js | 4 +- tools/spectral/ipa/rulesets/IPA-113.yaml | 4 +- tools/spectral/ipa/rulesets/README.md | 4 +- .../functions/utils/resourceEvaluation.js | 18 +++++--- 5 files changed, 56 insertions(+), 18 deletions(-) diff --git a/tools/spectral/ipa/__tests__/__helpers__/readOnlySchemaCases.js b/tools/spectral/ipa/__tests__/__helpers__/readOnlySchemaCases.js index f1fc8ea940..2e45c5726c 100644 --- a/tools/spectral/ipa/__tests__/__helpers__/readOnlySchemaCases.js +++ b/tools/spectral/ipa/__tests__/__helpers__/readOnlySchemaCases.js @@ -20,9 +20,9 @@ export const readOnlySchemaCases = [ expected: true, })), { - description: 'an explicitly read-only empty object', + description: 'an empty response object with a root read-only annotation', schema: { type: 'object', readOnly: true }, - expected: true, + expected: false, }, ...[true, {}, { type: 'string' }].flatMap((additionalProperties) => [ { @@ -31,19 +31,53 @@ export const readOnlySchemaCases = [ expected: false, }, { - description: `an explicitly read-only object with additionalProperties ${JSON.stringify(additionalProperties)}`, + description: `a response object with a root read-only annotation and additionalProperties ${JSON.stringify(additionalProperties)}`, schema: { ...readOnlyObject, readOnly: true, additionalProperties }, - expected: true, + expected: false, }, ]), { - description: 'an explicitly read-only object with writable named and additional fields', + description: 'a response object with a root read-only annotation and writable fields', schema: { type: 'object', readOnly: true, properties: { name: { type: 'string' } }, additionalProperties: { type: 'string' }, }, + expected: false, + }, + { + description: 'a response object with a root read-only annotation and a writable oneOf branch', + schema: { + type: 'object', + readOnly: true, + oneOf: [{ type: 'object', properties: { name: { type: 'string' } } }], + }, + expected: false, + }, + { + description: 'a response object with a root read-only annotation and only read-only fields', + schema: { ...readOnlyObject, readOnly: true }, + expected: true, + }, + { + description: 'an explicitly read-only object-valued property with writable named and additional fields', + schema: { + type: 'object', + properties: { + metadata: { + type: 'object', + readOnly: true, + properties: { name: { type: 'string' } }, + additionalProperties: { type: 'string' }, + }, + }, + }, + expected: true, + }, + { + description: 'an explicitly read-only empty object-valued property', + schema: { type: 'object', properties: { metadata: { type: 'object', readOnly: true } } }, expected: true, }, { diff --git a/tools/spectral/ipa/__tests__/utils/resourceEvaluation.test.js b/tools/spectral/ipa/__tests__/utils/resourceEvaluation.test.js index fe00647fae..75360385b9 100644 --- a/tools/spectral/ipa/__tests__/utils/resourceEvaluation.test.js +++ b/tools/spectral/ipa/__tests__/utils/resourceEvaluation.test.js @@ -711,7 +711,7 @@ describe('tools/spectral/ipa/rulesets/functions/utils/resourceEvaluation.js', () const schema = { type: 'object', readOnly: true }; schema.additionalProperties = schema; - expect(allPropertiesAreReadOnly(schema)).toEqual(true); + expect(allPropertiesAreReadOnly({ properties: { metadata: schema } })).toEqual(true); }); it('checks a shared schema independently in named and additional properties', () => { @@ -755,7 +755,7 @@ describe('tools/spectral/ipa/rulesets/functions/utils/resourceEvaluation.js', () const schema = { readOnly: true, allOf: [] }; schema.allOf.push(schema); - expect(isReadOnlyResource(resourceForSchema(schema))).toEqual(true); + expect(isReadOnlyResource(resourceForSchema({ properties: { metadata: schema } }))).toEqual(true); }); const testCases = [ diff --git a/tools/spectral/ipa/rulesets/IPA-113.yaml b/tools/spectral/ipa/rulesets/IPA-113.yaml index 02771738fb..cdfdc3d5b8 100644 --- a/tools/spectral/ipa/rulesets/IPA-113.yaml +++ b/tools/spectral/ipa/rulesets/IPA-113.yaml @@ -50,8 +50,8 @@ rules: ##### Implementation details Rule checks for the following conditions: - Applies only to singleton resources - - Excludes read-only singleton resources (where the GET response schema is marked as readOnly or contains only read-only fields) - - An explicit `readOnly: true` annotation covers the whole object, including additional properties + - Excludes read-only singleton resources (where the GET response schema contains only read-only fields) + - An explicit `readOnly: true` annotation on an object-valued property covers that property, including its additional properties; an annotation on the response schema itself does not establish a read-only resource - Otherwise, checks declared properties, array items, and explicit additionalProperties schemas recursively; writable dictionary entries prevent read-only inference - Omitted additionalProperties does not override read-only declared fields, but an empty or constraint-only schema alone does not prove read-only status - An explicitly closed empty object (`additionalProperties: false`) has no writable fields; use of that keyword is validated separately by IPA-118 diff --git a/tools/spectral/ipa/rulesets/README.md b/tools/spectral/ipa/rulesets/README.md index eb682fc4a3..17b585323d 100644 --- a/tools/spectral/ipa/rulesets/README.md +++ b/tools/spectral/ipa/rulesets/README.md @@ -891,8 +891,8 @@ 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 the GET response schema is marked as readOnly or contains only read-only fields) - - An explicit `readOnly: true` annotation covers the whole object, including additional properties + - Excludes read-only singleton resources (where the GET response schema contains only read-only fields) + - An explicit `readOnly: true` annotation on an object-valued property covers that property, including its additional properties; an annotation on the response schema itself does not establish a read-only resource - Otherwise, checks declared properties, array items, and explicit additionalProperties schemas recursively; writable dictionary entries prevent read-only inference - Omitted additionalProperties does not override read-only declared fields, but an empty or constraint-only schema alone does not prove read-only status - An explicitly closed empty object (`additionalProperties: false`) has no writable fields; use of that keyword is validated separately by IPA-118 diff --git a/tools/spectral/ipa/rulesets/functions/utils/resourceEvaluation.js b/tools/spectral/ipa/rulesets/functions/utils/resourceEvaluation.js index 34619b9f39..38907044e9 100644 --- a/tools/spectral/ipa/rulesets/functions/utils/resourceEvaluation.js +++ b/tools/spectral/ipa/rulesets/functions/utils/resourceEvaluation.js @@ -249,7 +249,7 @@ export function removePrefix(path) { * @returns {boolean} true if all properties are readOnly, false otherwise */ export function allPropertiesAreReadOnly(schema, visiting = new Set()) { - return getReadOnlyState(schema, visiting) === true; + return getReadOnlyState(schema, visiting, false) === true; } /** @@ -257,12 +257,13 @@ export function allPropertiesAreReadOnly(schema, visiting = new Set()) { * constraints that do not define fields. A constraint-only schema cannot prove read-only * status on its own, but can accompany field definitions in a composition. */ -function getReadOnlyState(schema, visiting) { +function getReadOnlyState(schema, visiting, isField) { if (!schema || typeof schema !== 'object') { return false; } - if (schema.readOnly === true) { + // OpenAPI readOnly applies to a field, not to an entire response component. + if (isField && schema.readOnly === true) { return true; } @@ -279,14 +280,14 @@ function getReadOnlyState(schema, visiting) { let hasReadOnlyFields = false; if (schema.properties && schema.type !== 'array') { const properties = Object.values(schema.properties); - if (!properties.every((property) => allPropertiesAreReadOnly(property, visiting))) { + if (!properties.every((property) => getReadOnlyState(property, visiting, true) === true)) { return false; } hasReadOnlyFields = properties.length > 0; } if (schema.items && schema.type !== 'object') { - if (!allPropertiesAreReadOnly(schema.items, visiting)) { + if (getReadOnlyState(schema.items, visiting, true) !== true) { return false; } hasReadOnlyFields = true; @@ -295,7 +296,10 @@ function getReadOnlyState(schema, visiting) { // Infer from declared fields, retaining the convention that omitted additionalProperties // does not override read-only named fields. Explicit dictionaries must be checked. if (schema.additionalProperties !== undefined && schema.type !== 'array') { - if (schema.additionalProperties !== false && !allPropertiesAreReadOnly(schema.additionalProperties, visiting)) { + if ( + schema.additionalProperties !== false && + getReadOnlyState(schema.additionalProperties, visiting, true) !== true + ) { return false; } hasReadOnlyFields = true; @@ -306,7 +310,7 @@ function getReadOnlyState(schema, visiting) { if (!Array.isArray(branches)) { continue; } - const states = branches.map((subSchema) => getReadOnlyState(subSchema, visiting)); + const states = branches.map((subSchema) => getReadOnlyState(subSchema, visiting, isField)); if (states.length === 0 || states.includes(false)) { return false; }