diff --git a/NESTED_REFS_SOLUTION.md b/NESTED_REFS_SOLUTION.md new file mode 100644 index 0000000..a8d7dfe --- /dev/null +++ b/NESTED_REFS_SOLUTION.md @@ -0,0 +1,302 @@ +# Nested Broken References Detection - Solution Summary + +## Problem Statement + +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`. + +```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 +``` + +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 + + +## Root Cause Analysis + +### 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 does not allow this and suggests ignoring such properties if they are present. + +```yaml +commonAnomaly: + $ref: '#/components/schemas/CommonAnomalyPolicy' + 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 +``` + +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 + +Properties listed in the `required` array but not defined in the `properties` object: + +```yaml +ThresholdAnomaly: + 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 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. + - 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 + +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 + +**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 clean error messages + +**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 `#/`) +- 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` 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 + +**Files**: +- [functions/validateRefSiblings.spec.js](/functions/validateRefSiblings.spec.js) +- [functions/validateRequiredProperties.spec.js](/functions/validateRequiredProperties.spec.js) + +**Coverage**: +- 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 + +**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 and 3.1.0 scenarios +- Real-world scenarios from `manage.yaml` +- Request/response/parameter schema validation +- Mixed valid and invalid scenarios +- External references handling +- Complex nested structures + +#### Test Resource Files + +**Directory**: [test/resources/](/test/resources/) + +**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 + +### 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 +- Both rules **work** and catch issues effectively + +### OpenAPI 3.1.x + +- `$ref` **CAN** have sibling properties +- Structure is **VALID** per specification +- Both rules are **ESSENTIAL** to catch nested broken refs and undefined properties + +## Usage + +### Testing the Solution + +```bash +# 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 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/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 + +**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-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/undefined-required-properties/undefined-required-properties-3.0.x.yml + +/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 and properties**: +```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! +``` + +## Real-World Impact + +### Issues Found in `manage.yaml` + +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 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) +``` + +## Technical Details + +### The `resolved: false` Flag + +This is the **critical** configuration that makes the broken references 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 + +### 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. + diff --git a/functions/validateRefSiblings.js b/functions/validateRefSiblings.js new file mode 100644 index 0000000..09d6453 --- /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 '${ref}'`, + }; + } + } + + // Reference is valid + return null; +} + diff --git a/functions/validateRefSiblings.spec.js b/functions/validateRefSiblings.spec.js new file mode 100644 index 0000000..d0df260 --- /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 '#/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 '#/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 '#/components/schemas/NonExistent1'" + }); + expect(result).toContainEqual({ + message: "broken reference '#/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 '#/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 '#/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 '#/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 '#/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 '#/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 '#/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/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/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/unknown_keyword_validation.md b/unknown_keyword_validation.md new file mode 100644 index 0000000..d209e61 --- /dev/null +++ b/unknown_keyword_validation.md @@ -0,0 +1,507 @@ +# Unknown Keyword Validation in OpenAPI Schemas + +## Overview + +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 + +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. + +--- + +## 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. 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"] +``` + +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. + +--- + +## 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 when additionalProperties are not allowed + +### 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 # ✅ additionalProperties are NOT 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 +``` + +### 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** - 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: + properties: + commonAnomaly: + type: string + # severities ignored + # 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`) + +--- + +## Scenario 6: Indentation Error Example when additionalProperties are allowed + +### 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 # ❌ 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 +``` + +### Validation Result + +```bash +$ spectral lint -r b.spectral.yaml minimal.yaml -D +No results with a severity of 'error' found! +``` + +### Analysis + +- ❌ **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 + +--- + +## 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`) + +--- + +## 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: + +### 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 + +--- + +## 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. diff --git a/validation.js b/validation.js index a9042da..c975ee1 100644 --- a/validation.js +++ b/validation.js @@ -19,6 +19,9 @@ 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': [ [ @@ -45,5 +48,35 @@ export default { }, ], }, + + 'broken-internal-refs': { + 'description': 'internal references should exist', + 'message': '{{description}}; {{error}}', + 'severity': 'error', + 'resolved': false, + 'given': '$..', + 'then': [ + { + 'function': validateRefSiblings, + }, + ], + }, + + '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, + }, + ], + }, }, };