diff --git a/NESTED_REFS_SOLUTION.md b/NESTED_REFS_SOLUTION.md new file mode 100644 index 0000000..afeefc1 --- /dev/null +++ b/NESTED_REFS_SOLUTION.md @@ -0,0 +1,195 @@ +# 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`. +The original issue #48 reported is [here](https://wwwin-github.cisco.com/DevNet/api-insights-support/issues/48). + +## 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 `#/`) + +### 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 + + +### 4. 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, + }, + ], + }, }, };