diff --git a/tools/spectral/ipa/__tests__/IPA118NoAdditionalPropertiesFalse.test.js b/tools/spectral/ipa/__tests__/IPA118NoAdditionalPropertiesFalse.test.js new file mode 100644 index 0000000000..e5ce9dce7a --- /dev/null +++ b/tools/spectral/ipa/__tests__/IPA118NoAdditionalPropertiesFalse.test.js @@ -0,0 +1,218 @@ +import testRule from './__helpers__/testRule'; +import { DiagnosticSeverity } from '@stoplight/types'; + +testRule('xgen-IPA-118-no-additional-properties-false', [ + { + name: 'valid without additionalProperties', + document: { + components: { + schemas: { + ExampleSchema: { + type: 'object', + properties: { + name: { type: 'string' }, + value: { type: 'integer' }, + }, + }, + }, + }, + }, + errors: [], + }, + { + name: 'valid with additionalProperties: true', + document: { + components: { + schemas: { + ExampleSchema: { + type: 'object', + properties: { + name: { type: 'string' }, + }, + additionalProperties: true, + }, + }, + }, + }, + errors: [], + }, + { + name: 'valid with additionalProperties as schema', + document: { + components: { + schemas: { + ExampleSchema: { + type: 'object', + properties: { + name: { type: 'string' }, + }, + additionalProperties: { + type: 'string', + }, + }, + }, + }, + }, + errors: [], + }, + { + name: 'invalid with additionalProperties: false', + document: { + components: { + schemas: { + ExampleSchema: { + type: 'object', + properties: { + name: { type: 'string' }, + }, + additionalProperties: false, + }, + ReferencedSchema: { + type: 'object', + properties: { + property: { $ref: '#/components/schemas/ExampleSchema' }, + }, + }, + }, + }, + }, + errors: [ + { + code: 'xgen-IPA-118-no-additional-properties-false', + message: + "Schema must not use 'additionalProperties: false'. Consider using 'additionalProperties: true' or omitting the property.", + path: ['components', 'schemas', 'ExampleSchema'], + severity: DiagnosticSeverity.Warning, + }, + ], + }, + { + name: 'invalid with nested additionalProperties: false', + document: { + components: { + schemas: { + ParentSchema: { + type: 'object', + properties: { + child: { + type: 'object', + properties: { + name: { type: 'string' }, + }, + additionalProperties: false, + }, + }, + }, + }, + }, + }, + errors: [ + { + code: 'xgen-IPA-118-no-additional-properties-false', + message: + "Schema must not use 'additionalProperties: false'. Consider using 'additionalProperties: true' or omitting the property.", + path: ['components', 'schemas', 'ParentSchema', 'properties', 'child'], + severity: DiagnosticSeverity.Warning, + }, + ], + }, + { + name: 'invalid with multiple nested additionalProperties: false', + document: { + components: { + schemas: { + ParentSchema: { + type: 'object', + properties: { + child: { + type: 'object', + properties: { + name: { type: 'string' }, + }, + additionalProperties: false, + }, + }, + additionalProperties: false, + }, + }, + }, + }, + errors: [ + { + code: 'xgen-IPA-118-no-additional-properties-false', + message: + "Schema must not use 'additionalProperties: false'. Consider using 'additionalProperties: true' or omitting the property.", + path: ['components', 'schemas', 'ParentSchema'], + severity: DiagnosticSeverity.Warning, + }, + { + code: 'xgen-IPA-118-no-additional-properties-false', + message: + "Schema must not use 'additionalProperties: false'. Consider using 'additionalProperties: true' or omitting the property.", + path: ['components', 'schemas', 'ParentSchema', 'properties', 'child'], + severity: DiagnosticSeverity.Warning, + }, + ], + }, + { + name: 'with exception', + document: { + components: { + schemas: { + ExampleSchema: { + type: 'object', + 'x-xgen-IPA-exception': { + 'xgen-IPA-118-no-additional-properties-false': 'Exception reason', + }, + properties: { + name: { type: 'string' }, + }, + additionalProperties: false, + }, + ParentSchema: { + type: 'object', + 'x-xgen-IPA-exception': { + 'xgen-IPA-118-no-additional-properties-false': 'Exception reason', + }, + properties: { + child: { + type: 'object', + properties: { + name: { type: 'string' }, + }, + additionalProperties: false, + }, + }, + }, + }, + }, + }, + errors: [], + }, + { + name: 'invalid with multiple nested additionalProperties: false - exceptions', + document: { + components: { + schemas: { + ParentSchema: { + type: 'object', + properties: { + child: { + type: 'object', + properties: { + name: { type: 'string' }, + }, + additionalProperties: false, + }, + }, + additionalProperties: false, + 'x-xgen-IPA-exception': { + 'xgen-IPA-118-no-additional-properties-false': 'Exception reason', + }, + }, + }, + }, + }, + errors: [], + }, +]); diff --git a/tools/spectral/ipa/__tests__/utils/compareUtils.test.js b/tools/spectral/ipa/__tests__/utils/compareUtils.test.js index 1fe52c9d92..3f223e2e13 100644 --- a/tools/spectral/ipa/__tests__/utils/compareUtils.test.js +++ b/tools/spectral/ipa/__tests__/utils/compareUtils.test.js @@ -1,5 +1,6 @@ import { describe, expect, it } from '@jest/globals'; import { + findAdditionalPropertiesFalsePaths, isDeepEqual, removePropertiesByFlag, removePropertyKeys, @@ -329,3 +330,157 @@ describe('removeRequestProperties', () => { expect(removeRequestProperties(input)).toEqual(expected); }); }); + +describe('findAdditionalPropertiesFalsePaths', () => { + it('finds additionalProperties:false at root level', () => { + const schema = { + type: 'object', + properties: { + name: { type: 'string' }, + }, + additionalProperties: false, + }; + + const results = findAdditionalPropertiesFalsePaths(schema, ['root']); + expect(results).toEqual([['root']]); + }); + + it('finds additionalProperties:false in nested properties', () => { + const schema = { + type: 'object', + properties: { + user: { + type: 'object', + properties: { + address: { + type: 'object', + additionalProperties: false, + }, + }, + }, + }, + }; + + const results = findAdditionalPropertiesFalsePaths(schema, ['root']); + expect(results).toEqual([['root', 'properties', 'user', 'properties', 'address']]); + }); + + it('finds additionalProperties:false in array items', () => { + const schema = { + type: 'array', + items: { + type: 'object', + additionalProperties: false, + }, + }; + + const results = findAdditionalPropertiesFalsePaths(schema, ['root']); + expect(results).toEqual([['root', 'items']]); + }); + + it('finds additionalProperties:false in composition keywords', () => { + const schema = { + oneOf: [ + { + type: 'object', + additionalProperties: false, + }, + { + type: 'object', + properties: { + name: { type: 'string' }, + }, + }, + ], + }; + + const results = findAdditionalPropertiesFalsePaths(schema, ['root']); + expect(results).toEqual([['root', 'oneOf', '0']]); + }); + + it('finds multiple additionalProperties:false occurrences', () => { + const schema = { + type: 'object', + additionalProperties: false, + properties: { + user: { + type: 'object', + additionalProperties: false, + properties: { + addresses: { + type: 'array', + items: { + type: 'object', + additionalProperties: false, + }, + }, + }, + }, + metadata: { + allOf: [ + { + type: 'object', + additionalProperties: false, + }, + ], + }, + }, + }; + + const results = findAdditionalPropertiesFalsePaths(schema, ['root']); + expect(results).toEqual([ + ['root'], + ['root', 'properties', 'user'], + ['root', 'properties', 'user', 'properties', 'addresses', 'items'], + ['root', 'properties', 'metadata', 'allOf', '0'], + ]); + }); + + it('handles deeply nested structures', () => { + const schema = { + type: 'object', + properties: { + level1: { + type: 'object', + properties: { + level2: { + type: 'object', + properties: { + level3: { + type: 'object', + properties: { + level4: { + type: 'object', + additionalProperties: false, + }, + }, + }, + }, + }, + }, + }, + }, + }; + + const results = findAdditionalPropertiesFalsePaths(schema, ['root']); + expect(results).toEqual([ + ['root', 'properties', 'level1', 'properties', 'level2', 'properties', 'level3', 'properties', 'level4'], + ]); + }); + + it('does not find additionalProperties:true or schema objects', () => { + const schema = { + type: 'object', + additionalProperties: true, + properties: { + user: { + type: 'object', + additionalProperties: { type: 'string' }, + }, + }, + }; + + const results = findAdditionalPropertiesFalsePaths(schema, ['root']); + expect(results).toEqual([]); + }); +}); diff --git a/tools/spectral/ipa/ipa-spectral.yaml b/tools/spectral/ipa/ipa-spectral.yaml index 78887b9858..c496d1dd0c 100644 --- a/tools/spectral/ipa/ipa-spectral.yaml +++ b/tools/spectral/ipa/ipa-spectral.yaml @@ -12,6 +12,7 @@ extends: - ./rulesets/IPA-113.yaml - ./rulesets/IPA-114.yaml - ./rulesets/IPA-117.yaml + - ./rulesets/IPA-118.yaml - ./rulesets/IPA-123.yaml - ./rulesets/IPA-125.yaml @@ -41,3 +42,15 @@ overrides: - '**#/components/schemas/UserSecurity/properties/customerX509' # unable to document exceptions, to be covered by CLOUDP-308286 rules: xgen-IPA-112-field-names-are-camel-case: 'off' + - files: + - '**#/components/schemas/DataLakeS3StoreSettings/allOf/1/properties/additionalStorageClasses' # unable to document exceptions, to be covered by CLOUDP-293178 + - '**#/components/schemas/DataLakeDatabaseDataSourceSettings/properties/databaseRegex' # unable to document exceptions, to be covered by CLOUDP-293178 + - '**#/components/schemas/DataLakeDatabaseDataSourceSettings/properties/collectionRegex' # unable to document exceptions, to be covered by CLOUDP-293178 + rules: + xgen-IPA-117-description-should-not-use-inline-links: 'off' + - files: + - '**#/paths/~1api~1atlas~1v2~1unauth~1openapi~1versions' # external reference, to be covered by CLOUDP-309694 + - '**#/paths/~1api~1atlas~1v2~1openapi~1info' # external reference, to be covered by CLOUDP-309694 + - '**#/paths/~1rest~1unauth~1version' # external reference, to be covered by CLOUDP-309694 + rules: + xgen-IPA-114-error-responses-refer-to-api-error: 'off' diff --git a/tools/spectral/ipa/rulesets/IPA-118.yaml b/tools/spectral/ipa/rulesets/IPA-118.yaml new file mode 100644 index 0000000000..3634c71737 --- /dev/null +++ b/tools/spectral/ipa/rulesets/IPA-118.yaml @@ -0,0 +1,20 @@ +# IPA-118: Extensible by Default +# http://go/ipa/118 + +functions: + - IPA118NoAdditionalPropertiesFalse + +rules: + xgen-IPA-118-no-additional-properties-false: + description: | + Schemas must not use `additionalProperties: false` + + ##### Implementation details + This rule checks that schemas don't restrict additional properties by setting `additionalProperties: false`. + Schemas without explicit `additionalProperties` settings (which default to true) or with `additionalProperties` set to `true` are compliant. + This rule checks all nested schemas, but only parent schemas can be marked for exception. + message: '{{error}} https://mdb.link/mongodb-atlas-openapi-validation#xgen-IPA-118-no-additional-properties-false' + severity: warn + given: '$.components.schemas[*]' + then: + function: 'IPA118NoAdditionalPropertiesFalse' diff --git a/tools/spectral/ipa/rulesets/README.md b/tools/spectral/ipa/rulesets/README.md index 75a3bf4435..8c8a49fb7b 100644 --- a/tools/spectral/ipa/rulesets/README.md +++ b/tools/spectral/ipa/rulesets/README.md @@ -772,6 +772,22 @@ The rule checks for the presence of the `schema`, `examples` or `example` proper +### IPA-118 + +Rules are based on [http://go/ipa/IPA-118](http://go/ipa/IPA-118). + +#### xgen-IPA-118-no-additional-properties-false + + ![warn](https://img.shields.io/badge/warning-yellow) +Schemas must not use `additionalProperties: false` + +##### Implementation details +This rule checks that schemas don't restrict additional properties by setting `additionalProperties: false`. +Schemas without explicit `additionalProperties` settings (which default to true) or with `additionalProperties` set to `true` are compliant. +This rule checks all nested schemas, but only parent schemas can be marked for exception. + + + ### IPA-123 Rules are based on [http://go/ipa/IPA-123](http://go/ipa/IPA-123). diff --git a/tools/spectral/ipa/rulesets/functions/IPA118NoAdditionalPropertiesFalse.js b/tools/spectral/ipa/rulesets/functions/IPA118NoAdditionalPropertiesFalse.js new file mode 100644 index 0000000000..e72a647350 --- /dev/null +++ b/tools/spectral/ipa/rulesets/functions/IPA118NoAdditionalPropertiesFalse.js @@ -0,0 +1,53 @@ +import { hasException } from './utils/exceptions.js'; +import { + collectAdoption, + collectAndReturnViolation, + collectException, + handleInternalError, +} from './utils/collectionUtils.js'; +import { resolveObject } from './utils/componentUtils.js'; +import { findAdditionalPropertiesFalsePaths } from './utils/compareUtils.js'; + +const RULE_NAME = 'xgen-IPA-118-no-additional-properties-false'; +const ERROR_MESSAGE = `Schema must not use 'additionalProperties: false'. Consider using 'additionalProperties: true' or omitting the property.`; +/** + * Validates that schemas don't use additionalProperties: false + * + * @param {object} input - The schema object to check + * @param {object} _ - Rule options (unused) + * @param {object} context - The context object containing path and document information + */ +export default (input, _, { path, documentInventory }) => { + const oas = documentInventory.unresolved; + const schemaObject = resolveObject(oas, path); + + if (hasException(input, RULE_NAME)) { + collectException(input, RULE_NAME, path); + return; + } + + const errors = checkViolationsAndReturnErrors(schemaObject, path); + if (errors.length > 0) { + return collectAndReturnViolation(path, RULE_NAME, errors); + } + + collectAdoption(path, RULE_NAME); +}; + +function checkViolationsAndReturnErrors(schemaObject, path) { + try { + const errors = []; + + const results = findAdditionalPropertiesFalsePaths(schemaObject, path); + for (const resultPath of results) { + errors.push({ + message: ERROR_MESSAGE, + path: resultPath, + }); + } + + return errors; + } catch (e) { + handleInternalError(RULE_NAME, path, e); + } +} diff --git a/tools/spectral/ipa/rulesets/functions/utils/compareUtils.js b/tools/spectral/ipa/rulesets/functions/utils/compareUtils.js index 3b0021d6f9..d8e1a74683 100644 --- a/tools/spectral/ipa/rulesets/functions/utils/compareUtils.js +++ b/tools/spectral/ipa/rulesets/functions/utils/compareUtils.js @@ -147,3 +147,49 @@ export function removeRequestProperties(schema) { let result = removePropertiesByFlag(schema, 'writeOnly'); return removePropertyKeys(result, 'title', 'description', 'required'); } + +/** + * Recursively finds all paths where additionalProperties is set to false in a schema + * @param {object} schema - The schema to analyze + * @param {Array} currentPath - The current path (for recursion) + * @param {Array>} results - Accumulator for paths with additionalProperties: false + * @returns {Array>} Array of paths where additionalProperties: false was found + */ +export function findAdditionalPropertiesFalsePaths(schema, currentPath = [], results = []) { + if (!schema || typeof schema !== 'object') { + return results; + } + + // Check if this schema has additionalProperties: false + if (schema.additionalProperties === false) { + results.push([...currentPath]); + } + + // Check properties + if (schema.properties && typeof schema.properties === 'object') { + for (const [propName, propValue] of Object.entries(schema.properties)) { + if (typeof propValue === 'object') { + findAdditionalPropertiesFalsePaths(propValue, [...currentPath, 'properties', propName], results); + } + } + } + + // Check items for arrays + if (schema.items && typeof schema.items === 'object') { + findAdditionalPropertiesFalsePaths(schema.items, [...currentPath, 'items'], results); + } + + // Check other schema composition keywords + const compositionKeywords = ['allOf', 'anyOf', 'oneOf']; + for (const keyword of compositionKeywords) { + if (Array.isArray(schema[keyword])) { + schema[keyword].forEach((subSchema, index) => { + if (typeof subSchema === 'object') { + findAdditionalPropertiesFalsePaths(subSchema, [...currentPath, keyword, index.toString()], results); + } + }); + } + } + + return results; +}