From 4524e8cc7a6c5dafb71644a3d5fe744d4a7ae4a4 Mon Sep 17 00:00:00 2001 From: Abhishek Kumar Date: Tue, 30 Sep 2025 12:57:52 +0530 Subject: [PATCH 1/7] issue#48: fix for broken internal ref --- NESTED_REFS_SOLUTION.md | 194 ++++++ functions/validateRefSiblings.js | 138 +++++ functions/validateRefSiblings.spec.js | 417 +++++++++++++ .../broken-internal-refs-3.0.x.yml | 28 + .../broken-internal-refs-3.1.x.yml | 28 + .../broken-refs/no-ref-siblings-3.0.x.yml | 30 + .../broken-refs/no-ref-siblings-3.1.x.yml | 30 + test/validation-nested-refs.spec.js | 575 ++++++++++++++++++ validation.js | 16 + 9 files changed, 1456 insertions(+) create mode 100644 NESTED_REFS_SOLUTION.md create mode 100644 functions/validateRefSiblings.js create mode 100644 functions/validateRefSiblings.spec.js create mode 100644 test/resources/broken-refs/broken-internal-refs-3.0.x.yml create mode 100644 test/resources/broken-refs/broken-internal-refs-3.1.x.yml create mode 100644 test/resources/broken-refs/no-ref-siblings-3.0.x.yml create mode 100644 test/resources/broken-refs/no-ref-siblings-3.1.x.yml create mode 100644 test/validation-nested-refs.spec.js diff --git a/NESTED_REFS_SOLUTION.md b/NESTED_REFS_SOLUTION.md new file mode 100644 index 0000000..55e3d43 --- /dev/null +++ b/NESTED_REFS_SOLUTION.md @@ -0,0 +1,194 @@ +# Nested Broken References Detection - Solution Summary + +## Problem Statement + +The Spectral validator `validation.js` was not catching the broken `$ref` references like `#/components/schemas/PolicySeverity` in the [spec file](/test/resources/broken-refs/broken-internal-refs-3.0.x.yml), specifically when they appeared in nested structures within sibling properties of another `$ref`. + +## Root Cause Analysis + +### The Issue + +In OpenAPI 3.1.x, the specification allows `$ref` to have sibling properties at the same level while OAS 3.0.x doesn't allow this: + +```yaml +commonAnomaly: + $ref: '#/components/schemas/CommonAnomalyPolicy' + severities: # Sibling property - valid in OAS 3.1.x but invalid in OAS 3.0.x and are genrally ignored by toolings + type: array + items: + $ref: '#/components/schemas/PolicySeverity' # Nested ref +``` +The broken refrence `#/components/schemas/PolicySeverity` was not being caught in both OAS 3.0.x and 3.1.x specs. Ideally, a broken reference should be reported as `invalid-ref` error as part of `oas3-schema ruleset`. + +### Why It Wasn't Caught + +1. **In OAS 3.0.x**: With respect to the parent commonAnomaly, Spectral simply ignored sibling property `severities` of `#ref`. Hence, validation never reached to `#/components/schemas/PolicySeverity`. +2. **In OAS 3.1.x**: Spectral resolves `$ref` references during document parsing. When a `$ref` has sibling properties, Spectral merges them during resolution. By the time inbuilt `invalid-ref` validation rule runs, the nested and broken `$ref` has been absorbed into the resolved schema structure and hence is not caught as an error. + +## Solution Implemented + +### 1. Added a rule `no-$ref-siblings` in validation.js +It warns that `$ref must not be placed next to any other properties` in case of OAS 3.0.x specs. + +### 2. Created `validateRefSiblings.js` Preprocessor + +**File**: [functions/validateRefSiblings.js](/functions/validateRefSiblings.js) + +**Purpose**: Catches broken `$ref` references that are nested within sibling properties of another `$ref`. + +**How It Works**: +1. Runs on the **unresolved document** (before Spectral processes `$ref` references) +2. Identifies objects that have both a `$ref` and sibling properties +3. Recursively searches sibling properties for nested `$ref` references +4. Validates each nested `$ref` against the document data +5. Reports broken references with detailed path information + +**Key Features**: +- Works with `resolved: false` flag to run before reference resolution +- Handles deeply nested structures +- Supports arrays and complex object hierarchies +- Ignores external references (HTTP/HTTPS/file paths) +- Only validates internal references (starting with `#/`) + +### 2. Updated `validation.js` Ruleset + +**File**: [validation.js](/validation.js) + +**Changes**: +- Added import for `validateRefSiblings` +- Created new rule: `broken-refs-in-siblings` +- Configured with `resolved: false` to run on unresolved document +- Uses `$..' JSONPath to match all objects +- Error severity for broken references + + +### 3. Comprehensive Test Suites + +#### Unit Tests: `validateRefSiblings.spec.js` + +**File**: [functions/validateRefSiblings.spec.js](/functions/validateRefSiblings.spec.js) + +**Coverage**: +- Basic validation (null inputs, missing $ref, no siblings) +- Valid nested references (should pass) +- Broken nested references (should fail) +- Multiple broken references +- Deeply nested structures +- External references (should be ignored) +- Edge cases (null values, primitives, empty objects) + +#### Integration Tests: `validation-nested-refs.spec.js` + +**File**: [test/validation-nested-refs.spec.js](/test/validation-nested-refs.spec.js) + +**Coverage**: +- OpenAPI 3.0.3 scenarios +- OpenAPI 3.1.0 scenarios (where $ref+siblings is valid per spec) +- Real-world ThresholdAnomaly/PolicySeverity scenario +- Multiple broken references +- Mixed valid and broken references +- External references handling +- Edge cases and complex nested structures + + +#### Test Resource Files + +**Directory**: [test/resources/broken-refs/](/test/resources/broken-refs/) + +Test files for different scenarios: + +1. [broken-internal-refs-3.0.x.yml](/test/resources/broken-refs/broken-internal-refs-3.0.x.yml) + - OpenAPI 3.0.3 spec with broken `PolicySeverity` reference + - Has `$ref` with sibling properties (technically invalid in 3.0.3) + - Used to verify preprocessor catches broken nested refs + +2. [broken-internal-refs-3.1.x.yml](/test/resources/broken-refs/broken-internal-refs-3.1.x.yml) + - OpenAPI 3.1.0 spec with broken `PolicySeverity` reference + - Has `$ref` with sibling properties (valid in 3.1.0) + - Used to verify preprocessor catches broken nested refs + +3. [no-ref-siblings-3.0.x.yml](/test/resources/broken-refs/no-ref-siblings-3.0.x.yml) + - OpenAPI 3.0.3 spec with valid `PolicySeverity` reference + - Has `$ref` with sibling properties + - Used to verify no false positives when ref is valid + +4. [no-ref-siblings-3.1.x.yml](/test/resources/broken-refs/no-ref-siblings-3.1.x.yml) + - OpenAPI 3.1.0 spec with valid `PolicySeverity` reference + - Has `$ref` with sibling properties + - Used to verify no false positives when ref is valid + + +## OpenAPI Version Differences + +### OpenAPI 3.0.x + +- `$ref` **SHOULD** be the only property at its level per specification +- Having sibling properties is **discouraged** but parsers may allow it +- Our preprocessor **works** and catches broken nested refs + +### OpenAPI 3.1.x + +- `$ref` **CAN** have sibling properties +- Structure is **VALID** per specification +- Our preprocessor is **ESSENTIAL** to catch nested broken refs + +## Usage + +### Testing the Solution + +```bash +# Test on OpenAPI 3.0.3 with broken refs +spectral lint -r validation.js test/resources/broken-refs/broken-internal-refs-3.0.x.yml + +# Test on OpenAPI 3.1.0 with broken refs +spectral lint -r validation.js test/resources/broken-refs/broken-internal-refs-3.1.x.yml + +# Run unit tests +npm test -- functions/validateRefSiblings.spec.js + +# Run integration tests +npm test -- test/validation-nested-refs.spec.js +``` + +### Expected Behavior + +**For files with broken nested refs**: +```bash +$ spectral lint -r validation.js test/resources/broken-refs/broken-internal-refs-3.0.x.yml + +/path/to/broken-internal-refs-3.0.x.yml + 22:23 error broken-refs-in-siblings Broken reference in sibling property 'severities.items.$ref': #/components/schemas/PolicySeverity components.schemas.TestSchema.properties.commonAnomaly + 24:22 warning no-$ref-siblings $ref must not be placed next to any other properties components.schemas.TestSchema.properties.commonAnomaly.severities + ✖ 2 problems (1 error, 1 warning, 0 infos, 0 hints) +``` +```bash +$ spectral lint -r validation.js test/resources/broken-refs/broken-internal-refs-3.1.x.yml + +/path/to/broken-internal-refs-3.1.x.yml + 22:23 error broken-refs-in-siblings Broken reference in sibling property 'severities.items.$ref': #/components/schemas/PolicySeverity + ✖ 1 problem (1 error, 0 warnings, 0 infos, 0 hints) +``` + +**For files with valid refs**: +```bash +$ spectral lint -r validation.js test/resources/broken-refs/no-ref-siblings-3.0.x.yml +26:22 warning no-$ref-siblings $ref must not be placed next to any other properties components.schemas.TestSchema.properties.commonAnomaly.severities +✖ 1 problem (0 error, 1 warnings, 0 infos, 0 hints) +``` +```bash +$ spectral lint -r validation.js test/resources/broken-refs/no-ref-siblings-3.0.x.yml +No results with a severity of 'error' found! +``` + +## Technical Details + +### The `resolved: false` Flag + +This is the **critical** configuration that makes the solution work: + +```javascript +'resolved': false // Run on unresolved document +``` + +- **Without this**: Rule runs on resolved document, nested refs are already merged and hidden +- **With this**: Rule runs on raw parsed YAML, before Spectral processes any `$ref` references diff --git a/functions/validateRefSiblings.js b/functions/validateRefSiblings.js new file mode 100644 index 0000000..9a9a87f --- /dev/null +++ b/functions/validateRefSiblings.js @@ -0,0 +1,138 @@ +/** + * Copyright 2022 Cisco Systems, Inc. and its affiliates. + * + * Licensed under the Apache License, Version 2.0 (the "License"); + * you may not use this file except in compliance with the License. + * You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + * + * SPDX-License-Identifier: Apache-2.0 + */ + +'use strict'; + +/** + * Preprocessor function that validates $ref references in sibling properties + * This catches broken references that would be hidden during schema resolution + * @param {Object} input - The object containing a $ref + * @param {Object} options - Function options + * @param {Object} context - Spectral context with document and path info + * @returns {Array} Array of validation results + */ +export default function validateRefSiblings(input, options, context) { + if (!input || typeof input !== 'object' || !input.$ref) { + return []; + } + + // Check if context has document data + if (!context || !context.document || !context.document.data) { + return []; + } + + const results = []; + + // Get all sibling properties (excluding $ref itself) + const siblingKeys = Object.keys(input).filter(key => key !== '$ref'); + + if (siblingKeys.length === 0) { + return []; + } + + // Recursively find and validate all $ref references in sibling properties + for (const key of siblingKeys) { + const siblingValue = input[key]; + const nestedResults = findAndValidateRefs( + siblingValue, + context.document.data, + `${key}` + ); + results.push(...nestedResults); + } + + return results; +} + +/** + * Recursively find all $ref values and validate them + * @param {*} obj - Object to search + * @param {Object} documentData - The full document data for validation + * @param {string} pathPrefix - Path prefix for error messages + * @returns {Array} Array of validation errors + */ +function findAndValidateRefs(obj, documentData, pathPrefix = '') { + const results = []; + + if (!obj || typeof obj !== 'object') { + return results; + } + + if (Array.isArray(obj)) { + obj.forEach((item, index) => { + const itemResults = findAndValidateRefs( + item, + documentData, + `${pathPrefix}[${index}]` + ); + results.push(...itemResults); + }); + } else { + for (const [key, value] of Object.entries(obj)) { + const currentPath = pathPrefix ? `${pathPrefix}.${key}` : key; + + if (key === '$ref' && typeof value === 'string') { + // Found a $ref - validate it + const validationResult = validateRef(value, documentData, currentPath); + if (validationResult) { + results.push(validationResult); + } + } else if (typeof value === 'object') { + // Recurse into nested objects + const nestedResults = findAndValidateRefs(value, documentData, currentPath); + results.push(...nestedResults); + } + } + } + + return results; +} + +/** + * Validate a single $ref reference + * @param {string} ref - The $ref value to validate + * @param {Object} documentData - The full document data + * @param {string} pathContext - Path context for error messages + * @returns {Object|null} Validation error object or null if valid + */ +function validateRef(ref, documentData, pathContext) { + // Only validate internal references (starting with #/) + if (!ref || typeof ref !== 'string' || !ref.startsWith('#/')) { + return null; + } + + // Parse the reference path + const path = ref.substring(2).split('/'); + + // Try to resolve the reference + let current = documentData; + for (const segment of path) { + if (current && typeof current === 'object' && segment in current) { + current = current[segment]; + } else { + // Broken reference found + return { + message: `Broken reference in sibling property '${pathContext}': ${ref}`, + }; + } + } + + // Reference is valid + return null; +} + diff --git a/functions/validateRefSiblings.spec.js b/functions/validateRefSiblings.spec.js new file mode 100644 index 0000000..6339bb6 --- /dev/null +++ b/functions/validateRefSiblings.spec.js @@ -0,0 +1,417 @@ +/** + * Copyright 2022 Cisco Systems, Inc. and its affiliates. + * + * Licensed under the Apache License, Version 2.0 (the "License"); + * you may not use this file except in compliance with the License. + * You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + * + * SPDX-License-Identifier: Apache-2.0 + */ + +import { + describe, expect, it, +} from '@jest/globals'; +import validateRefSiblings from './validateRefSiblings.js'; + +describe('validateRefSiblings', () => { + const mockContext = { + document: { + data: { + components: { + schemas: { + User: { + type: 'object', + properties: { + id: { type: 'string' }, + name: { type: 'string' } + } + }, + Organization: { + type: 'object', + properties: { + id: { type: 'string' }, + name: { type: 'string' } + } + }, + CommonPolicy: { + type: 'object', + properties: { + details: { type: 'string' }, + isActive: { type: 'boolean' } + } + } + } + } + } + } + }; + + describe('basic validation', () => { + it('should return empty array when input has no $ref', () => { + const input = { + type: 'object', + properties: { + name: { type: 'string' } + } + }; + const result = validateRefSiblings(input, {}, mockContext); + expect(result).toEqual([]); + }); + + it('should return empty array when input is null', () => { + const result = validateRefSiblings(null, {}, mockContext); + expect(result).toEqual([]); + }); + + it('should return empty array when input is not an object', () => { + const result = validateRefSiblings('string', {}, mockContext); + expect(result).toEqual([]); + }); + + it('should return empty array when $ref has no sibling properties', () => { + const input = { + $ref: '#/components/schemas/User' + }; + const result = validateRefSiblings(input, {}, mockContext); + expect(result).toEqual([]); + }); + + it('should return empty array when context is missing', () => { + const input = { + $ref: '#/components/schemas/User', + description: 'A user' + }; + const result = validateRefSiblings(input, {}, null); + expect(result).toEqual([]); + }); + + it('should return empty array when context.document.data is missing', () => { + const input = { + $ref: '#/components/schemas/User', + description: 'A user' + }; + const result = validateRefSiblings(input, {}, { document: {} }); + expect(result).toEqual([]); + }); + }); + + describe('valid nested references in siblings', () => { + it('should return empty array when nested $ref in sibling is valid', () => { + const input = { + $ref: '#/components/schemas/CommonPolicy', + additionalProp: { + $ref: '#/components/schemas/User' + } + }; + const result = validateRefSiblings(input, {}, mockContext); + expect(result).toEqual([]); + }); + + it('should return empty array when nested $ref in array items is valid', () => { + const input = { + $ref: '#/components/schemas/CommonPolicy', + items: { + type: 'array', + items: { + $ref: '#/components/schemas/User' + } + } + }; + const result = validateRefSiblings(input, {}, mockContext); + expect(result).toEqual([]); + }); + + it('should return empty array when multiple valid nested $refs exist', () => { + const input = { + $ref: '#/components/schemas/CommonPolicy', + users: { + type: 'array', + items: { + $ref: '#/components/schemas/User' + } + }, + organization: { + $ref: '#/components/schemas/Organization' + } + }; + const result = validateRefSiblings(input, {}, mockContext); + expect(result).toEqual([]); + }); + + it('should return empty array when deeply nested valid $ref exists', () => { + const input = { + $ref: '#/components/schemas/CommonPolicy', + properties: { + data: { + properties: { + user: { + $ref: '#/components/schemas/User' + } + } + } + } + }; + const result = validateRefSiblings(input, {}, mockContext); + expect(result).toEqual([]); + }); + }); + + describe('broken nested references in siblings', () => { + it('should detect broken $ref in direct sibling property', () => { + const input = { + $ref: '#/components/schemas/CommonPolicy', + additionalProp: { + $ref: '#/components/schemas/NonExistent' + } + }; + const result = validateRefSiblings(input, {}, mockContext); + expect(result).toEqual([ + { + message: "Broken reference in sibling property 'additionalProp.$ref': #/components/schemas/NonExistent" + } + ]); + }); + + it('should detect broken $ref in array items sibling', () => { + const input = { + $ref: '#/components/schemas/CommonPolicy', + severities: { + type: 'array', + items: { + $ref: '#/components/schemas/PolicySeverity' + } + } + }; + const result = validateRefSiblings(input, {}, mockContext); + expect(result).toEqual([ + { + message: "Broken reference in sibling property 'severities.items.$ref': #/components/schemas/PolicySeverity" + } + ]); + }); + + it('should detect multiple broken $refs in siblings', () => { + const input = { + $ref: '#/components/schemas/CommonPolicy', + prop1: { + $ref: '#/components/schemas/NonExistent1' + }, + prop2: { + $ref: '#/components/schemas/NonExistent2' + } + }; + const result = validateRefSiblings(input, {}, mockContext); + expect(result).toHaveLength(2); + expect(result).toContainEqual({ + message: "Broken reference in sibling property 'prop1.$ref': #/components/schemas/NonExistent1" + }); + expect(result).toContainEqual({ + message: "Broken reference in sibling property 'prop2.$ref': #/components/schemas/NonExistent2" + }); + }); + + it('should detect deeply nested broken $ref', () => { + const input = { + $ref: '#/components/schemas/CommonPolicy', + properties: { + nested: { + deep: { + value: { + $ref: '#/components/schemas/DeepNonExistent' + } + } + } + } + }; + const result = validateRefSiblings(input, {}, mockContext); + expect(result).toEqual([ + { + message: "Broken reference in sibling property 'properties.nested.deep.value.$ref': #/components/schemas/DeepNonExistent" + } + ]); + }); + + it('should detect broken $ref in array of objects', () => { + const input = { + $ref: '#/components/schemas/CommonPolicy', + arrayProp: [ + { + $ref: '#/components/schemas/NonExistent' + } + ] + }; + const result = validateRefSiblings(input, {}, mockContext); + expect(result).toEqual([ + { + message: "Broken reference in sibling property 'arrayProp[0].$ref': #/components/schemas/NonExistent" + } + ]); + }); + }); + + describe('mixed valid and broken references', () => { + it('should only report broken references when mix exists', () => { + const input = { + $ref: '#/components/schemas/CommonPolicy', + validProp: { + $ref: '#/components/schemas/User' + }, + brokenProp: { + $ref: '#/components/schemas/NonExistent' + } + }; + const result = validateRefSiblings(input, {}, mockContext); + expect(result).toHaveLength(1); + expect(result).toEqual([ + { + message: "Broken reference in sibling property 'brokenProp.$ref': #/components/schemas/NonExistent" + } + ]); + }); + + it('should validate all references in complex nested structure', () => { + const input = { + $ref: '#/components/schemas/CommonPolicy', + users: { + type: 'array', + items: { + $ref: '#/components/schemas/User' // valid + } + }, + policies: { + type: 'array', + items: { + $ref: '#/components/schemas/Policy' // broken + } + }, + organization: { + $ref: '#/components/schemas/Organization' // valid + } + }; + const result = validateRefSiblings(input, {}, mockContext); + expect(result).toHaveLength(1); + expect(result).toEqual([ + { + message: "Broken reference in sibling property 'policies.items.$ref': #/components/schemas/Policy" + } + ]); + }); + }); + + describe('external references', () => { + it('should ignore external HTTP references', () => { + const input = { + $ref: '#/components/schemas/CommonPolicy', + externalProp: { + $ref: 'http://example.com/schema.json' + } + }; + const result = validateRefSiblings(input, {}, mockContext); + expect(result).toEqual([]); + }); + + it('should ignore external HTTPS references', () => { + const input = { + $ref: '#/components/schemas/CommonPolicy', + externalProp: { + $ref: 'https://example.com/schema.json' + } + }; + const result = validateRefSiblings(input, {}, mockContext); + expect(result).toEqual([]); + }); + + it('should ignore relative file references', () => { + const input = { + $ref: '#/components/schemas/CommonPolicy', + externalProp: { + $ref: './schemas/user.json' + } + }; + const result = validateRefSiblings(input, {}, mockContext); + expect(result).toEqual([]); + }); + + it('should validate internal refs but ignore external refs', () => { + const input = { + $ref: '#/components/schemas/CommonPolicy', + internalBroken: { + $ref: '#/components/schemas/NonExistent' + }, + externalRef: { + $ref: 'http://example.com/schema.json' + } + }; + const result = validateRefSiblings(input, {}, mockContext); + expect(result).toHaveLength(1); + expect(result).toEqual([ + { + message: "Broken reference in sibling property 'internalBroken.$ref': #/components/schemas/NonExistent" + } + ]); + }); + }); + + describe('edge cases', () => { + it('should handle sibling with null value', () => { + const input = { + $ref: '#/components/schemas/CommonPolicy', + nullProp: null + }; + const result = validateRefSiblings(input, {}, mockContext); + expect(result).toEqual([]); + }); + + it('should handle sibling with primitive value', () => { + const input = { + $ref: '#/components/schemas/CommonPolicy', + stringProp: 'some value', + numberProp: 42 + }; + const result = validateRefSiblings(input, {}, mockContext); + expect(result).toEqual([]); + }); + + it('should handle empty object sibling', () => { + const input = { + $ref: '#/components/schemas/CommonPolicy', + emptyProp: {} + }; + const result = validateRefSiblings(input, {}, mockContext); + expect(result).toEqual([]); + }); + + it('should handle empty array sibling', () => { + const input = { + $ref: '#/components/schemas/CommonPolicy', + emptyArray: [] + }; + const result = validateRefSiblings(input, {}, mockContext); + expect(result).toEqual([]); + }); + + it('should handle $ref pointing to non-existent path segment', () => { + const input = { + $ref: '#/components/schemas/CommonPolicy', + brokenPath: { + $ref: '#/nonexistent/path/schema' + } + }; + const result = validateRefSiblings(input, {}, mockContext); + expect(result).toEqual([ + { + message: "Broken reference in sibling property 'brokenPath.$ref': #/nonexistent/path/schema" + } + ]); + }); + }); +}); + diff --git a/test/resources/broken-refs/broken-internal-refs-3.0.x.yml b/test/resources/broken-refs/broken-internal-refs-3.0.x.yml new file mode 100644 index 0000000..83d00cd --- /dev/null +++ b/test/resources/broken-refs/broken-internal-refs-3.0.x.yml @@ -0,0 +1,28 @@ +openapi: 3.0.3 +info: + title: Test API + version: 1.0.0 +paths: + /test: + get: + responses: + '200': + description: OK + content: + application/json: + schema: + $ref: '#/components/schemas/TestSchema' +components: + schemas: + CommonAnomalyPolicy: + type: object + TestSchema: + type: object + properties: + commonAnomaly: + $ref: '#/components/schemas/CommonAnomalyPolicy' + severities: # no-$ref-siblings should warn here for openapi 3.0.x + description: severity threshold values + items: + $ref: '#/components/schemas/PolicySeverity' # a broken $ref here should be caught + type: array diff --git a/test/resources/broken-refs/broken-internal-refs-3.1.x.yml b/test/resources/broken-refs/broken-internal-refs-3.1.x.yml new file mode 100644 index 0000000..eeb7cf5 --- /dev/null +++ b/test/resources/broken-refs/broken-internal-refs-3.1.x.yml @@ -0,0 +1,28 @@ +openapi: 3.1.0 +info: + title: Test API + version: 1.0.0 +paths: + /test: + get: + responses: + '200': + description: OK + content: + application/json: + schema: + $ref: '#/components/schemas/TestSchema' +components: + schemas: + CommonAnomalyPolicy: + type: object + TestSchema: + type: object + properties: + commonAnomaly: + $ref: '#/components/schemas/CommonAnomalyPolicy' + severities: # no no-$ref-siblings warning for openapi 3.1.x + description: severity threshold values + items: + $ref: '#/components/schemas/PolicySeverity' # a broken $ref here should be caught + type: array diff --git a/test/resources/broken-refs/no-ref-siblings-3.0.x.yml b/test/resources/broken-refs/no-ref-siblings-3.0.x.yml new file mode 100644 index 0000000..d69b3db --- /dev/null +++ b/test/resources/broken-refs/no-ref-siblings-3.0.x.yml @@ -0,0 +1,30 @@ +openapi: 3.0.3 +info: + title: Test API + version: 1.0.0 +paths: + /test: + get: + responses: + '200': + description: OK + content: + application/json: + schema: + $ref: '#/components/schemas/TestSchema' +components: + schemas: + PolicySeverity: + type: object + CommonAnomalyPolicy: + type: object + TestSchema: + type: object + properties: + commonAnomaly: + $ref: '#/components/schemas/CommonAnomalyPolicy' + severities: # no-$ref-siblings should warn here for openapi 3.0.x + description: severity threshold values + items: + $ref: '#/components/schemas/PolicySeverity' # Not a broken $ref + type: array diff --git a/test/resources/broken-refs/no-ref-siblings-3.1.x.yml b/test/resources/broken-refs/no-ref-siblings-3.1.x.yml new file mode 100644 index 0000000..5c8a0d7 --- /dev/null +++ b/test/resources/broken-refs/no-ref-siblings-3.1.x.yml @@ -0,0 +1,30 @@ +openapi: 3.1.0 +info: + title: Test API + version: 1.0.0 +paths: + /test: + get: + responses: + '200': + description: OK + content: + application/json: + schema: + $ref: '#/components/schemas/TestSchema' +components: + schemas: + PolicySeverity: + type: object + CommonAnomalyPolicy: + type: object + TestSchema: + type: object + properties: + commonAnomaly: + $ref: '#/components/schemas/CommonAnomalyPolicy' + severities: # no no-$ref-siblings warning for openapi 3.1.x + description: severity threshold values + items: + $ref: '#/components/schemas/PolicySeverity' # Not a broken $ref + type: array diff --git a/test/validation-nested-refs.spec.js b/test/validation-nested-refs.spec.js new file mode 100644 index 0000000..805d20c --- /dev/null +++ b/test/validation-nested-refs.spec.js @@ -0,0 +1,575 @@ +/** + * Copyright 2022 Cisco Systems, Inc. and its affiliates. + * + * Licensed under the Apache License, Version 2.0 (the "License"); + * you may not use this file except in compliance with the License. + * You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + * + * SPDX-License-Identifier: Apache-2.0 + */ + +import { + describe, expect, it, beforeAll, +} from '@jest/globals'; +import { Spectral } from '@stoplight/spectral-core'; +import { bundleAndLoadRuleset } from '@stoplight/spectral-ruleset-bundler/with-loader'; +import * as fs from 'fs'; +import * as path from 'path'; + +// __dirname is provided by Jest in CommonJS mode +const rulesetPath = path.join(__dirname, '..', 'validation.js'); + +describe('validation.js - nested broken refs detection', () => { + let spectral; + let ruleset; + + beforeAll(async () => { + spectral = new Spectral(); + ruleset = await bundleAndLoadRuleset(rulesetPath, { fs, fetch }); + spectral.setRuleset(ruleset); + }); + + describe('OpenAPI 3.0.3 - broken refs in standard locations', () => { + it('should catch broken direct $ref', async () => { + const spec = ` +openapi: 3.0.3 +info: + title: Test API + version: 1.0.0 +paths: + /test: + get: + responses: + '200': + description: OK + content: + application/json: + schema: + $ref: '#/components/schemas/NonExistent' +components: + schemas: + User: + type: object +`; + const results = await spectral.run(spec); + const brokenRefErrors = results.filter(r => + r.code === 'broken-refs-in-siblings' || + r.code === 'invalid-ref' + ); + + expect(brokenRefErrors.length).toBeGreaterThan(0); + }); + + it('should catch broken $ref in array items', async () => { + const spec = ` +openapi: 3.0.3 +info: + title: Test API + version: 1.0.0 +paths: + /test: + get: + responses: + '200': + description: OK + content: + application/json: + schema: + type: array + items: + $ref: '#/components/schemas/NonExistent' +components: + schemas: + User: + type: object +`; + const results = await spectral.run(spec); + const brokenRefErrors = results.filter(r => + r.code === 'broken-refs-in-siblings' || + r.code === 'invalid-ref' + ); + + expect(brokenRefErrors.length).toBeGreaterThan(0); + }); + + it('should catch broken refs in siblings even in 3.0.3 (our preprocessor works)', async () => { + const spec = ` +openapi: 3.0.3 +info: + title: Test API + version: 1.0.0 +paths: {} +components: + schemas: + CommonPolicy: + type: object + TestSchema: + type: object + properties: + commonAnomaly: + $ref: '#/components/schemas/CommonPolicy' + severities: + type: array + items: + $ref: '#/components/schemas/PolicySeverity' +`; + const results = await spectral.run(spec); + + // Our preprocessor should catch the broken ref even in 3.0.3 + // (even though the structure itself is technically invalid per spec) + const brokenRefErrors = results.filter(r => + r.code === 'broken-refs-in-siblings' + ); + expect(brokenRefErrors.length).toBeGreaterThan(0); + expect(brokenRefErrors[0].message).toContain('PolicySeverity'); + }); + }); + + describe('OpenAPI 3.1.0 - broken refs in sibling properties', () => { + it('should catch broken $ref in sibling property', async () => { + const spec = ` +openapi: 3.1.0 +info: + title: Test API + version: 1.0.0 +paths: + /test: + get: + responses: + '200': + description: OK + content: + application/json: + schema: + $ref: '#/components/schemas/TestSchema' +components: + schemas: + CommonPolicy: + type: object + TestSchema: + type: object + properties: + commonAnomaly: + $ref: '#/components/schemas/CommonPolicy' + severities: + type: array + items: + $ref: '#/components/schemas/PolicySeverity' +`; + const results = await spectral.run(spec); + const brokenRefErrors = results.filter(r => + r.code === 'broken-refs-in-siblings' + ); + + expect(brokenRefErrors.length).toBeGreaterThan(0); + expect(brokenRefErrors[0].message).toContain('PolicySeverity'); + }); + + it('should catch multiple broken refs in sibling properties', async () => { + const spec = ` +openapi: 3.1.0 +info: + title: Test API + version: 1.0.0 +paths: + /test: + get: + responses: + '200': + description: OK + content: + application/json: + schema: + $ref: '#/components/schemas/TestSchema' +components: + schemas: + CommonPolicy: + type: object + TestSchema: + type: object + properties: + property1: + $ref: '#/components/schemas/CommonPolicy' + nested1: + $ref: '#/components/schemas/NonExistent1' + property2: + $ref: '#/components/schemas/CommonPolicy' + nested2: + $ref: '#/components/schemas/NonExistent2' +`; + const results = await spectral.run(spec); + const brokenRefErrors = results.filter(r => + r.code === 'broken-refs-in-siblings' + ); + + expect(brokenRefErrors.length).toBeGreaterThanOrEqual(2); + }); + + it('should catch deeply nested broken ref in sibling', async () => { + const spec = ` +openapi: 3.1.0 +info: + title: Test API + version: 1.0.0 +paths: + /test: + get: + responses: + '200': + description: OK + content: + application/json: + schema: + $ref: '#/components/schemas/TestSchema' +components: + schemas: + CommonPolicy: + type: object + TestSchema: + type: object + properties: + commonAnomaly: + $ref: '#/components/schemas/CommonPolicy' + properties: + nested: + properties: + deep: + $ref: '#/components/schemas/DeepNonExistent' +`; + const results = await spectral.run(spec); + const brokenRefErrors = results.filter(r => + r.code === 'broken-refs-in-siblings' + ); + + expect(brokenRefErrors.length).toBeGreaterThan(0); + expect(brokenRefErrors[0].message).toContain('DeepNonExistent'); + }); + + it('should NOT report errors when nested refs in siblings are valid', async () => { + const spec = ` +openapi: 3.1.0 +info: + title: Test API + version: 1.0.0 +paths: + /test: + get: + responses: + '200': + description: OK + content: + application/json: + schema: + $ref: '#/components/schemas/TestSchema' +components: + schemas: + CommonPolicy: + type: object + User: + type: object + TestSchema: + type: object + properties: + commonAnomaly: + $ref: '#/components/schemas/CommonPolicy' + users: + type: array + items: + $ref: '#/components/schemas/User' +`; + const results = await spectral.run(spec); + const brokenRefErrors = results.filter(r => + r.code === 'broken-refs-in-siblings' + ); + + expect(brokenRefErrors.length).toBe(0); + }); + }); + + describe('OpenAPI 3.1.0 - real-world scenario: ThresholdAnomaly', () => { + it('should catch broken PolicySeverity ref in ThresholdAnomaly structure', async () => { + const spec = ` +openapi: 3.1.0 +info: + title: Anomaly API + version: 1.0.0 +paths: + /anomalies: + get: + responses: + '200': + description: OK + content: + application/json: + schema: + $ref: '#/components/schemas/ThresholdAnomaly' +components: + schemas: + AnomalyTypeFormat: + type: string + enum: [basic, regex, threshold] + ThresholdAnomalyType: + type: string + enum: [cpu, memory, disk] + CommonAnomalyPolicy: + type: object + properties: + details: + type: string + isActive: + type: boolean + ThresholdAnomaly: + type: object + properties: + anomalyFormatType: + $ref: '#/components/schemas/AnomalyTypeFormat' + anomalyType: + $ref: '#/components/schemas/ThresholdAnomalyType' + commonAnomaly: + $ref: '#/components/schemas/CommonAnomalyPolicy' + severities: + description: severity threshold values + type: array + items: + $ref: '#/components/schemas/PolicySeverity' + required: + - anomalyFormatType + - anomalyType + - severities +`; + const results = await spectral.run(spec); + const brokenRefErrors = results.filter(r => + r.code === 'broken-refs-in-siblings' + ); + + expect(brokenRefErrors.length).toBeGreaterThan(0); + expect(brokenRefErrors.some(e => e.message.includes('PolicySeverity'))).toBe(true); + }); + + it('should NOT report error when PolicySeverity is defined', async () => { + const spec = ` +openapi: 3.1.0 +info: + title: Anomaly API + version: 1.0.0 +paths: + /anomalies: + get: + responses: + '200': + description: OK + content: + application/json: + schema: + $ref: '#/components/schemas/ThresholdAnomaly' +components: + schemas: + AnomalyTypeFormat: + type: string + enum: [basic, regex, threshold] + ThresholdAnomalyType: + type: string + enum: [cpu, memory, disk] + CommonAnomalyPolicy: + type: object + properties: + details: + type: string + isActive: + type: boolean + PolicySeverity: + type: string + enum: [low, medium, high, critical] + ThresholdAnomaly: + type: object + properties: + anomalyFormatType: + $ref: '#/components/schemas/AnomalyTypeFormat' + anomalyType: + $ref: '#/components/schemas/ThresholdAnomalyType' + commonAnomaly: + $ref: '#/components/schemas/CommonAnomalyPolicy' + severities: + description: severity threshold values + type: array + items: + $ref: '#/components/schemas/PolicySeverity' + required: + - anomalyFormatType + - anomalyType + - severities +`; + const results = await spectral.run(spec); + const brokenRefErrors = results.filter(r => + r.code === 'broken-refs-in-siblings' + ); + + expect(brokenRefErrors.length).toBe(0); + }); + }); + + describe('OpenAPI 3.1.0 - external references', () => { + it('should NOT report errors for external HTTP refs in siblings', async () => { + const spec = ` +openapi: 3.1.0 +info: + title: Test API + version: 1.0.0 +paths: + /test: + get: + responses: + '200': + description: OK + content: + application/json: + schema: + $ref: '#/components/schemas/TestSchema' +components: + schemas: + CommonPolicy: + type: object + TestSchema: + type: object + properties: + commonAnomaly: + $ref: '#/components/schemas/CommonPolicy' + externalRef: + $ref: 'http://example.com/schemas/external.json' +`; + const results = await spectral.run(spec); + const brokenRefErrors = results.filter(r => + r.code === 'broken-refs-in-siblings' + ); + + expect(brokenRefErrors.length).toBe(0); + }); + + it('should catch internal broken refs but ignore external refs', async () => { + const spec = ` +openapi: 3.1.0 +info: + title: Test API + version: 1.0.0 +paths: + /test: + get: + responses: + '200': + description: OK + content: + application/json: + schema: + $ref: '#/components/schemas/TestSchema' +components: + schemas: + CommonPolicy: + type: object + TestSchema: + type: object + properties: + commonAnomaly: + $ref: '#/components/schemas/CommonPolicy' + internalBroken: + $ref: '#/components/schemas/NonExistent' + externalRef: + $ref: 'https://example.com/schemas/external.json' +`; + const results = await spectral.run(spec); + const brokenRefErrors = results.filter(r => + r.code === 'broken-refs-in-siblings' + ); + + expect(brokenRefErrors.length).toBeGreaterThan(0); + expect(brokenRefErrors.some(e => e.message.includes('NonExistent'))).toBe(true); + expect(brokenRefErrors.some(e => e.message.includes('example.com'))).toBe(false); + }); + }); + + describe('Edge cases', () => { + it('should handle $ref with non-object siblings', async () => { + const spec = ` +openapi: 3.1.0 +info: + title: Test API + version: 1.0.0 +paths: + /test: + get: + responses: + '200': + description: OK + content: + application/json: + schema: + $ref: '#/components/schemas/TestSchema' +components: + schemas: + CommonPolicy: + type: object + TestSchema: + type: object + properties: + prop: + $ref: '#/components/schemas/CommonPolicy' + description: "Some description" + example: "example value" +`; + const results = await spectral.run(spec); + const brokenRefErrors = results.filter(r => + r.code === 'broken-refs-in-siblings' + ); + + // Should not error - no nested $refs + expect(brokenRefErrors.length).toBe(0); + }); + + it('should handle multiple levels of $ref with siblings', async () => { + const spec = ` +openapi: 3.1.0 +info: + title: Test API + version: 1.0.0 +paths: + /test: + get: + responses: + '200': + description: OK + content: + application/json: + schema: + $ref: '#/components/schemas/Level1' +components: + schemas: + Base: + type: object + Level1: + type: object + properties: + level1Prop: + $ref: '#/components/schemas/Base' + level2: + $ref: '#/components/schemas/Base' + level3: + $ref: '#/components/schemas/NonExistent' +`; + const results = await spectral.run(spec); + const brokenRefErrors = results.filter(r => + r.code === 'broken-refs-in-siblings' + ); + + expect(brokenRefErrors.length).toBeGreaterThan(0); + expect(brokenRefErrors.some(e => e.message.includes('NonExistent'))).toBe(true); + }); + }); +}); + diff --git a/validation.js b/validation.js index a9042da..a530938 100644 --- a/validation.js +++ b/validation.js @@ -19,6 +19,8 @@ import { oas } from '@stoplight/spectral-rulesets'; import { oas3_1 } from '@stoplight/spectral-formats'; import defaultInEnum from './functions/defaultInEnum.js'; +import validateRefSiblings from './functions/validateRefSiblings.js'; + export default { 'extends': [ [ @@ -29,6 +31,7 @@ export default { 'rules': { 'oas3-schema': 'error', 'oas2-schema': 'error', + 'no-$ref-siblings': 'warn', 'oas3-operation-security-defined': 'error', 'oas2-operation-security-defined': 'error', 'server-variable-default': { @@ -45,5 +48,18 @@ export default { }, ], }, + + 'broken-refs-in-siblings': { + 'description': 'Internal $ref references in sibling properties of $ref should point to existing schema components', + 'message': '{{error}}', + 'severity': 'error', + 'resolved': false, + 'given': '$..', + 'then': [ + { + 'function': validateRefSiblings, + }, + ], + }, }, }; From d6b5279bb177fd0611975f07c8e5b19788c98f36 Mon Sep 17 00:00:00 2001 From: abkum3 Date: Mon, 6 Oct 2025 18:58:49 +0530 Subject: [PATCH 2/7] adding link of the issue --- NESTED_REFS_SOLUTION.md | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) diff --git a/NESTED_REFS_SOLUTION.md b/NESTED_REFS_SOLUTION.md index 55e3d43..afeefc1 100644 --- a/NESTED_REFS_SOLUTION.md +++ b/NESTED_REFS_SOLUTION.md @@ -3,6 +3,7 @@ ## Problem Statement The Spectral validator `validation.js` was not catching the broken `$ref` references like `#/components/schemas/PolicySeverity` in the [spec file](/test/resources/broken-refs/broken-internal-refs-3.0.x.yml), specifically when they appeared in nested structures within sibling properties of another `$ref`. +The original issue #48 reported is [here](https://wwwin-github.cisco.com/DevNet/api-insights-support/issues/48). ## Root Cause Analysis @@ -50,7 +51,7 @@ It warns that `$ref must not be placed next to any other properties` in case of - Ignores external references (HTTP/HTTPS/file paths) - Only validates internal references (starting with `#/`) -### 2. Updated `validation.js` Ruleset +### 3. Updated `validation.js` Ruleset **File**: [validation.js](/validation.js) @@ -62,7 +63,7 @@ It warns that `$ref must not be placed next to any other properties` in case of - Error severity for broken references -### 3. Comprehensive Test Suites +### 4. Comprehensive Test Suites #### Unit Tests: `validateRefSiblings.spec.js` From f54cc49255176c99ea31413d14a5b811f1530944 Mon Sep 17 00:00:00 2001 From: abkum3 Date: Thu, 9 Oct 2025 20:15:46 +0530 Subject: [PATCH 3/7] fixing undefined required property issues --- NESTED_REFS_SOLUTION.md | 209 +++--- functions/validateRefSiblings.js | 2 +- functions/validateRefSiblings.spec.js | 20 +- functions/validateRequiredProperties.js | 58 ++ functions/validateRequiredProperties.spec.js | 222 +++++++ test/integration/broken-internal-refs.test.js | 598 ++++++++++++++++++ test/integration/broken-refs-yaml.test.js | 174 +++++ .../undefined-properties-yaml.test.js | 261 ++++++++ .../undefined-required-properties.test.js | 446 +++++++++++++ .../base-permit-deny-entry-3.0.x.yml | 29 + .../comprehensive-test-3.0.x.yml | 82 +++ .../device-interface-complete-3.0.x.yml | 81 +++ .../device-interface-scope-3.0.x.yml | 28 + .../reports-summary-complete-3.0.x.yml | 74 +++ .../request-response-schemas-3.0.x.yml | 55 ++ .../security-policies-complete-3.0.x.yml | 76 +++ .../smart-switch-complete-3.0.x.yml | 86 +++ .../smart-switch-integration-3.0.x.yml | 28 + .../summary-properties-3.0.x.yml | 28 + .../undefined-required-properties-3.0.x.yml | 26 + .../undefined-required-properties-3.1.x.yml | 26 + .../valid-schema-3.0.x.yml | 34 + test/validation-nested-refs.spec.js | 575 ----------------- validation.js | 25 +- 24 files changed, 2580 insertions(+), 663 deletions(-) create mode 100644 functions/validateRequiredProperties.js create mode 100644 functions/validateRequiredProperties.spec.js create mode 100644 test/integration/broken-internal-refs.test.js create mode 100644 test/integration/broken-refs-yaml.test.js create mode 100644 test/integration/undefined-properties-yaml.test.js create mode 100644 test/integration/undefined-required-properties.test.js create mode 100644 test/resources/undefined-required-properties/base-permit-deny-entry-3.0.x.yml create mode 100644 test/resources/undefined-required-properties/comprehensive-test-3.0.x.yml create mode 100644 test/resources/undefined-required-properties/device-interface-complete-3.0.x.yml create mode 100644 test/resources/undefined-required-properties/device-interface-scope-3.0.x.yml create mode 100644 test/resources/undefined-required-properties/reports-summary-complete-3.0.x.yml create mode 100644 test/resources/undefined-required-properties/request-response-schemas-3.0.x.yml create mode 100644 test/resources/undefined-required-properties/security-policies-complete-3.0.x.yml create mode 100644 test/resources/undefined-required-properties/smart-switch-complete-3.0.x.yml create mode 100644 test/resources/undefined-required-properties/smart-switch-integration-3.0.x.yml create mode 100644 test/resources/undefined-required-properties/summary-properties-3.0.x.yml create mode 100644 test/resources/undefined-required-properties/undefined-required-properties-3.0.x.yml create mode 100644 test/resources/undefined-required-properties/undefined-required-properties-3.1.x.yml create mode 100644 test/resources/undefined-required-properties/valid-schema-3.0.x.yml delete mode 100644 test/validation-nested-refs.spec.js diff --git a/NESTED_REFS_SOLUTION.md b/NESTED_REFS_SOLUTION.md index afeefc1..6f22b0b 100644 --- a/NESTED_REFS_SOLUTION.md +++ b/NESTED_REFS_SOLUTION.md @@ -2,36 +2,59 @@ ## Problem Statement -The Spectral validator `validation.js` was not catching the broken `$ref` references like `#/components/schemas/PolicySeverity` in the [spec file](/test/resources/broken-refs/broken-internal-refs-3.0.x.yml), specifically when they appeared in nested structures within sibling properties of another `$ref`. +The Spectral validator `validation.js` was not catching two types of issues: + +1. **Broken `$ref` references** like `#/components/schemas/PolicySeverity` in nested structures within sibling properties of another `$ref` +2. **Undefined required properties** where properties are listed in the `required` array but not defined in the `properties` object + The original issue #48 reported is [here](https://wwwin-github.cisco.com/DevNet/api-insights-support/issues/48). ## Root Cause Analysis -### The Issue +### Issue 1: Broken Nested References In OpenAPI 3.1.x, the specification allows `$ref` to have sibling properties at the same level while OAS 3.0.x doesn't allow this: ```yaml commonAnomaly: $ref: '#/components/schemas/CommonAnomalyPolicy' - severities: # Sibling property - valid in OAS 3.1.x but invalid in OAS 3.0.x and are genrally ignored by toolings + severities: # Sibling property - valid in OAS 3.1.x but invalid in OAS 3.0.x type: array items: $ref: '#/components/schemas/PolicySeverity' # Nested ref ``` -The broken refrence `#/components/schemas/PolicySeverity` was not being caught in both OAS 3.0.x and 3.1.x specs. Ideally, a broken reference should be reported as `invalid-ref` error as part of `oas3-schema ruleset`. -### Why It Wasn't Caught +The broken reference `#/components/schemas/PolicySeverity` was not being caught in both OAS 3.0.x and 3.1.x specs. -1. **In OAS 3.0.x**: With respect to the parent commonAnomaly, Spectral simply ignored sibling property `severities` of `#ref`. Hence, validation never reached to `#/components/schemas/PolicySeverity`. -2. **In OAS 3.1.x**: Spectral resolves `$ref` references during document parsing. When a `$ref` has sibling properties, Spectral merges them during resolution. By the time inbuilt `invalid-ref` validation rule runs, the nested and broken `$ref` has been absorbed into the resolved schema structure and hence is not caught as an error. +### Issue 2: Undefined Required Properties -## Solution Implemented +Properties listed in the `required` array but not defined in the `properties` object: + +```yaml +TestSchema: + type: object + properties: + commonAnomaly: + type: object + # severities property is missing but listed in required + required: + - commonAnomaly + - severities # This property is required but not defined in properties +``` + +### Why These Issues Weren't Caught + +1. **Broken References**: + - **In OAS 3.0.x**: Spectral ignored sibling properties of `$ref`, so validation never reached nested `$ref` + - **In OAS 3.1.x**: Spectral resolves `$ref` references during parsing, merging siblings. By the time validation runs, nested broken `$ref` is absorbed into resolved schema + +2. **Undefined Required Properties**: + - Spectral's default `oas3-schema` ruleset does not validate that all properties in `required` array are defined in `properties` object + - This is a common issue in real-world OpenAPI specifications -### 1. Added a rule `no-$ref-siblings` in validation.js -It warns that `$ref must not be placed next to any other properties` in case of OAS 3.0.x specs. +## Solution Implemented -### 2. Created `validateRefSiblings.js` Preprocessor +### 1. `broken-internal-refs` Rule **File**: [functions/validateRefSiblings.js](/functions/validateRefSiblings.js) @@ -42,7 +65,7 @@ It warns that `$ref must not be placed next to any other properties` in case of 2. Identifies objects that have both a `$ref` and sibling properties 3. Recursively searches sibling properties for nested `$ref` references 4. Validates each nested `$ref` against the document data -5. Reports broken references with detailed path information +5. Reports broken references with clean error messages **Key Features**: - Works with `resolved: false` flag to run before reference resolution @@ -50,74 +73,88 @@ It warns that `$ref must not be placed next to any other properties` in case of - Supports arrays and complex object hierarchies - Ignores external references (HTTP/HTTPS/file paths) - Only validates internal references (starting with `#/`) +- Clean error message format: `"broken reference '#/components/schemas/Reference'"` + +### 2. `undefined-required-properties` Rule + +**File**: [functions/validateRequiredProperties.js](/functions/validateRequiredProperties.js) + +**Purpose**: Validates that all properties listed in the `required` array are defined in the `properties` object. + +**How It Works**: +1. Identifies object schemas with both `required` array and `properties` object +2. Compares required properties against defined properties +3. Reports missing properties with clear error messages +4. Works across all schema locations (components, responses, request bodies, parameters) + +**Key Features**: +- Validates all OpenAPI schema locations +- Clean error message format: `"'propertyName' is not defined"` +- Handles edge cases (empty arrays, non-object schemas) +- Works with both OpenAPI 3.0.x and 3.1.x ### 3. Updated `validation.js` Ruleset **File**: [validation.js](/validation.js) **Changes**: -- Added import for `validateRefSiblings` -- Created new rule: `broken-refs-in-siblings` -- Configured with `resolved: false` to run on unresolved document -- Uses `$..' JSONPath to match all objects -- Error severity for broken references - +- Added import for `validateRefSiblings` and `validateRequiredProperties` +- Created `broken-internal-refs` rule with `resolved: false` +- Created `undefined-required-properties` rule for comprehensive schema validation +- Configured both rules with appropriate `given` paths and error severity ### 4. Comprehensive Test Suites -#### Unit Tests: `validateRefSiblings.spec.js` +#### Unit Tests -**File**: [functions/validateRefSiblings.spec.js](/functions/validateRefSiblings.spec.js) +**Files**: +- [functions/validateRefSiblings.spec.js](/functions/validateRefSiblings.spec.js) +- [functions/validateRequiredProperties.spec.js](/functions/validateRequiredProperties.spec.js) **Coverage**: -- Basic validation (null inputs, missing $ref, no siblings) -- Valid nested references (should pass) -- Broken nested references (should fail) -- Multiple broken references +- Basic validation (null inputs, missing data) +- Valid references/properties (should pass) +- Broken references/missing properties (should fail) +- Multiple errors in single schema - Deeply nested structures - External references (should be ignored) - Edge cases (null values, primitives, empty objects) -#### Integration Tests: `validation-nested-refs.spec.js` +#### Integration Tests -**File**: [test/validation-nested-refs.spec.js](/test/validation-nested-refs.spec.js) +**Files**: +- [test/integration/broken-internal-refs.test.js](/test/integration/broken-internal-refs.test.js) +- [test/integration/broken-refs-yaml.test.js](/test/integration/broken-refs-yaml.test.js) +- [test/integration/undefined-required-properties.test.js](/test/integration/undefined-required-properties.test.js) +- [test/integration/undefined-properties-yaml.test.js](/test/integration/undefined-properties-yaml.test.js) **Coverage**: -- OpenAPI 3.0.3 scenarios -- OpenAPI 3.1.0 scenarios (where $ref+siblings is valid per spec) -- Real-world ThresholdAnomaly/PolicySeverity scenario -- Multiple broken references -- Mixed valid and broken references +- OpenAPI 3.0.3 and 3.1.0 scenarios +- Real-world scenarios from `manage.yaml` +- Request/response/parameter schema validation +- Mixed valid and invalid scenarios - External references handling -- Edge cases and complex nested structures - +- Complex nested structures #### Test Resource Files -**Directory**: [test/resources/broken-refs/](/test/resources/broken-refs/) - -Test files for different scenarios: - -1. [broken-internal-refs-3.0.x.yml](/test/resources/broken-refs/broken-internal-refs-3.0.x.yml) - - OpenAPI 3.0.3 spec with broken `PolicySeverity` reference - - Has `$ref` with sibling properties (technically invalid in 3.0.3) - - Used to verify preprocessor catches broken nested refs - -2. [broken-internal-refs-3.1.x.yml](/test/resources/broken-refs/broken-internal-refs-3.1.x.yml) - - OpenAPI 3.1.0 spec with broken `PolicySeverity` reference - - Has `$ref` with sibling properties (valid in 3.1.0) - - Used to verify preprocessor catches broken nested refs - -3. [no-ref-siblings-3.0.x.yml](/test/resources/broken-refs/no-ref-siblings-3.0.x.yml) - - OpenAPI 3.0.3 spec with valid `PolicySeverity` reference - - Has `$ref` with sibling properties - - Used to verify no false positives when ref is valid +**Directory**: [test/resources/](/test/resources/) -4. [no-ref-siblings-3.1.x.yml](/test/resources/broken-refs/no-ref-siblings-3.1.x.yml) - - OpenAPI 3.1.0 spec with valid `PolicySeverity` reference - - Has `$ref` with sibling properties - - Used to verify no false positives when ref is valid +**Broken References Tests**: +- [broken-refs/broken-internal-refs-3.0.x.yml](/test/resources/broken-refs/broken-internal-refs-3.0.x.yml) +- [broken-refs/broken-internal-refs-3.1.x.yml](/test/resources/broken-refs/broken-internal-refs-3.1.x.yml) +- [broken-refs/no-ref-siblings-3.0.x.yml](/test/resources/broken-refs/no-ref-siblings-3.0.x.yml) +- [broken-refs/no-ref-siblings-3.1.x.yml](/test/resources/broken-refs/no-ref-siblings-3.1.x.yml) +**Undefined Required Properties Tests**: +- [undefined-required-properties/undefined-required-properties-3.0.x.yml](/test/resources/undefined-required-properties/undefined-required-properties-3.0.x.yml) +- [undefined-required-properties/undefined-required-properties-3.1.x.yml](/test/resources/undefined-required-properties/undefined-required-properties-3.1.x.yml) +- [undefined-required-properties/device-interface-scope-3.0.x.yml](/test/resources/undefined-required-properties/device-interface-scope-3.0.x.yml) +- [undefined-required-properties/smart-switch-integration-3.0.x.yml](/test/resources/undefined-required-properties/smart-switch-integration-3.0.x.yml) +- [undefined-required-properties/security-policies-complete-3.0.x.yml](/test/resources/undefined-required-properties/security-policies-complete-3.0.x.yml) +- [undefined-required-properties/reports-summary-complete-3.0.x.yml](/test/resources/undefined-required-properties/reports-summary-complete-3.0.x.yml) +- [undefined-required-properties/comprehensive-test-3.0.x.yml](/test/resources/undefined-required-properties/comprehensive-test-3.0.x.yml) +- [undefined-required-properties/valid-schema-3.0.x.yml](/test/resources/undefined-required-properties/valid-schema-3.0.x.yml) ## OpenAPI Version Differences @@ -125,30 +162,39 @@ Test files for different scenarios: - `$ref` **SHOULD** be the only property at its level per specification - Having sibling properties is **discouraged** but parsers may allow it -- Our preprocessor **works** and catches broken nested refs +- Both rules **work** and catch issues effectively ### OpenAPI 3.1.x - `$ref` **CAN** have sibling properties - Structure is **VALID** per specification -- Our preprocessor is **ESSENTIAL** to catch nested broken refs +- Both rules are **ESSENTIAL** to catch nested broken refs and undefined properties ## Usage ### Testing the Solution ```bash -# Test on OpenAPI 3.0.3 with broken refs +# Test broken references on OpenAPI 3.0.3 spectral lint -r validation.js test/resources/broken-refs/broken-internal-refs-3.0.x.yml -# Test on OpenAPI 3.1.0 with broken refs +# Test broken references on OpenAPI 3.1.0 spectral lint -r validation.js test/resources/broken-refs/broken-internal-refs-3.1.x.yml +# Test undefined required properties +spectral lint -r validation.js test/resources/undefined-required-properties/undefined-required-properties-3.0.x.yml + # Run unit tests npm test -- functions/validateRefSiblings.spec.js +npm test -- functions/validateRequiredProperties.spec.js # Run integration tests -npm test -- test/validation-nested-refs.spec.js +npm test -- test/integration/broken-internal-refs.test.js +npm test -- test/integration/undefined-required-properties.test.js +npm test -- test/integration/undefined-properties-yaml.test.js + +# Run all tests +npm test ``` ### Expected Behavior @@ -158,34 +204,50 @@ npm test -- test/validation-nested-refs.spec.js $ spectral lint -r validation.js test/resources/broken-refs/broken-internal-refs-3.0.x.yml /path/to/broken-internal-refs-3.0.x.yml - 22:23 error broken-refs-in-siblings Broken reference in sibling property 'severities.items.$ref': #/components/schemas/PolicySeverity components.schemas.TestSchema.properties.commonAnomaly - 24:22 warning no-$ref-siblings $ref must not be placed next to any other properties components.schemas.TestSchema.properties.commonAnomaly.severities - ✖ 2 problems (1 error, 1 warning, 0 infos, 0 hints) + 22:23 error broken-internal-refs internal references should exist; broken reference '#/components/schemas/PolicySeverity' components.schemas.TestSchema.properties.commonAnomaly + ✖ 1 problem (1 error, 0 warnings, 0 infos, 0 hints) ``` + +**For files with undefined required properties**: ```bash -$ spectral lint -r validation.js test/resources/broken-refs/broken-internal-refs-3.1.x.yml +$ spectral lint -r validation.js test/resources/undefined-required-properties/undefined-required-properties-3.0.x.yml -/path/to/broken-internal-refs-3.1.x.yml - 22:23 error broken-refs-in-siblings Broken reference in sibling property 'severities.items.$ref': #/components/schemas/PolicySeverity - ✖ 1 problem (1 error, 0 warnings, 0 infos, 0 hints) +/path/to/undefined-required-properties-3.0.x.yml + 17:22 error undefined-required-properties required properties must be defined; 'severities' is not defined components.schemas.ThresholdAnomaly + ✖ 1 problem (1 error, 0 warnings, 0 infos, 0 hints) ``` -**For files with valid refs**: +**For files with valid refs and properties**: ```bash $ spectral lint -r validation.js test/resources/broken-refs/no-ref-siblings-3.0.x.yml -26:22 warning no-$ref-siblings $ref must not be placed next to any other properties components.schemas.TestSchema.properties.commonAnomaly.severities -✖ 1 problem (0 error, 1 warnings, 0 infos, 0 hints) +No results with a severity of 'error' found! ``` + +## Real-World Impact + +### Issues Found in `manage.yaml` openAPI spec that was shared along with [issue #48](https://wwwin-github.cisco.com/DevNet/api-insights-support/issues/48) + +The rules successfully detect real-world issues: + ```bash -$ spectral lint -r validation.js test/resources/broken-refs/no-ref-siblings-3.0.x.yml -No results with a severity of 'error' found! +$ spectral lint -r validation.js manage.yaml + +/path/to/manage.yaml + 11980:22 error undefined-required-properties required properties must be defined; 'severities' is not defined components.schemas.ThresholdAnomaly + 11988:23 error broken-internal-refs internal references should point to existing components; broken reference '#/components/schemas/PolicySeverity' components.schemas.ThresholdAnomaly.properties.commonAnomaly + 14023:26 error undefined-required-properties required properties must be defined; 'switchid' is not defined components.schemas.deviceInterfaceScope + 16736:34 error undefined-required-properties required properties must be defined; 'switchName' is not defined components.schemas.smartSwitchIntegrationIdData + 22814:25 error undefined-required-properties required properties must be defined; 'protocol' is not defined components.schemas.basePermitDenyEntry + 25083:23 error undefined-required-properties required properties must be defined; 'type' is not defined components.schemas.summaryProperties + +✖ 6 problems (6 errors, 0 warnings, 0 infos, 0 hints) ``` ## Technical Details ### The `resolved: false` Flag -This is the **critical** configuration that makes the solution work: +This is the **critical** configuration that makes the broken references solution work: ```javascript 'resolved': false // Run on unresolved document @@ -193,3 +255,4 @@ This is the **critical** configuration that makes the solution work: - **Without this**: Rule runs on resolved document, nested refs are already merged and hidden - **With this**: Rule runs on raw parsed YAML, before Spectral processes any `$ref` references + diff --git a/functions/validateRefSiblings.js b/functions/validateRefSiblings.js index 9a9a87f..09d6453 100644 --- a/functions/validateRefSiblings.js +++ b/functions/validateRefSiblings.js @@ -127,7 +127,7 @@ function validateRef(ref, documentData, pathContext) { } else { // Broken reference found return { - message: `Broken reference in sibling property '${pathContext}': ${ref}`, + message: `broken reference '${ref}'`, }; } } diff --git a/functions/validateRefSiblings.spec.js b/functions/validateRefSiblings.spec.js index 6339bb6..d0df260 100644 --- a/functions/validateRefSiblings.spec.js +++ b/functions/validateRefSiblings.spec.js @@ -175,7 +175,7 @@ describe('validateRefSiblings', () => { const result = validateRefSiblings(input, {}, mockContext); expect(result).toEqual([ { - message: "Broken reference in sibling property 'additionalProp.$ref': #/components/schemas/NonExistent" + message: "broken reference '#/components/schemas/NonExistent'" } ]); }); @@ -193,7 +193,7 @@ describe('validateRefSiblings', () => { const result = validateRefSiblings(input, {}, mockContext); expect(result).toEqual([ { - message: "Broken reference in sibling property 'severities.items.$ref': #/components/schemas/PolicySeverity" + message: "broken reference '#/components/schemas/PolicySeverity'" } ]); }); @@ -211,10 +211,10 @@ describe('validateRefSiblings', () => { const result = validateRefSiblings(input, {}, mockContext); expect(result).toHaveLength(2); expect(result).toContainEqual({ - message: "Broken reference in sibling property 'prop1.$ref': #/components/schemas/NonExistent1" + message: "broken reference '#/components/schemas/NonExistent1'" }); expect(result).toContainEqual({ - message: "Broken reference in sibling property 'prop2.$ref': #/components/schemas/NonExistent2" + message: "broken reference '#/components/schemas/NonExistent2'" }); }); @@ -234,7 +234,7 @@ describe('validateRefSiblings', () => { const result = validateRefSiblings(input, {}, mockContext); expect(result).toEqual([ { - message: "Broken reference in sibling property 'properties.nested.deep.value.$ref': #/components/schemas/DeepNonExistent" + message: "broken reference '#/components/schemas/DeepNonExistent'" } ]); }); @@ -251,7 +251,7 @@ describe('validateRefSiblings', () => { const result = validateRefSiblings(input, {}, mockContext); expect(result).toEqual([ { - message: "Broken reference in sibling property 'arrayProp[0].$ref': #/components/schemas/NonExistent" + message: "broken reference '#/components/schemas/NonExistent'" } ]); }); @@ -272,7 +272,7 @@ describe('validateRefSiblings', () => { expect(result).toHaveLength(1); expect(result).toEqual([ { - message: "Broken reference in sibling property 'brokenProp.$ref': #/components/schemas/NonExistent" + message: "broken reference '#/components/schemas/NonExistent'" } ]); }); @@ -300,7 +300,7 @@ describe('validateRefSiblings', () => { expect(result).toHaveLength(1); expect(result).toEqual([ { - message: "Broken reference in sibling property 'policies.items.$ref': #/components/schemas/Policy" + message: "broken reference '#/components/schemas/Policy'" } ]); }); @@ -354,7 +354,7 @@ describe('validateRefSiblings', () => { expect(result).toHaveLength(1); expect(result).toEqual([ { - message: "Broken reference in sibling property 'internalBroken.$ref': #/components/schemas/NonExistent" + message: "broken reference '#/components/schemas/NonExistent'" } ]); }); @@ -408,7 +408,7 @@ describe('validateRefSiblings', () => { const result = validateRefSiblings(input, {}, mockContext); expect(result).toEqual([ { - message: "Broken reference in sibling property 'brokenPath.$ref': #/nonexistent/path/schema" + message: "broken reference '#/nonexistent/path/schema'" } ]); }); diff --git a/functions/validateRequiredProperties.js b/functions/validateRequiredProperties.js new file mode 100644 index 0000000..4daa9d3 --- /dev/null +++ b/functions/validateRequiredProperties.js @@ -0,0 +1,58 @@ +/** + * Copyright 2022 Cisco Systems, Inc. and its affiliates. + * + * Licensed under the Apache License, Version 2.0 (the "License"); + * you may not use this file except in compliance with the License. + * You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE/2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + * + * SPDX-License-Identifier: Apache-2.0 + */ + +/** + * Validates that all properties listed in the required array are defined in the properties object + * @param {Object} input - The schema object to validate + * @param {Object} options - Function options + * @param {Object} context - Spectral context with document and path info + * @returns {Array} Array of validation results + */ +export default function validateRequiredProperties(input, options, context) { + if (!input || typeof input !== 'object') { + return []; + } + + // Only validate object schemas + if (input.type && input.type !== 'object') { + return []; + } + + // Skip if no required array or properties object + if (!input.required || !Array.isArray(input.required)) { + return []; + } + + if (!input.properties || typeof input.properties !== 'object') { + return []; + } + + const results = []; + const definedProperties = Object.keys(input.properties); + + // Check each required property + for (const requiredProp of input.required) { + if (!definedProperties.includes(requiredProp)) { + results.push({ + message: `'${requiredProp}' is not defined`, + }); + } + } + + return results; +} diff --git a/functions/validateRequiredProperties.spec.js b/functions/validateRequiredProperties.spec.js new file mode 100644 index 0000000..b100388 --- /dev/null +++ b/functions/validateRequiredProperties.spec.js @@ -0,0 +1,222 @@ +/** + * Copyright 2022 Cisco Systems, Inc. and its affiliates. + * + * Licensed under the Apache License, Version 2.0 (the "License"); + * you may not use this file except in compliance with the License. + * You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + * + * SPDX-License-Identifier: Apache-2.0 + */ + +import { + describe, expect, it, +} from '@jest/globals'; +import validateRequiredProperties from './validateRequiredProperties.js'; + +describe('validateRequiredProperties', () => { + const mockContext = { + document: { + data: {} + } + }; + + describe('basic validation', () => { + it('should return empty array when input is null', () => { + const result = validateRequiredProperties(null, {}, mockContext); + expect(result).toEqual([]); + }); + + it('should return empty array when input is not an object', () => { + const result = validateRequiredProperties('string', {}, mockContext); + expect(result).toEqual([]); + }); + + it('should return empty array when schema is not an object type', () => { + const input = { + type: 'string', + required: ['name'], + properties: { + name: { type: 'string' } + } + }; + const result = validateRequiredProperties(input, {}, mockContext); + expect(result).toEqual([]); + }); + + it('should return empty array when no required array', () => { + const input = { + type: 'object', + properties: { + name: { type: 'string' } + } + }; + const result = validateRequiredProperties(input, {}, mockContext); + expect(result).toEqual([]); + }); + + it('should return empty array when no properties object', () => { + const input = { + type: 'object', + required: ['name'] + }; + const result = validateRequiredProperties(input, {}, mockContext); + expect(result).toEqual([]); + }); + + it('should return empty array when all required properties are defined', () => { + const input = { + type: 'object', + required: ['name', 'email'], + properties: { + name: { type: 'string' }, + email: { type: 'string' } + } + }; + const result = validateRequiredProperties(input, {}, mockContext); + expect(result).toEqual([]); + }); + }); + + describe('missing required properties', () => { + it('should detect single missing required property', () => { + const input = { + type: 'object', + required: ['name', 'email'], + properties: { + name: { type: 'string' } + // email is missing + } + }; + const result = validateRequiredProperties(input, {}, mockContext); + expect(result).toEqual([ + { + message: "'email' is not defined" + } + ]); + }); + + it('should detect multiple missing required properties', () => { + const input = { + type: 'object', + required: ['name', 'email', 'age'], + properties: { + name: { type: 'string' } + // email and age are missing + } + }; + const result = validateRequiredProperties(input, {}, mockContext); + expect(result).toHaveLength(2); + expect(result).toContainEqual({ + message: "'email' is not defined" + }); + expect(result).toContainEqual({ + message: "'age' is not defined" + }); + }); + + it('should detect missing required property when some are defined', () => { + const input = { + type: 'object', + required: ['name', 'email', 'age'], + properties: { + name: { type: 'string' }, + age: { type: 'number' } + // email is missing + } + }; + const result = validateRequiredProperties(input, {}, mockContext); + expect(result).toEqual([ + { + message: "'email' is not defined" + } + ]); + }); + + it('should detect missing required property with complex schema', () => { + const input = { + type: 'object', + required: ['commonAnomaly', 'severities'], + properties: { + commonAnomaly: { + $ref: '#/components/schemas/CommonAnomalyPolicy' + } + // severities is missing + } + }; + const result = validateRequiredProperties(input, {}, mockContext); + expect(result).toEqual([ + { + message: "'severities' is not defined" + } + ]); + }); + }); + + describe('edge cases', () => { + it('should handle empty required array', () => { + const input = { + type: 'object', + required: [], + properties: { + name: { type: 'string' } + } + }; + const result = validateRequiredProperties(input, {}, mockContext); + expect(result).toEqual([]); + }); + + it('should handle empty properties object', () => { + const input = { + type: 'object', + required: ['name'], + properties: {} + }; + const result = validateRequiredProperties(input, {}, mockContext); + expect(result).toEqual([ + { + message: "'name' is not defined" + } + ]); + }); + + it('should handle required array with duplicate properties', () => { + const input = { + type: 'object', + required: ['name', 'name', 'email'], + properties: { + name: { type: 'string' } + // email is missing + } + }; + const result = validateRequiredProperties(input, {}, mockContext); + expect(result).toEqual([ + { + message: "'email' is not defined" + } + ]); + }); + + it('should handle schema without explicit type (defaults to object)', () => { + const input = { + required: ['name'], + properties: { + // name is missing + } + }; + const result = validateRequiredProperties(input, {}, mockContext); + expect(result).toEqual([ + { + message: "'name' is not defined" + } + ]); + }); + }); +}); diff --git a/test/integration/broken-internal-refs.test.js b/test/integration/broken-internal-refs.test.js new file mode 100644 index 0000000..e121ba7 --- /dev/null +++ b/test/integration/broken-internal-refs.test.js @@ -0,0 +1,598 @@ +/** + * Copyright 2022 Cisco Systems, Inc. and its affiliates. + * + * Licensed under the Apache License, Version 2.0 (the "License"); + * you may not use this file except in compliance with the License. + * You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE/2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + * + * SPDX-License-Identifier: Apache-2.0 + */ + +const { describe, expect, it } = require('@jest/globals'); +const { Spectral } = require('@stoplight/spectral-core'); +const ruleset = require('../../validation.js').default; + +describe('Integration Tests - broken-internal-refs', () => { + let spectral; + + beforeEach(() => { + spectral = new Spectral(); + spectral.setRuleset(ruleset); + }); + + describe('should detect broken internal references', () => { + it('should catch broken $ref in sibling properties', async () => { + const document = { + openapi: '3.0.3', + info: { + title: 'Test API', + version: '1.0.0' + }, + paths: { + '/test': { + get: { + responses: { + '200': { + description: 'OK', + content: { + 'application/json': { + schema: { + $ref: '#/components/schemas/TestSchema' + } + } + } + } + } + } + } + }, + components: { + schemas: { + CommonAnomalyPolicy: { + type: 'object', + properties: { + name: { type: 'string' } + } + }, + TestSchema: { + type: 'object', + properties: { + commonAnomaly: { + $ref: '#/components/schemas/CommonAnomalyPolicy', + severities: { + type: 'array', + items: { + $ref: '#/components/schemas/PolicySeverity' // This reference is broken + } + } + } + } + } + } + } + }; + + const results = await spectral.run(document); + + const brokenRefErrors = results.filter( + result => result.code === 'broken-internal-refs' + ); + + expect(brokenRefErrors.length).toBeGreaterThanOrEqual(1); + + // Check that we have the expected error message + const hasPolicySeverityError = brokenRefErrors.some( + error => error.message.includes("broken reference '#/components/schemas/PolicySeverity'") + ); + expect(hasPolicySeverityError).toBe(true); + }); + + it('should catch multiple broken $refs in different locations', async () => { + const document = { + openapi: '3.0.3', + info: { + title: 'Test API', + version: '1.0.0' + }, + paths: { + '/test': { + get: { + responses: { + '200': { + description: 'OK', + content: { + 'application/json': { + schema: { + $ref: '#/components/schemas/TestSchema' + } + } + } + } + } + } + } + }, + components: { + schemas: { + ValidSchema: { + type: 'object', + properties: { + name: { type: 'string' } + } + }, + TestSchema: { + type: 'object', + properties: { + prop1: { + $ref: '#/components/schemas/ValidSchema', + nested: { + $ref: '#/components/schemas/NonExistent1' // Broken reference 1 + } + }, + prop2: { + $ref: '#/components/schemas/ValidSchema', + items: { + type: 'array', + items: { + $ref: '#/components/schemas/NonExistent2' // Broken reference 2 + } + } + } + } + } + } + } + }; + + const results = await spectral.run(document); + + const brokenRefErrors = results.filter( + result => result.code === 'broken-internal-refs' + ); + + expect(brokenRefErrors.length).toBeGreaterThanOrEqual(2); + + // Check that we have the expected error messages + const errorMessages = brokenRefErrors.map(error => error.message); + const hasNonExistent1Error = errorMessages.some(msg => + msg.includes("broken reference '#/components/schemas/NonExistent1'") + ); + const hasNonExistent2Error = errorMessages.some(msg => + msg.includes("broken reference '#/components/schemas/NonExistent2'") + ); + + expect(hasNonExistent1Error).toBe(true); + expect(hasNonExistent2Error).toBe(true); + }); + + it('should catch broken $refs in deeply nested structures', async () => { + const document = { + openapi: '3.0.3', + info: { + title: 'Test API', + version: '1.0.0' + }, + paths: { + '/test': { + get: { + responses: { + '200': { + description: 'OK', + content: { + 'application/json': { + schema: { + $ref: '#/components/schemas/TestSchema' + } + } + } + } + } + } + } + }, + components: { + schemas: { + ValidSchema: { + type: 'object', + properties: { + name: { type: 'string' } + } + }, + TestSchema: { + type: 'object', + properties: { + data: { + $ref: '#/components/schemas/ValidSchema', + nested: { + deep: { + deeper: { + value: { + $ref: '#/components/schemas/DeepNonExistent' // Deeply nested broken reference + } + } + } + } + } + } + } + } + } + }; + + const results = await spectral.run(document); + + const brokenRefErrors = results.filter( + result => result.code === 'broken-internal-refs' + ); + + expect(brokenRefErrors.length).toBeGreaterThanOrEqual(1); + + // Check that we have the expected error message + const hasDeepError = brokenRefErrors.some( + error => error.message.includes("broken reference '#/components/schemas/DeepNonExistent'") + ); + expect(hasDeepError).toBe(true); + }); + + it('should catch broken $refs in array items', async () => { + const document = { + openapi: '3.0.3', + info: { + title: 'Test API', + version: '1.0.0' + }, + paths: { + '/test': { + get: { + responses: { + '200': { + description: 'OK', + content: { + 'application/json': { + schema: { + $ref: '#/components/schemas/TestSchema' + } + } + } + } + } + } + } + }, + components: { + schemas: { + ValidSchema: { + type: 'object', + properties: { + name: { type: 'string' } + } + }, + TestSchema: { + type: 'object', + properties: { + items: { + $ref: '#/components/schemas/ValidSchema', + arrayProp: [ + { + $ref: '#/components/schemas/ArrayNonExistent' // Broken reference in array + } + ] + } + } + } + } + } + }; + + const results = await spectral.run(document); + + const brokenRefErrors = results.filter( + result => result.code === 'broken-internal-refs' + ); + + expect(brokenRefErrors.length).toBeGreaterThanOrEqual(1); + + // Check that we have the expected error message + const hasArrayError = brokenRefErrors.some( + error => error.message.includes("broken reference '#/components/schemas/ArrayNonExistent'") + ); + expect(hasArrayError).toBe(true); + }); + + it('should catch broken $refs in request body schemas', async () => { + const document = { + openapi: '3.0.3', + info: { + title: 'Test API', + version: '1.0.0' + }, + paths: { + '/test': { + post: { + requestBody: { + content: { + 'application/json': { + schema: { + $ref: '#/components/schemas/ValidSchema', + additionalProp: { + $ref: '#/components/schemas/RequestNonExistent' // Broken reference in request body + } + } + } + } + }, + responses: { + '201': { + description: 'Created' + } + } + } + } + }, + components: { + schemas: { + ValidSchema: { + type: 'object', + properties: { + name: { type: 'string' } + } + } + } + } + }; + + const results = await spectral.run(document); + + const brokenRefErrors = results.filter( + result => result.code === 'broken-internal-refs' + ); + + expect(brokenRefErrors.length).toBeGreaterThanOrEqual(1); + + // Check that we have the expected error message + const hasRequestError = brokenRefErrors.some( + error => error.message.includes("broken reference '#/components/schemas/RequestNonExistent'") + ); + expect(hasRequestError).toBe(true); + }); + + it('should catch broken $refs in response schemas', async () => { + const document = { + openapi: '3.0.3', + info: { + title: 'Test API', + version: '1.0.0' + }, + paths: { + '/test': { + get: { + responses: { + '200': { + description: 'OK', + content: { + 'application/json': { + schema: { + $ref: '#/components/schemas/ValidSchema', + responseProp: { + $ref: '#/components/schemas/ResponseNonExistent' // Broken reference in response + } + } + } + } + } + } + } + } + }, + components: { + schemas: { + ValidSchema: { + type: 'object', + properties: { + name: { type: 'string' } + } + } + } + } + }; + + const results = await spectral.run(document); + + const brokenRefErrors = results.filter( + result => result.code === 'broken-internal-refs' + ); + + expect(brokenRefErrors.length).toBeGreaterThanOrEqual(1); + + // Check that we have the expected error message + const hasResponseError = brokenRefErrors.some( + error => error.message.includes("broken reference '#/components/schemas/ResponseNonExistent'") + ); + expect(hasResponseError).toBe(true); + }); + }); + + describe('should not flag valid references', () => { + it('should pass when all $refs are valid', async () => { + const document = { + openapi: '3.0.3', + info: { + title: 'Test API', + version: '1.0.0' + }, + paths: { + '/test': { + get: { + responses: { + '200': { + description: 'OK', + content: { + 'application/json': { + schema: { + $ref: '#/components/schemas/TestSchema' + } + } + } + } + } + } + } + }, + components: { + schemas: { + ValidSchema1: { + type: 'object', + properties: { + name: { type: 'string' } + } + }, + ValidSchema2: { + type: 'object', + properties: { + id: { type: 'string' } + } + }, + TestSchema: { + type: 'object', + properties: { + prop1: { + $ref: '#/components/schemas/ValidSchema1', + nested: { + $ref: '#/components/schemas/ValidSchema2' + } + } + } + } + } + } + }; + + const results = await spectral.run(document); + + const brokenRefErrors = results.filter( + result => result.code === 'broken-internal-refs' + ); + + expect(brokenRefErrors).toHaveLength(0); + }); + + it('should pass when $ref has no sibling properties', async () => { + const document = { + openapi: '3.0.3', + info: { + title: 'Test API', + version: '1.0.0' + }, + paths: { + '/test': { + get: { + responses: { + '200': { + description: 'OK', + content: { + 'application/json': { + schema: { + $ref: '#/components/schemas/TestSchema' + } + } + } + } + } + } + } + }, + components: { + schemas: { + ValidSchema: { + type: 'object', + properties: { + name: { type: 'string' } + } + }, + TestSchema: { + type: 'object', + properties: { + prop1: { + $ref: '#/components/schemas/ValidSchema' // No sibling properties + } + } + } + } + } + }; + + const results = await spectral.run(document); + + const brokenRefErrors = results.filter( + result => result.code === 'broken-internal-refs' + ); + + expect(brokenRefErrors).toHaveLength(0); + }); + + it('should ignore external references', async () => { + const document = { + openapi: '3.0.3', + info: { + title: 'Test API', + version: '1.0.0' + }, + paths: { + '/test': { + get: { + responses: { + '200': { + description: 'OK', + content: { + 'application/json': { + schema: { + $ref: '#/components/schemas/TestSchema' + } + } + } + } + } + } + } + }, + components: { + schemas: { + ValidSchema: { + type: 'object', + properties: { + name: { type: 'string' } + } + }, + TestSchema: { + type: 'object', + properties: { + prop1: { + $ref: '#/components/schemas/ValidSchema', + externalRef: { + $ref: 'http://example.com/schema.json' // External reference - should be ignored + } + } + } + } + } + } + }; + + const results = await spectral.run(document); + + const brokenRefErrors = results.filter( + result => result.code === 'broken-internal-refs' + ); + + expect(brokenRefErrors).toHaveLength(0); + }); + }); +}); diff --git a/test/integration/broken-refs-yaml.test.js b/test/integration/broken-refs-yaml.test.js new file mode 100644 index 0000000..aad816b --- /dev/null +++ b/test/integration/broken-refs-yaml.test.js @@ -0,0 +1,174 @@ +/** + * Copyright 2022 Cisco Systems, Inc. and its affiliates. + * + * Licensed under the Apache License, Version 2.0 (the "License"); + * you may not use this file except in compliance with the License. + * You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + * + * SPDX-License-Identifier: Apache-2.0 + */ + +const { describe, expect, it } = require('@jest/globals'); +const { Spectral } = require('@stoplight/spectral-core'); +const { readFileSync } = require('fs'); +const { join } = require('path'); +const ruleset = require('../../validation.js').default; + +describe('Integration Tests - broken-internal-refs YAML Files', () => { + let spectral; + + beforeEach(() => { + spectral = new Spectral(); + spectral.setRuleset(ruleset); + }); + + describe('broken-internal-refs rule', () => { + it('should detect broken $ref in 3.0.x file', async () => { + const yamlPath = join(__dirname, '../resources/broken-refs/broken-internal-refs-3.0.x.yml'); + const yamlContent = readFileSync(yamlPath, 'utf8'); + + const results = await spectral.run(yamlContent); + + const brokenRefErrors = results.filter( + result => result.code === 'broken-internal-refs' + ); + + expect(brokenRefErrors.length).toBeGreaterThanOrEqual(1); + + // Check that we have the expected error message + const hasPolicySeverityError = brokenRefErrors.some( + error => error.message.includes("broken reference '#/components/schemas/PolicySeverity'") + ); + expect(hasPolicySeverityError).toBe(true); + }); + + it('should detect broken $ref in 3.1.x file', async () => { + const yamlPath = join(__dirname, '../resources/broken-refs/broken-internal-refs-3.1.x.yml'); + const yamlContent = readFileSync(yamlPath, 'utf8'); + + const results = await spectral.run(yamlContent); + + const brokenRefErrors = results.filter( + result => result.code === 'broken-internal-refs' + ); + + expect(brokenRefErrors.length).toBeGreaterThanOrEqual(1); + + // Check that we have the expected error message + const hasPolicySeverityError = brokenRefErrors.some( + error => error.message.includes("broken reference '#/components/schemas/PolicySeverity'") + ); + expect(hasPolicySeverityError).toBe(true); + }); + + it('should not detect broken $ref in no-ref-siblings 3.0.x file (all refs are valid)', async () => { + const yamlPath = join(__dirname, '../resources/broken-refs/no-ref-siblings-3.0.x.yml'); + const yamlContent = readFileSync(yamlPath, 'utf8'); + + const results = await spectral.run(yamlContent); + + const brokenRefErrors = results.filter( + result => result.code === 'broken-internal-refs' + ); + + // All references in this file are valid, so no broken-internal-refs errors should occur + expect(brokenRefErrors).toHaveLength(0); + }); + + it('should not detect broken $ref in no-ref-siblings 3.1.x file (all refs are valid)', async () => { + const yamlPath = join(__dirname, '../resources/broken-refs/no-ref-siblings-3.1.x.yml'); + const yamlContent = readFileSync(yamlPath, 'utf8'); + + const results = await spectral.run(yamlContent); + + const brokenRefErrors = results.filter( + result => result.code === 'broken-internal-refs' + ); + + // All references in this file are valid, so no broken-internal-refs errors should occur + expect(brokenRefErrors).toHaveLength(0); + }); + + it('should correctly identify valid vs broken references', async () => { + // Test that the rule can distinguish between valid and broken references + const document = { + openapi: '3.0.3', + info: { + title: 'Test API', + version: '1.0.0' + }, + paths: { + '/test': { + get: { + responses: { + '200': { + description: 'OK', + content: { + 'application/json': { + schema: { + $ref: '#/components/schemas/TestSchema' + } + } + } + } + } + } + } + }, + components: { + schemas: { + ValidSchema: { + type: 'object', + properties: { + name: { type: 'string' } + } + }, + TestSchema: { + type: 'object', + properties: { + validRef: { + $ref: '#/components/schemas/ValidSchema', // This should be valid + description: 'Valid reference' + }, + brokenRef: { + $ref: '#/components/schemas/ValidSchema', + nested: { + $ref: '#/components/schemas/NonExistentSchema' // This should be broken + } + } + } + } + } + } + }; + + const results = await spectral.run(document); + + const brokenRefErrors = results.filter( + result => result.code === 'broken-internal-refs' + ); + + // Should only detect the broken reference, not the valid one + expect(brokenRefErrors.length).toBeGreaterThanOrEqual(1); + + const hasBrokenError = brokenRefErrors.some( + error => error.message.includes("broken reference '#/components/schemas/NonExistentSchema'") + ); + expect(hasBrokenError).toBe(true); + + // Should not have errors for the valid reference + const hasValidError = brokenRefErrors.some( + error => error.message.includes("broken reference '#/components/schemas/ValidSchema'") + ); + expect(hasValidError).toBe(false); + }); + }); +}); diff --git a/test/integration/undefined-properties-yaml.test.js b/test/integration/undefined-properties-yaml.test.js new file mode 100644 index 0000000..12a5a9e --- /dev/null +++ b/test/integration/undefined-properties-yaml.test.js @@ -0,0 +1,261 @@ +/** + * Copyright 2022 Cisco Systems, Inc. and its affiliates. + * + * Licensed under the Apache License, Version 2.0 (the "License"); + * you may not use this file except in compliance with the License. + * You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE/2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + * + * SPDX-License-Identifier: Apache-2.0 + */ + +const { describe, expect, it } = require('@jest/globals'); +const { Spectral } = require('@stoplight/spectral-core'); +const { readFileSync } = require('fs'); +const { join } = require('path'); +const ruleset = require('../../validation.js').default; + +describe('Integration Tests - undefined-required-properties YAML Files', () => { + let spectral; + + beforeEach(() => { + spectral = new Spectral(); + spectral.setRuleset(ruleset); + }); + + describe('undefined-required-properties rule', () => { + it('should detect undefined required properties in 3.0.x file', async () => { + const yamlPath = join(__dirname, '../resources/undefined-required-properties/undefined-required-properties-3.0.x.yml'); + const yamlContent = readFileSync(yamlPath, 'utf8'); + + const results = await spectral.run(yamlContent); + + const undefinedRequiredErrors = results.filter( + result => result.code === 'undefined-required-properties' + ); + + expect(undefinedRequiredErrors.length).toBeGreaterThanOrEqual(1); + + // Check that we have the expected error message + const hasSeveritiesError = undefinedRequiredErrors.some( + error => error.message.includes("'severities' is not defined") + ); + expect(hasSeveritiesError).toBe(true); + }); + + it('should detect undefined required properties in 3.1.x file', async () => { + const yamlPath = join(__dirname, '../resources/undefined-required-properties/undefined-required-properties-3.1.x.yml'); + const yamlContent = readFileSync(yamlPath, 'utf8'); + + const results = await spectral.run(yamlContent); + + const undefinedRequiredErrors = results.filter( + result => result.code === 'undefined-required-properties' + ); + + expect(undefinedRequiredErrors.length).toBeGreaterThanOrEqual(1); + + // Check that we have the expected error message + const hasSeveritiesError = undefinedRequiredErrors.some( + error => error.message.includes("'severities' is not defined") + ); + expect(hasSeveritiesError).toBe(true); + }); + + it('should detect undefined required properties in device interface scope', async () => { + const yamlPath = join(__dirname, '../resources/undefined-required-properties/device-interface-scope-3.0.x.yml'); + const yamlContent = readFileSync(yamlPath, 'utf8'); + + const results = await spectral.run(yamlContent); + + const undefinedRequiredErrors = results.filter( + result => result.code === 'undefined-required-properties' + ); + + expect(undefinedRequiredErrors.length).toBeGreaterThanOrEqual(1); + + // Check that we have the expected error message + const hasSwitchIdError = undefinedRequiredErrors.some( + error => error.message.includes("'switchid' is not defined") + ); + expect(hasSwitchIdError).toBe(true); + }); + + it('should detect undefined required properties in smart switch integration', async () => { + const yamlPath = join(__dirname, '../resources/undefined-required-properties/smart-switch-integration-3.0.x.yml'); + const yamlContent = readFileSync(yamlPath, 'utf8'); + + const results = await spectral.run(yamlContent); + + const undefinedRequiredErrors = results.filter( + result => result.code === 'undefined-required-properties' + ); + + expect(undefinedRequiredErrors.length).toBeGreaterThanOrEqual(1); + + // Check that we have the expected error message + const hasSwitchNameError = undefinedRequiredErrors.some( + error => error.message.includes("'switchName' is not defined") + ); + expect(hasSwitchNameError).toBe(true); + }); + + it('should detect undefined required properties in base permit deny entry', async () => { + const yamlPath = join(__dirname, '../resources/undefined-required-properties/base-permit-deny-entry-3.0.x.yml'); + const yamlContent = readFileSync(yamlPath, 'utf8'); + + const results = await spectral.run(yamlContent); + + const undefinedRequiredErrors = results.filter( + result => result.code === 'undefined-required-properties' + ); + + expect(undefinedRequiredErrors.length).toBeGreaterThanOrEqual(1); + + // Check that we have the expected error message + const hasProtocolError = undefinedRequiredErrors.some( + error => error.message.includes("'protocol' is not defined") + ); + expect(hasProtocolError).toBe(true); + }); + + it('should detect undefined required properties in summary properties', async () => { + const yamlPath = join(__dirname, '../resources/undefined-required-properties/summary-properties-3.0.x.yml'); + const yamlContent = readFileSync(yamlPath, 'utf8'); + + const results = await spectral.run(yamlContent); + + const undefinedRequiredErrors = results.filter( + result => result.code === 'undefined-required-properties' + ); + + expect(undefinedRequiredErrors.length).toBeGreaterThanOrEqual(1); + + // Check that we have the expected error message + const hasTypeError = undefinedRequiredErrors.some( + error => error.message.includes("'type' is not defined") + ); + expect(hasTypeError).toBe(true); + }); + + it('should detect undefined required properties in device interface complete scenario', async () => { + const yamlPath = join(__dirname, '../resources/undefined-required-properties/device-interface-complete-3.0.x.yml'); + const yamlContent = readFileSync(yamlPath, 'utf8'); + + const results = await spectral.run(yamlContent); + + const undefinedRequiredErrors = results.filter( + result => result.code === 'undefined-required-properties' + ); + + expect(undefinedRequiredErrors.length).toBeGreaterThanOrEqual(4); + + // Check that we have the expected error messages + const errorMessages = undefinedRequiredErrors.map(error => error.message); + const hasSwitchIdError = errorMessages.some(msg => msg.includes("'switchid' is not defined")); + const hasVlanIdError = errorMessages.some(msg => msg.includes("'vlanId' is not defined")); + const hasPortNumberError = errorMessages.some(msg => msg.includes("'portNumber' is not defined")); + const hasConfigurationError = errorMessages.some(msg => msg.includes("'configuration' is not defined")); + + expect(hasSwitchIdError).toBe(true); + expect(hasVlanIdError).toBe(true); + expect(hasPortNumberError).toBe(true); + expect(hasConfigurationError).toBe(true); + }); + + it('should detect undefined required properties in smart switch complete scenario', async () => { + const yamlPath = join(__dirname, '../resources/undefined-required-properties/smart-switch-complete-3.0.x.yml'); + const yamlContent = readFileSync(yamlPath, 'utf8'); + + const results = await spectral.run(yamlContent); + + const undefinedRequiredErrors = results.filter( + result => result.code === 'undefined-required-properties' + ); + + expect(undefinedRequiredErrors.length).toBeGreaterThanOrEqual(4); + + // Check that we have the expected error messages + const errorMessages = undefinedRequiredErrors.map(error => error.message); + const hasSwitchNameError = errorMessages.some(msg => msg.includes("'switchName' is not defined")); + const hasIncludeMetricsError = errorMessages.some(msg => msg.includes("'includeMetrics' is not defined")); + const hasConfigurationError = errorMessages.some(msg => msg.includes("'configuration' is not defined")); + const hasResultError = errorMessages.some(msg => msg.includes("'result' is not defined")); + + expect(hasSwitchNameError).toBe(true); + expect(hasIncludeMetricsError).toBe(true); + expect(hasConfigurationError).toBe(true); + expect(hasResultError).toBe(true); + }); + + it('should detect undefined required properties in security policies complete scenario', async () => { + const yamlPath = join(__dirname, '../resources/undefined-required-properties/security-policies-complete-3.0.x.yml'); + const yamlContent = readFileSync(yamlPath, 'utf8'); + + const results = await spectral.run(yamlContent); + + const undefinedRequiredErrors = results.filter( + result => result.code === 'undefined-required-properties' + ); + + expect(undefinedRequiredErrors.length).toBeGreaterThanOrEqual(4); + + // Check that we have the expected error messages + const errorMessages = undefinedRequiredErrors.map(error => error.message); + const hasProtocolError = errorMessages.some(msg => msg.includes("'protocol' is not defined")); + const hasPriorityError = errorMessages.some(msg => msg.includes("'priority' is not defined")); + const hasDestinationError = errorMessages.some(msg => msg.includes("'destination' is not defined")); + const hasRulesError = errorMessages.some(msg => msg.includes("'rules' is not defined")); + + expect(hasProtocolError).toBe(true); + expect(hasPriorityError).toBe(true); + expect(hasDestinationError).toBe(true); + expect(hasRulesError).toBe(true); + }); + + it('should detect undefined required properties in reports summary complete scenario', async () => { + const yamlPath = join(__dirname, '../resources/undefined-required-properties/reports-summary-complete-3.0.x.yml'); + const yamlContent = readFileSync(yamlPath, 'utf8'); + + const results = await spectral.run(yamlContent); + + const undefinedRequiredErrors = results.filter( + result => result.code === 'undefined-required-properties' + ); + + expect(undefinedRequiredErrors.length).toBeGreaterThanOrEqual(4); + + // Check that we have the expected error messages + const errorMessages = undefinedRequiredErrors.map(error => error.message); + const hasTypeError = errorMessages.some(msg => msg.includes("'type' is not defined")); + const hasGroupByError = errorMessages.some(msg => msg.includes("'groupBy' is not defined")); + const hasFormatError = errorMessages.some(msg => msg.includes("'format' is not defined")); + const hasMetadataError = errorMessages.some(msg => msg.includes("'metadata' is not defined")); + + expect(hasTypeError).toBe(true); + expect(hasGroupByError).toBe(true); + expect(hasFormatError).toBe(true); + expect(hasMetadataError).toBe(true); + }); + + it('should pass for valid schema with all required properties defined', async () => { + const yamlPath = join(__dirname, '../resources/undefined-required-properties/valid-schema-3.0.x.yml'); + const yamlContent = readFileSync(yamlPath, 'utf8'); + + const results = await spectral.run(yamlContent); + + const undefinedRequiredErrors = results.filter( + result => result.code === 'undefined-required-properties' + ); + + expect(undefinedRequiredErrors).toHaveLength(0); + }); + }); +}); \ No newline at end of file diff --git a/test/integration/undefined-required-properties.test.js b/test/integration/undefined-required-properties.test.js new file mode 100644 index 0000000..db26e75 --- /dev/null +++ b/test/integration/undefined-required-properties.test.js @@ -0,0 +1,446 @@ +/** + * Copyright 2022 Cisco Systems, Inc. and its affiliates. + * + * Licensed under the Apache License, Version 2.0 (the "License"); + * you may not use this file except in compliance with the License. + * You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + * + * SPDX-License-Identifier: Apache-2.0 + */ + +const { describe, expect, it } = require('@jest/globals'); +const { Spectral } = require('@stoplight/spectral-core'); +const ruleset = require('../../validation.js').default; + +describe('Integration Tests - undefined-required-properties', () => { + let spectral; + + beforeEach(() => { + spectral = new Spectral(); + spectral.setRuleset(ruleset); + }); + + describe('should detect undefined required properties', () => { + it('should catch missing severities property', async () => { + const document = { + openapi: '3.0.3', + info: { + title: 'Test API', + version: '1.0.0' + }, + paths: { + '/test': { + get: { + responses: { + '200': { + description: 'OK', + content: { + 'application/json': { + schema: { + $ref: '#/components/schemas/ThresholdAnomaly' + } + } + } + } + } + } + } + }, + components: { + schemas: { + ThresholdAnomaly: { + type: 'object', + properties: { + commonAnomaly: { + type: 'object', + description: 'Common anomaly configuration' + } + // severities property is missing but listed in required + }, + required: ['commonAnomaly', 'severities'] + } + } + } + }; + + const results = await spectral.run(document); + + // Should have at least 1 error for undefined-required-properties + // The rule runs on multiple paths, so we might get duplicates + const undefinedRequiredErrors = results.filter( + result => result.code === 'undefined-required-properties' + ); + + expect(undefinedRequiredErrors.length).toBeGreaterThanOrEqual(1); + + // Check that we have the expected error message + const hasSeveritiesError = undefinedRequiredErrors.some( + error => error.message.includes("'severities' is not defined") + ); + expect(hasSeveritiesError).toBe(true); + + // Check that at least one error points to the correct schema + const schemaErrors = undefinedRequiredErrors.filter( + error => error.path.includes('components') && + error.path.includes('schemas') && + error.path.includes('ThresholdAnomaly') + ); + expect(schemaErrors.length).toBeGreaterThanOrEqual(1); + }); + + it('should catch multiple missing required properties', async () => { + const document = { + openapi: '3.0.3', + info: { + title: 'Test API', + version: '1.0.0' + }, + paths: { + '/test': { + get: { + responses: { + '200': { + description: 'OK', + content: { + 'application/json': { + schema: { + $ref: '#/components/schemas/TestSchema' + } + } + } + } + } + } + } + }, + components: { + schemas: { + TestSchema: { + type: 'object', + properties: { + name: { + type: 'string' + } + // email and age are missing but listed in required + }, + required: ['name', 'email', 'age'] + } + } + } + }; + + const results = await spectral.run(document); + + // Should have at least 2 errors for undefined-required-properties + // The rule runs on multiple paths, so we might get duplicates + const undefinedRequiredErrors = results.filter( + result => result.code === 'undefined-required-properties' + ); + + expect(undefinedRequiredErrors.length).toBeGreaterThanOrEqual(2); + + // Check that we have the expected error messages + const errorMessages = undefinedRequiredErrors.map(error => error.message); + const hasEmailError = errorMessages.some(msg => msg.includes("'email' is not defined")); + const hasAgeError = errorMessages.some(msg => msg.includes("'age' is not defined")); + + expect(hasEmailError).toBe(true); + expect(hasAgeError).toBe(true); + }); + + it('should catch missing properties in response schemas', async () => { + const document = { + openapi: '3.0.3', + info: { + title: 'Test API', + version: '1.0.0' + }, + paths: { + '/test': { + get: { + responses: { + '200': { + description: 'OK', + content: { + 'application/json': { + schema: { + type: 'object', + properties: { + id: { + type: 'string' + } + // name is missing but listed in required + }, + required: ['id', 'name'] + } + } + } + } + } + } + } + } + }; + + const results = await spectral.run(document); + + const undefinedRequiredErrors = results.filter( + result => result.code === 'undefined-required-properties' + ); + + expect(undefinedRequiredErrors).toHaveLength(1); + expect(undefinedRequiredErrors[0].message).toContain("'name' is not defined"); + }); + + it('should catch missing properties in request body schemas', async () => { + const document = { + openapi: '3.0.3', + info: { + title: 'Test API', + version: '1.0.0' + }, + paths: { + '/test': { + post: { + requestBody: { + content: { + 'application/json': { + schema: { + type: 'object', + properties: { + title: { + type: 'string' + } + // description is missing but listed in required + }, + required: ['title', 'description'] + } + } + } + }, + responses: { + '201': { + description: 'Created' + } + } + } + } + } + }; + + const results = await spectral.run(document); + + const undefinedRequiredErrors = results.filter( + result => result.code === 'undefined-required-properties' + ); + + expect(undefinedRequiredErrors).toHaveLength(1); + expect(undefinedRequiredErrors[0].message).toContain("'description' is not defined"); + }); + + it('should catch missing properties in parameter schemas', async () => { + const document = { + openapi: '3.0.3', + info: { + title: 'Test API', + version: '1.0.0' + }, + paths: { + '/test': { + get: { + parameters: [ + { + name: 'filter', + in: 'query', + schema: { + type: 'object', + properties: { + category: { + type: 'string' + } + // status is missing but listed in required + }, + required: ['category', 'status'] + } + } + ], + responses: { + '200': { + description: 'OK' + } + } + } + } + } + }; + + const results = await spectral.run(document); + + const undefinedRequiredErrors = results.filter( + result => result.code === 'undefined-required-properties' + ); + + expect(undefinedRequiredErrors).toHaveLength(1); + expect(undefinedRequiredErrors[0].message).toContain("'status' is not defined"); + }); + }); + + describe('should not flag valid schemas', () => { + it('should pass when all required properties are defined', async () => { + const document = { + openapi: '3.0.3', + info: { + title: 'Test API', + version: '1.0.0' + }, + paths: { + '/test': { + get: { + responses: { + '200': { + description: 'OK', + content: { + 'application/json': { + schema: { + $ref: '#/components/schemas/ValidSchema' + } + } + } + } + } + } + } + }, + components: { + schemas: { + ValidSchema: { + type: 'object', + properties: { + name: { + type: 'string' + }, + email: { + type: 'string', + format: 'email' + }, + age: { + type: 'integer' + } + }, + required: ['name', 'email', 'age'] + } + } + } + }; + + const results = await spectral.run(document); + + const undefinedRequiredErrors = results.filter( + result => result.code === 'undefined-required-properties' + ); + + expect(undefinedRequiredErrors).toHaveLength(0); + }); + + it('should pass when schema has no required array', async () => { + const document = { + openapi: '3.0.3', + info: { + title: 'Test API', + version: '1.0.0' + }, + paths: { + '/test': { + get: { + responses: { + '200': { + description: 'OK', + content: { + 'application/json': { + schema: { + $ref: '#/components/schemas/NoRequiredSchema' + } + } + } + } + } + } + } + }, + components: { + schemas: { + NoRequiredSchema: { + type: 'object', + properties: { + name: { + type: 'string' + } + } + // No required array + } + } + } + }; + + const results = await spectral.run(document); + + const undefinedRequiredErrors = results.filter( + result => result.code === 'undefined-required-properties' + ); + + expect(undefinedRequiredErrors).toHaveLength(0); + }); + + it('should pass when schema is not an object type', async () => { + const document = { + openapi: '3.0.3', + info: { + title: 'Test API', + version: '1.0.0' + }, + paths: { + '/test': { + get: { + responses: { + '200': { + description: 'OK', + content: { + 'application/json': { + schema: { + $ref: '#/components/schemas/StringSchema' + } + } + } + } + } + } + } + }, + components: { + schemas: { + StringSchema: { + type: 'string', + required: ['someProperty'] // This should be ignored for non-object schemas + } + } + } + }; + + const results = await spectral.run(document); + + const undefinedRequiredErrors = results.filter( + result => result.code === 'undefined-required-properties' + ); + + expect(undefinedRequiredErrors).toHaveLength(0); + }); + }); +}); diff --git a/test/resources/undefined-required-properties/base-permit-deny-entry-3.0.x.yml b/test/resources/undefined-required-properties/base-permit-deny-entry-3.0.x.yml new file mode 100644 index 0000000..4bf4105 --- /dev/null +++ b/test/resources/undefined-required-properties/base-permit-deny-entry-3.0.x.yml @@ -0,0 +1,29 @@ +openapi: 3.0.3 +info: + title: Test API + version: 1.0.0 +paths: + /test: + get: + responses: + '200': + description: OK + content: + application/json: + schema: + $ref: '#/components/schemas/BasePermitDenyEntry' +components: + schemas: + BasePermitDenyEntry: + type: object + properties: + action: + type: string + enum: [permit, deny] + description: Action to take + source: + type: string + description: Source address + required: + - action + - protocol # This property is required but not defined in properties diff --git a/test/resources/undefined-required-properties/comprehensive-test-3.0.x.yml b/test/resources/undefined-required-properties/comprehensive-test-3.0.x.yml new file mode 100644 index 0000000..6f69d12 --- /dev/null +++ b/test/resources/undefined-required-properties/comprehensive-test-3.0.x.yml @@ -0,0 +1,82 @@ +openapi: 3.0.3 +info: + title: Comprehensive Test API + version: 1.0.0 +paths: + # Test case 1: Missing properties in response schema + /test/response: + get: + responses: + '200': + description: OK + content: + application/json: + schema: + type: object + properties: + id: + type: string + # name is missing but listed in required + required: + - id + - name # This property is required but not defined in response schema + + # Test case 2: Missing properties in request body schema + /test/request: + post: + requestBody: + content: + application/json: + schema: + type: object + properties: + title: + type: string + # description is missing but listed in required + required: + - title + - description # This property is required but not defined in request body schema + responses: + '201': + description: Created + + # Test case 3: Missing properties in parameter schema + /test/parameter: + get: + parameters: + - name: filter + in: query + schema: + type: object + properties: + category: + type: string + # status is missing but listed in required + required: + - category + - status # This property is required but not defined in parameter schema + responses: + '200': + description: OK + + # Test case 4: Missing properties in component schema + /test/component: + get: + responses: + '200': + description: OK + content: + application/json: + schema: + $ref: '#/components/schemas/TestComponent' +components: + schemas: + TestComponent: + type: object + properties: + id: + type: string + # name is missing but listed in required + required: + - id + - name # This property is required but not defined in component schema diff --git a/test/resources/undefined-required-properties/device-interface-complete-3.0.x.yml b/test/resources/undefined-required-properties/device-interface-complete-3.0.x.yml new file mode 100644 index 0000000..f6c670e --- /dev/null +++ b/test/resources/undefined-required-properties/device-interface-complete-3.0.x.yml @@ -0,0 +1,81 @@ +openapi: 3.0.3 +info: + title: Test API + version: 1.0.0 +paths: + /devices/{deviceId}/interfaces: + get: + parameters: + - name: deviceId + in: path + required: true + schema: + type: string + - name: filter + in: query + schema: + type: object + properties: + interfaceType: + type: string + description: Type of interface + # vlanId is missing but listed in required + required: + - interfaceType + - vlanId # This property is required but not defined in parameter schema + responses: + '200': + description: OK + content: + application/json: + schema: + $ref: '#/components/schemas/DeviceInterfaceScope' + post: + parameters: + - name: deviceId + in: path + required: true + schema: + type: string + requestBody: + content: + application/json: + schema: + type: object + properties: + interfaceName: + type: string + description: Name of the interface + # portNumber is missing but listed in required + required: + - interfaceName + - portNumber # This property is required but not defined in request body schema + responses: + '201': + description: Created + content: + application/json: + schema: + type: object + properties: + interfaceId: + type: string + description: Created interface ID + # configuration is missing but listed in required + required: + - interfaceId + - configuration # This property is required but not defined in response schema +components: + schemas: + DeviceInterfaceScope: + type: object + properties: + interfaceName: + type: string + description: Name of the interface + vlanId: + type: integer + description: VLAN ID + required: + - interfaceName + - switchid # This property is required but not defined in properties diff --git a/test/resources/undefined-required-properties/device-interface-scope-3.0.x.yml b/test/resources/undefined-required-properties/device-interface-scope-3.0.x.yml new file mode 100644 index 0000000..eec4102 --- /dev/null +++ b/test/resources/undefined-required-properties/device-interface-scope-3.0.x.yml @@ -0,0 +1,28 @@ +openapi: 3.0.3 +info: + title: Test API + version: 1.0.0 +paths: + /test: + get: + responses: + '200': + description: OK + content: + application/json: + schema: + $ref: '#/components/schemas/DeviceInterfaceScope' +components: + schemas: + DeviceInterfaceScope: + type: object + properties: + interfaceName: + type: string + description: Name of the interface + vlanId: + type: integer + description: VLAN ID + required: + - interfaceName + - switchid # This property is required but not defined in properties diff --git a/test/resources/undefined-required-properties/reports-summary-complete-3.0.x.yml b/test/resources/undefined-required-properties/reports-summary-complete-3.0.x.yml new file mode 100644 index 0000000..341ec6f --- /dev/null +++ b/test/resources/undefined-required-properties/reports-summary-complete-3.0.x.yml @@ -0,0 +1,74 @@ +openapi: 3.0.3 +info: + title: Test API + version: 1.0.0 +paths: + /reports/summary: + get: + parameters: + - name: filters + in: query + schema: + type: object + properties: + dateRange: + type: string + description: Date range for the report + # groupBy is missing but listed in required + required: + - dateRange + - groupBy # This property is required but not defined in parameter schema + responses: + '200': + description: OK + content: + application/json: + schema: + $ref: '#/components/schemas/SummaryProperties' + post: + requestBody: + content: + application/json: + schema: + type: object + properties: + reportName: + type: string + description: Name of the report + count: + type: integer + description: Number of items + # format is missing but listed in required + required: + - reportName + - count + - format # This property is required but not defined in request body schema + responses: + '201': + description: Created + content: + application/json: + schema: + type: object + properties: + reportId: + type: string + description: Created report ID + # metadata is missing but listed in required + required: + - reportId + - metadata # This property is required but not defined in response schema +components: + schemas: + SummaryProperties: + type: object + properties: + count: + type: integer + description: Number of items + total: + type: integer + description: Total count + required: + - count + - type # This property is required but not defined in properties diff --git a/test/resources/undefined-required-properties/request-response-schemas-3.0.x.yml b/test/resources/undefined-required-properties/request-response-schemas-3.0.x.yml new file mode 100644 index 0000000..5454c57 --- /dev/null +++ b/test/resources/undefined-required-properties/request-response-schemas-3.0.x.yml @@ -0,0 +1,55 @@ +openapi: 3.0.3 +info: + title: Test API + version: 1.0.0 +paths: + /anomalyRules/complianceRules: + get: + responses: + '200': + description: OK + content: + application/json: + schema: + $ref: '#/components/schemas/ThresholdAnomaly' + post: + requestBody: + content: + application/json: + schema: + type: object + properties: + ruleName: + type: string + description: Name of the compliance rule + # severity is missing but listed in required + required: + - ruleName + - severity # This property is required but not defined in request body schema + responses: + '201': + description: Created + content: + application/json: + schema: + type: object + properties: + id: + type: string + description: Created rule ID + # status is missing but listed in required + required: + - id + - status # This property is required but not defined in response schema +components: + schemas: + ThresholdAnomaly: + type: object + properties: + commonAnomaly: + type: object + description: Common anomaly configuration + # severities property is missing from properties but listed in required + required: + - commonAnomaly + - severities # This property is required but not defined in properties diff --git a/test/resources/undefined-required-properties/security-policies-complete-3.0.x.yml b/test/resources/undefined-required-properties/security-policies-complete-3.0.x.yml new file mode 100644 index 0000000..0a76dda --- /dev/null +++ b/test/resources/undefined-required-properties/security-policies-complete-3.0.x.yml @@ -0,0 +1,76 @@ +openapi: 3.0.3 +info: + title: Test API + version: 1.0.0 +paths: + /security/policies: + get: + parameters: + - name: policyType + in: query + schema: + type: object + properties: + category: + type: string + description: Policy category + # priority is missing but listed in required + required: + - category + - priority # This property is required but not defined in parameter schema + responses: + '200': + description: OK + content: + application/json: + schema: + $ref: '#/components/schemas/BasePermitDenyEntry' + post: + requestBody: + content: + application/json: + schema: + type: object + properties: + action: + type: string + enum: [permit, deny] + description: Action to take + source: + type: string + description: Source address + # destination is missing but listed in required + required: + - action + - source + - destination # This property is required but not defined in request body schema + responses: + '201': + description: Created + content: + application/json: + schema: + type: object + properties: + policyId: + type: string + description: Created policy ID + # rules is missing but listed in required + required: + - policyId + - rules # This property is required but not defined in response schema +components: + schemas: + BasePermitDenyEntry: + type: object + properties: + action: + type: string + enum: [permit, deny] + description: Action to take + source: + type: string + description: Source address + required: + - action + - protocol # This property is required but not defined in properties diff --git a/test/resources/undefined-required-properties/smart-switch-complete-3.0.x.yml b/test/resources/undefined-required-properties/smart-switch-complete-3.0.x.yml new file mode 100644 index 0000000..e9f1222 --- /dev/null +++ b/test/resources/undefined-required-properties/smart-switch-complete-3.0.x.yml @@ -0,0 +1,86 @@ +openapi: 3.0.3 +info: + title: Test API + version: 1.0.0 +paths: + /smartSwitches/{switchId}/integrations: + get: + parameters: + - name: switchId + in: path + required: true + schema: + type: string + - name: options + in: query + schema: + type: object + properties: + includeStatus: + type: boolean + description: Include status information + # includeMetrics is missing but listed in required + required: + - includeStatus + - includeMetrics # This property is required but not defined in parameter schema + responses: + '200': + description: OK + content: + application/json: + schema: + $ref: '#/components/schemas/SmartSwitchIntegrationIdData' + put: + parameters: + - name: switchId + in: path + required: true + schema: + type: string + requestBody: + content: + application/json: + schema: + type: object + properties: + integrationId: + type: string + description: Integration ID + status: + type: string + description: Integration status + # configuration is missing but listed in required + required: + - integrationId + - status + - configuration # This property is required but not defined in request body schema + responses: + '200': + description: Updated + content: + application/json: + schema: + type: object + properties: + updatedAt: + type: string + format: date-time + description: Last update timestamp + # result is missing but listed in required + required: + - updatedAt + - result # This property is required but not defined in response schema +components: + schemas: + SmartSwitchIntegrationIdData: + type: object + properties: + integrationId: + type: string + description: Integration ID + status: + type: string + description: Integration status + required: + - integrationId + - switchName # This property is required but not defined in properties diff --git a/test/resources/undefined-required-properties/smart-switch-integration-3.0.x.yml b/test/resources/undefined-required-properties/smart-switch-integration-3.0.x.yml new file mode 100644 index 0000000..c1e11d5 --- /dev/null +++ b/test/resources/undefined-required-properties/smart-switch-integration-3.0.x.yml @@ -0,0 +1,28 @@ +openapi: 3.0.3 +info: + title: Test API + version: 1.0.0 +paths: + /test: + get: + responses: + '200': + description: OK + content: + application/json: + schema: + $ref: '#/components/schemas/SmartSwitchIntegrationIdData' +components: + schemas: + SmartSwitchIntegrationIdData: + type: object + properties: + integrationId: + type: string + description: Integration ID + status: + type: string + description: Integration status + required: + - integrationId + - switchName # This property is required but not defined in properties diff --git a/test/resources/undefined-required-properties/summary-properties-3.0.x.yml b/test/resources/undefined-required-properties/summary-properties-3.0.x.yml new file mode 100644 index 0000000..a221bbe --- /dev/null +++ b/test/resources/undefined-required-properties/summary-properties-3.0.x.yml @@ -0,0 +1,28 @@ +openapi: 3.0.3 +info: + title: Test API + version: 1.0.0 +paths: + /test: + get: + responses: + '200': + description: OK + content: + application/json: + schema: + $ref: '#/components/schemas/SummaryProperties' +components: + schemas: + SummaryProperties: + type: object + properties: + count: + type: integer + description: Number of items + total: + type: integer + description: Total count + required: + - count + - type # This property is required but not defined in properties diff --git a/test/resources/undefined-required-properties/undefined-required-properties-3.0.x.yml b/test/resources/undefined-required-properties/undefined-required-properties-3.0.x.yml new file mode 100644 index 0000000..352fcb8 --- /dev/null +++ b/test/resources/undefined-required-properties/undefined-required-properties-3.0.x.yml @@ -0,0 +1,26 @@ +openapi: 3.0.3 +info: + title: Test API + version: 1.0.0 +paths: + /test: + get: + responses: + '200': + description: OK + content: + application/json: + schema: + $ref: '#/components/schemas/ThresholdAnomaly' +components: + schemas: + ThresholdAnomaly: + type: object + properties: + commonAnomaly: + type: object + description: Common anomaly configuration + # severities property is missing from properties but listed in required + required: + - commonAnomaly + - severities # This property is required but not defined in properties diff --git a/test/resources/undefined-required-properties/undefined-required-properties-3.1.x.yml b/test/resources/undefined-required-properties/undefined-required-properties-3.1.x.yml new file mode 100644 index 0000000..bd22c8c --- /dev/null +++ b/test/resources/undefined-required-properties/undefined-required-properties-3.1.x.yml @@ -0,0 +1,26 @@ +openapi: 3.1.0 +info: + title: Test API + version: 1.0.0 +paths: + /test: + get: + responses: + '200': + description: OK + content: + application/json: + schema: + $ref: '#/components/schemas/ThresholdAnomaly' +components: + schemas: + ThresholdAnomaly: + type: object + properties: + commonAnomaly: + type: object + description: Common anomaly configuration + # severities property is missing from properties but listed in required + required: + - commonAnomaly + - severities # This property is required but not defined in properties diff --git a/test/resources/undefined-required-properties/valid-schema-3.0.x.yml b/test/resources/undefined-required-properties/valid-schema-3.0.x.yml new file mode 100644 index 0000000..100ad93 --- /dev/null +++ b/test/resources/undefined-required-properties/valid-schema-3.0.x.yml @@ -0,0 +1,34 @@ +openapi: 3.0.3 +info: + title: Test API + version: 1.0.0 +paths: + /test: + get: + responses: + '200': + description: OK + content: + application/json: + schema: + $ref: '#/components/schemas/ValidSchema' +components: + schemas: + ValidSchema: + type: object + properties: + name: + type: string + description: Name of the item + email: + type: string + format: email + description: Email address + age: + type: integer + minimum: 0 + description: Age in years + required: + - name + - email + - age # All required properties are defined in properties diff --git a/test/validation-nested-refs.spec.js b/test/validation-nested-refs.spec.js deleted file mode 100644 index 805d20c..0000000 --- a/test/validation-nested-refs.spec.js +++ /dev/null @@ -1,575 +0,0 @@ -/** - * Copyright 2022 Cisco Systems, Inc. and its affiliates. - * - * Licensed under the Apache License, Version 2.0 (the "License"); - * you may not use this file except in compliance with the License. - * You may obtain a copy of the License at - * - * http://www.apache.org/licenses/LICENSE-2.0 - * - * Unless required by applicable law or agreed to in writing, software - * distributed under the License is distributed on an "AS IS" BASIS, - * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. - * See the License for the specific language governing permissions and - * limitations under the License. - * - * SPDX-License-Identifier: Apache-2.0 - */ - -import { - describe, expect, it, beforeAll, -} from '@jest/globals'; -import { Spectral } from '@stoplight/spectral-core'; -import { bundleAndLoadRuleset } from '@stoplight/spectral-ruleset-bundler/with-loader'; -import * as fs from 'fs'; -import * as path from 'path'; - -// __dirname is provided by Jest in CommonJS mode -const rulesetPath = path.join(__dirname, '..', 'validation.js'); - -describe('validation.js - nested broken refs detection', () => { - let spectral; - let ruleset; - - beforeAll(async () => { - spectral = new Spectral(); - ruleset = await bundleAndLoadRuleset(rulesetPath, { fs, fetch }); - spectral.setRuleset(ruleset); - }); - - describe('OpenAPI 3.0.3 - broken refs in standard locations', () => { - it('should catch broken direct $ref', async () => { - const spec = ` -openapi: 3.0.3 -info: - title: Test API - version: 1.0.0 -paths: - /test: - get: - responses: - '200': - description: OK - content: - application/json: - schema: - $ref: '#/components/schemas/NonExistent' -components: - schemas: - User: - type: object -`; - const results = await spectral.run(spec); - const brokenRefErrors = results.filter(r => - r.code === 'broken-refs-in-siblings' || - r.code === 'invalid-ref' - ); - - expect(brokenRefErrors.length).toBeGreaterThan(0); - }); - - it('should catch broken $ref in array items', async () => { - const spec = ` -openapi: 3.0.3 -info: - title: Test API - version: 1.0.0 -paths: - /test: - get: - responses: - '200': - description: OK - content: - application/json: - schema: - type: array - items: - $ref: '#/components/schemas/NonExistent' -components: - schemas: - User: - type: object -`; - const results = await spectral.run(spec); - const brokenRefErrors = results.filter(r => - r.code === 'broken-refs-in-siblings' || - r.code === 'invalid-ref' - ); - - expect(brokenRefErrors.length).toBeGreaterThan(0); - }); - - it('should catch broken refs in siblings even in 3.0.3 (our preprocessor works)', async () => { - const spec = ` -openapi: 3.0.3 -info: - title: Test API - version: 1.0.0 -paths: {} -components: - schemas: - CommonPolicy: - type: object - TestSchema: - type: object - properties: - commonAnomaly: - $ref: '#/components/schemas/CommonPolicy' - severities: - type: array - items: - $ref: '#/components/schemas/PolicySeverity' -`; - const results = await spectral.run(spec); - - // Our preprocessor should catch the broken ref even in 3.0.3 - // (even though the structure itself is technically invalid per spec) - const brokenRefErrors = results.filter(r => - r.code === 'broken-refs-in-siblings' - ); - expect(brokenRefErrors.length).toBeGreaterThan(0); - expect(brokenRefErrors[0].message).toContain('PolicySeverity'); - }); - }); - - describe('OpenAPI 3.1.0 - broken refs in sibling properties', () => { - it('should catch broken $ref in sibling property', async () => { - const spec = ` -openapi: 3.1.0 -info: - title: Test API - version: 1.0.0 -paths: - /test: - get: - responses: - '200': - description: OK - content: - application/json: - schema: - $ref: '#/components/schemas/TestSchema' -components: - schemas: - CommonPolicy: - type: object - TestSchema: - type: object - properties: - commonAnomaly: - $ref: '#/components/schemas/CommonPolicy' - severities: - type: array - items: - $ref: '#/components/schemas/PolicySeverity' -`; - const results = await spectral.run(spec); - const brokenRefErrors = results.filter(r => - r.code === 'broken-refs-in-siblings' - ); - - expect(brokenRefErrors.length).toBeGreaterThan(0); - expect(brokenRefErrors[0].message).toContain('PolicySeverity'); - }); - - it('should catch multiple broken refs in sibling properties', async () => { - const spec = ` -openapi: 3.1.0 -info: - title: Test API - version: 1.0.0 -paths: - /test: - get: - responses: - '200': - description: OK - content: - application/json: - schema: - $ref: '#/components/schemas/TestSchema' -components: - schemas: - CommonPolicy: - type: object - TestSchema: - type: object - properties: - property1: - $ref: '#/components/schemas/CommonPolicy' - nested1: - $ref: '#/components/schemas/NonExistent1' - property2: - $ref: '#/components/schemas/CommonPolicy' - nested2: - $ref: '#/components/schemas/NonExistent2' -`; - const results = await spectral.run(spec); - const brokenRefErrors = results.filter(r => - r.code === 'broken-refs-in-siblings' - ); - - expect(brokenRefErrors.length).toBeGreaterThanOrEqual(2); - }); - - it('should catch deeply nested broken ref in sibling', async () => { - const spec = ` -openapi: 3.1.0 -info: - title: Test API - version: 1.0.0 -paths: - /test: - get: - responses: - '200': - description: OK - content: - application/json: - schema: - $ref: '#/components/schemas/TestSchema' -components: - schemas: - CommonPolicy: - type: object - TestSchema: - type: object - properties: - commonAnomaly: - $ref: '#/components/schemas/CommonPolicy' - properties: - nested: - properties: - deep: - $ref: '#/components/schemas/DeepNonExistent' -`; - const results = await spectral.run(spec); - const brokenRefErrors = results.filter(r => - r.code === 'broken-refs-in-siblings' - ); - - expect(brokenRefErrors.length).toBeGreaterThan(0); - expect(brokenRefErrors[0].message).toContain('DeepNonExistent'); - }); - - it('should NOT report errors when nested refs in siblings are valid', async () => { - const spec = ` -openapi: 3.1.0 -info: - title: Test API - version: 1.0.0 -paths: - /test: - get: - responses: - '200': - description: OK - content: - application/json: - schema: - $ref: '#/components/schemas/TestSchema' -components: - schemas: - CommonPolicy: - type: object - User: - type: object - TestSchema: - type: object - properties: - commonAnomaly: - $ref: '#/components/schemas/CommonPolicy' - users: - type: array - items: - $ref: '#/components/schemas/User' -`; - const results = await spectral.run(spec); - const brokenRefErrors = results.filter(r => - r.code === 'broken-refs-in-siblings' - ); - - expect(brokenRefErrors.length).toBe(0); - }); - }); - - describe('OpenAPI 3.1.0 - real-world scenario: ThresholdAnomaly', () => { - it('should catch broken PolicySeverity ref in ThresholdAnomaly structure', async () => { - const spec = ` -openapi: 3.1.0 -info: - title: Anomaly API - version: 1.0.0 -paths: - /anomalies: - get: - responses: - '200': - description: OK - content: - application/json: - schema: - $ref: '#/components/schemas/ThresholdAnomaly' -components: - schemas: - AnomalyTypeFormat: - type: string - enum: [basic, regex, threshold] - ThresholdAnomalyType: - type: string - enum: [cpu, memory, disk] - CommonAnomalyPolicy: - type: object - properties: - details: - type: string - isActive: - type: boolean - ThresholdAnomaly: - type: object - properties: - anomalyFormatType: - $ref: '#/components/schemas/AnomalyTypeFormat' - anomalyType: - $ref: '#/components/schemas/ThresholdAnomalyType' - commonAnomaly: - $ref: '#/components/schemas/CommonAnomalyPolicy' - severities: - description: severity threshold values - type: array - items: - $ref: '#/components/schemas/PolicySeverity' - required: - - anomalyFormatType - - anomalyType - - severities -`; - const results = await spectral.run(spec); - const brokenRefErrors = results.filter(r => - r.code === 'broken-refs-in-siblings' - ); - - expect(brokenRefErrors.length).toBeGreaterThan(0); - expect(brokenRefErrors.some(e => e.message.includes('PolicySeverity'))).toBe(true); - }); - - it('should NOT report error when PolicySeverity is defined', async () => { - const spec = ` -openapi: 3.1.0 -info: - title: Anomaly API - version: 1.0.0 -paths: - /anomalies: - get: - responses: - '200': - description: OK - content: - application/json: - schema: - $ref: '#/components/schemas/ThresholdAnomaly' -components: - schemas: - AnomalyTypeFormat: - type: string - enum: [basic, regex, threshold] - ThresholdAnomalyType: - type: string - enum: [cpu, memory, disk] - CommonAnomalyPolicy: - type: object - properties: - details: - type: string - isActive: - type: boolean - PolicySeverity: - type: string - enum: [low, medium, high, critical] - ThresholdAnomaly: - type: object - properties: - anomalyFormatType: - $ref: '#/components/schemas/AnomalyTypeFormat' - anomalyType: - $ref: '#/components/schemas/ThresholdAnomalyType' - commonAnomaly: - $ref: '#/components/schemas/CommonAnomalyPolicy' - severities: - description: severity threshold values - type: array - items: - $ref: '#/components/schemas/PolicySeverity' - required: - - anomalyFormatType - - anomalyType - - severities -`; - const results = await spectral.run(spec); - const brokenRefErrors = results.filter(r => - r.code === 'broken-refs-in-siblings' - ); - - expect(brokenRefErrors.length).toBe(0); - }); - }); - - describe('OpenAPI 3.1.0 - external references', () => { - it('should NOT report errors for external HTTP refs in siblings', async () => { - const spec = ` -openapi: 3.1.0 -info: - title: Test API - version: 1.0.0 -paths: - /test: - get: - responses: - '200': - description: OK - content: - application/json: - schema: - $ref: '#/components/schemas/TestSchema' -components: - schemas: - CommonPolicy: - type: object - TestSchema: - type: object - properties: - commonAnomaly: - $ref: '#/components/schemas/CommonPolicy' - externalRef: - $ref: 'http://example.com/schemas/external.json' -`; - const results = await spectral.run(spec); - const brokenRefErrors = results.filter(r => - r.code === 'broken-refs-in-siblings' - ); - - expect(brokenRefErrors.length).toBe(0); - }); - - it('should catch internal broken refs but ignore external refs', async () => { - const spec = ` -openapi: 3.1.0 -info: - title: Test API - version: 1.0.0 -paths: - /test: - get: - responses: - '200': - description: OK - content: - application/json: - schema: - $ref: '#/components/schemas/TestSchema' -components: - schemas: - CommonPolicy: - type: object - TestSchema: - type: object - properties: - commonAnomaly: - $ref: '#/components/schemas/CommonPolicy' - internalBroken: - $ref: '#/components/schemas/NonExistent' - externalRef: - $ref: 'https://example.com/schemas/external.json' -`; - const results = await spectral.run(spec); - const brokenRefErrors = results.filter(r => - r.code === 'broken-refs-in-siblings' - ); - - expect(brokenRefErrors.length).toBeGreaterThan(0); - expect(brokenRefErrors.some(e => e.message.includes('NonExistent'))).toBe(true); - expect(brokenRefErrors.some(e => e.message.includes('example.com'))).toBe(false); - }); - }); - - describe('Edge cases', () => { - it('should handle $ref with non-object siblings', async () => { - const spec = ` -openapi: 3.1.0 -info: - title: Test API - version: 1.0.0 -paths: - /test: - get: - responses: - '200': - description: OK - content: - application/json: - schema: - $ref: '#/components/schemas/TestSchema' -components: - schemas: - CommonPolicy: - type: object - TestSchema: - type: object - properties: - prop: - $ref: '#/components/schemas/CommonPolicy' - description: "Some description" - example: "example value" -`; - const results = await spectral.run(spec); - const brokenRefErrors = results.filter(r => - r.code === 'broken-refs-in-siblings' - ); - - // Should not error - no nested $refs - expect(brokenRefErrors.length).toBe(0); - }); - - it('should handle multiple levels of $ref with siblings', async () => { - const spec = ` -openapi: 3.1.0 -info: - title: Test API - version: 1.0.0 -paths: - /test: - get: - responses: - '200': - description: OK - content: - application/json: - schema: - $ref: '#/components/schemas/Level1' -components: - schemas: - Base: - type: object - Level1: - type: object - properties: - level1Prop: - $ref: '#/components/schemas/Base' - level2: - $ref: '#/components/schemas/Base' - level3: - $ref: '#/components/schemas/NonExistent' -`; - const results = await spectral.run(spec); - const brokenRefErrors = results.filter(r => - r.code === 'broken-refs-in-siblings' - ); - - expect(brokenRefErrors.length).toBeGreaterThan(0); - expect(brokenRefErrors.some(e => e.message.includes('NonExistent'))).toBe(true); - }); - }); -}); - diff --git a/validation.js b/validation.js index a530938..c975ee1 100644 --- a/validation.js +++ b/validation.js @@ -20,6 +20,7 @@ import { oas } from '@stoplight/spectral-rulesets'; import { oas3_1 } from '@stoplight/spectral-formats'; import defaultInEnum from './functions/defaultInEnum.js'; import validateRefSiblings from './functions/validateRefSiblings.js'; +import validateRequiredProperties from './functions/validateRequiredProperties.js'; export default { 'extends': [ @@ -31,7 +32,6 @@ export default { 'rules': { 'oas3-schema': 'error', 'oas2-schema': 'error', - 'no-$ref-siblings': 'warn', 'oas3-operation-security-defined': 'error', 'oas2-operation-security-defined': 'error', 'server-variable-default': { @@ -49,9 +49,9 @@ export default { ], }, - 'broken-refs-in-siblings': { - 'description': 'Internal $ref references in sibling properties of $ref should point to existing schema components', - 'message': '{{error}}', + 'broken-internal-refs': { + 'description': 'internal references should exist', + 'message': '{{description}}; {{error}}', 'severity': 'error', 'resolved': false, 'given': '$..', @@ -61,5 +61,22 @@ export default { }, ], }, + + 'undefined-required-properties': { + 'description': 'required properties must be defined', + 'message': '{{description}}; {{error}}', + 'severity': 'error', + 'given': [ + '$.components.schemas.*', + '$.paths.*.*.responses.*.content.*.schema', + '$.paths.*.*.requestBody.content.*.schema', + '$.paths.*.*.parameters.*.schema' + ], + 'then': [ + { + 'function': validateRequiredProperties, + }, + ], + }, }, }; From e8666d0046378499960f97b2c8316e58044b760c Mon Sep 17 00:00:00 2001 From: abkum3 Date: Fri, 10 Oct 2025 20:25:29 +0530 Subject: [PATCH 4/7] updating documentation of approach --- NESTED_REFS_SOLUTION.md | 48 +++++++++++++++++++++++++++++------------ 1 file changed, 34 insertions(+), 14 deletions(-) diff --git a/NESTED_REFS_SOLUTION.md b/NESTED_REFS_SOLUTION.md index 6f22b0b..b474965 100644 --- a/NESTED_REFS_SOLUTION.md +++ b/NESTED_REFS_SOLUTION.md @@ -13,7 +13,11 @@ The original issue #48 reported is [here](https://wwwin-github.cisco.com/DevNet/ ### Issue 1: Broken Nested References -In OpenAPI 3.1.x, the specification allows `$ref` to have sibling properties at the same level while OAS 3.0.x doesn't allow this: +The OpenAPI spec file from issue #48 (`manage.yaml`) has an indentation issue in the `CommonAnomaly` property of `components.schemas.ThresholdAnomaly`. The `severities` property should have been defined one level up as a sibling to `commonAnomaly`. + +However, this cannot be identified as a schema structure problem because, even with the incorrect indentation in `manage.yaml`, `severities` can rightfully be considered as a property of `commonAnomaly`. + +In OpenAPI 3.1.x, the specification allows `$ref` to have sibling properties at the same level, while OAS 3.0.x does not allow this and suggests ignoring such properties if they are present. ```yaml commonAnomaly: @@ -24,7 +28,7 @@ commonAnomaly: $ref: '#/components/schemas/PolicySeverity' # Nested ref ``` -The broken reference `#/components/schemas/PolicySeverity` was not being caught in both OAS 3.0.x and 3.1.x specs. +Broken nested references like `#/components/schemas/PolicySeverity` were not being caught in either OAS 3.0.x or 3.1.x specs by the existing validator. While the validator correctly identifies broken references at the top level, it fails to detect them when they are nested within sibling properties of a `$ref`. ### Issue 2: Undefined Required Properties @@ -45,12 +49,12 @@ TestSchema: ### Why These Issues Weren't Caught 1. **Broken References**: - - **In OAS 3.0.x**: Spectral ignored sibling properties of `$ref`, so validation never reached nested `$ref` - - **In OAS 3.1.x**: Spectral resolves `$ref` references during parsing, merging siblings. By the time validation runs, nested broken `$ref` is absorbed into resolved schema + - **In OAS 3.0.x**: Spectral ignores sibling properties of `$ref`, so validation never reaches nested `$ref` references + - **In OAS 3.1.x**: Spectral resolves `$ref` references during parsing and merges sibling properties. By the time validation runs, the nested broken `$ref` has been absorbed into the resolved schema structure, making it undetectable 2. **Undefined Required Properties**: - - Spectral's default `oas3-schema` ruleset does not validate that all properties in `required` array are defined in `properties` object - - This is a common issue in real-world OpenAPI specifications + - Spectral's default `oas3-schema` ruleset does not validate that all properties in the `required` array are actually defined in the `properties` object + - This validation gap is a common source of errors in real-world OpenAPI specifications ## Solution Implemented @@ -225,20 +229,20 @@ No results with a severity of 'error' found! ## Real-World Impact -### Issues Found in `manage.yaml` openAPI spec that was shared along with [issue #48](https://wwwin-github.cisco.com/DevNet/api-insights-support/issues/48) +### Issues Found in `manage.yaml` -The rules successfully detect real-world issues: +Using the OpenAPI spec file shared with [issue #48](https://wwwin-github.cisco.com/DevNet/api-insights-support/issues/48), the rules successfully detect the following real-world issues: ```bash $ spectral lint -r validation.js manage.yaml /path/to/manage.yaml - 11980:22 error undefined-required-properties required properties must be defined; 'severities' is not defined components.schemas.ThresholdAnomaly - 11988:23 error broken-internal-refs internal references should point to existing components; broken reference '#/components/schemas/PolicySeverity' components.schemas.ThresholdAnomaly.properties.commonAnomaly - 14023:26 error undefined-required-properties required properties must be defined; 'switchid' is not defined components.schemas.deviceInterfaceScope - 16736:34 error undefined-required-properties required properties must be defined; 'switchName' is not defined components.schemas.smartSwitchIntegrationIdData - 22814:25 error undefined-required-properties required properties must be defined; 'protocol' is not defined components.schemas.basePermitDenyEntry - 25083:23 error undefined-required-properties required properties must be defined; 'type' is not defined components.schemas.summaryProperties + 11980:22 error undefined-required-properties required properties must be defined; 'severities' is not defined components.schemas.ThresholdAnomaly + 11988:23 error broken-internal-refs internal references should exist; broken reference '#/components/schemas/PolicySeverity' components.schemas.ThresholdAnomaly.properties.commonAnomaly + 14023:26 error undefined-required-properties required properties must be defined; 'switchid' is not defined components.schemas.deviceInterfaceScope + 16736:34 error undefined-required-properties required properties must be defined; 'switchName' is not defined components.schemas.smartSwitchIntegrationIdData + 22814:25 error undefined-required-properties required properties must be defined; 'protocol' is not defined components.schemas.basePermitDenyEntry + 25083:23 error undefined-required-properties required properties must be defined; 'type' is not defined components.schemas.summaryProperties ✖ 6 problems (6 errors, 0 warnings, 0 infos, 0 hints) ``` @@ -256,3 +260,19 @@ This is the **critical** configuration that makes the broken references solution - **Without this**: Rule runs on resolved document, nested refs are already merged and hidden - **With this**: Rule runs on raw parsed YAML, before Spectral processes any `$ref` references +### Error Message Formats + +Both rules provide clear, actionable error messages to help developers quickly identify and fix issues: + +**Broken Internal References**: +``` +internal references should exist; broken reference '#/components/schemas/Reference' +``` + +**Undefined Required Properties**: +``` +required properties must be defined; 'propertyName' is not defined +``` + +These concise formats focus on the essential information needed to resolve the issues without cluttering the output with unnecessary details. + From d80719a5eb345ca13afe5c41051ce5d347ad771a Mon Sep 17 00:00:00 2001 From: abkum3 Date: Tue, 14 Oct 2025 20:00:59 +0530 Subject: [PATCH 5/7] updating approach docs with possible impact --- NESTED_REFS_SOLUTION.md | 41 +++++++++++++++++++++++++++++++++-------- 1 file changed, 33 insertions(+), 8 deletions(-) diff --git a/NESTED_REFS_SOLUTION.md b/NESTED_REFS_SOLUTION.md index b474965..5a358f3 100644 --- a/NESTED_REFS_SOLUTION.md +++ b/NESTED_REFS_SOLUTION.md @@ -2,21 +2,40 @@ ## Problem Statement -The Spectral validator `validation.js` was not catching two types of issues: +The OpenAPI spec file (`manage.yaml`) from issue [#48](https://wwwin-github.cisco.com/DevNet/api-insights-support/issues/48) has an indentation issue in the `CommonAnomaly` property of `components.schemas.ThresholdAnomaly`. The `severities` property should have been defined one level up as a sibling to `commonAnomaly`. + +However, this cannot be identified as a schema structure problem because, even with the incorrect indentation in `manage.yaml`, `severities` can rightfully be considered as a property of `commonAnomaly` as in OpenAPI 3.1.x, the specification allows `$ref` to have sibling properties at the same level, while OAS 3.0.x does not allow this and suggests ignoring such properties if they are present. + +```yaml +components: + schemas: + CommonAnomalyPolicy: + type: object + ThresholdAnomaly: + type: object + properties: + commonAnomaly: + $ref: '#/components/schemas/CommonAnomalyPolicy' + severities: # indentation issue; it was intended at one level up + description: severity threshold values + items: + $ref: '#/components/schemas/PolicySeverity' # a broken $ref which be caught if 'severities' is at correct level + type: array + required: + - commonAnomaly + - severities +``` + +However, to minimize false positive cases of successfully validating such specs with indentation issues, we have focused on catching below two related issues at the time validation: 1. **Broken `$ref` references** like `#/components/schemas/PolicySeverity` in nested structures within sibling properties of another `$ref` -2. **Undefined required properties** where properties are listed in the `required` array but not defined in the `properties` object +2. **Undefined required properties** where properties are listed in the `required` array but not defined in the `properties` object like the `severities` property in this case The original issue #48 reported is [here](https://wwwin-github.cisco.com/DevNet/api-insights-support/issues/48). ## Root Cause Analysis ### Issue 1: Broken Nested References - -The OpenAPI spec file from issue #48 (`manage.yaml`) has an indentation issue in the `CommonAnomaly` property of `components.schemas.ThresholdAnomaly`. The `severities` property should have been defined one level up as a sibling to `commonAnomaly`. - -However, this cannot be identified as a schema structure problem because, even with the incorrect indentation in `manage.yaml`, `severities` can rightfully be considered as a property of `commonAnomaly`. - In OpenAPI 3.1.x, the specification allows `$ref` to have sibling properties at the same level, while OAS 3.0.x does not allow this and suggests ignoring such properties if they are present. ```yaml @@ -35,7 +54,7 @@ Broken nested references like `#/components/schemas/PolicySeverity` were not bei Properties listed in the `required` array but not defined in the `properties` object: ```yaml -TestSchema: +ThresholdAnomaly: type: object properties: commonAnomaly: @@ -56,6 +75,12 @@ TestSchema: - Spectral's default `oas3-schema` ruleset does not validate that all properties in the `required` array are actually defined in the `properties` object - This validation gap is a common source of errors in real-world OpenAPI specifications +## Possible Impact of Implemented Solution + +The fix for detecting **Broken References** at nested levels will have a positive impact on existing validations by correctly flagging these issues during the validation step. + +However, the fix for **Undefined Required Properties** may impact the validation behavior of existing OpenAPI specifications. Some specifications that previously passed validation successfully may now fail due to the presence of properties that are listed as required but not actually defined in the schema. + ## Solution Implemented ### 1. `broken-internal-refs` Rule From efd3ad0a72af696214b5c14843c4488931563395 Mon Sep 17 00:00:00 2001 From: abkum3 Date: Fri, 17 Oct 2025 18:38:41 +0530 Subject: [PATCH 6/7] research findings to catch indentation problem --- unknown_keyword_validation.md | 320 ++++++++++++++++++++++++++++++++++ 1 file changed, 320 insertions(+) create mode 100644 unknown_keyword_validation.md diff --git a/unknown_keyword_validation.md b/unknown_keyword_validation.md new file mode 100644 index 0000000..5a33913 --- /dev/null +++ b/unknown_keyword_validation.md @@ -0,0 +1,320 @@ +# Unknown Keyword Validation in OpenAPI Schemas + +## Overview + +This document explains how OpenAPI/JSON Schema validators handle **unknown keywords** in schema definitions, and why **indentation errors** can go undetected during validation. Understanding this behavior is critical for avoiding subtle bugs in OpenAPI specifications. + +## Key Concept: Unknown Keywords Are Ignored + +According to JSON Schema specifications, validators **MUST NOT raise errors** for unknown keywords. This design choice allows for: +- **Custom extensions** (like `x-custom-field`) +- **Future keyword compatibility** without breaking existing validators +- **Annotation support** for tooling + +However, this behavior can **mask indentation errors** where properties are accidentally placed at the wrong schema level. + +--- + +## Test Setup + +All tests use a simple Spectral ruleset that extends standard OAS rules: + +**b.spectral.yaml:** +```yaml +extends: ["spectral:oas"] +``` + +This includes built-in rules like `oas3-valid-schema-example` and `oas3-valid-media-example`. + +--- + +## Scenario 1: Unknown Keyword at Wrong Level + +### Schema Definition + +```yaml +openapi: 3.1.0 +info: + title: My API + version: 1.0.0 + +components: + schemas: + ThresholdAnomaly: + description: Threshold type Anomaly policy definition + properties: + commonAnomaly: + severities: # ❌ Unknown keyword - NOT inside 'properties' + type: string + specialAnolamy: # ✅ Correctly defined property + type: string + type: object + example: + commonAnomaly: + severities: 4 + specialAnolamy: 10 +``` + +### Validation Result + +```bash +$ spectral lint -r b.spectral.yaml minimal.yaml -D + +/Users/abkum3/work/minimal.yaml + 20:25 error oas3-valid-schema-example "specialAnolamy" property type must be string + components.schemas.ThresholdAnomaly.example.specialAnolamy + +✖ 1 problem (1 error, 0 warnings, 0 infos, 0 hints) +``` + +### Analysis + +- ✅ **Type error caught** for `specialAnolamy` (expected string, got integer `10`) +- ❌ **No error for `severities`** - it's silently ignored as an unknown keyword +- The validator treats `commonAnomaly` as having **no defined properties** +- `severities` in the example is also ignored because the resolved schema doesn't recognize it + +--- + +## Scenario 2: Adding `additionalProperties: false` + +### Schema Definition + +```yaml +openapi: 3.1.0 +info: + title: My API + version: 1.0.0 + +components: + schemas: + ThresholdAnomaly: + description: Threshold type Anomaly policy definition + properties: + commonAnomaly: + additionalProperties: false # ← Strict mode: no extra properties allowed + severities: # ← Still an unknown keyword (ignored) + type: string + specialAnolamy: + type: string + type: object + example: + commonAnomaly: + severities: 4 # ← Now violates additionalProperties! + specialAnolamy: 10 +``` + +### Validation Result + +```bash +$ spectral lint -r b.spectral.yaml minimal.yaml -D + +/Users/abkum3/work/minimal.yaml + 19:23 error oas3-valid-schema-example Property "severities" is not expected to be here + components.schemas.ThresholdAnomaly.example.commonAnomaly + +✖ 1 problem (1 error, 0 warnings, 0 infos, 0 hints) +``` + +### Analysis + +- ✅ **Now catches `severities` error** in the example +- Why? The resolved schema is: + ```yaml + commonAnomaly: + additionalProperties: false + # severities keyword ignored + # No 'properties' defined + ``` +- Since `additionalProperties: false` and no properties are defined, `severities` in the example is **not allowed** +- This confirms that `severities` was **never recognized** as a property definition + +--- + +## Scenario 3: Correct Schema Structure + +### Schema Definition + +```yaml +openapi: 3.1.0 +info: + title: My API + version: 1.0.0 + +components: + schemas: + ThresholdAnomaly: + description: Threshold type Anomaly policy definition + properties: + commonAnomaly: + additionalProperties: false + properties: # ✅ Correct: properties keyword added + severities: # ✅ Now a proper property definition + type: string + specialAnolamy: + type: string + type: object + example: + commonAnomaly: + severities: 4 # ❌ Wrong type (integer vs string) + specialAnolamy: "10" # ✅ Correct type +``` + +### Validation Result + +```bash +$ spectral lint -r b.spectral.yaml minimal.yaml -D + +/Users/abkum3/work/minimal.yaml + 21:23 error oas3-valid-schema-example "severities" property type must be string + components.schemas.ThresholdAnomaly.example.commonAnomaly.severities + +✖ 1 problem (1 error, 0 warnings, 0 infos, 0 hints) +``` + +### Analysis + +- ✅ **Type error correctly caught** for `severities` property +- Now `severities` is properly defined within `properties` +- The example validation works as expected + +--- + +## Scenario 4: Correct Top-Level Properties + +### Schema Definition + +```yaml +openapi: 3.1.0 +info: + title: My API + version: 1.0.0 + +components: + schemas: + ThresholdAnomaly: + description: Threshold type Anomaly policy definition + additionalProperties: false + properties: + commonAnomaly: + type: string + severities: # ✅ Top-level property (sibling to commonAnomaly) + type: string + type: object + example: + commonAnomaly: "4" + severities: "10" +``` + +### Validation Result + +```bash +$ spectral lint -r b.spectral.yaml minimal.yaml -D + +No results with a severity of 'error' found! +``` + +### Analysis + +- ✅ **No errors** - schema and example are both correct +- `severities` is properly defined as a property of `ThresholdAnomaly` +- Both properties are siblings at the same level + +--- + +## Scenario 5: Indentation Error Example + +### Schema Definition + +```yaml +openapi: 3.1.0 +info: + title: My API + version: 1.0.0 + +components: + schemas: + ThresholdAnomaly: + description: Threshold type Anomaly policy definition + additionalProperties: false + properties: + commonAnomaly: + type: string + severities: # ❌ Indentation error: moved one level too deep + type: string + type: object + example: + commonAnomaly: "4" + severities: "10" # ❌ Not defined at this level +``` + +### Validation Result + +```bash +$ spectral lint -r b.spectral.yaml minimal.yaml -D + +/Users/abkum3/work/minimal.yaml + 17:15 error oas3-valid-schema-example Property "severities" is not expected to be here + components.schemas.ThresholdAnomaly.example + +✖ 1 problem (1 error, 0 warnings, 0 infos, 0 hints) +``` + +### Analysis + +- ✅ **Example validation catches the issue** - `severities` not defined at `ThresholdAnomaly` level +- ❌ **Schema structure error not caught** - `severities` under `commonAnomaly` is ignored as unknown keyword +- The resolved schema has: + ```yaml + ThresholdAnomaly: + properties: + commonAnomaly: + type: string + # severities ignored + # No severities property defined + ``` +- This is a **common indentation mistake** that validation doesn't catch directly + +--- + +## Official Specification References + +### OpenAPI 3.0.3 + +**Schema Object Definition:** +[https://spec.openapis.org/oas/v3.0.3.html#schema-object](https://spec.openapis.org/oas/v3.0.3.html#schema-object) + +**JSON Schema Core (Draft Wright 00), Section 4.4:** +[https://datatracker.ietf.org/doc/html/draft-wright-json-schema-00#section-4.4](https://datatracker.ietf.org/doc/html/draft-wright-json-schema-00#section-4.4) + +> "A JSON Schema MAY contain properties which are not schema keywords. Unknown keywords SHOULD be ignored." + +### OpenAPI 3.1.0 + +**Schema Object Definition:** +[https://spec.openapis.org/oas/v3.1.0.html#schema-object](https://spec.openapis.org/oas/v3.1.0.html#schema-object) + +**JSON Schema Core (Draft Bhutton 01), Section 4.3.1:** +[https://datatracker.ietf.org/doc/html/draft-bhutton-json-schema-01#section-4.3.1](https://datatracker.ietf.org/doc/html/draft-bhutton-json-schema-01#section-4.3.1) + +> "A JSON Schema MAY contain properties which are not schema keywords. Unknown keywords SHOULD be treated as annotations, where the value of the keyword is the value of the annotation." + +--- + +## Why a Custom Validation Rule Is Challenging + +While it's technically possible to create a custom Spectral rule to detect unknown keywords in schemas, this approach has significant limitations: + +### Challenges: + +1. **Keyword Inventory**: Would require maintaining a comprehensive list of all valid JSON Schema keywords +2. **Version Differences**: OpenAPI 3.0.x and 3.1.0 support different sets of keywords +3. **JSON Schema Evolution**: New keywords are added in each JSON Schema version +4. **Extension Keywords**: Need to handle `x-*` custom extensions properly +5. **Maintenance Burden**: List would need constant updates as specifications evolve + +### Alternative Approach: + +**Example Validation**: The `oas3-valid-schema-example` and `oas3-valid-media-example` rules could be turned on in the spectral validation. This would help identify such indentaion issues provided example(s) have been added in the spec file. This will also report an error if a required property is missing in the example. + + From 69bec1716c4e2388cad1f726a577e05e54b8bd6f Mon Sep 17 00:00:00 2001 From: abkum3 Date: Mon, 20 Oct 2025 16:04:12 +0530 Subject: [PATCH 7/7] updated both the docs --- NESTED_REFS_SOLUTION.md | 19 ++- unknown_keyword_validation.md | 241 ++++++++++++++++++++++++++++++---- 2 files changed, 223 insertions(+), 37 deletions(-) diff --git a/NESTED_REFS_SOLUTION.md b/NESTED_REFS_SOLUTION.md index 5a358f3..a8d7dfe 100644 --- a/NESTED_REFS_SOLUTION.md +++ b/NESTED_REFS_SOLUTION.md @@ -4,8 +4,6 @@ The OpenAPI spec file (`manage.yaml`) from issue [#48](https://wwwin-github.cisco.com/DevNet/api-insights-support/issues/48) has an indentation issue in the `CommonAnomaly` property of `components.schemas.ThresholdAnomaly`. The `severities` property should have been defined one level up as a sibling to `commonAnomaly`. -However, this cannot be identified as a schema structure problem because, even with the incorrect indentation in `manage.yaml`, `severities` can rightfully be considered as a property of `commonAnomaly` as in OpenAPI 3.1.x, the specification allows `$ref` to have sibling properties at the same level, while OAS 3.0.x does not allow this and suggests ignoring such properties if they are present. - ```yaml components: schemas: @@ -26,12 +24,12 @@ components: - severities ``` -However, to minimize false positive cases of successfully validating such specs with indentation issues, we have focused on catching below two related issues at the time validation: - -1. **Broken `$ref` references** like `#/components/schemas/PolicySeverity` in nested structures within sibling properties of another `$ref` -2. **Undefined required properties** where properties are listed in the `required` array but not defined in the `properties` object like the `severities` property in this case +To minimize cases of successfully validating such specs with indentation issues, we have identified two different approaches: +1. Identify and flag the **unknown keyword**: Wrong indentation, like in the issue#48, would cause one sibling property to move inside another property as its child making it an `unknown keyword`. The reasearch on this and approach to identify `unknown keyword` has been documented in [unknown_keyword_validation.md](unknown_keyword_validation.md). +2. Identify and flag **Broken `$ref` references** and **Undefined required properties** + - **Broken `$ref` references** like `#/components/schemas/PolicySeverity` in nested structures within sibling properties of another `$ref` + - **Undefined required properties** where properties are listed in the `required` array but not defined in the `properties` object like the `severities` property in this case -The original issue #48 reported is [here](https://wwwin-github.cisco.com/DevNet/api-insights-support/issues/48). ## Root Cause Analysis @@ -68,11 +66,12 @@ ThresholdAnomaly: ### Why These Issues Weren't Caught 1. **Broken References**: - - **In OAS 3.0.x**: Spectral ignores sibling properties of `$ref`, so validation never reaches nested `$ref` references - - **In OAS 3.1.x**: Spectral resolves `$ref` references during parsing and merges sibling properties. By the time validation runs, the nested broken `$ref` has been absorbed into the resolved schema structure, making it undetectable + - **In OAS 3.0.x**: Spectral ignores sibling properties of `$ref` as well as any unknown keyword, so validation never reaches nested `$ref` references + - **In OAS 3.1.x**: Spectral resolves `$ref` references during parsing and merges sibling properties. By the time validation runs, the nested broken `$ref` has either been absorbed into the resolved schema structure, making it undetectable or that `$ref` has been ignored as its parent was an unknown keyword due to indentation issue. 2. **Undefined Required Properties**: - - Spectral's default `oas3-schema` ruleset does not validate that all properties in the `required` array are actually defined in the `properties` object + - Spectral's default `oas3-schema` ruleset does not validate that all properties in the `required` array are actually defined in the `properties` object. + - Required properties are validated for their presence in the examples of a schema, not in the schema definition - This validation gap is a common source of errors in real-world OpenAPI specifications ## Possible Impact of Implemented Solution diff --git a/unknown_keyword_validation.md b/unknown_keyword_validation.md index 5a33913..d209e61 100644 --- a/unknown_keyword_validation.md +++ b/unknown_keyword_validation.md @@ -2,7 +2,79 @@ ## Overview -This document explains how OpenAPI/JSON Schema validators handle **unknown keywords** in schema definitions, and why **indentation errors** can go undetected during validation. Understanding this behavior is critical for avoiding subtle bugs in OpenAPI specifications. +This document investigates a critical aspect of OpenAPI/JSON Schema validation: **how validators handle unknown keywords in schema definitions**. Understanding this behavior is essential because it directly impacts the detection of indentation errors in YAML-based specifications. + +### Why This Matters + +YAML indentation errors are particularly dangerous because they can silently corrupt schema structures: +- A misplaced property moves to an incorrect schema level +- Validators treat it as an unknown keyword and ignore it (per JSON Schema specification) +- The specification passes validation successfully +- However, the schema behavior during analysis differs from what was intended + +### What This Document Covers + +This analysis demonstrates: +1. How unknown keywords are handled in OpenAPI 3.0.3 and 3.1.0 +2. Why standard validators cannot catch indentation-related structural errors +3. Spectral's validation behavior confirming this +4. How example validation can reveal these hidden issues to a great extent + +## Problem Statement + +### The Core Question + +**Can validators detect the indentation error in Spec 2, given that Spec 1 represents the intended schema structure?** + +### The Issue + +In Spec 2, the `severities` property has incorrect indentation, moving it from being a property of `ThresholdAnomaly` (as intended in Spec 1) to being a child attribute of the `commonAnomaly` property. + +This creates a critical distinction: +- In **Spec 1**: `severities` is a **property** of `ThresholdAnomaly` (correctly defined under `properties`) +- In **Spec 2**: `severities` becomes an **unknown keyword** of the `commonAnomaly` schema object (at the same level as `type`) + +The fundamental question becomes: **How do OpenAPI/JSON Schema validators handle unknown keywords?** Do they raise errors, issue warnings, or silently ignore them? + +### Spec 1 + +```yaml +openapi: 3.1.0 +info: + title: My API + version: 1.0.0 +components: + schemas: + ThresholdAnomaly: + description: Threshold type Anomaly policy definition + properties: + commonAnomaly: + type: string + severities: # ✅ Top-level property (sibling to commonAnomaly) + type: string + type: object +``` + +### Spec 2 + +```yaml +openapi: 3.1.0 +info: + title: My API + version: 1.0.0 +components: + schemas: + ThresholdAnomaly: + description: Threshold type Anomaly policy definition + properties: + commonAnomaly: + type: string + severities: # ❌ Indentation error: moved one level too deep + type: string + type: object +``` + + ## Key Concept: Unknown Keywords Are Ignored @@ -15,16 +87,40 @@ However, this behavior can **mask indentation errors** where properties are acci --- -## Test Setup +## Official Specification References + +### OpenAPI 3.0.3 + +**Schema Object Definition:** +[https://spec.openapis.org/oas/v3.0.3.html#schema-object](https://spec.openapis.org/oas/v3.0.3.html#schema-object) + +**JSON Schema Core (Draft Wright 00), Section 4.4:** +[https://datatracker.ietf.org/doc/html/draft-wright-json-schema-00#section-4.4](https://datatracker.ietf.org/doc/html/draft-wright-json-schema-00#section-4.4) + +> "A JSON Schema MAY contain properties which are not schema keywords. Unknown keywords SHOULD be ignored." + +### OpenAPI 3.1.0 + +**Schema Object Definition:** +[https://spec.openapis.org/oas/v3.1.0.html#schema-object](https://spec.openapis.org/oas/v3.1.0.html#schema-object) + +**JSON Schema Core (Draft Bhutton 01), Section 4.3.1:** +[https://datatracker.ietf.org/doc/html/draft-bhutton-json-schema-01#section-4.3.1](https://datatracker.ietf.org/doc/html/draft-bhutton-json-schema-01#section-4.3.1) + +> "A JSON Schema MAY contain properties which are not schema keywords. Unknown keywords SHOULD be treated as annotations, where the value of the keyword is the value of the annotation." + +--- + +## Spectral Test Setup -All tests use a simple Spectral ruleset that extends standard OAS rules: +All tests use a simple Spectral ruleset that extends standard OAS rules. The ruleset of `b.spectral.yaml` used in the test setup is a superset of the set of OAS rules used by the api-insights validator. **b.spectral.yaml:** ```yaml extends: ["spectral:oas"] ``` -This includes built-in rules like `oas3-valid-schema-example` and `oas3-valid-media-example`. +Through various scenarios, we have tried to confirm that unknown keywords are indeed ignored by the Spectral validator. However, it can identify the unknown keywords provided additionalProperties are not allowed (`additionalProperties: false`) and an example schema has been provided either at the media level or at the schema level. --- @@ -222,7 +318,7 @@ No results with a severity of 'error' found! --- -## Scenario 5: Indentation Error Example +## Scenario 5: Indentation Error Example when additionalProperties are not allowed ### Schema Definition @@ -236,16 +332,16 @@ components: schemas: ThresholdAnomaly: description: Threshold type Anomaly policy definition - additionalProperties: false + additionalProperties: false # ✅ additionalProperties are NOT allowed properties: commonAnomaly: type: string - severities: # ❌ Indentation error: moved one level too deep + severities: # ❌ Indentation error: moved one level too deep type: string type: object example: commonAnomaly: "4" - severities: "10" # ❌ Not defined at this level + severities: "10" # ❌ Not defined at this level ``` ### Validation Result @@ -262,8 +358,8 @@ $ spectral lint -r b.spectral.yaml minimal.yaml -D ### Analysis -- ✅ **Example validation catches the issue** - `severities` not defined at `ThresholdAnomaly` level -- ❌ **Schema structure error not caught** - `severities` under `commonAnomaly` is ignored as unknown keyword +- ✅ **Example validation catches the issue** - In the schema, `severities` is not defined at the `ThresholdAnomaly` level, but it has moved as an unknown keyword of `ThresholdAnomaly`, hence `severities` has been ignored altogether in the schema structure. However, in the schema example, as `ThresholdAnomaly` can have just one property named `commonAnomaly`, it now catches this issue. +- ❌ **Schema structure error not caught** - `severities` under `commonAnomaly` is ignored as an unknown keyword - The resolved schema has: ```yaml ThresholdAnomaly: @@ -274,34 +370,122 @@ $ spectral lint -r b.spectral.yaml minimal.yaml -D # No severities property defined ``` - This is a **common indentation mistake** that validation doesn't catch directly +- This issue could be caught by doing all of the below: + - Enable `oas3-valid-schema-example` rule in validation + - Provide a schema example + - Disable additionalProperties (`additionalProperties: false`) --- -## Official Specification References +## Scenario 6: Indentation Error Example when additionalProperties are allowed -### OpenAPI 3.0.3 +### Schema Definition -**Schema Object Definition:** -[https://spec.openapis.org/oas/v3.0.3.html#schema-object](https://spec.openapis.org/oas/v3.0.3.html#schema-object) +```yaml +openapi: 3.1.0 +info: + title: My API + version: 1.0.0 -**JSON Schema Core (Draft Wright 00), Section 4.4:** -[https://datatracker.ietf.org/doc/html/draft-wright-json-schema-00#section-4.4](https://datatracker.ietf.org/doc/html/draft-wright-json-schema-00#section-4.4) +components: + schemas: + ThresholdAnomaly: + description: Threshold type Anomaly policy definition + # additionalProperties: false # ❌ additionalProperties are allowed + properties: + commonAnomaly: + type: string + severities: # ❌ Indentation error: moved one level too deep + type: string + type: object + example: + commonAnomaly: "4" + severities: "10" # ✅ Not defined at this level but will not be caught as additionalProperties are allowed +``` -> "A JSON Schema MAY contain properties which are not schema keywords. Unknown keywords SHOULD be ignored." +### Validation Result -### OpenAPI 3.1.0 +```bash +$ spectral lint -r b.spectral.yaml minimal.yaml -D +No results with a severity of 'error' found! +``` -**Schema Object Definition:** -[https://spec.openapis.org/oas/v3.1.0.html#schema-object](https://spec.openapis.org/oas/v3.1.0.html#schema-object) +### Analysis -**JSON Schema Core (Draft Bhutton 01), Section 4.3.1:** -[https://datatracker.ietf.org/doc/html/draft-bhutton-json-schema-01#section-4.3.1](https://datatracker.ietf.org/doc/html/draft-bhutton-json-schema-01#section-4.3.1) +- ❌ **Example validation does NOT catch the issue** - In the schema, `severities` is not defined at the `ThresholdAnomaly` level, but it has moved as an unknown keyword of `ThresholdAnomaly`, hence `severities` has been ignored altogether in the schema structure. Also, in the schema example, as `ThresholdAnomaly` can have properties additional to the defined one, i.e., `commonAnomaly`, `severities` at the example level is not caught as an incorrect schema example. +- ❌ **Schema structure error not caught** - `severities` under `commonAnomaly` is ignored as an unknown keyword -> "A JSON Schema MAY contain properties which are not schema keywords. Unknown keywords SHOULD be treated as annotations, where the value of the keyword is the value of the annotation." +--- + +## Scenario 7: Indentation Error Example at media level + +### Schema Definition + +```yaml +openapi: 3.1.0 +info: + title: My API + version: 1.0.0 + +components: + schemas: + ThresholdAnomaly: + description: Threshold type Anomaly policy definition + additionalProperties: false + properties: + commonAnomaly: + type: string + severities: + type: string + type: object +paths: + /anomaly-policies: + get: + summary: Get anomaly policy + responses: + '200': + description: Successful response + content: + application/json: + schema: + $ref: '#/components/schemas/ThresholdAnomaly' # ❌ Problematic schema being used here + example: + commonAnomaly: "4" + severities: "10" # ❌ Not defined at this level +``` + +### Validation Result + +```bash +$ spectral lint -r b.spectral.yaml minimal.yaml -D + +/Users/abkum3/work/minimal.yaml + 28:23 error oas3-valid-media-example Property "severities" is not expected to be here paths./anomaly-policies.get.responses[200].content.application/json.example + +✖ 1 problem (1 error, 0 warnings, 0 infos, 0 hints) +``` + +### Analysis + +- ✅ **Example validation catches the issue** - In the schema, `severities` is not defined at the `ThresholdAnomaly` level, but it has moved as an unknown keyword of `ThresholdAnomaly`, hence `severities` has been ignored altogether in the schema structure. However, in the schema example, as `ThresholdAnomaly` can have just one property named `commonAnomaly`, it now catches this issue. + +- The resolved schema has: + ```yaml + ThresholdAnomaly: + properties: + commonAnomaly: + type: string + # severities ignored + # No severities property defined + ``` +- This **common indentation mistake** could be caught by doing all of the below: + - Enable `oas3-valid-schema-example` and `oas3-valid-media-example` rules in validation + - Provide a schema example at either the schema definition level or at the media level + - Disable additionalProperties (`additionalProperties: false`) --- -## Why a Custom Validation Rule Is Challenging +## Challenge of Custom Validation Rule While it's technically possible to create a custom Spectral rule to detect unknown keywords in schemas, this approach has significant limitations: @@ -313,8 +497,11 @@ While it's technically possible to create a custom Spectral rule to detect unkno 4. **Extension Keywords**: Need to handle `x-*` custom extensions properly 5. **Maintenance Burden**: List would need constant updates as specifications evolve -### Alternative Approach: - -**Example Validation**: The `oas3-valid-schema-example` and `oas3-valid-media-example` rules could be turned on in the spectral validation. This would help identify such indentaion issues provided example(s) have been added in the spec file. This will also report an error if a required property is missing in the example. +--- +## Possible Solution +**Example Validation**: As demonstrated in Scenario 5 and Scenario 7, implementing `example validation` will help catch indentation issues to a greater extent. It can be implemented by doing the following: +- The `oas3-valid-schema-example` and `oas3-valid-media-example` rules have to be enabled in the Spectral validation. +- The OpenAPI specifications under validation should provide example(s) at either the schema definition level or at the schema consumption (media) level. +- additionalProperties should not be allowed. For that, additionalProperties shall have to be set to false (`additionalProperties: false`) in the OpenAPI specs that are under validation.