diff --git a/.changeset/skip-4xx-safe-methods.md b/.changeset/skip-4xx-safe-methods.md new file mode 100644 index 0000000000..9a14ece2ac --- /dev/null +++ b/.changeset/skip-4xx-safe-methods.md @@ -0,0 +1,9 @@ +--- +'@redocly/openapi-core': patch +'@redocly/cli': patch +--- + +Make the `operation-4xx-response` rule configurable to exclude safe HTTP +methods by default (get, head, options). This allows projects to avoid +requiring 4XX responses for read-only operations while keeping the default +behavior conservative. diff --git a/docs/@v2/rules/oas/operation-4xx-response.md b/docs/@v2/rules/oas/operation-4xx-response.md index 3c7fc29dba..6fef190727 100644 --- a/docs/@v2/rules/oas/operation-4xx-response.md +++ b/docs/@v2/rules/oas/operation-4xx-response.md @@ -23,10 +23,11 @@ While this thinking has mostly changed (for the better in our opinion), it does ## Configuration -| Option | Type | Description | -| ---------------- | ------- | ----------------------------------------------------------------------------------------- | -| severity | string | Possible values: `off`, `warn`, `error`. Default `warn` (in `recommended` configuration). | -| validateWebhooks | boolean | Determines if responses inside webhooks are validated. Default `false`. | +| Option | Type | Description | +| ---------------- | ------- | ----------------------------------------------------------------------------------------------------------- | +| severity | string | Possible values: `off`, `warn`, `error`. Default `warn` (in `recommended` configuration). | +| validateWebhooks | boolean | Determines if responses inside webhooks are validated. Default `false`. | +| excludeMethods | array | List of HTTP methods (case-insensitive) to exclude from 4XX validation. Default: `['get','head','options']` | An example configuration: @@ -44,6 +45,15 @@ rules: validateWebhooks: true ``` +To exclude additional methods from 4XX validation: + +```yaml +rules: + operation-4xx-response: + severity: error + excludeMethods: [get, head, options, trace] +``` + ## Examples Given this configuration: diff --git a/packages/core/src/rules/common/__tests__/operation-4xx-response.exclude.test.ts b/packages/core/src/rules/common/__tests__/operation-4xx-response.exclude.test.ts new file mode 100644 index 0000000000..a7161befeb --- /dev/null +++ b/packages/core/src/rules/common/__tests__/operation-4xx-response.exclude.test.ts @@ -0,0 +1,71 @@ +import { outdent } from 'outdent'; + +import { parseYamlToDocument, replaceSourceWithRef } from '../../../../__tests__/utils.js'; +import { createConfig } from '../../../config/index.js'; +import { lintDocument } from '../../../lint.js'; +import { BaseResolver } from '../../../resolve.js'; + +describe('Oas3 operation-4xx-response (exclude methods)', () => { + it('should not report for excluded methods by default (GET)', async () => { + const document = parseYamlToDocument( + outdent` + openapi: 3.0.0 + paths: + '/test': + get: + responses: + 200: + description: ok response + `, + 'foobar.yaml' + ); + + const results = await lintDocument({ + externalRefResolver: new BaseResolver(), + document, + config: await createConfig({ rules: { 'operation-4xx-response': 'error' } }), + }); + + expect(replaceSourceWithRef(results)).toMatchInlineSnapshot(`[]`); + }); + + it('should report for non-excluded methods (POST) when missing 4xx', async () => { + const document = parseYamlToDocument( + outdent` + openapi: 3.0.0 + paths: + '/test': + post: + responses: + 200: + description: ok response + `, + 'foobar.yaml' + ); + + const results = await lintDocument({ + externalRefResolver: new BaseResolver(), + document, + config: await createConfig({ rules: { 'operation-4xx-response': 'error' } }), + }); + + expect(replaceSourceWithRef(results)).toMatchInlineSnapshot(` + [ + { + "location": [ + { + "pointer": "#/paths/~1test/post/responses", + "reportOnKey": true, + "source": "foobar.yaml", + }, + ], + "message": "Operation must have at least one \`4XX\` response.", + "reference": "https://redocly.com/docs/cli/rules/oas/operation-4xx-response", + "ruleId": "operation-4xx-response", + "severity": "error", + "suggest": [], + }, + ] + `); + }); +}); diff --git a/packages/core/src/rules/common/operation-4xx-response.ts b/packages/core/src/rules/common/operation-4xx-response.ts index e61247b772..163612d8a6 100644 --- a/packages/core/src/rules/common/operation-4xx-response.ts +++ b/packages/core/src/rules/common/operation-4xx-response.ts @@ -2,32 +2,55 @@ import type { Oas3Rule, Oas2Rule } from '../../visitors.js'; import type { UserContext } from '../../walk.js'; import { validateResponseCodes } from '../utils.js'; -export const Operation4xxResponse: Oas3Rule | Oas2Rule = ({ validateWebhooks }) => { +export const Operation4xxResponse: Oas3Rule | Oas2Rule = (opts: any = {}) => { + const { validateWebhooks, excludeMethods: rawExcludeMethods } = opts || {}; + const defaultExcluded = ['get', 'head', 'options']; + const excludeMethods = Array.isArray(rawExcludeMethods) + ? rawExcludeMethods.map((m: string) => String(m).toLowerCase()) + : defaultExcluded; + return { Paths: { - Responses(responses: Record, { report }: UserContext) { - const codes = Object.keys(responses || {}); - - validateResponseCodes({ - responseCodes: codes, - codeRange: '4XX', - report: report as UserContext['report'], - reference: 'https://redocly.com/docs/cli/rules/oas/operation-4xx-response', - }); + Operation: { + leave(operation: Record, { report, key, location }: UserContext) { + const method = String(key).toLowerCase(); + if (excludeMethods.includes(method)) return; + + const codes = Object.keys((operation.responses as Record) || {}); + + // keep the reported location consistent with previous implementation + const childReport: UserContext['report'] = (problem) => + report({ ...problem, location: location.child(['responses']).key() }); + + validateResponseCodes({ + responseCodes: codes, + codeRange: '4XX', + report: childReport, + reference: 'https://redocly.com/docs/cli/rules/oas/operation-4xx-response', + }); + }, }, }, WebhooksMap: { - Responses(responses: Record, { report }: UserContext) { - if (!validateWebhooks) return; + Operation: { + leave(operation: Record, { report, key, location }: UserContext) { + if (!validateWebhooks) return; + + const method = String(key).toLowerCase(); + if (excludeMethods.includes(method)) return; + + const codes = Object.keys((operation.responses as Record) || {}); - const codes = Object.keys(responses || {}); + const childReport: UserContext['report'] = (problem) => + report({ ...problem, location: location.child(['responses']).key() }); - validateResponseCodes({ - responseCodes: codes, - codeRange: '4XX', - report: report as UserContext['report'], - reference: 'https://redocly.com/docs/cli/rules/oas/operation-4xx-response', - }); + validateResponseCodes({ + responseCodes: codes, + codeRange: '4XX', + report: childReport, + reference: 'https://redocly.com/docs/cli/rules/oas/operation-4xx-response', + }); + }, }, }, };