Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Original file line number Diff line number Diff line change
@@ -0,0 +1,110 @@
import testRule from './__helpers__/testRule.js';
import { DiagnosticSeverity } from '@stoplight/types';

testRule('xgen-IPA-114-parameterized-paths-have-404-not-found', [
{
name: 'valid parameterized path with 404 response',
document: {
paths: {
'/resources/{resourceId}': {
get: {
responses: {
200: { description: 'Success' },
404: { description: 'Not Found' },
},
},
},
},
},
errors: [],
},
{
name: 'invalid parameterized path missing 404 response',
document: {
paths: {
'/resources/{resourceId}': {
get: {
responses: {
200: { description: 'Success' },
},
},
},
},
},
errors: [
{
code: 'xgen-IPA-114-parameterized-paths-have-404-not-found',
message: 'Parameterized path must define a 404 response.',
path: ['paths', '/resources/{resourceId}', 'get'],
severity: DiagnosticSeverity.Warning,
},
],
},
{
name: 'non-parameterized path without 404 response (valid)',
document: {
paths: {
'/resources': {
get: {
responses: {
200: { description: 'Success' },
},
},
},
},
},
errors: [],
},
{
name: 'parameterized path with multiple parameters and 404 response',
document: {
paths: {
'/resources/{resourceId}/items/{itemId}': {
get: {
responses: {
200: { description: 'Success' },
404: { description: 'Not Found' },
},
},
},
},
},
errors: [],
},
{
name: 'no responses',
document: {
paths: {
'/resources/{resourceId}': {
get: {},
},
},
},
errors: [
{
code: 'xgen-IPA-114-parameterized-paths-have-404-not-found',
message: 'Parameterized path must define a 404 response.',
path: ['paths', '/resources/{resourceId}', 'get'],
severity: DiagnosticSeverity.Warning,
},
],
},
{
name: 'with exception',
document: {
paths: {
'/resources/{resourceId}': {
get: {
'x-xgen-IPA-exception': {
'xgen-IPA-114-parameterized-paths-have-404-not-found': 'Reason',
},
responses: {
200: { description: 'Success' },
},
},
},
},
},
errors: [],
},
]);
14 changes: 14 additions & 0 deletions tools/spectral/ipa/rulesets/IPA-114.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,7 @@ functions:
- IPA114ErrorResponsesReferToApiError
- IPA114ApiErrorHasBadRequestDetail
- IPA114AuthenticatedEndpointsHaveAuthErrors
- IPA114ParameterizedPathsHave404NotFound

rules:
xgen-IPA-114-error-responses-refer-to-api-error:
Expand Down Expand Up @@ -45,3 +46,16 @@ rules:
given: '$.paths[*][get,put,post,delete,options,head,patch,trace]'
then:
function: 'IPA114AuthenticatedEndpointsHaveAuthErrors'
xgen-IPA-114-parameterized-paths-have-404-not-found:
description: |
Paths with parameters must define 404 responses.

##### Implementation details
This rule checks that all endpoints with path parameters (identified by '{param}'
in the path) include a 404 response to handle the case when the requested resource
is not found.
message: '{{error}} https://mdb.link/mongodb-atlas-openapi-validation#xgen-IPA-114-parameterized-paths-have-not-found'
severity: warn
given: '$.paths[*][get,put,post,delete,options,head,patch,trace]'
Copy link
Member

@wtrocki wtrocki Apr 1, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

[Outside PR] Delete has it's own rule covering 404. If if introduce generic rule (which I think makes sense) we should remove dedicated 404 rule for delete. Is that planned? WDYT?

Copy link
Member

@wtrocki wtrocki Apr 1, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

[Question] Separately - what is the purpose of mentioning all of the methods here.
I guess you wanted to exclude exceptions? If yes the same filtering can be done in the code.

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yes, I wanted to exclude exceptions and all the stuff other than HTTP verbs. I agree that it can be handled in the code, but it is widely used approach at the moment, so I followed it

We can remove the dedicated 404 rule for delete, as you said it will be covered by this one. I will share the PR for removing it in a separate PR, if that is okay

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thank you.

then:
function: 'IPA114ParameterizedPathsHave404NotFound'
10 changes: 10 additions & 0 deletions tools/spectral/ipa/rulesets/README.md
Original file line number Diff line number Diff line change
Expand Up @@ -636,6 +636,16 @@ Authenticated endpoints must define 401 and 403 responses.
This rule checks that all authenticated endpoints (those without explicit 'security: []'
and not containing '/unauth' in the path) include 401 and 403 responses.

#### xgen-IPA-114-parameterized-paths-have-404-not-found

![warn](https://img.shields.io/badge/warning-yellow)
Paths with parameters must define 404 responses.

##### Implementation details
This rule checks that all endpoints with path parameters (identified by '{param}'
in the path) include a 404 response to handle the case when the requested resource
is not found.



### IPA-117
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,73 @@
import { hasException } from './utils/exceptions.js';
import {
collectAdoption,
collectAndReturnViolation,
collectException,
handleInternalError,
} from './utils/collectionUtils.js';

const RULE_NAME = 'xgen-IPA-114-parameterized-paths-have-404-not-found';
const ERROR_MESSAGE = `Parameterized path must define a 404 response.`;
/**
* Validates that paths with parameters include a 404 response
*
* @param {object} input - The operation object to check
* @param {object} _ - Rule options (unused)
* @param {object} context - The context object containing path and document information
*/
export default (input, _, { path }) => {
// Path components: [paths, pathName, methodName, ...]
const pathName = path[1];

const pathParamRegex = /{[^{}]+}/;
if (!pathParamRegex.test(pathName)) {
return;
}

// Check for exception at operation level
if (hasException(input, RULE_NAME)) {
collectException(input, RULE_NAME, path);
return;
}

const errors = checkViolationsAndReturnErrors(input.responses, path);
if (errors.length > 0) {
return collectAndReturnViolation(path, RULE_NAME, errors);
}

collectAdoption(path, RULE_NAME);
};

/**
* Check for violations in response structure
*
* @param {object} responses - The responses object to validate
* @param {Array} path - Path to the responses in the document
* @returns {Array} - Array of error objects
*/
function checkViolationsAndReturnErrors(responses, path) {
try {
if (!responses) {
return [
{
path,
message: ERROR_MESSAGE,
},
];
}

// Check for 404 Not Found response
if (!responses['404']) {
return [
{
path,
message: ERROR_MESSAGE,
},
];
}

return [];
} catch (e) {
handleInternalError(RULE_NAME, path, e);
}
}
Loading