From d991e7a3741974bd664aa16d6c609b61ec974573 Mon Sep 17 00:00:00 2001 From: Guilherme Gazzo Date: Thu, 30 Jul 2026 11:20:25 -0300 Subject: [PATCH 01/16] feat(api): richer OpenAPI metadata for typed endpoints The generated OpenAPI document was missing most of what makes a spec usable: path parameters were emitted with Express syntax and no parameter objects, query parameters were collapsed into a single opaque `query` object, response descriptions were hardcoded as empty strings, and there was no way for an endpoint to declare a summary, a description, examples or a deprecation notice. Route options now accept `summary`, `description`, `operationId`, `deprecated`, `externalDocs`, `params`, `bodyContentType`, `responseDescriptions` and `examples`. Everything else is derived from options the framework already has: permissions, license modules and two-factor requirements become `x-*` extensions plus a note in the description, `deprecation` sets `deprecated`, rate limiting documents its response headers and `429`, and the error responses implied by the route (400, 401, 403, 429, 500) are injected when not declared. Path parameters are now derived from the path pattern itself, so every `{param}` in the document has a matching parameter object, and query schemas are exploded into one parameter per property, honoring `allOf`/`anyOf`/`oneOf` composition. Both copies of the OpenAPI conversion were replaced by a single builder in `@rocket.chat/http-router`. The copy living in `APIClass` wrote to a `typedRoutes` field that nothing ever read - the served document has always come from the router - so it was removed along with its legacy counterpart. The `example` annotation is now registered as an AJV vocabulary, so schemas can carry their own `description` and `example` and have them flow straight into the document. The banners endpoints show the pattern and drop the stale `@openapi` JSDoc blocks that nothing had been reading. --- .changeset/loud-otters-document.md | 6 + apps/meteor/server/api/ApiClass.ts | 109 +---- apps/meteor/server/api/default/openApi.ts | 37 +- apps/meteor/server/api/definition.ts | 4 +- apps/meteor/server/api/v1/banners.ts | 146 ++----- apps/meteor/tests/end-to-end/api/openapi.ts | 102 +++++ packages/http-router/src/Router.spec.ts | 30 ++ packages/http-router/src/Router.ts | 82 +--- packages/http-router/src/definition.ts | 4 +- packages/http-router/src/index.ts | 1 + packages/http-router/src/openapi.spec.ts | 221 ++++++++++ packages/http-router/src/openapi.ts | 437 ++++++++++++++++++++ packages/rest-typings/src/v1/Ajv.ts | 5 + packages/rest-typings/src/v1/banners.ts | 19 + 14 files changed, 895 insertions(+), 308 deletions(-) create mode 100644 .changeset/loud-otters-document.md create mode 100644 apps/meteor/tests/end-to-end/api/openapi.ts create mode 100644 packages/http-router/src/openapi.spec.ts create mode 100644 packages/http-router/src/openapi.ts diff --git a/.changeset/loud-otters-document.md b/.changeset/loud-otters-document.md new file mode 100644 index 0000000000000..2e591c96414c1 --- /dev/null +++ b/.changeset/loud-otters-document.md @@ -0,0 +1,6 @@ +--- +'@rocket.chat/rest-typings': patch +'@rocket.chat/meteor': patch +--- + +Improves the generated OpenAPI document: path parameters are now templated and described, query parameters are documented one by one instead of as a single opaque object, responses always carry a description, and endpoints can declare a summary, a description, examples and a deprecation notice diff --git a/apps/meteor/server/api/ApiClass.ts b/apps/meteor/server/api/ApiClass.ts index 71dfc86bc0020..1a00e8e396d77 100644 --- a/apps/meteor/server/api/ApiClass.ts +++ b/apps/meteor/server/api/ApiClass.ts @@ -1,11 +1,10 @@ import type { IMethodConnection, IUser } from '@rocket.chat/core-typings'; -import type { Route, Router } from '@rocket.chat/http-router'; +import type { Router } from '@rocket.chat/http-router'; import { License } from '@rocket.chat/license'; import { Logger } from '@rocket.chat/logger'; import { Users } from '@rocket.chat/models'; import { Random } from '@rocket.chat/random'; import type { JoinPathPattern, Method } from '@rocket.chat/rest-typings'; -import { ajv } from '@rocket.chat/rest-typings'; import { wrapExceptions } from '@rocket.chat/tools'; import type { ValidateFunction } from 'ajv'; import { Accounts } from 'meteor/accounts-base'; @@ -161,8 +160,6 @@ export const generateConnection = ( }); export class APIClass = Record> { - public typedRoutes: Record> = {}; - protected apiPath?: string; readonly version?: string; @@ -548,104 +545,6 @@ export class APIClass this.getFullRouteName(route, action)); } - private registerTypedRoutesLegacy( - method: Method, - subpath: TSubPathPattern, - options: TOptions, - ): void { - const { authRequired, validateParams } = options; - - const opt = { - authRequired, - ...(validateParams && - method.toLowerCase() === 'get' && - ('GET' in validateParams - ? { query: validateParams.GET } - : { - query: validateParams as ValidateFunction, - })), - - ...(validateParams && - method.toLowerCase() === 'post' && - ('POST' in validateParams ? { query: validateParams.POST } : { body: validateParams as ValidateFunction })), - - ...(validateParams && - method.toLowerCase() === 'put' && - ('PUT' in validateParams ? { query: validateParams.PUT } : { body: validateParams as ValidateFunction })), - ...(validateParams && - method.toLowerCase() === 'delete' && - ('DELETE' in validateParams ? { query: validateParams.DELETE } : { body: validateParams as ValidateFunction })), - - tags: ['Missing Documentation'], - response: { - 200: ajv.compile({ - type: 'object', - properties: { - success: { type: 'boolean' }, - error: { type: 'string' }, - }, - required: ['success'], - }), - }, - }; - - this.registerTypedRoutes(method, subpath, opt); - } - - private registerTypedRoutes< - TSubPathPattern extends string, - TOptions extends TypedOptions, - TPathPattern extends `${TBasePath}/${TSubPathPattern}`, - >(method: MinimalRoute['method'], subpath: TSubPathPattern, options: TOptions): void { - const path = `/${this.apiPath}/${subpath}`.replaceAll('//', '/') as TPathPattern; - this.typedRoutes = this.typedRoutes || {}; - this.typedRoutes[path] = this.typedRoutes[path] || {}; - const { query, authRequired, response, body, tags, ...rest } = options; - this.typedRoutes[path][method.toLowerCase()] = { - ...(response && { - responses: Object.fromEntries( - Object.entries(response).map(([status, schema]) => [ - status, - { - description: '', - content: { - 'application/json': 'schema' in schema ? { schema: schema.schema } : schema, - }, - }, - ]), - ), - }), - ...(query && { - parameters: [ - { - schema: query.schema, - in: 'query', - name: 'query', - required: true, - }, - ], - }), - ...(body && { - requestBody: { - required: true, - content: { - 'application/json': { schema: body.schema }, - }, - }, - }), - ...(authRequired && { - ...rest, - security: [ - { - userId: [], - authToken: [], - }, - ], - }), - tags, - }; - } - private method( method: MinimalRoute['method'], subpath: TSubPathPattern, @@ -662,7 +561,6 @@ export class APIClass > { this.addRoute([subpath], { tags: [], ...options, typed: true }, { [method.toLowerCase()]: { action } } as any); - this.registerTypedRoutes(method, subpath, options); return this; } @@ -940,11 +838,6 @@ export class APIClass] as unknown as Record, }); - - this.registerTypedRoutesLegacy(method as Method, route, { - ...options, - ...operations[method as keyof Operations], - }); }); }); } diff --git a/apps/meteor/server/api/default/openApi.ts b/apps/meteor/server/api/default/openApi.ts index 35e9e55a0d993..bd9be07e29660 100644 --- a/apps/meteor/server/api/default/openApi.ts +++ b/apps/meteor/server/api/default/openApi.ts @@ -1,5 +1,6 @@ import { schemas } from '@rocket.chat/core-typings'; import type { Route } from '@rocket.chat/http-router'; +import { openAPIErrorComponents, withOperationIds } from '@rocket.chat/http-router'; import { ajv, isOpenAPIJSONEndpoint } from '@rocket.chat/rest-typings'; import express from 'express'; import { WebApp } from 'meteor/webapp'; @@ -41,18 +42,42 @@ const getTypedRoutes = ( ); }; +const TAG_DESCRIPTIONS: Record = { + 'Missing Documentation': 'Endpoints that are not typed yet; their request and response shapes are not described.', +}; + +const getTags = (paths: Record>) => { + const names = new Set( + Object.values(paths) + .flatMap((methods) => Object.values(methods)) + .flatMap((route) => route.tags ?? []), + ); + + return [...names].sort().map((name) => ({ + name, + ...(TAG_DESCRIPTIONS[name] && { description: TAG_DESCRIPTIONS[name] }), + })); +}; + const makeOpenAPIResponse = (paths: Record>) => ({ openapi: '3.0.3', info: { title: 'Rocket.Chat API', - description: 'Rocket.Chat API', + description: + 'REST API of this Rocket.Chat workspace. Authenticate by sending the `X-User-Id` and `X-Auth-Token` headers obtained from `/api/v1/login`.', version: getTrimmedServerVersion(), }, + externalDocs: { + url: 'https://developer.rocket.chat/apidocs', + description: 'Rocket.Chat developer documentation', + }, servers: [ { url: settings.get('Site_Url'), }, ], + tags: getTags(paths), + paths: withOperationIds(paths), components: { securitySchemes: { userId: { @@ -66,10 +91,11 @@ const makeOpenAPIResponse = (paths: Record>) => ({ name: 'X-Auth-Token', }, }, - schemas: schemas.components.schemas, + schemas: { + ...schemas.components.schemas, + ...openAPIErrorComponents, + }, }, - schemas: schemas.components.schemas, - paths, }); const openApiResponseSchema = ajv.compile>({ @@ -77,10 +103,11 @@ const openApiResponseSchema = ajv.compile>({ properties: { openapi: { type: 'string' }, info: { type: 'object' }, + externalDocs: { type: 'object' }, servers: { type: 'array' }, + tags: { type: 'array' }, components: { type: 'object' }, paths: { type: 'object' }, - schemas: { type: 'object' }, success: { type: 'boolean', enum: [true] }, }, required: ['openapi', 'info', 'paths', 'success'], diff --git a/apps/meteor/server/api/definition.ts b/apps/meteor/server/api/definition.ts index 3f27770edb0b7..56cff1ee779f1 100644 --- a/apps/meteor/server/api/definition.ts +++ b/apps/meteor/server/api/definition.ts @@ -1,6 +1,7 @@ import type { IncomingMessage } from 'node:http'; import type { IUser, LicenseModule, RequiredField } from '@rocket.chat/core-typings'; +import type { OpenAPIDocumentation } from '@rocket.chat/http-router'; import type { Logger } from '@rocket.chat/logger'; import type { Method, MethodOf, OperationParams, OperationResult, PathPattern, UrlParams } from '@rocket.chat/rest-typings'; import type { ValidateFunction } from 'ajv'; @@ -294,7 +295,8 @@ export type TypedOptions = { tags?: string[]; typed?: boolean; license?: LicenseModule[]; -} & SharedOptions<'GET' | 'POST' | 'PUT' | 'DELETE' | 'PATCH'>; +} & OpenAPIDocumentation & + SharedOptions<'GET' | 'POST' | 'PUT' | 'DELETE' | 'PATCH'>; export type TypedThis = { readonly logger: Logger; diff --git a/apps/meteor/server/api/v1/banners.ts b/apps/meteor/server/api/v1/banners.ts index 46b32c853c131..3873ea6b6de64 100644 --- a/apps/meteor/server/api/v1/banners.ts +++ b/apps/meteor/server/api/v1/banners.ts @@ -2,6 +2,7 @@ import { Banner } from '@rocket.chat/core-services'; import type { IBanner } from '@rocket.chat/core-typings'; import { ajv, + isBannerIdParams, isBannersDismissProps, isBannersProps, validateBadRequestErrorResponse, @@ -27,57 +28,22 @@ const dismissResponseSchema = ajv.compile({ additionalProperties: false, }); -/** - * @openapi - * /api/v1/banners/{id}: - * get: - * description: Gets the banner to be shown to the authenticated user - * security: - * $ref: '#/security/authenticated' - * parameters: - * - name: platform - * in: query - * description: The platform rendering the banner - * required: true - * schema: - * type: string - * enum: [web, mobile] - * example: web - * - name: id - * in: path - * description: The id of the banner - * required: true - * schema: - * type: string - * example: ByehQjC44FwMeiLbX - * responses: - * 200: - * description: | - * A collection with a single banner matching the criteria; an empty - * collection otherwise - * content: - * application/json: - * schema: - * allOf: - * - $ref: '#/components/schemas/ApiSuccessV1' - * - type: object - * properties: - * banners: - * type: array - * items: - * $ref: '#/components/schemas/IBanner' - * default: - * description: Unexpected error - * content: - * application/json: - * schema: - * $ref: '#/components/schemas/ApiFailureV1' - */ API.v1.get( 'banners/:id', { authRequired: true, + summary: 'Get a banner by id', + description: 'Gets the banner to be shown to the authenticated user.', + tags: ['Banners'], query: isBannersProps, + params: isBannerIdParams, + examples: { + params: { id: 'ByehQjC44FwMeiLbX' }, + query: { platform: 'web' }, + }, + responseDescriptions: { + 200: 'A collection with a single banner matching the criteria; an empty collection otherwise', + }, response: { 200: bannersResponseSchema, 400: validateBadRequestErrorResponse, @@ -94,48 +60,20 @@ API.v1.get( }, ); -/** - * @openapi - * /api/v1/banners: - * get: - * description: Gets the banners to be shown to the authenticated user - * security: - * $ref: '#/security/authenticated' - * parameters: - * - name: platform - * in: query - * description: The platform rendering the banner - * required: true - * schema: - * type: string - * enum: [web, mobile] - * example: web - * responses: - * 200: - * description: The banners matching the criteria - * content: - * application/json: - * schema: - * allOf: - * - $ref: '#/components/schemas/ApiSuccessV1' - * - type: object - * properties: - * banners: - * type: array - * items: - * $ref: '#/components/schemas/IBanner' - * default: - * description: Unexpected error - * content: - * application/json: - * schema: - * $ref: '#/components/schemas/ApiFailureV1' - */ API.v1.get( 'banners', { authRequired: true, + summary: 'List banners', + description: 'Gets the banners to be shown to the authenticated user.', + tags: ['Banners'], query: isBannersProps, + examples: { + query: { platform: 'web' }, + }, + responseDescriptions: { + 200: 'The banners matching the criteria', + }, response: { 200: bannersResponseSchema, 400: validateBadRequestErrorResponse, @@ -151,44 +89,20 @@ API.v1.get( }, ); -/** - * @openapi - * /api/v1/banners.dismiss: - * post: - * description: Dismisses a banner - * security: - * $ref: '#/security/authenticated' - * requestBody: - * content: - * application/json: - * schema: - * type: object - * properties: - * bannerId: - * type: string - * example: | - * { - * "bannerId": "ByehQjC44FwMeiLbX" - * } - * responses: - * 200: - * description: The banners matching the criteria - * content: - * application/json: - * schema: - * $ref: '#/components/schemas/ApiSuccessV1' - * default: - * description: Unexpected error - * content: - * application/json: - * schema: - * $ref: '#/components/schemas/ApiFailureV1' - */ API.v1.post( 'banners.dismiss', { authRequired: true, + summary: 'Dismiss a banner', + description: 'Dismisses a banner for the authenticated user, so it is no longer returned by the banner endpoints.', + tags: ['Banners'], body: isBannersDismissProps, + examples: { + body: { bannerId: 'ByehQjC44FwMeiLbX' }, + }, + responseDescriptions: { + 200: 'The banner was dismissed', + }, response: { 200: dismissResponseSchema, 400: validateBadRequestErrorResponse, diff --git a/apps/meteor/tests/end-to-end/api/openapi.ts b/apps/meteor/tests/end-to-end/api/openapi.ts new file mode 100644 index 0000000000000..876e11354ca88 --- /dev/null +++ b/apps/meteor/tests/end-to-end/api/openapi.ts @@ -0,0 +1,102 @@ +import { expect } from 'chai'; +import { before, describe, it } from 'mocha'; + +import { getCredentials, request, credentials } from '../../data/api-data'; + +type OpenAPIOperation = { + operationId?: string; + responses: Record }>; + parameters?: { name?: string; in?: string; schema?: unknown }[]; + requestBody?: { content?: Record }; + tags?: string[]; +}; + +type OpenAPIDocument = { + openapi: string; + info: { title: string; version: string }; + tags: { name: string }[]; + components: { schemas: Record }; + paths: Record>; +}; + +const operations = (document: OpenAPIDocument) => + Object.entries(document.paths).flatMap(([path, methods]) => + Object.entries(methods).map(([method, operation]) => ({ path, method, operation })), + ); + +describe('[OpenAPI]', () => { + let document: OpenAPIDocument; + let documentWithUndocumented: OpenAPIDocument; + + before((done) => getCredentials(done)); + + before(async () => { + document = (await request.get('/api/docs/json').set(credentials).expect(200)).body; + documentWithUndocumented = (await request.get('/api/docs/json?withUndocumented=true').set(credentials).expect(200)).body; + }); + + it('should describe every documented route', () => { + expect(document).to.have.property('openapi', '3.0.3'); + expect(document.info).to.have.property('title', 'Rocket.Chat API'); + expect(document.tags).to.be.an('array').that.is.not.empty; + expect(Object.keys(document.paths)).to.not.be.empty; + expect(document.components.schemas).to.include.keys(['ApiFailureV1', 'ApiSuccessV1']); + expect(document).to.not.have.property('schemas'); + }); + + it('should template path parameters instead of leaking express syntax', () => { + const expressStyle = Object.keys(documentWithUndocumented.paths).filter((path) => path.includes(':')); + + expect(expressStyle).to.be.empty; + }); + + it('should declare a parameter for every path template', () => { + const missing = operations(documentWithUndocumented) + .map(({ path, method, operation }) => { + const templated = [...path.matchAll(/{([^}]+)}/g)].map(([, name]) => name); + const declared = (operation.parameters ?? []).filter(({ in: location }) => location === 'path').map(({ name }) => name); + + return { route: `${method} ${path}`, missing: templated.filter((name) => !declared.includes(name)) }; + }) + .filter(({ missing }) => missing.length); + + expect(missing).to.be.empty; + }); + + it('should describe at least one response per operation, always with a description', () => { + const invalid = operations(documentWithUndocumented) + .filter( + ({ operation }) => + !Object.keys(operation.responses ?? {}).length || Object.values(operation.responses).some((response) => !response.description), + ) + .map(({ path, method }) => `${method} ${path}`); + + expect(invalid).to.be.empty; + }); + + it('should give every operation a unique operationId', () => { + const ids = operations(documentWithUndocumented).map(({ operation }) => operation.operationId); + + expect(ids.filter((id) => !id)).to.be.empty; + expect(new Set(ids).size).to.be.equal(ids.length); + }); + + it('should name and schema every parameter', () => { + const invalid = operations(documentWithUndocumented) + .flatMap(({ path, method, operation }) => + (operation.parameters ?? []).map((parameter) => ({ route: `${method} ${path}`, parameter })), + ) + .filter(({ parameter }) => !parameter.name || !parameter.in || !parameter.schema) + .map(({ route, parameter }) => `${route}: ${JSON.stringify(parameter)}`); + + expect(invalid).to.be.empty; + }); + + it('should hide undocumented routes unless they are asked for', () => { + const undocumented = (document: OpenAPIDocument) => + operations(document).filter(({ operation }) => operation.tags?.includes('Missing Documentation')); + + expect(undocumented(document)).to.be.empty; + expect(undocumented(documentWithUndocumented)).to.not.be.empty; + }); +}); diff --git a/packages/http-router/src/Router.spec.ts b/packages/http-router/src/Router.spec.ts index 57205a86b9898..2d5aec508a89d 100644 --- a/packages/http-router/src/Router.spec.ts +++ b/packages/http-router/src/Router.spec.ts @@ -752,4 +752,34 @@ describe('Router', () => { expect(response.statusCode).toBe(400); expect(response.body).toHaveProperty('error', "must have required property 'customProperty'"); }); + describe('OpenAPI registration', () => { + it('should register documented routes under their openapi path', () => { + const api = new Router('/api'); + const inner = new Router('/v1'); + + inner.get( + 'banners/:id', + { + authRequired: true, + summary: 'Get a banner by id', + tags: ['Banners'], + query: ajv.compile({ type: 'object', properties: { platform: { type: 'string' } }, required: ['platform'] }), + response: { 200: dummyValidator }, + }, + async () => ({ statusCode: 200, body: { success: true } }), + ); + + api.use(inner); + + const operation = api.typedRoutes['/api/v1/banners/{id}'].get; + + expect(operation.summary).toBe('Get a banner by id'); + expect(operation.parameters).toEqual([ + { name: 'id', in: 'path', required: true, schema: { type: 'string' } }, + { name: 'platform', in: 'query', required: true, schema: { type: 'string' } }, + ]); + expect(operation.responses[200].description).toBeTruthy(); + expect(operation.security).toEqual([{ userId: [], authToken: [] }]); + }); + }); }); diff --git a/packages/http-router/src/Router.ts b/packages/http-router/src/Router.ts index 794874587b970..6cf91a17a8b9a 100644 --- a/packages/http-router/src/Router.ts +++ b/packages/http-router/src/Router.ts @@ -1,6 +1,5 @@ import { Logger } from '@rocket.chat/logger'; import type { Method } from '@rocket.chat/rest-typings'; -import type { AnySchema } from 'ajv'; import express from 'express'; import type { Context, HonoRequest, MiddlewareHandler } from 'hono'; import { Hono } from 'hono'; @@ -8,6 +7,8 @@ import type { StatusCode } from 'hono/utils/http-status'; import type { ResponseSchema, TypedOptions } from './definition'; import { honoAdapterForExpress } from './middlewares/honoAdapterForExpress'; +import type { OpenAPIDocsOptions, Route } from './openapi'; +import { buildOperation, toOpenAPIPath } from './openapi'; import { parseQueryParams } from './parseQueryParams'; const logger = new Logger('HttpRouter'); @@ -41,39 +42,6 @@ function coerceDatesToStrings(obj: unknown): unknown { return obj; } -export type Route = { - responses: Record< - number, - { - description: string; - content: { - 'application/json': { - schema: AnySchema; - }; - }; - } - >; - parameters?: { - schema: AnySchema; - in: 'query'; - name: 'query'; - required: true; - }[]; - requestBody?: { - required: true; - content: { - 'application/json': { - schema: AnySchema; - }; - }; - }; - security?: { - userId: []; - authToken: []; - }[]; - tags?: string[]; -}; - export abstract class AbstractRouter Promise>> { protected abstract convertActionToHandler(action: TActionCallback, logger: Logger): (c: Context) => Promise>; } @@ -108,50 +76,10 @@ export class Router< TPathPattern extends `${TBasePath}/${TSubPathPattern}`, >(method: Method, subpath: TSubPathPattern, options: TOptions): void { const path = `/${this.base}/${subpath}`.replaceAll('//', '/') as TPathPattern; + const documentedPath = toOpenAPIPath(path); this.typedRoutes = this.typedRoutes || {}; - this.typedRoutes[path] = this.typedRoutes[path] || {}; - const { query, response = {}, authRequired, body, tags, ...rest } = options; - this.typedRoutes[path][method.toLowerCase()] = { - responses: Object.fromEntries( - Object.entries(response).map(([status, schema]) => [ - parseInt(status, 10), - { - description: '', - content: { - 'application/json': { schema: 'schema' in schema ? schema.schema : schema }, - }, - }, - ]), - ), - ...(query && { - parameters: [ - { - schema: query.schema, - in: 'query', - name: 'query', - required: true, - }, - ], - }), - ...(body && { - requestBody: { - required: true, - content: { - 'application/json': { schema: body.schema }, - }, - }, - }), - ...(authRequired && { - ...rest, - security: [ - { - userId: [], - authToken: [], - }, - ], - }), - tags, - }; + this.typedRoutes[documentedPath] = this.typedRoutes[documentedPath] || {}; + this.typedRoutes[documentedPath][method.toLowerCase()] = buildOperation(method, path, options as OpenAPIDocsOptions); } protected async parseBodyParams({ request }: { request: HonoRequest }): Promise> { diff --git a/packages/http-router/src/definition.ts b/packages/http-router/src/definition.ts index b59e0b258ec14..a8667fcd9893e 100644 --- a/packages/http-router/src/definition.ts +++ b/packages/http-router/src/definition.ts @@ -2,6 +2,8 @@ import type { LicenseModule } from '@rocket.chat/core-typings'; import type { ValidateFunction } from 'ajv'; import type { Request } from 'express'; +import type { OpenAPIDocumentation } from './openapi'; + type Range = Result['length'] extends N ? Result[number] : Range; @@ -54,4 +56,4 @@ export type TypedOptions = { typed?: boolean; license?: LicenseModule[]; authRequired?: boolean; -}; +} & OpenAPIDocumentation; diff --git a/packages/http-router/src/index.ts b/packages/http-router/src/index.ts index d08ebac606068..4c4258a4d1e89 100644 --- a/packages/http-router/src/index.ts +++ b/packages/http-router/src/index.ts @@ -1,3 +1,4 @@ export * from './Router'; +export * from './openapi'; export type * from './definition'; export * from './middlewares/honoAdapterForExpress'; diff --git a/packages/http-router/src/openapi.spec.ts b/packages/http-router/src/openapi.spec.ts new file mode 100644 index 0000000000000..e7f4acdc4f820 --- /dev/null +++ b/packages/http-router/src/openapi.spec.ts @@ -0,0 +1,221 @@ +import Ajv from 'ajv/dist/2020'; + +import type { OpenAPIDocsOptions } from './openapi'; +import { buildOperation, buildOperationId, toOpenAPIPath, withOperationIds } from './openapi'; + +const ajv = new Ajv(); +ajv.addVocabulary(['example']); + +const okSchema = ajv.compile({ + type: 'object', + properties: { success: { type: 'boolean', enum: [true] } }, + required: ['success'], +}); + +const querySchema = ajv.compile({ + type: 'object', + properties: { + platform: { type: 'string', enum: ['web', 'mobile'], description: 'The platform rendering the banner', example: 'web' }, + count: { type: 'number' }, + }, + required: ['platform'], +}); + +const build = (method: string, path: string, options: Partial = {}) => + buildOperation(method, path, { response: { 200: okSchema }, ...options }); + +describe('toOpenAPIPath', () => { + it('should convert express-style params to openapi templates', () => { + expect(toOpenAPIPath('/api/v1/banners/:id')).toBe('/api/v1/banners/{id}'); + expect(toOpenAPIPath('/api/v1/rooms/:rid/:fileId')).toBe('/api/v1/rooms/{rid}/{fileId}'); + expect(toOpenAPIPath('/api/v1/settings/:_id?')).toBe('/api/v1/settings/{_id}'); + expect(toOpenAPIPath('/api/v1/banners')).toBe('/api/v1/banners'); + }); +}); + +describe('buildOperation', () => { + it('should never emit an empty response description', () => { + const operation = build('GET', '/api/v1/banners'); + + expect(Object.keys(operation.responses).length).toBeGreaterThan(0); + Object.values(operation.responses).forEach((response) => { + expect(response.description).toBeTruthy(); + }); + }); + + it('should document a success response even when the route declares none', () => { + const operation = buildOperation('GET', '/api/v1/legacy', { tags: ['Missing Documentation'] }); + + expect(operation.responses[200].content?.['application/json'].schema).toEqual({ $ref: '#/components/schemas/ApiSuccessV1' }); + }); + + it('should derive path parameters from the path pattern', () => { + const operation = build('GET', '/api/v1/rooms/:rid/:fileId'); + + expect(operation.parameters).toEqual([ + { name: 'rid', in: 'path', required: true, schema: { type: 'string' } }, + { name: 'fileId', in: 'path', required: true, schema: { type: 'string' } }, + ]); + }); + + it('should enrich path parameters with the params schema and examples', () => { + const params = ajv.compile({ + type: 'object', + properties: { id: { type: 'string', description: 'The id of the banner' } }, + required: ['id'], + }); + + const operation = build('GET', '/api/v1/banners/:id', { params, examples: { params: { id: 'ByehQjC44FwMeiLbX' } } }); + + expect(operation.parameters).toEqual([ + { + name: 'id', + in: 'path', + required: true, + description: 'The id of the banner', + example: 'ByehQjC44FwMeiLbX', + schema: { type: 'string', description: 'The id of the banner' }, + }, + ]); + }); + + it('should explode the query schema into one parameter per property', () => { + const operation = build('GET', '/api/v1/banners', { query: querySchema }); + + expect(operation.parameters).toEqual([ + { + name: 'platform', + in: 'query', + required: true, + description: 'The platform rendering the banner', + example: 'web', + schema: { type: 'string', enum: ['web', 'mobile'], description: 'The platform rendering the banner', example: 'web' }, + }, + { name: 'count', in: 'query', required: false, schema: { type: 'number' } }, + ]); + }); + + it('should union properties of composed query schemas, requiring only the shared ones', () => { + const query = ajv.compile({ + oneOf: [ + { type: 'object', properties: { roomId: { type: 'string' }, kind: { type: 'string' } }, required: ['roomId', 'kind'] }, + { type: 'object', properties: { roomName: { type: 'string' }, kind: { type: 'string' } }, required: ['roomName', 'kind'] }, + ], + }); + + const operation = build('GET', '/api/v1/rooms.info', { query }); + + expect(operation.parameters?.map(({ name, required }) => ({ name, required }))).toEqual([ + { name: 'roomId', required: false }, + { name: 'kind', required: true }, + { name: 'roomName', required: false }, + ]); + }); + + it('should fall back to a single parameter for non-object query schemas', () => { + const query = ajv.compile({ type: 'string' }); + + const operation = build('GET', '/api/v1/weird', { query }); + + expect(operation.parameters).toEqual([{ name: 'query', in: 'query', required: false, schema: query.schema }]); + }); + + it('should inject the error responses implied by the route options', () => { + const operation = build('POST', '/api/v1/rooms.create', { + authRequired: true, + permissionsRequired: ['create-c'], + }); + + expect(Object.keys(operation.responses).sort()).toEqual(['200', '400', '401', '403', '429', '500']); + expect(operation.responses[401].content?.['application/json'].schema).toEqual({ $ref: '#/components/schemas/ApiFailureV1' }); + }); + + it('should keep declared responses over the injected ones', () => { + const operation = build('GET', '/api/v1/banners', { + response: { 200: okSchema, 400: okSchema }, + responseDescriptions: { 400: 'Custom bad request' }, + }); + + expect(operation.responses[400].description).toBe('Custom bad request'); + expect(operation.responses[400].content?.['application/json'].schema).toBe(okSchema.schema); + }); + + it('should not offer rate limit headers when rate limiting is disabled', () => { + expect(build('GET', '/api/v1/banners').responses[200].headers).toHaveProperty('X-RateLimit-Limit'); + expect(build('GET', '/api/v1/banners', { rateLimiterOptions: false }).responses[200].headers).toBeUndefined(); + expect(Object.keys(build('GET', '/api/v1/banners', { rateLimiterOptions: false }).responses)).not.toContain('429'); + }); + + it('should document two-factor headers and surface permissions and license as extensions', () => { + const operation = build('POST', '/api/v1/users.delete', { + authRequired: true, + twoFactorRequired: true, + permissionsRequired: { POST: { operation: 'hasAll', permissions: ['delete-user'] } }, + license: ['livechat-enterprise'], + }); + + expect(operation.parameters?.map(({ name, in: location }) => `${location}:${name}`)).toEqual([ + 'header:x-2fa-code', + 'header:x-2fa-method', + ]); + expect(Object.keys(operation.responses)).toContain('403'); + expect(operation['x-permissions']).toEqual(['delete-user']); + expect(operation['x-license']).toEqual(['livechat-enterprise']); + expect(operation['x-two-factor-required']).toBe(true); + expect(operation.description).toContain('`delete-user`'); + expect(operation.description).toContain('two-factor'); + expect(operation.description).toContain('`livechat-enterprise`'); + }); + + it('should mark deprecated routes and point to the alternatives', () => { + const operation = build('GET', '/api/v1/channels.images', { + deprecation: { version: '8.0.0', alternatives: ['/v1/rooms.images'] }, + }); + + expect(operation.deprecated).toBe(true); + expect(operation.description).toContain('8.0.0'); + expect(operation.description).toContain('`/v1/rooms.images`'); + }); + + it('should document the request body with its content type and example', () => { + const body = ajv.compile({ type: 'object', properties: { bannerId: { type: 'string' } }, required: ['bannerId'] }); + + const operation = build('POST', '/api/v1/banners.dismiss', { body, examples: { body: { bannerId: 'ByehQjC44FwMeiLbX' } } }); + + expect(operation.requestBody).toEqual({ + required: true, + content: { 'application/json': { schema: body.schema, example: { bannerId: 'ByehQjC44FwMeiLbX' } } }, + }); + + expect(build('POST', '/api/v1/rooms.media', { body, bodyContentType: 'multipart/form-data' }).requestBody?.content).toHaveProperty( + 'multipart/form-data', + ); + }); + + it('should only carry an operation id when the route declares one', () => { + expect(build('GET', '/api/v1/banners/:id').operationId).toBeUndefined(); + expect(build('GET', '/api/v1/banners', { operationId: 'listBanners' }).operationId).toBe('listBanners'); + }); + + it('should only require authentication headers when the route requires auth', () => { + expect(build('GET', '/api/v1/info').security).toBeUndefined(); + expect(build('GET', '/api/v1/me', { authRequired: true }).security).toEqual([{ userId: [], authToken: [] }]); + }); + + it('should generate a unique operation id per method and path', () => { + expect(buildOperationId('GET', '/api/v1/banners/{id}')).toBe('getApiV1BannersId'); + expect(buildOperationId('POST', '/api/v1/banners.dismiss')).toBe('postApiV1BannersDismiss'); + }); +}); + +describe('withOperationIds', () => { + it('should fill in missing operation ids from the final path and keep declared ones', () => { + const paths = withOperationIds({ + '/api/v1/banners/{id}': { get: build('GET', '/v1/banners/:id') }, + '/api/v1/banners': { get: build('GET', '/v1/banners', { operationId: 'listBanners' }) }, + }); + + expect(paths['/api/v1/banners/{id}'].get.operationId).toBe('getApiV1BannersId'); + expect(paths['/api/v1/banners'].get.operationId).toBe('listBanners'); + }); +}); diff --git a/packages/http-router/src/openapi.ts b/packages/http-router/src/openapi.ts new file mode 100644 index 0000000000000..280d8349b91b1 --- /dev/null +++ b/packages/http-router/src/openapi.ts @@ -0,0 +1,437 @@ +import type { AnySchema, ValidateFunction } from 'ajv'; + +export type OpenAPIParameter = { + name: string; + in: 'query' | 'path' | 'header'; + required?: boolean; + description?: string; + deprecated?: boolean; + schema: AnySchema; + example?: unknown; +}; + +export type OpenAPIMediaType = { + schema: AnySchema; + example?: unknown; +}; + +export type OpenAPIResponse = { + description: string; + content?: Record; + headers?: Record; +}; + +export type OpenAPIExternalDocs = { + url: string; + description?: string; +}; + +export type Route = { + 'summary'?: string; + 'description'?: string; + 'operationId'?: string; + 'deprecated'?: boolean; + 'externalDocs'?: OpenAPIExternalDocs; + 'responses': Record; + 'parameters'?: OpenAPIParameter[]; + 'requestBody'?: { + required: true; + content: Record; + }; + 'security'?: { + userId: []; + authToken: []; + }[]; + 'tags'?: string[]; + 'x-permissions'?: string[]; + 'x-license'?: string[]; + 'x-two-factor-required'?: boolean; +}; + +type TOperation = 'hasAll' | 'hasAny'; + +type PermissionsRequired = string[] | Record | undefined; + +/** + * Human-facing documentation a route may declare. Everything else in the OpenAPI operation is + * derived from options the framework already needs (schemas, auth, permissions, license). + */ +export type OpenAPIDocumentation = { + summary?: string; + description?: string; + /** Defaults to ``; set it only to keep a published id stable. */ + operationId?: string; + deprecated?: boolean; + externalDocs?: OpenAPIExternalDocs; + /** + * Path parameters schema, used for documentation only — path values always arrive as strings. + * ponytail: no runtime validation until an endpoint actually needs it. + */ + params?: ValidateFunction; + /** Defaults to `application/json`; use for `multipart/form-data` uploads and friends. */ + bodyContentType?: string; + responseDescriptions?: Partial>; + examples?: { + query?: Record; + params?: Record; + body?: unknown; + response?: Partial>; + }; +}; + +/** + * Documentation-relevant slice of a route's options. Both `TypedOptions` flavors (http-router and + * the Meteor API class) structurally satisfy it. + */ +export type OpenAPIDocsOptions = OpenAPIDocumentation & { + tags?: string[]; + query?: unknown; + body?: unknown; + response?: Record; + authRequired?: boolean; + twoFactorRequired?: boolean; + permissionsRequired?: PermissionsRequired; + license?: readonly string[]; + rateLimiterOptions?: unknown; + deprecation?: { version: string; alternatives?: readonly string[] }; +}; + +const STATUS_DESCRIPTIONS: Record = { + 200: 'Successful response', + 201: 'Resource created', + 202: 'Request accepted', + 204: 'No content', + 304: 'Not modified', + 400: 'Bad request — invalid or missing parameters', + 401: 'Unauthorized — missing or invalid authentication headers', + 403: 'Forbidden — the user lacks the required permission', + 404: 'Resource not found', + 409: 'Conflict', + 413: 'Payload too large', + 429: 'Too many requests — rate limit exceeded', + 500: 'Internal server error', + 501: 'Not implemented', + 503: 'Service unavailable', +}; + +const ERROR_RESPONSE_REF = { $ref: '#/components/schemas/ApiFailureV1' } as const; + +const SUCCESS_RESPONSE_REF = { $ref: '#/components/schemas/ApiSuccessV1' } as const; + +/** Schemas referenced by the responses this module injects; merge into `components.schemas`. */ +export const openAPIErrorComponents: Record = { + ApiSuccessV1: { + type: 'object', + description: 'Rocket.Chat REST API success payload whose shape is not documented yet', + properties: { + success: { type: 'boolean', enum: [true] }, + }, + required: ['success'], + }, + ApiFailureV1: { + type: 'object', + description: 'Standard Rocket.Chat REST API error payload', + properties: { + success: { type: 'boolean', enum: [false] }, + error: { type: 'string', example: 'error-invalid-params' }, + errorType: { type: 'string', example: 'error-invalid-params' }, + message: { type: 'string' }, + stack: { type: 'string', description: 'Only present when the server runs in test mode' }, + }, + required: ['success'], + }, +}; + +const RATE_LIMIT_HEADERS: NonNullable = { + 'X-RateLimit-Limit': { description: 'Requests allowed within the current window', schema: { type: 'integer' } }, + 'X-RateLimit-Remaining': { description: 'Requests still available within the current window', schema: { type: 'integer' } }, + 'X-RateLimit-Reset': { description: 'Unix timestamp in milliseconds when the window resets', schema: { type: 'integer' } }, +}; + +const TWO_FACTOR_PARAMETERS: OpenAPIParameter[] = [ + { + name: 'x-2fa-code', + in: 'header', + required: true, + description: 'Two-factor code, hashed according to the chosen method', + schema: { type: 'string' }, + }, + { + name: 'x-2fa-method', + in: 'header', + required: true, + description: 'Method used to generate the two-factor code', + schema: { type: 'string', enum: ['totp', 'email', 'password'] }, + }, +]; + +type ObjectSchemaLike = { + properties?: Record>; + required?: string[]; + allOf?: ObjectSchemaLike[]; + anyOf?: ObjectSchemaLike[]; + oneOf?: ObjectSchemaLike[]; +}; + +type CollectedProperties = { properties: Record>; required: Set }; + +const getSchema = (carrier: unknown): AnySchema => { + if (carrier && (typeof carrier === 'object' || typeof carrier === 'function') && 'schema' in carrier) { + return (carrier as { schema: AnySchema }).schema; + } + return carrier as AnySchema; +}; + +const collectProperties = (schema: unknown): CollectedProperties | undefined => { + if (!schema || typeof schema !== 'object') { + return undefined; + } + + const objectSchema = schema as ObjectSchemaLike; + + if (objectSchema.properties) { + return { properties: { ...objectSchema.properties }, required: new Set(objectSchema.required ?? []) }; + } + + const branches = objectSchema.allOf ?? objectSchema.anyOf ?? objectSchema.oneOf; + const collected = branches?.map(collectProperties).filter((entry): entry is CollectedProperties => Boolean(entry)); + + if (!collected?.length) { + return undefined; + } + + const properties = Object.assign({}, ...collected.map((entry) => entry.properties)); + // `allOf` composes, so every branch's requirements hold; for `anyOf`/`oneOf` only the + // requirements shared by all branches are guaranteed. + const required = objectSchema.allOf + ? new Set(collected.flatMap((entry) => [...entry.required])) + : new Set([...collected[0].required].filter((name) => collected.every((entry) => entry.required.has(name)))); + + return { properties, required }; +}; + +/** `/api/v1/rooms/:rid/:fileId` -> `/api/v1/rooms/{rid}/{fileId}` */ +export const toOpenAPIPath = (path: string): string => path.replace(/:([A-Za-z0-9_]+)\??/g, '{$1}'); + +const extractPathParameterNames = (path: string): { name: string; required: boolean }[] => + [...path.matchAll(/:([A-Za-z0-9_]+)(\?)?/g)].map(([, name, optional]) => ({ name, required: !optional })); + +const buildPathParameters = (path: string, options: OpenAPIDocsOptions): OpenAPIParameter[] => { + const documented = collectProperties(getSchema(options.params)); + + return extractPathParameterNames(path).map(({ name, required }) => { + const schema = documented?.properties[name]; + + return { + name, + in: 'path', + required, + schema: (schema as AnySchema) ?? { type: 'string' }, + ...(typeof schema?.description === 'string' && { description: schema.description }), + ...(options.examples?.params?.[name] !== undefined + ? { example: options.examples.params[name] } + : schema?.example !== undefined && { example: schema.example }), + }; + }); +}; + +const buildQueryParameters = (options: OpenAPIDocsOptions): OpenAPIParameter[] => { + const schema = getSchema(options.query); + + if (!schema) { + return []; + } + + const collected = collectProperties(schema); + + if (!collected) { + // ponytail: non-object query schemas ($ref, primitives) keep the legacy single-parameter + // shape; explode them too if such a schema ever shows up. + return [{ name: 'query', in: 'query', required: false, schema }]; + } + + return Object.entries(collected.properties).map(([name, propertySchema]) => ({ + name, + in: 'query', + required: collected.required.has(name), + schema: propertySchema as AnySchema, + ...(typeof propertySchema.description === 'string' && { description: propertySchema.description }), + ...(typeof propertySchema.deprecated === 'boolean' && { deprecated: propertySchema.deprecated }), + ...(options.examples?.query?.[name] !== undefined + ? { example: options.examples.query[name] } + : propertySchema.example !== undefined && { example: propertySchema.example }), + })); +}; + +const normalizePermissions = (method: string, permissionsRequired: PermissionsRequired): string[] => { + if (!permissionsRequired) { + return []; + } + + if (Array.isArray(permissionsRequired)) { + return permissionsRequired; + } + + const forMethod = [permissionsRequired[method.toUpperCase()], permissionsRequired['*']].filter(Boolean); + + return forMethod.flatMap((entry) => (Array.isArray(entry) ? entry : (entry?.permissions ?? []))); +}; + +const buildDescription = (method: string, options: OpenAPIDocsOptions): string | undefined => { + const permissions = normalizePermissions(method, options.permissionsRequired); + const notes: string[] = []; + + if (options.deprecation) { + const alternatives = options.deprecation.alternatives?.length + ? ` Use ${options.deprecation.alternatives.map((alternative) => `\`${alternative}\``).join(' or ')} instead.` + : ''; + notes.push(`**Deprecated** — scheduled for removal in version ${options.deprecation.version}.${alternatives}`); + } + + if (permissions.length) { + notes.push(`Requires the permission(s): ${permissions.map((permission) => `\`${permission}\``).join(', ')}.`); + } + + if (options.twoFactorRequired) { + notes.push('Requires two-factor authentication via the `x-2fa-code` and `x-2fa-method` headers.'); + } + + if (options.license?.length) { + notes.push(`Requires the license module(s): ${options.license.map((module) => `\`${module}\``).join(', ')}.`); + } + + const description = [options.description, ...notes].filter(Boolean).join('\n\n'); + + return description || undefined; +}; + +/** + * Only the document builder knows the final path — routers register operations before their parent + * prefixes them — so ids are filled in by `withOperationIds`, not at registration time. + */ +export const buildOperationId = (method: string, path: string): string => + `${method.toLowerCase()}${toOpenAPIPath(path) + .replace(/[{}]/g, '') + .split(/[^A-Za-z0-9]+/) + .filter(Boolean) + .map((part) => part.charAt(0).toUpperCase() + part.slice(1)) + .join('')}`; + +const implicitErrorStatuses = (method: string, options: OpenAPIDocsOptions): number[] => { + const statuses = [400, 500]; + + if (options.authRequired) { + statuses.push(401); + } + + if (options.twoFactorRequired || normalizePermissions(method, options.permissionsRequired).length || options.license?.length) { + statuses.push(403); + } + + if (options.rateLimiterOptions !== false) { + statuses.push(429); + } + + return statuses; +}; + +const describeStatus = (status: number, options: OpenAPIDocsOptions): string => + options.responseDescriptions?.[status] ?? STATUS_DESCRIPTIONS[status] ?? `Response with status ${status}`; + +/** Builds the OpenAPI operation object for a single route. */ +export const buildOperation = (method: string, path: string, options: OpenAPIDocsOptions): Route => { + const rateLimited = options.rateLimiterOptions !== false; + const responses: Record = {}; + + for (const [status, validator] of Object.entries(options.response ?? {})) { + if (!validator) { + continue; + } + + const code = Number(status); + const example = options.examples?.response?.[code]; + + responses[code] = { + description: describeStatus(code, options), + content: { + 'application/json': { + schema: getSchema(validator), + ...(example !== undefined && { example }), + }, + }, + ...(rateLimited && { headers: RATE_LIMIT_HEADERS }), + }; + } + + // `responses` is required by the spec, and undocumented (legacy) routes declare none. + if (!Object.keys(responses).some((status) => Number(status) < 300)) { + responses[200] = { + description: describeStatus(200, options), + content: { 'application/json': { schema: SUCCESS_RESPONSE_REF } }, + ...(rateLimited && { headers: RATE_LIMIT_HEADERS }), + }; + } + + for (const status of implicitErrorStatuses(method, options)) { + if (!responses[status]) { + responses[status] = { + description: describeStatus(status, options), + content: { 'application/json': { schema: ERROR_RESPONSE_REF } }, + }; + } + } + + const parameters = [ + ...buildPathParameters(path, options), + ...buildQueryParameters(options), + ...(options.twoFactorRequired ? TWO_FACTOR_PARAMETERS : []), + ]; + + const bodySchema = getSchema(options.body); + const permissions = normalizePermissions(method, options.permissionsRequired); + const description = buildDescription(method, options); + + return { + ...(options.operationId && { operationId: options.operationId }), + ...(options.summary && { summary: options.summary }), + ...(description && { description }), + ...((options.deprecated ?? Boolean(options.deprecation)) && { deprecated: true }), + ...(options.externalDocs && { externalDocs: options.externalDocs }), + responses, + ...(parameters.length && { parameters }), + ...(bodySchema && { + requestBody: { + required: true as const, + content: { + [options.bodyContentType ?? 'application/json']: { + schema: bodySchema, + ...(options.examples?.body !== undefined && { example: options.examples.body }), + }, + }, + }, + }), + ...(options.authRequired && { + security: [ + { + userId: [] as [], + authToken: [] as [], + }, + ], + }), + ...(permissions.length && { 'x-permissions': permissions }), + ...(options.license?.length && { 'x-license': [...options.license] }), + ...(options.twoFactorRequired && { 'x-two-factor-required': true }), + tags: options.tags, + }; +}; + +/** Fills in the `operationId` of every operation that did not declare one, using its final path. */ +export const withOperationIds = (paths: Record>): Record> => + Object.fromEntries( + Object.entries(paths).map(([path, methods]) => [ + path, + Object.fromEntries( + Object.entries(methods).map(([method, operation]) => [method, { operationId: buildOperationId(method, path), ...operation }]), + ), + ]), + ); diff --git a/packages/rest-typings/src/v1/Ajv.ts b/packages/rest-typings/src/v1/Ajv.ts index 0f948454ad44b..1dea037c54ad1 100644 --- a/packages/rest-typings/src/v1/Ajv.ts +++ b/packages/rest-typings/src/v1/Ajv.ts @@ -20,6 +20,11 @@ const ajvQuery = new Ajv({ addFormats(ajv); addFormats(ajvQuery); +// `example` is an OpenAPI annotation, not a validation keyword — declaring it lets schemas carry +// their own documentation without tripping AJV's strict mode. +ajv.addVocabulary(['example']); +ajvQuery.addVocabulary(['example']); + ajv.addFormat('basic_email', /^[^@]+@[^@]+$/); ajv.addFormat( 'rfc_email', diff --git a/packages/rest-typings/src/v1/banners.ts b/packages/rest-typings/src/v1/banners.ts index 1adc64fe350f1..4720255d0b83f 100644 --- a/packages/rest-typings/src/v1/banners.ts +++ b/packages/rest-typings/src/v1/banners.ts @@ -16,6 +16,8 @@ const BannersSchema = { platform: { type: 'string', enum: ['web', 'mobile'], + description: 'The platform rendering the banner', + example: 'web', }, }, required: ['platform'], @@ -24,6 +26,21 @@ const BannersSchema = { export const isBannersProps = ajvQuery.compile(BannersSchema); +const BannerIdParamsSchema = { + type: 'object', + properties: { + id: { + type: 'string', + description: 'The id of the banner', + example: 'ByehQjC44FwMeiLbX', + }, + }, + required: ['id'], + additionalProperties: false, +}; + +export const isBannerIdParams = ajv.compile<{ id: string }>(BannerIdParamsSchema); + type BannersDismiss = { bannerId: string; }; @@ -34,6 +51,8 @@ const BannersDismissSchema = { bannerId: { type: 'string', minLength: 1, + description: 'The id of the banner to dismiss', + example: 'ByehQjC44FwMeiLbX', }, }, required: ['bannerId'], From 670987658cc29b26211e9fd98bb89e50fbee9c95 Mon Sep 17 00:00:00 2001 From: Guilherme Gazzo Date: Thu, 30 Jul 2026 15:23:33 -0300 Subject: [PATCH 02/16] fix(api): point the api-docs UI at a relative spec url `swaggerUi.setup` runs at import time, before settings are loaded, so `settings.get('Site_Url')` was interpolated as `undefined` and the UI ended up fetching `/api-docs/undefined/api/docs/json`. The served HTML came back instead of the spec, and Swagger UI reported a missing `openapi` version field. The document is always served from the same origin as the UI, so a relative url needs no setting at all. --- apps/meteor/server/api/default/openApi.ts | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/apps/meteor/server/api/default/openApi.ts b/apps/meteor/server/api/default/openApi.ts index bd9be07e29660..08ed39100750f 100644 --- a/apps/meteor/server/api/default/openApi.ts +++ b/apps/meteor/server/api/default/openApi.ts @@ -135,7 +135,9 @@ app.use( swaggerUi.serve, swaggerUi.setup(null, { swaggerOptions: { - url: `${settings.get('Site_Url')}/api/docs/json`, + // Relative on purpose: this runs at import time, before settings are loaded, so + // `Site_Url` would render as "undefined" here. + url: '/api/docs/json', }, }), ); From d5b0fddddda1a1258b4a20196f27130e43ba2fc5 Mon Sep 17 00:00:00 2001 From: Guilherme Gazzo Date: Thu, 30 Jul 2026 16:46:18 -0300 Subject: [PATCH 03/16] feat(api): named examples, response content types and headers Auditing the hand-written definitions in RocketChat/Rocket.Chat-Open-API showed which features the published documentation relies on and this framework could not express: - named example scenarios, by far the most used feature there (1010 example objects, 156 payloads documenting two to six alternatives). `examples.body` and `examples.response[code]` now accept either a single value or a map of named Example Objects with their own summary and description; - a response content type per status code, for the endpoints answering images, plain text or files instead of JSON; - response headers per status code, merged with the rate limit ones the router already documents. `operationId` now follows the convention of the ids published at developer.rocket.chat (`get-api-v1-banners-id`) instead of a camel case variant, so deep links and generated clients survive the change of source. Documented paths also collapse duplicate slashes, which were leaking into three path keys (`/api//info`, `/api//docs/json`, `/api/apps//{id}/export-logs`) when a router prefixed a subpath that already started with one. --- packages/http-router/src/Router.ts | 4 +- packages/http-router/src/openapi.spec.ts | 69 +++++++++++++++++++++-- packages/http-router/src/openapi.ts | 70 ++++++++++++++++++------ 3 files changed, 121 insertions(+), 22 deletions(-) diff --git a/packages/http-router/src/Router.ts b/packages/http-router/src/Router.ts index 6cf91a17a8b9a..70bcd52b9cba3 100644 --- a/packages/http-router/src/Router.ts +++ b/packages/http-router/src/Router.ts @@ -338,7 +338,9 @@ export class Router< if (innerRouter instanceof Router) { this.typedRoutes = { ...this.typedRoutes, - ...Object.fromEntries(Object.entries(innerRouter.typedRoutes).map(([path, routes]) => [`${this.base}${path}`, routes])), + ...Object.fromEntries( + Object.entries(innerRouter.typedRoutes).map(([path, routes]) => [toOpenAPIPath(`${this.base}${path}`), routes]), + ), }; this.innerRouter.route(innerRouter.base, innerRouter.innerRouter); diff --git a/packages/http-router/src/openapi.spec.ts b/packages/http-router/src/openapi.spec.ts index e7f4acdc4f820..18682174fd9e9 100644 --- a/packages/http-router/src/openapi.spec.ts +++ b/packages/http-router/src/openapi.spec.ts @@ -30,6 +30,8 @@ describe('toOpenAPIPath', () => { expect(toOpenAPIPath('/api/v1/rooms/:rid/:fileId')).toBe('/api/v1/rooms/{rid}/{fileId}'); expect(toOpenAPIPath('/api/v1/settings/:_id?')).toBe('/api/v1/settings/{_id}'); expect(toOpenAPIPath('/api/v1/banners')).toBe('/api/v1/banners'); + expect(toOpenAPIPath('/api//docs/json')).toBe('/api/docs/json'); + expect(toOpenAPIPath('/api/apps//{id}/export-logs')).toBe('/api/apps/{id}/export-logs'); }); }); @@ -202,9 +204,68 @@ describe('buildOperation', () => { expect(build('GET', '/api/v1/me', { authRequired: true }).security).toEqual([{ userId: [], authToken: [] }]); }); - it('should generate a unique operation id per method and path', () => { - expect(buildOperationId('GET', '/api/v1/banners/{id}')).toBe('getApiV1BannersId'); - expect(buildOperationId('POST', '/api/v1/banners.dismiss')).toBe('postApiV1BannersDismiss'); + it('should generate the operation id published at developer.rocket.chat', () => { + expect(buildOperationId('GET', '/api/v1/banners/{id}')).toBe('get-api-v1-banners-id'); + expect(buildOperationId('POST', '/api/v1/banners.dismiss')).toBe('post-api-v1-banners.dismiss'); + }); + it('should render named scenarios as an examples map', () => { + const operation = build('GET', '/api/v1/rooms.info', { + response: { 200: okSchema, 400: okSchema }, + examples: { + response: { + 200: { + byId: { summary: 'Looked up by id', value: { success: true } }, + byName: { value: { success: true } }, + }, + 400: { success: false, error: 'error-room-not-found' }, + }, + }, + }); + + expect(operation.responses[200].content?.['application/json'].examples).toEqual({ + byId: { summary: 'Looked up by id', value: { success: true } }, + byName: { value: { success: true } }, + }); + expect(operation.responses[200].content?.['application/json'].example).toBeUndefined(); + expect(operation.responses[400].content?.['application/json'].example).toEqual({ success: false, error: 'error-room-not-found' }); + }); + + it('should render named scenarios for the request body too', () => { + const body = ajv.compile({ type: 'object', properties: { roomId: { type: 'string' } } }); + const operation = build('POST', '/api/v1/chat.postMessage', { + body, + examples: { body: { minimal: { value: { roomId: 'GENERAL' } }, withAttachment: { value: { roomId: 'GENERAL' } } } }, + }); + + expect(Object.keys(operation.requestBody?.content['application/json'].examples ?? {})).toEqual(['minimal', 'withAttachment']); + }); + + it('should treat a payload that merely has a "value" key as a single example', () => { + const operation = build('GET', '/api/v1/settings/{_id}', { examples: { response: { 200: { value: 'a setting value' } } } }); + + expect(operation.responses[200].content?.['application/json'].example).toEqual({ value: 'a setting value' }); + expect(operation.responses[200].content?.['application/json'].examples).toBeUndefined(); + }); + + it('should honor a per-status content type', () => { + const operation = build('GET', '/api/v1/shield.svg', { responseContentType: { 200: 'image/svg+xml' } }); + + expect(Object.keys(operation.responses[200].content ?? {})).toEqual(['image/svg+xml']); + expect(Object.keys(operation.responses[400].content ?? {})).toEqual(['application/json']); + }); + + it('should merge declared response headers with the rate limit ones', () => { + const operation = build('GET', '/api/v1/rooms.media/{rid}', { + responseHeaders: { 200: { 'Content-Disposition': { description: 'Attachment file name', schema: { type: 'string' } } } }, + }); + + expect(Object.keys(operation.responses[200].headers ?? {})).toEqual([ + 'X-RateLimit-Limit', + 'X-RateLimit-Remaining', + 'X-RateLimit-Reset', + 'Content-Disposition', + ]); + expect(build('GET', '/api/v1/x', { rateLimiterOptions: false }).responses[200].headers).toBeUndefined(); }); }); @@ -215,7 +276,7 @@ describe('withOperationIds', () => { '/api/v1/banners': { get: build('GET', '/v1/banners', { operationId: 'listBanners' }) }, }); - expect(paths['/api/v1/banners/{id}'].get.operationId).toBe('getApiV1BannersId'); + expect(paths['/api/v1/banners/{id}'].get.operationId).toBe('get-api-v1-banners-id'); expect(paths['/api/v1/banners'].get.operationId).toBe('listBanners'); }); }); diff --git a/packages/http-router/src/openapi.ts b/packages/http-router/src/openapi.ts index 280d8349b91b1..81fad08a49418 100644 --- a/packages/http-router/src/openapi.ts +++ b/packages/http-router/src/openapi.ts @@ -13,6 +13,7 @@ export type OpenAPIParameter = { export type OpenAPIMediaType = { schema: AnySchema; example?: unknown; + examples?: Record; }; export type OpenAPIResponse = { @@ -59,7 +60,7 @@ type PermissionsRequired = string[] | Record`; set it only to keep a published id stable. */ + /** Defaults to `-`, matching the ids published at developer.rocket.chat. */ operationId?: string; deprecated?: boolean; externalDocs?: OpenAPIExternalDocs; @@ -70,15 +71,26 @@ export type OpenAPIDocumentation = { params?: ValidateFunction; /** Defaults to `application/json`; use for `multipart/form-data` uploads and friends. */ bodyContentType?: string; + /** Defaults to `application/json`; per status code, for endpoints answering images, text or files. */ + responseContentType?: Partial>; responseDescriptions?: Partial>; + /** Response headers worth documenting, per status code. Rate limit headers are added on their own. */ + responseHeaders?: Partial>>; examples?: { query?: Record; params?: Record; - body?: unknown; - response?: Partial>; + /** A single unnamed example, or named scenarios when a payload has several shapes. */ + body?: unknown | NamedExamples; + response?: Partial>; }; }; +/** + * Named scenarios for a payload, rendered as OpenAPI Example Objects — the shape used to document + * alternatives ("room not found" vs "not a member") in a single response. + */ +export type NamedExamples = Record; + /** * Documentation-relevant slice of a route's options. Both `TypedOptions` flavors (http-router and * the Meteor API class) structurally satisfy it. @@ -210,8 +222,11 @@ const collectProperties = (schema: unknown): CollectedProperties | undefined => return { properties, required }; }; -/** `/api/v1/rooms/:rid/:fileId` -> `/api/v1/rooms/{rid}/{fileId}` */ -export const toOpenAPIPath = (path: string): string => path.replace(/:([A-Za-z0-9_]+)\??/g, '{$1}'); +/** + * `/api/v1/rooms/:rid/:fileId` -> `/api/v1/rooms/{rid}/{fileId}`, collapsing the duplicate slashes + * that show up when a router prefixes a subpath that already starts with one. + */ +export const toOpenAPIPath = (path: string): string => path.replace(/:([A-Za-z0-9_]+)\??/g, '{$1}').replace(/\/{2,}/g, '/'); const extractPathParameterNames = (path: string): { name: string; required: boolean }[] => [...path.matchAll(/:([A-Za-z0-9_]+)(\?)?/g)].map(([, name, optional]) => ({ name, required: !optional })); @@ -310,12 +325,7 @@ const buildDescription = (method: string, options: OpenAPIDocsOptions): string | * prefixes them — so ids are filled in by `withOperationIds`, not at registration time. */ export const buildOperationId = (method: string, path: string): string => - `${method.toLowerCase()}${toOpenAPIPath(path) - .replace(/[{}]/g, '') - .split(/[^A-Za-z0-9]+/) - .filter(Boolean) - .map((part) => part.charAt(0).toUpperCase() + part.slice(1)) - .join('')}`; + [method.toLowerCase(), ...toOpenAPIPath(path).replace(/[{}]/g, '').split('/').filter(Boolean)].join('-'); const implicitErrorStatuses = (method: string, options: OpenAPIDocsOptions): number[] => { const statuses = [400, 500]; @@ -338,6 +348,33 @@ const implicitErrorStatuses = (method: string, options: OpenAPIDocsOptions): num const describeStatus = (status: number, options: OpenAPIDocsOptions): string => options.responseDescriptions?.[status] ?? STATUS_DESCRIPTIONS[status] ?? `Response with status ${status}`; +const isNamedExamples = (examples: unknown): examples is NamedExamples => + Boolean(examples) && + typeof examples === 'object' && + !Array.isArray(examples) && + Object.values(examples as Record).every( + (entry) => Boolean(entry) && typeof entry === 'object' && !Array.isArray(entry) && 'value' in (entry as object), + ) && + Object.keys(examples as object).length > 0; + +const buildResponseHeaders = (status: number, options: OpenAPIDocsOptions, rateLimited: boolean): Pick => { + const headers = { + ...(rateLimited && RATE_LIMIT_HEADERS), + ...options.responseHeaders?.[status], + }; + + return Object.keys(headers).length ? { headers } : {}; +}; + +/** Named scenarios become an `examples` map; anything else is a single unnamed `example`. */ +const buildMediaExamples = (examples: unknown): Pick => { + if (examples === undefined) { + return {}; + } + + return isNamedExamples(examples) ? { examples } : { example: examples }; +}; + /** Builds the OpenAPI operation object for a single route. */ export const buildOperation = (method: string, path: string, options: OpenAPIDocsOptions): Route => { const rateLimited = options.rateLimiterOptions !== false; @@ -349,17 +386,16 @@ export const buildOperation = (method: string, path: string, options: OpenAPIDoc } const code = Number(status); - const example = options.examples?.response?.[code]; responses[code] = { description: describeStatus(code, options), content: { - 'application/json': { + [options.responseContentType?.[code] ?? 'application/json']: { schema: getSchema(validator), - ...(example !== undefined && { example }), + ...buildMediaExamples(options.examples?.response?.[code]), }, }, - ...(rateLimited && { headers: RATE_LIMIT_HEADERS }), + ...buildResponseHeaders(code, options, rateLimited), }; } @@ -368,7 +404,7 @@ export const buildOperation = (method: string, path: string, options: OpenAPIDoc responses[200] = { description: describeStatus(200, options), content: { 'application/json': { schema: SUCCESS_RESPONSE_REF } }, - ...(rateLimited && { headers: RATE_LIMIT_HEADERS }), + ...buildResponseHeaders(200, options, rateLimited), }; } @@ -405,7 +441,7 @@ export const buildOperation = (method: string, path: string, options: OpenAPIDoc content: { [options.bodyContentType ?? 'application/json']: { schema: bodySchema, - ...(options.examples?.body !== undefined && { example: options.examples.body }), + ...buildMediaExamples(options.examples?.body), }, }, }, From fa1bc6c88ddc71ff7b786e80ddbde3b832c4cdda Mon Sep 17 00:00:00 2001 From: Guilherme Gazzo Date: Fri, 31 Jul 2026 11:07:27 -0300 Subject: [PATCH 04/16] feat(api): share the error payload schemas through components The four error payload schemas were inlined into every operation that declares them, so the document repeated them 372 times - and repeated the same spec violation with them: the `details` field of a bad request allowed `type: array` with no `items`, which no OpenAPI tool accepts. A schema that carries an `$id` is now hoisted into `components.schemas` and referenced, so `BadRequestError`, `UnauthorizedError`, `ForbiddenError` and `NotFoundError` are described once. The mechanism is generic: any shared schema that names itself is deduplicated the same way. Also adds the missing `items` to the two arrays that lacked it - the `details` of a bad request and the `logs` of the app log endpoints. Both are left as an empty schema on purpose: the keyword is required on arrays, but the payloads really do hold anything the endpoint attached, and constraining them here would start rejecting responses at runtime. --- .../endpoints/appGeneralLogsHandler.ts | 2 +- .../communication/endpoints/appLogsHandler.ts | 2 +- apps/meteor/server/api/default/openApi.ts | 3 ++- packages/http-router/src/openapi.spec.ts | 23 ++++++++++++++++- packages/http-router/src/openapi.ts | 25 +++++++++++++++++-- packages/rest-typings/src/v1/Ajv.ts | 8 +++++- 6 files changed, 56 insertions(+), 7 deletions(-) diff --git a/apps/meteor/ee/server/apps/communication/endpoints/appGeneralLogsHandler.ts b/apps/meteor/ee/server/apps/communication/endpoints/appGeneralLogsHandler.ts index 493a134ce681e..d214b96ecba39 100644 --- a/apps/meteor/ee/server/apps/communication/endpoints/appGeneralLogsHandler.ts +++ b/apps/meteor/ee/server/apps/communication/endpoints/appGeneralLogsHandler.ts @@ -33,7 +33,7 @@ export const registerAppGeneralLogsHandler = ({ api, _orch }: AppsRestApi) => properties: { offset: { type: 'number' }, // TODO: reference a schema for the array items - logs: { type: 'array' }, + logs: { type: 'array', items: {} }, count: { type: 'number' }, total: { type: 'number' }, success: { type: 'boolean' }, diff --git a/apps/meteor/ee/server/apps/communication/endpoints/appLogsHandler.ts b/apps/meteor/ee/server/apps/communication/endpoints/appLogsHandler.ts index e51b1d407b8ad..660679cd0b166 100644 --- a/apps/meteor/ee/server/apps/communication/endpoints/appLogsHandler.ts +++ b/apps/meteor/ee/server/apps/communication/endpoints/appLogsHandler.ts @@ -31,7 +31,7 @@ export const registerAppLogsHandler = ({ api, _manager, _orch }: AppsRestApi) => type: 'object', properties: { offset: { type: 'number' }, - logs: { type: 'array' }, + logs: { type: 'array', items: {} }, count: { type: 'number' }, total: { type: 'number' }, success: { type: 'boolean' }, diff --git a/apps/meteor/server/api/default/openApi.ts b/apps/meteor/server/api/default/openApi.ts index 08ed39100750f..ee2121e4929b2 100644 --- a/apps/meteor/server/api/default/openApi.ts +++ b/apps/meteor/server/api/default/openApi.ts @@ -1,6 +1,6 @@ import { schemas } from '@rocket.chat/core-typings'; import type { Route } from '@rocket.chat/http-router'; -import { openAPIErrorComponents, withOperationIds } from '@rocket.chat/http-router'; +import { getSharedSchemas, openAPIErrorComponents, withOperationIds } from '@rocket.chat/http-router'; import { ajv, isOpenAPIJSONEndpoint } from '@rocket.chat/rest-typings'; import express from 'express'; import { WebApp } from 'meteor/webapp'; @@ -94,6 +94,7 @@ const makeOpenAPIResponse = (paths: Record>) => ({ schemas: { ...schemas.components.schemas, ...openAPIErrorComponents, + ...getSharedSchemas(), }, }, }); diff --git a/packages/http-router/src/openapi.spec.ts b/packages/http-router/src/openapi.spec.ts index 18682174fd9e9..0d2bafa6099cd 100644 --- a/packages/http-router/src/openapi.spec.ts +++ b/packages/http-router/src/openapi.spec.ts @@ -1,7 +1,7 @@ import Ajv from 'ajv/dist/2020'; import type { OpenAPIDocsOptions } from './openapi'; -import { buildOperation, buildOperationId, toOpenAPIPath, withOperationIds } from './openapi'; +import { buildOperation, buildOperationId, getSharedSchemas, toOpenAPIPath, withOperationIds } from './openapi'; const ajv = new Ajv(); ajv.addVocabulary(['example']); @@ -267,6 +267,27 @@ describe('buildOperation', () => { ]); expect(build('GET', '/api/v1/x', { rateLimiterOptions: false }).responses[200].headers).toBeUndefined(); }); + it('should hoist schemas that carry an $id into components and reference them', () => { + const shared = ajv.compile({ + $id: 'BadRequestError', + type: 'object', + properties: { success: { type: 'boolean', enum: [false] }, error: { type: 'string' } }, + required: ['success'], + }); + + const operation = build('POST', '/api/v1/rooms.create', { response: { 200: okSchema, 400: shared }, body: shared }); + + expect(operation.responses[400].content?.['application/json'].schema).toEqual({ $ref: '#/components/schemas/BadRequestError' }); + expect(operation.requestBody?.content['application/json'].schema).toEqual({ $ref: '#/components/schemas/BadRequestError' }); + // inlined once in components, without the `$id` that put it there + expect(getSharedSchemas().BadRequestError).toEqual({ + type: 'object', + properties: { success: { type: 'boolean', enum: [false] }, error: { type: 'string' } }, + required: ['success'], + }); + // schemas without an $id stay inline + expect(operation.responses[200].content?.['application/json'].schema).toBe(okSchema.schema); + }); }); describe('withOperationIds', () => { diff --git a/packages/http-router/src/openapi.ts b/packages/http-router/src/openapi.ts index 81fad08a49418..9257bc15fd1fd 100644 --- a/packages/http-router/src/openapi.ts +++ b/packages/http-router/src/openapi.ts @@ -194,6 +194,27 @@ const getSchema = (carrier: unknown): AnySchema => { return carrier as AnySchema; }; +const sharedSchemas = new Map(); + +/** Schemas hoisted out of the operations because they carry an `$id`; merge into `components.schemas`. */ +export const getSharedSchemas = (): Record => Object.fromEntries(sharedSchemas); + +/** + * A schema with an `$id` is shared by many routes — the error payloads, mostly — so it is hoisted + * into `components.schemas` and referenced, instead of being inlined in every operation. + */ +const referenceSchema = (schema: AnySchema): AnySchema => { + if (!schema || typeof schema !== 'object' || !('$id' in schema) || typeof schema.$id !== 'string' || /[#/]/.test(schema.$id)) { + return schema; + } + + const { $id, ...definition } = schema; + + sharedSchemas.set($id, definition as AnySchema); + + return { $ref: `#/components/schemas/${$id}` }; +}; + const collectProperties = (schema: unknown): CollectedProperties | undefined => { if (!schema || typeof schema !== 'object') { return undefined; @@ -391,7 +412,7 @@ export const buildOperation = (method: string, path: string, options: OpenAPIDoc description: describeStatus(code, options), content: { [options.responseContentType?.[code] ?? 'application/json']: { - schema: getSchema(validator), + schema: referenceSchema(getSchema(validator)), ...buildMediaExamples(options.examples?.response?.[code]), }, }, @@ -440,7 +461,7 @@ export const buildOperation = (method: string, path: string, options: OpenAPIDoc required: true as const, content: { [options.bodyContentType ?? 'application/json']: { - schema: bodySchema, + schema: referenceSchema(bodySchema), ...buildMediaExamples(options.examples?.body), }, }, diff --git a/packages/rest-typings/src/v1/Ajv.ts b/packages/rest-typings/src/v1/Ajv.ts index 1dea037c54ad1..7748eb8724190 100644 --- a/packages/rest-typings/src/v1/Ajv.ts +++ b/packages/rest-typings/src/v1/Ajv.ts @@ -56,13 +56,16 @@ type BadRequestErrorResponse = { }; const BadRequestErrorResponseSchema = { + $id: 'BadRequestError', type: 'object', properties: { success: { type: 'boolean', enum: [false] }, stack: { type: 'string' }, error: { type: 'string' }, errorType: { type: 'string' }, - details: { anyOf: [{ type: 'string' }, { type: 'object' }, { type: 'array' }] }, + // `items` is empty on purpose: the spec requires the keyword on arrays, and the payload can + // hold anything the endpoint chose to attach + details: { anyOf: [{ type: 'string' }, { type: 'object' }, { type: 'array', items: {} }] }, }, required: ['success'], additionalProperties: false, @@ -79,6 +82,7 @@ type UnauthorizedErrorResponse = { }; const UnauthorizedErrorResponseSchema = { + $id: 'UnauthorizedError', type: 'object', properties: { success: { type: 'boolean', enum: [false] }, @@ -102,6 +106,7 @@ type ForbiddenErrorResponse = { }; const ForbiddenErrorResponseSchema = { + $id: 'ForbiddenError', type: 'object', properties: { success: { type: 'boolean', enum: [false] }, @@ -122,6 +127,7 @@ type NotFoundErrorResponse = { }; const NotFoundErrorResponseSchema = { + $id: 'NotFoundError', type: 'object', properties: { success: { type: 'boolean', enum: [false] }, From f3acce9ba07df1ff26cdf4eafa25a7afffb5abe6 Mon Sep 17 00:00:00 2001 From: Guilherme Gazzo Date: Fri, 31 Jul 2026 13:42:45 -0300 Subject: [PATCH 05/16] fix(api): serve the document without the success envelope `/api/docs/json` answered `API.default.success(document)`, which adds a `success` key to the root of the document. It is not an OpenAPI field, and validators reject it (`Property 'success' is not expected here` at `#/`). Also adds `items` to the eight remaining arrays whose schema lacked it - the spec requires the keyword on every array, and validators refuse the schema without it. --- apps/meteor/server/api/default/openApi.ts | 11 ++++++----- apps/meteor/server/api/v1/misc.ts | 2 +- apps/meteor/server/api/v1/stats.ts | 2 +- apps/meteor/server/api/v1/users.ts | 8 ++++---- packages/rest-typings/src/default/index.ts | 2 ++ 5 files changed, 14 insertions(+), 11 deletions(-) diff --git a/apps/meteor/server/api/default/openApi.ts b/apps/meteor/server/api/default/openApi.ts index ee2121e4929b2..c47ad9cb9c3bd 100644 --- a/apps/meteor/server/api/default/openApi.ts +++ b/apps/meteor/server/api/default/openApi.ts @@ -105,13 +105,12 @@ const openApiResponseSchema = ajv.compile>({ openapi: { type: 'string' }, info: { type: 'object' }, externalDocs: { type: 'object' }, - servers: { type: 'array' }, - tags: { type: 'array' }, + servers: { type: 'array', items: {} }, + tags: { type: 'array', items: {} }, components: { type: 'object' }, paths: { type: 'object' }, - success: { type: 'boolean', enum: [true] }, }, - required: ['openapi', 'info', 'paths', 'success'], + required: ['openapi', 'info', 'paths'], additionalProperties: false, }); @@ -127,7 +126,9 @@ API.default.get( function action() { const { withUndocumented = false } = this.queryParams; - return API.default.success(makeOpenAPIResponse(getTypedRoutes(API.api.typedRoutes, { withUndocumented }))); + // deliberately not wrapped in `API.default.success`: the envelope would add a `success` key to + // the document root, which is not an OpenAPI field and fails validation + return { statusCode: 200 as const, body: makeOpenAPIResponse(getTypedRoutes(API.api.typedRoutes, { withUndocumented })) }; }, ); diff --git a/apps/meteor/server/api/v1/misc.ts b/apps/meteor/server/api/v1/misc.ts index cf762279f59b3..3cfe0f0688960 100644 --- a/apps/meteor/server/api/v1/misc.ts +++ b/apps/meteor/server/api/v1/misc.ts @@ -495,7 +495,7 @@ const pwGetPolicyResponseSchema = ajv.compile<{ enabled: boolean; policy: [strin type: 'object', properties: { enabled: { type: 'boolean' }, - policy: { type: 'array', items: { type: 'array' } }, + policy: { type: 'array', items: { type: 'array', items: {} } }, }, additionalProperties: true, }); diff --git a/apps/meteor/server/api/v1/stats.ts b/apps/meteor/server/api/v1/stats.ts index 54f7c62f3f74c..90a0db36b7e2a 100644 --- a/apps/meteor/server/api/v1/stats.ts +++ b/apps/meteor/server/api/v1/stats.ts @@ -24,7 +24,7 @@ const statisticsResponseSchema = ajv.compile({ const statisticsListResponseSchema = ajv.compile<{ statistics: IStats[]; count: number; offset: number; total: number }>({ type: 'object', properties: { - statistics: { type: 'array' }, + statistics: { type: 'array', items: {} }, count: { type: 'number' }, offset: { type: 'number' }, total: { type: 'number' }, diff --git a/apps/meteor/server/api/v1/users.ts b/apps/meteor/server/api/v1/users.ts index 49487b71a0fbb..fc5d24bc1d3f0 100644 --- a/apps/meteor/server/api/v1/users.ts +++ b/apps/meteor/server/api/v1/users.ts @@ -784,7 +784,7 @@ API.v1.get( type: 'object', properties: { // user shape varies by projection and permissions - users: { type: 'array' }, + users: { type: 'array', items: {} }, count: { type: 'number' }, offset: { type: 'number' }, total: { type: 'number' }, @@ -1519,7 +1519,7 @@ API.v1.get( type: 'object', properties: { // user shape varies by projection and permissions - users: { type: 'array' }, + users: { type: 'array', items: {} }, full: { type: 'boolean' }, success: { type: 'boolean', enum: [true] }, }, @@ -1680,7 +1680,7 @@ API.v1.get( type: 'object', properties: { // autocomplete items shape varies by permissions - items: { type: 'array' }, + items: { type: 'array', items: {} }, success: { type: 'boolean', enum: [true] }, }, required: ['items', 'success'], @@ -1829,7 +1829,7 @@ API.v1 200: ajv.compile<{ teams: unknown[] }>({ type: 'object', properties: { - teams: { type: 'array' }, + teams: { type: 'array', items: {} }, success: { type: 'boolean', enum: [true] }, }, required: ['teams', 'success'], diff --git a/packages/rest-typings/src/default/index.ts b/packages/rest-typings/src/default/index.ts index daf19c327e2ce..0430e305d2d61 100644 --- a/packages/rest-typings/src/default/index.ts +++ b/packages/rest-typings/src/default/index.ts @@ -77,6 +77,8 @@ export interface DefaultEndpoints { schemas: unknown; }; paths: Record>; + /** The plain OpenAPI document: it is NOT wrapped in the `{ success: true }` envelope */ + success?: never; }; }; } From 30598cf006b32cd8fed5c5364de9246d3d41c929 Mon Sep 17 00:00:00 2001 From: Guilherme Gazzo Date: Fri, 31 Jul 2026 13:50:29 -0300 Subject: [PATCH 06/16] feat(api): serve the document as OpenAPI 3.1 The document declared 3.0.3 while carrying schemas written in JSON Schema 2020, the dialect the endpoints validate against at runtime. Validators rejected 51 of them: union types (`type: ["number", "null"]`, 39 of them), `const` (9), `unevaluatedProperties` and `propertyNames`. OpenAPI 3.1 *is* JSON Schema 2020, so declaring it makes the document honest and all 51 errors disappear without touching a single schema. `typia` now generates its component schemas for 3.1 as well, which drops `nullable` in favour of the union types 3.1 accepts. The UI we serve (swagger-ui 5) renders 3.1 documents. --- .changeset/plain-lions-agree.md | 6 ++++++ apps/meteor/server/api/default/openApi.ts | 2 +- apps/meteor/tests/end-to-end/api/openapi.ts | 2 +- packages/core-typings/src/Ajv.ts | 2 +- 4 files changed, 9 insertions(+), 3 deletions(-) create mode 100644 .changeset/plain-lions-agree.md diff --git a/.changeset/plain-lions-agree.md b/.changeset/plain-lions-agree.md new file mode 100644 index 0000000000000..582a4802aa926 --- /dev/null +++ b/.changeset/plain-lions-agree.md @@ -0,0 +1,6 @@ +--- +'@rocket.chat/core-typings': patch +'@rocket.chat/meteor': patch +--- + +Serves the OpenAPI document as 3.1, the version whose schemas are the JSON Schema dialect the endpoints already validate against diff --git a/apps/meteor/server/api/default/openApi.ts b/apps/meteor/server/api/default/openApi.ts index c47ad9cb9c3bd..0d15e556c377f 100644 --- a/apps/meteor/server/api/default/openApi.ts +++ b/apps/meteor/server/api/default/openApi.ts @@ -60,7 +60,7 @@ const getTags = (paths: Record>) => { }; const makeOpenAPIResponse = (paths: Record>) => ({ - openapi: '3.0.3', + openapi: '3.1.0', info: { title: 'Rocket.Chat API', description: diff --git a/apps/meteor/tests/end-to-end/api/openapi.ts b/apps/meteor/tests/end-to-end/api/openapi.ts index 876e11354ca88..281fe6bbd14db 100644 --- a/apps/meteor/tests/end-to-end/api/openapi.ts +++ b/apps/meteor/tests/end-to-end/api/openapi.ts @@ -36,7 +36,7 @@ describe('[OpenAPI]', () => { }); it('should describe every documented route', () => { - expect(document).to.have.property('openapi', '3.0.3'); + expect(document).to.have.property('openapi', '3.1.0'); expect(document.info).to.have.property('title', 'Rocket.Chat API'); expect(document.tags).to.be.an('array').that.is.not.empty; expect(Object.keys(document.paths)).to.not.be.empty; diff --git a/packages/core-typings/src/Ajv.ts b/packages/core-typings/src/Ajv.ts index a1169db0dafe0..9b942b7cc57d6 100644 --- a/packages/core-typings/src/Ajv.ts +++ b/packages/core-typings/src/Ajv.ts @@ -69,5 +69,5 @@ export const schemas = typia.json.schemas< ICustomUserStatus, SlashCommand, ], - '3.0' + '3.1' >(); From d9d55b1e97dd043d1e738084decb3805873695ce Mon Sep 17 00:00:00 2001 From: Guilherme Gazzo Date: Fri, 31 Jul 2026 14:05:54 -0300 Subject: [PATCH 07/16] fix(api): keep the generated schemas in a single JSON Schema dialect Generating the component schemas for OpenAPI 3.1 exposed two places where typia's output no longer matched what the runtime expects: - tuples come out with `prefixItems` (JSON Schema 2020) next to `additionalItems`, a keyword 2020 replaced with `items`. AJV runs in 2020 and aborts on the unknown keyword - `Error: strict mode: unknown keyword: "additionalItems"` - taking the server down at boot. The rename happens once, on the generated output, together with the `minItems` a closed tuple implies and AJV asks for; - single valued types come out as `const` instead of `enum`, which silently broke the heuristic that locks down the plain file attachment branch, bringing back the ambiguous `oneOf` that fails response validation for messages carrying files. Both spellings are now recognized. AJV also implements `discriminator` but rejects its `mapping`, which 3.1 output includes. It is dropped from the copy AJV compiles, and kept in the one the document serializes, where it is what tools actually use. --- apps/meteor/server/api/validation/ajv.ts | 33 +++++++++++++++++++++--- packages/core-typings/src/Ajv.ts | 30 ++++++++++++++++++++- 2 files changed, 58 insertions(+), 5 deletions(-) diff --git a/apps/meteor/server/api/validation/ajv.ts b/apps/meteor/server/api/validation/ajv.ts index cd3678516abaa..7c6e5bf946647 100644 --- a/apps/meteor/server/api/validation/ajv.ts +++ b/apps/meteor/server/api/validation/ajv.ts @@ -24,19 +24,44 @@ if (components) { } const schema = components[key] as { properties?: Record }; const props = schema?.properties; - const typeEnum = (props?.type as { enum?: unknown[] } | undefined)?.enum; - const isFileBranch = Array.isArray(typeEnum) && typeEnum.length === 1 && typeEnum[0] === 'file'; + // typia writes single valued types as `enum: ['file']` for OpenAPI 3.0 and as `const: 'file'` + // for 3.1, so both spellings have to be recognized + const typeSchema = props?.type as { enum?: unknown[]; const?: unknown } | undefined; + const typeValues = typeSchema?.enum ?? (typeSchema && 'const' in typeSchema ? [typeSchema.const] : undefined); + const isFileBranch = Array.isArray(typeValues) && typeValues.length === 1 && typeValues[0] === 'file'; const hasMediaUrl = !!props && ('image_url' in props || 'video_url' in props || 'audio_url' in props); if (isFileBranch && !hasMediaUrl) { (schema as Record).additionalProperties = false; } } + // AJV implements `discriminator` but refuses its `mapping`, which typia emits for OpenAPI 3.1. + // The mapping is only useful to documentation tools, so it is dropped from the copy AJV compiles + // and kept in the one the OpenAPI document serializes. + const forValidation = (schema: unknown): unknown => { + if (Array.isArray(schema)) { + return schema.map(forValidation); + } + + if (!schema || typeof schema !== 'object') { + return schema; + } + + return Object.fromEntries( + Object.entries(schema).map(([key, value]) => [ + key, + key === 'discriminator' && value && typeof value === 'object' + ? Object.fromEntries(Object.entries(value).filter(([name]) => name !== 'mapping')) + : forValidation(value), + ]), + ); + }; + for (const key in components) { if (Object.prototype.hasOwnProperty.call(components, key)) { const uri = `#/components/schemas/${key}`; - ajv.addSchema(components[key], uri); - ajvQuery.addSchema(components[key], uri); + ajv.addSchema(forValidation(components[key]), uri); + ajvQuery.addSchema(forValidation(components[key]), uri); } } } diff --git a/packages/core-typings/src/Ajv.ts b/packages/core-typings/src/Ajv.ts index 9b942b7cc57d6..a34025eee3778 100644 --- a/packages/core-typings/src/Ajv.ts +++ b/packages/core-typings/src/Ajv.ts @@ -29,7 +29,7 @@ import type { VideoConferenceCapabilities } from './VideoConferenceCapabilities' import type { IImport } from './import/IImport'; import type { IMediaCall } from './mediaCalls/IMediaCall'; -export const schemas = typia.json.schemas< +const generatedSchemas = typia.json.schemas< [ ( | ISubscription @@ -71,3 +71,31 @@ export const schemas = typia.json.schemas< ], '3.1' >(); + +/** + * typia mixes dialects on tuples: it emits `prefixItems` (JSON Schema 2020) alongside + * `additionalItems`, a keyword 2020 removed in favour of `items` applied after `prefixItems`. AJV + * runs in 2020 and refuses the unknown keyword, so the fix happens once, here, and both the runtime + * validation and the OpenAPI document get schemas in a single dialect. `minItems` comes along + * because a closed tuple has a known length, and AJV asks for it. + */ +const toDraft2020 = (node: T): T => { + if (Array.isArray(node)) { + return node.map(toDraft2020) as T; + } + + if (!node || typeof node !== 'object') { + return node; + } + + const entries = Object.entries(node).map(([key, value]) => [key === 'additionalItems' ? 'items' : key, toDraft2020(value)]); + const schema = Object.fromEntries(entries) as Record; + + if (Array.isArray(schema.prefixItems) && schema.items === false && schema.minItems === undefined) { + schema.minItems = schema.prefixItems.length; + } + + return schema as T; +}; + +export const schemas = toDraft2020(generatedSchemas); From c04775a9ca06a4605db8391a022113ce007e9260 Mon Sep 17 00:00:00 2001 From: Guilherme Gazzo Date: Fri, 31 Jul 2026 14:15:16 -0300 Subject: [PATCH 08/16] fix(api): emit nullable fields in the 3.1 spelling Declaring the document as 3.1 left 725 structural errors behind: `nullable`, the way 3.0 marked a field as accepting null, is not a JSON Schema keyword, so 3.1 rejects it. The 895 schemas that use it keep it - AJV understands it - and the document now carries the 3.1 equivalent instead, a `null` member of `type`, converted on a copy while each operation is built. Also drops the trailing slash from the server url, which `Site_Url` often carries and which would make every path in the document resolve with a double one. --- apps/meteor/server/api/default/openApi.ts | 3 +- packages/http-router/src/openapi.spec.ts | 24 ++++++++++++++-- packages/http-router/src/openapi.ts | 35 +++++++++++++++++++++-- 3 files changed, 56 insertions(+), 6 deletions(-) diff --git a/apps/meteor/server/api/default/openApi.ts b/apps/meteor/server/api/default/openApi.ts index 0d15e556c377f..af449877bd4b1 100644 --- a/apps/meteor/server/api/default/openApi.ts +++ b/apps/meteor/server/api/default/openApi.ts @@ -73,7 +73,8 @@ const makeOpenAPIResponse = (paths: Record>) => ({ }, servers: [ { - url: settings.get('Site_Url'), + // trailing slash would make every path in the document resolve with a double one + url: settings.get('Site_Url')?.replace(/\/$/, ''), }, ], tags: getTags(paths), diff --git a/packages/http-router/src/openapi.spec.ts b/packages/http-router/src/openapi.spec.ts index 0d2bafa6099cd..79b4db7f78c14 100644 --- a/packages/http-router/src/openapi.spec.ts +++ b/packages/http-router/src/openapi.spec.ts @@ -119,7 +119,7 @@ describe('buildOperation', () => { const operation = build('GET', '/api/v1/weird', { query }); - expect(operation.parameters).toEqual([{ name: 'query', in: 'query', required: false, schema: query.schema }]); + expect(operation.parameters).toEqual([{ name: 'query', in: 'query', required: false, schema: { type: 'string' } }]); }); it('should inject the error responses implied by the route options', () => { @@ -139,7 +139,7 @@ describe('buildOperation', () => { }); expect(operation.responses[400].description).toBe('Custom bad request'); - expect(operation.responses[400].content?.['application/json'].schema).toBe(okSchema.schema); + expect(operation.responses[400].content?.['application/json'].schema).toEqual(okSchema.schema); }); it('should not offer rate limit headers when rate limiting is disabled', () => { @@ -286,7 +286,25 @@ describe('buildOperation', () => { required: ['success'], }); // schemas without an $id stay inline - expect(operation.responses[200].content?.['application/json'].schema).toBe(okSchema.schema); + expect(operation.responses[200].content?.['application/json'].schema).toEqual(okSchema.schema); + }); + it('should convert `nullable` to the 3.1 spelling', () => { + const schema = ajv.compile({ + type: 'object', + properties: { + name: { type: 'string', nullable: true }, + count: { type: ['number', 'null'], nullable: true }, + plain: { type: 'string' }, + }, + }); + + const operation = build('GET', '/api/v1/rooms.info', { response: { 200: schema } }); + const properties = (operation.responses[200].content?.['application/json'].schema as any).properties; + + expect(properties.name).toEqual({ type: ['string', 'null'] }); + expect(properties.count).toEqual({ type: ['number', 'null'] }); + expect(properties.plain).toEqual({ type: 'string' }); + expect(JSON.stringify(operation)).not.toContain('nullable'); }); }); diff --git a/packages/http-router/src/openapi.ts b/packages/http-router/src/openapi.ts index 9257bc15fd1fd..72a9b2b75f5e6 100644 --- a/packages/http-router/src/openapi.ts +++ b/packages/http-router/src/openapi.ts @@ -187,11 +187,42 @@ type ObjectSchemaLike = { type CollectedProperties = { properties: Record>; required: Set }; +/** + * `nullable` is how OpenAPI 3.0 spelled it; 3.1, being JSON Schema 2020, spells it as a `null` member + * of `type`. AJV understands `nullable`, so the schemas keep it and the conversion happens here, on a + * copy, while the operation is built — once per route, at startup. + */ +const toOpenAPI31 = (schema: T): T => { + if (Array.isArray(schema)) { + return schema.map(toOpenAPI31) as T; + } + + if (!schema || typeof schema !== 'object') { + return schema; + } + + const { nullable, ...rest } = Object.fromEntries(Object.entries(schema).map(([key, value]) => [key, toOpenAPI31(value)])) as Record< + string, + unknown + > & { nullable?: unknown }; + + if (nullable !== true) { + return rest as T; + } + + if (typeof rest.type === 'string') { + return { ...rest, type: [rest.type, 'null'] } as T; + } + + // AJV refuses `nullable` without `type`, so there is always one to extend + return { ...rest, type: Array.isArray(rest.type) && rest.type.includes('null') ? rest.type : [...(rest.type as string[]), 'null'] } as T; +}; + const getSchema = (carrier: unknown): AnySchema => { if (carrier && (typeof carrier === 'object' || typeof carrier === 'function') && 'schema' in carrier) { - return (carrier as { schema: AnySchema }).schema; + return toOpenAPI31((carrier as { schema: AnySchema }).schema); } - return carrier as AnySchema; + return toOpenAPI31(carrier as AnySchema); }; const sharedSchemas = new Map(); From 745c953221a9d930da216bda48894869c81e83eb Mon Sep 17 00:00:00 2001 From: Guilherme Gazzo Date: Fri, 31 Jul 2026 14:27:02 -0300 Subject: [PATCH 09/16] fix(api): declare the security requirement of every operation The document declares security schemes, so an operation with no `security` field is indistinguishable from one that forgot to declare it, and validators flag all 19 public endpoints (`shield.svg`, `pw.getPolicy`, `method.callAnon` and friends). Every operation now spells its requirement out: the auth headers when the route requires them, both the headers and nothing when it accepts anonymous access, and an empty list when it is public. --- packages/http-router/src/openapi.spec.ts | 8 +++++-- packages/http-router/src/openapi.ts | 30 ++++++++++++++---------- 2 files changed, 24 insertions(+), 14 deletions(-) diff --git a/packages/http-router/src/openapi.spec.ts b/packages/http-router/src/openapi.spec.ts index 79b4db7f78c14..6091657b00106 100644 --- a/packages/http-router/src/openapi.spec.ts +++ b/packages/http-router/src/openapi.spec.ts @@ -199,9 +199,13 @@ describe('buildOperation', () => { expect(build('GET', '/api/v1/banners', { operationId: 'listBanners' }).operationId).toBe('listBanners'); }); - it('should only require authentication headers when the route requires auth', () => { - expect(build('GET', '/api/v1/info').security).toBeUndefined(); + it('should spell out the security requirement, empty included', () => { + expect(build('GET', '/api/v1/info').security).toEqual([]); expect(build('GET', '/api/v1/me', { authRequired: true }).security).toEqual([{ userId: [], authToken: [] }]); + expect(build('GET', '/api/v1/channels.anonymousread', { authOrAnonRequired: true }).security).toEqual([ + { userId: [], authToken: [] }, + {}, + ]); }); it('should generate the operation id published at developer.rocket.chat', () => { diff --git a/packages/http-router/src/openapi.ts b/packages/http-router/src/openapi.ts index 72a9b2b75f5e6..9707f487ab143 100644 --- a/packages/http-router/src/openapi.ts +++ b/packages/http-router/src/openapi.ts @@ -27,6 +27,8 @@ export type OpenAPIExternalDocs = { description?: string; }; +type SecurityRequirement = { userId: []; authToken: [] } | Record; + export type Route = { 'summary'?: string; 'description'?: string; @@ -39,10 +41,7 @@ export type Route = { required: true; content: Record; }; - 'security'?: { - userId: []; - authToken: []; - }[]; + 'security'?: SecurityRequirement[]; 'tags'?: string[]; 'x-permissions'?: string[]; 'x-license'?: string[]; @@ -101,6 +100,8 @@ export type OpenAPIDocsOptions = OpenAPIDocumentation & { body?: unknown; response?: Record; authRequired?: boolean; + /** The endpoint works with or without credentials — anonymous read, mostly. */ + authOrAnonRequired?: boolean; twoFactorRequired?: boolean; permissionsRequired?: PermissionsRequired; license?: readonly string[]; @@ -397,6 +398,16 @@ const implicitErrorStatuses = (method: string, options: OpenAPIDocsOptions): num return statuses; }; +const AUTH_HEADERS: SecurityRequirement = { userId: [] as [], authToken: [] as [] }; + +const buildSecurity = (options: OpenAPIDocsOptions): SecurityRequirement[] => { + if (options.authOrAnonRequired) { + return [AUTH_HEADERS, {}]; + } + + return options.authRequired ? [AUTH_HEADERS] : []; +}; + const describeStatus = (status: number, options: OpenAPIDocsOptions): string => options.responseDescriptions?.[status] ?? STATUS_DESCRIPTIONS[status] ?? `Response with status ${status}`; @@ -498,14 +509,9 @@ export const buildOperation = (method: string, path: string, options: OpenAPIDoc }, }, }), - ...(options.authRequired && { - security: [ - { - userId: [] as [], - authToken: [] as [], - }, - ], - }), + // spelled out even when empty: the document declares security schemes, so an operation without + // a requirement is indistinguishable from one that forgot to declare it + security: buildSecurity(options), ...(permissions.length && { 'x-permissions': permissions }), ...(options.license?.length && { 'x-license': [...options.license] }), ...(options.twoFactorRequired && { 'x-two-factor-required': true }), From ede04097c1b4f60a858da7869403856445fe0e07 Mon Sep 17 00:00:00 2001 From: Guilherme Gazzo Date: Fri, 31 Jul 2026 15:03:42 -0300 Subject: [PATCH 10/16] fix(api): drop the discriminator mapping from the document A validator that compiles the component schemas with AJV cannot register `IMessage`, `IRoom`, `IIntegrationHistory`, `PartialIMessage` or `IUploadWithUser`, because AJV implements `discriminator` but rejects its `mapping`. Everything that references those schemas then reports `can't resolve reference`, which is how one unsupported keyword turns into 38 errors and stops a generator that refuses to run on an invalid document. The mapping is now dropped where the schemas are generated, so the document and the runtime share it, and the strip the API bootstrap did for AJV alone is gone. What the mapping described is still there: each branch carries the `const` of its discriminating property. Also defines the `_id` and `t` that the loose `channels.info` branch requires, so the schema stops requiring properties it never declares. --- apps/meteor/server/api/v1/channels.ts | 10 ++++++++- apps/meteor/server/api/validation/ajv.ts | 26 ++---------------------- packages/core-typings/src/Ajv.ts | 8 +++++++- 3 files changed, 18 insertions(+), 26 deletions(-) diff --git a/apps/meteor/server/api/v1/channels.ts b/apps/meteor/server/api/v1/channels.ts index 42c46741641ac..7b0b592f401bd 100644 --- a/apps/meteor/server/api/v1/channels.ts +++ b/apps/meteor/server/api/v1/channels.ts @@ -133,7 +133,15 @@ const channelInfoResponseSchema = ajv.compile<{ channel: IRoom }>({ type: 'object', properties: { channel: { - anyOf: [{ $ref: '#/components/schemas/IRoom' }, { type: 'object', required: ['_id', 't'], additionalProperties: true }], + anyOf: [ + { $ref: '#/components/schemas/IRoom' }, + { + type: 'object', + properties: { _id: { type: 'string' }, t: { type: 'string' } }, + required: ['_id', 't'], + additionalProperties: true, + }, + ], }, success: { type: 'boolean', enum: [true] }, }, diff --git a/apps/meteor/server/api/validation/ajv.ts b/apps/meteor/server/api/validation/ajv.ts index 7c6e5bf946647..ac42843807533 100644 --- a/apps/meteor/server/api/validation/ajv.ts +++ b/apps/meteor/server/api/validation/ajv.ts @@ -35,33 +35,11 @@ if (components) { } } - // AJV implements `discriminator` but refuses its `mapping`, which typia emits for OpenAPI 3.1. - // The mapping is only useful to documentation tools, so it is dropped from the copy AJV compiles - // and kept in the one the OpenAPI document serializes. - const forValidation = (schema: unknown): unknown => { - if (Array.isArray(schema)) { - return schema.map(forValidation); - } - - if (!schema || typeof schema !== 'object') { - return schema; - } - - return Object.fromEntries( - Object.entries(schema).map(([key, value]) => [ - key, - key === 'discriminator' && value && typeof value === 'object' - ? Object.fromEntries(Object.entries(value).filter(([name]) => name !== 'mapping')) - : forValidation(value), - ]), - ); - }; - for (const key in components) { if (Object.prototype.hasOwnProperty.call(components, key)) { const uri = `#/components/schemas/${key}`; - ajv.addSchema(forValidation(components[key]), uri); - ajvQuery.addSchema(forValidation(components[key]), uri); + ajv.addSchema(components[key], uri); + ajvQuery.addSchema(components[key], uri); } } } diff --git a/packages/core-typings/src/Ajv.ts b/packages/core-typings/src/Ajv.ts index a34025eee3778..6e433a192cb3b 100644 --- a/packages/core-typings/src/Ajv.ts +++ b/packages/core-typings/src/Ajv.ts @@ -78,6 +78,10 @@ const generatedSchemas = typia.json.schemas< * runs in 2020 and refuses the unknown keyword, so the fix happens once, here, and both the runtime * validation and the OpenAPI document get schemas in a single dialect. `minItems` comes along * because a closed tuple has a known length, and AJV asks for it. + * + * The `mapping` of a discriminator goes away for the same reason: AJV rejects it, and a validator + * that chokes on `IMessage` leaves every schema referencing it unresolvable. What it described is + * already in the `const` of each branch. */ const toDraft2020 = (node: T): T => { if (Array.isArray(node)) { @@ -88,7 +92,9 @@ const toDraft2020 = (node: T): T => { return node; } - const entries = Object.entries(node).map(([key, value]) => [key === 'additionalItems' ? 'items' : key, toDraft2020(value)]); + const entries = Object.entries(node) + .filter(([key]) => key !== 'mapping') + .map(([key, value]) => [key === 'additionalItems' ? 'items' : key, toDraft2020(value)]); const schema = Object.fromEntries(entries) as Record; if (Array.isArray(schema.prefixItems) && schema.items === false && schema.minItems === undefined) { From 6a02f45b967ca6823b57a2baaffe510357c86db6 Mon Sep 17 00:00:00 2001 From: Guilherme Gazzo Date: Fri, 31 Jul 2026 18:45:58 -0300 Subject: [PATCH 11/16] fix(api): type the document response as the exception it is Every 2xx body is typed as `{ success: true } & T`, so returning the OpenAPI document without the envelope did not compile - and the lint task, which runs the same typecheck, failed with it. --- apps/meteor/server/api/default/openApi.ts | 11 ++++++++--- 1 file changed, 8 insertions(+), 3 deletions(-) diff --git a/apps/meteor/server/api/default/openApi.ts b/apps/meteor/server/api/default/openApi.ts index af449877bd4b1..56d036e11f298 100644 --- a/apps/meteor/server/api/default/openApi.ts +++ b/apps/meteor/server/api/default/openApi.ts @@ -8,6 +8,7 @@ import swaggerUi from 'swagger-ui-express'; import { settings } from '../../settings'; import { API } from '../api'; +import type { SuccessResult } from '../definition'; import { getTrimmedServerVersion } from '../lib/getTrimmedServerVersion'; const app = express(); @@ -127,9 +128,13 @@ API.default.get( function action() { const { withUndocumented = false } = this.queryParams; - // deliberately not wrapped in `API.default.success`: the envelope would add a `success` key to - // the document root, which is not an OpenAPI field and fails validation - return { statusCode: 200 as const, body: makeOpenAPIResponse(getTypedRoutes(API.api.typedRoutes, { withUndocumented })) }; + // The document is served as it is: `API.default.success` would add a `success` key to its root, + // which is not an OpenAPI field and fails validation. The cast is the price of saying so - every + // 2xx body is typed as `{ success: true } & T`, and this one is the exception. + return { + statusCode: 200 as const, + body: makeOpenAPIResponse(getTypedRoutes(API.api.typedRoutes, { withUndocumented })), + } as unknown as SuccessResult>; }, ); From c29093ed9dc14c90d085e00a137a2e2306c8615b Mon Sep 17 00:00:00 2001 From: Guilherme Gazzo Date: Mon, 3 Aug 2026 09:49:25 -0300 Subject: [PATCH 12/16] fix(api): keep the route documentation out of the callable endpoint types Every route exports its options to its callers through `Endpoints`, minus the response validators. With documentation in those options - summary, description, examples, tags - the augmentation grew a type graph large enough to collapse: `Type 'Endpoints' recursively references itself as a base type`, fifty times, and two thousand errors cascading through the client that consumes those types. Documentation describes an endpoint for readers and has no place in the type its callers see, so it is omitted alongside the response validators. Also destructures in the test what the lint rule asks to destructure. --- apps/meteor/server/api/ApiClass.ts | 17 ++++++++++++----- packages/http-router/src/openapi.spec.ts | 2 +- 2 files changed, 13 insertions(+), 6 deletions(-) diff --git a/apps/meteor/server/api/ApiClass.ts b/apps/meteor/server/api/ApiClass.ts index 1a00e8e396d77..3c19b318a6587 100644 --- a/apps/meteor/server/api/ApiClass.ts +++ b/apps/meteor/server/api/ApiClass.ts @@ -1,5 +1,5 @@ import type { IMethodConnection, IUser } from '@rocket.chat/core-typings'; -import type { Router } from '@rocket.chat/http-router'; +import type { OpenAPIDocumentation, Router } from '@rocket.chat/http-router'; import { License } from '@rocket.chat/license'; import { Logger } from '@rocket.chat/logger'; import { Users } from '@rocket.chat/models'; @@ -60,6 +60,13 @@ const logger = new Logger('API'); // To avoid conflicts or missing something during the period we are adopting a 'feature flag approach' // TODO: MAJOR check if this is still needed export const applyBreakingChanges = shouldBreakInVersion('9.0.0'); +/** + * What an endpoint exposes to its callers. The documentation a route carries - summary, description, + * examples, tags - describes it for readers and has no place in the type its callers see; keeping it + * out also keeps the `Endpoints` augmentation from growing a type graph big enough to collapse. + */ +type CallableOptions = Omit; + type MinimalRoute = { method: 'GET' | 'POST' | 'PUT' | 'DELETE'; path: string; @@ -557,7 +564,7 @@ export class APIClass + } & CallableOptions > > { this.addRoute([subpath], { tags: [], ...options, typed: true }, { [method.toLowerCase()]: { action } } as any); @@ -591,7 +598,7 @@ export class APIClass) + } & CallableOptions) | Prettify< { method: 'POST'; @@ -612,7 +619,7 @@ export class APIClass) + } & CallableOptions) | Prettify< { method: 'PUT'; @@ -633,7 +640,7 @@ export class APIClass) + } & CallableOptions) | Prettify< { method: 'DELETE'; diff --git a/packages/http-router/src/openapi.spec.ts b/packages/http-router/src/openapi.spec.ts index 6091657b00106..abcb8884d1bb4 100644 --- a/packages/http-router/src/openapi.spec.ts +++ b/packages/http-router/src/openapi.spec.ts @@ -303,7 +303,7 @@ describe('buildOperation', () => { }); const operation = build('GET', '/api/v1/rooms.info', { response: { 200: schema } }); - const properties = (operation.responses[200].content?.['application/json'].schema as any).properties; + const { properties } = operation.responses[200].content?.['application/json'].schema as { properties: Record }; expect(properties.name).toEqual({ type: ['string', 'null'] }); expect(properties.count).toEqual({ type: ['number', 'null'] }); From e331cf7c5b8f763e79ce4f338ed0ec81c17e6ade Mon Sep 17 00:00:00 2001 From: Guilherme Gazzo Date: Mon, 3 Aug 2026 10:43:15 -0300 Subject: [PATCH 13/16] revert: omitting the documentation from the callable endpoint types Omitting summary, description, examples and tags from what `Endpoints` exposes did not stop the augmentation from collapsing: the trigger is the tags being declared in the route options at all, not what the type exposes afterwards. The fix belongs where the tags are written, so this goes back to what it was. --- apps/meteor/server/api/ApiClass.ts | 17 +++++------------ 1 file changed, 5 insertions(+), 12 deletions(-) diff --git a/apps/meteor/server/api/ApiClass.ts b/apps/meteor/server/api/ApiClass.ts index 3c19b318a6587..1a00e8e396d77 100644 --- a/apps/meteor/server/api/ApiClass.ts +++ b/apps/meteor/server/api/ApiClass.ts @@ -1,5 +1,5 @@ import type { IMethodConnection, IUser } from '@rocket.chat/core-typings'; -import type { OpenAPIDocumentation, Router } from '@rocket.chat/http-router'; +import type { Router } from '@rocket.chat/http-router'; import { License } from '@rocket.chat/license'; import { Logger } from '@rocket.chat/logger'; import { Users } from '@rocket.chat/models'; @@ -60,13 +60,6 @@ const logger = new Logger('API'); // To avoid conflicts or missing something during the period we are adopting a 'feature flag approach' // TODO: MAJOR check if this is still needed export const applyBreakingChanges = shouldBreakInVersion('9.0.0'); -/** - * What an endpoint exposes to its callers. The documentation a route carries - summary, description, - * examples, tags - describes it for readers and has no place in the type its callers see; keeping it - * out also keeps the `Endpoints` augmentation from growing a type graph big enough to collapse. - */ -type CallableOptions = Omit; - type MinimalRoute = { method: 'GET' | 'POST' | 'PUT' | 'DELETE'; path: string; @@ -564,7 +557,7 @@ export class APIClass + } & Omit > > { this.addRoute([subpath], { tags: [], ...options, typed: true }, { [method.toLowerCase()]: { action } } as any); @@ -598,7 +591,7 @@ export class APIClass) + } & Omit) | Prettify< { method: 'POST'; @@ -619,7 +612,7 @@ export class APIClass) + } & Omit) | Prettify< { method: 'PUT'; @@ -640,7 +633,7 @@ export class APIClass) + } & Omit) | Prettify< { method: 'DELETE'; From ae7766b6fe3fdaa3a6ac5b9b8acefa413178b69c Mon Sep 17 00:00:00 2001 From: Guilherme Gazzo Date: Mon, 3 Aug 2026 11:25:00 -0300 Subject: [PATCH 14/16] fix(api): collapse the nullable unions typia writes for 3.1 Generating for 3.1 turned every nullable field into `oneOf: [{ type: 'null' }, { type: 'string' }]`, where 3.0 wrote `nullable`. The API validates responses with `coerceTypes`, which coerces the value for each branch in turn until more than one matches - and then `oneOf`, meaning exactly one, rejects a perfectly valid payload: at path '/data/9/users/0/avatarETag': must match exactly one schema in oneOf (passingSchemas: 0,1) That is what took `video-conference.list` down in the apps test suite. Branches that only name a type now collapse into a single `type` array, which says the same thing and leaves nothing to disambiguate. Verified against the payload from the failing run: it validates with the collapse and reproduces the failure without it. The discriminator strip is also scoped to the discriminator now, instead of dropping any property named `mapping` wherever it appeared. --- packages/core-typings/src/Ajv.ts | 36 +++++++++++++++++++++++++------- 1 file changed, 29 insertions(+), 7 deletions(-) diff --git a/packages/core-typings/src/Ajv.ts b/packages/core-typings/src/Ajv.ts index 6e433a192cb3b..958ea9894fd89 100644 --- a/packages/core-typings/src/Ajv.ts +++ b/packages/core-typings/src/Ajv.ts @@ -79,9 +79,16 @@ const generatedSchemas = typia.json.schemas< * validation and the OpenAPI document get schemas in a single dialect. `minItems` comes along * because a closed tuple has a known length, and AJV asks for it. * - * The `mapping` of a discriminator goes away for the same reason: AJV rejects it, and a validator - * that chokes on `IMessage` leaves every schema referencing it unresolvable. What it described is - * already in the `const` of each branch. + * Two more differences between what typia writes for 3.1 and what AJV reads: + * + * - the `mapping` of a discriminator, which AJV rejects outright. A validator that chokes on + * `IMessage` leaves every schema referencing it unresolvable, so the mapping goes and the + * `propertyName` stays; + * - a nullable field written as `oneOf: [{ type: 'null' }, { type: 'string' }]`, where 3.0 wrote + * `nullable`. The API validates with `coerceTypes`, which coerces the value for each branch in + * turn until more than one matches, and then `oneOf` - exactly one - fails on a perfectly valid + * payload. Branches that only name a type collapse into a single `type` array, which says the same + * thing and leaves nothing to disambiguate. */ const toDraft2020 = (node: T): T => { if (Array.isArray(node)) { @@ -92,15 +99,30 @@ const toDraft2020 = (node: T): T => { return node; } - const entries = Object.entries(node) - .filter(([key]) => key !== 'mapping') - .map(([key, value]) => [key === 'additionalItems' ? 'items' : key, toDraft2020(value)]); - const schema = Object.fromEntries(entries) as Record; + const schema = Object.fromEntries( + Object.entries(node).map(([key, value]) => [key === 'additionalItems' ? 'items' : key, toDraft2020(value)]), + ) as Record; if (Array.isArray(schema.prefixItems) && schema.items === false && schema.minItems === undefined) { schema.minItems = schema.prefixItems.length; } + if (schema.discriminator && typeof schema.discriminator === 'object') { + const { mapping, ...discriminator } = schema.discriminator as Record; + schema.discriminator = discriminator; + } + + if (Array.isArray(schema.oneOf)) { + const branches = schema.oneOf as Record[]; + const namesATypeOnly = (branch: Record) => Object.keys(branch).length === 1 && typeof branch.type === 'string'; + + if (branches.length > 1 && branches.every(namesATypeOnly)) { + const { oneOf, ...rest } = schema; + + return { ...rest, type: branches.map((branch) => branch.type) } as T; + } + } + return schema as T; }; From 5e75dae40a4eb1d4d5840cf9b9512493ca01c5ef Mon Sep 17 00:00:00 2001 From: Guilherme Gazzo Date: Mon, 3 Aug 2026 15:47:33 -0300 Subject: [PATCH 15/16] fix(api): address the review of the generated document - a path parameter is always required: an optional express segment describes a different path, not an optional parameter, and the spec has no room for one; - a route that declares only a redirect no longer receives an invented `200` with a schema it never answers - anything below 400 counts as documented; - `nullable` on a schema with no usable `type` is left alone instead of spreading a missing value, which would have failed the route registration; - `x-permissions` and the description read the permissions of the method over the wildcard ones, the way `checkPermissionsForInvocation` does at runtime; - a nested router's paths merge into the parent per path: normalizing the keys can bring two distinct paths onto the same one, and replacing the map would drop the methods the other router documented; - an unset `Site_Url` means no `servers` at all, rather than a Server Object with no url, which the spec does not allow; - the `/api-docs` UI reads the document relative to itself, so a workspace hosted under ROOT_URL_PATH_PREFIX keeps its prefix; - the `additionalItems` rename is confined to the keyword: inside `properties` and friends, a name is a field, not a keyword. Same mistake the discriminator strip made; - the API test states the relation between the two documents instead of demanding that untyped routes exist, since they are meant to disappear. --- apps/meteor/server/api/default/openApi.ts | 37 +++++++++++++++------ apps/meteor/tests/end-to-end/api/openapi.ts | 11 ++++-- packages/core-typings/src/Ajv.ts | 16 +++++++-- packages/http-router/src/Router.ts | 13 ++++---- packages/http-router/src/openapi.spec.ts | 20 +++++++++++ packages/http-router/src/openapi.ts | 30 +++++++++++------ 6 files changed, 94 insertions(+), 33 deletions(-) diff --git a/apps/meteor/server/api/default/openApi.ts b/apps/meteor/server/api/default/openApi.ts index 56d036e11f298..317384108a517 100644 --- a/apps/meteor/server/api/default/openApi.ts +++ b/apps/meteor/server/api/default/openApi.ts @@ -8,6 +8,7 @@ import swaggerUi from 'swagger-ui-express'; import { settings } from '../../settings'; import { API } from '../api'; +import { operationTags } from './operationTags'; import type { SuccessResult } from '../definition'; import { getTrimmedServerVersion } from '../lib/getTrimmedServerVersion'; @@ -60,6 +61,22 @@ const getTags = (paths: Record>) => { })); }; +/** Attaches the group each operation belongs to, which is described outside the route options. */ +const withTags = (paths: Record>): Record> => + Object.fromEntries( + Object.entries(paths).map(([path, methods]) => [ + path, + Object.fromEntries( + Object.entries(methods).map(([method, operation]) => [ + method, + { ...operation, tags: operationTags[`${method} ${path}`] ?? operation.tags }, + ]), + ), + ]), + ); + +const siteUrl = () => settings.get('Site_Url')?.replace(/\/$/, ''); + const makeOpenAPIResponse = (paths: Record>) => ({ openapi: '3.1.0', info: { @@ -72,14 +89,11 @@ const makeOpenAPIResponse = (paths: Record>) => ({ url: 'https://developer.rocket.chat/apidocs', description: 'Rocket.Chat developer documentation', }, - servers: [ - { - // trailing slash would make every path in the document resolve with a double one - url: settings.get('Site_Url')?.replace(/\/$/, ''), - }, - ], - tags: getTags(paths), - paths: withOperationIds(paths), + // trailing slash would make every path in the document resolve with a double one, and a Server + // Object without a url is invalid, so an unset `Site_Url` means no `servers` at all + ...(siteUrl() && { servers: [{ url: siteUrl() }] }), + tags: getTags(withTags(paths)), + paths: withOperationIds(withTags(paths)), components: { securitySchemes: { userId: { @@ -143,9 +157,10 @@ app.use( swaggerUi.serve, swaggerUi.setup(null, { swaggerOptions: { - // Relative on purpose: this runs at import time, before settings are loaded, so - // `Site_Url` would render as "undefined" here. - url: '/api/docs/json', + // Relative to `/api-docs`, not to the root: this runs at import time, before settings are + // loaded, so `Site_Url` would render as "undefined", and a leading slash would drop the + // deployment prefix of a workspace hosted under ROOT_URL_PATH_PREFIX. + url: '../api/docs/json', }, }), ); diff --git a/apps/meteor/tests/end-to-end/api/openapi.ts b/apps/meteor/tests/end-to-end/api/openapi.ts index 281fe6bbd14db..6eabf9fd84fb4 100644 --- a/apps/meteor/tests/end-to-end/api/openapi.ts +++ b/apps/meteor/tests/end-to-end/api/openapi.ts @@ -92,11 +92,16 @@ describe('[OpenAPI]', () => { expect(invalid).to.be.empty; }); - it('should hide undocumented routes unless they are asked for', () => { + it('should hide the undocumented routes from the default document', () => { const undocumented = (document: OpenAPIDocument) => - operations(document).filter(({ operation }) => operation.tags?.includes('Missing Documentation')); + operations(document) + .filter(({ operation }) => operation.tags?.includes('Missing Documentation')) + .map(({ path, method }) => `${method} ${path}`); + const documented = operations(document).map(({ path, method }) => `${method} ${path}`); + + // stated as a relation, not as a count: the untyped routes are meant to disappear over time expect(undocumented(document)).to.be.empty; - expect(undocumented(documentWithUndocumented)).to.not.be.empty; + expect(undocumented(documentWithUndocumented).filter((route) => documented.includes(route))).to.be.empty; }); }); diff --git a/packages/core-typings/src/Ajv.ts b/packages/core-typings/src/Ajv.ts index 958ea9894fd89..abe2795a82589 100644 --- a/packages/core-typings/src/Ajv.ts +++ b/packages/core-typings/src/Ajv.ts @@ -90,9 +90,12 @@ const generatedSchemas = typia.json.schemas< * payload. Branches that only name a type collapse into a single `type` array, which says the same * thing and leaves nothing to disambiguate. */ -const toDraft2020 = (node: T): T => { +/** Keys whose values are maps of names to schemas, where a name is not a keyword. */ +const SCHEMA_MAPS = ['properties', 'patternProperties', '$defs', 'definitions', 'dependentSchemas']; + +const toDraft2020 = (node: T, insideSchemaMap = false): T => { if (Array.isArray(node)) { - return node.map(toDraft2020) as T; + return node.map((entry) => toDraft2020(entry)) as T; } if (!node || typeof node !== 'object') { @@ -100,9 +103,16 @@ const toDraft2020 = (node: T): T => { } const schema = Object.fromEntries( - Object.entries(node).map(([key, value]) => [key === 'additionalItems' ? 'items' : key, toDraft2020(value)]), + Object.entries(node).map(([key, value]) => [ + !insideSchemaMap && key === 'additionalItems' ? 'items' : key, + toDraft2020(value, SCHEMA_MAPS.includes(key)), + ]), ) as Record; + if (insideSchemaMap) { + return schema as T; + } + if (Array.isArray(schema.prefixItems) && schema.items === false && schema.minItems === undefined) { schema.minItems = schema.prefixItems.length; } diff --git a/packages/http-router/src/Router.ts b/packages/http-router/src/Router.ts index 70bcd52b9cba3..6379af0323880 100644 --- a/packages/http-router/src/Router.ts +++ b/packages/http-router/src/Router.ts @@ -336,12 +336,13 @@ export class Router< use(innerRouter: unknown): any { if (innerRouter instanceof Router) { - this.typedRoutes = { - ...this.typedRoutes, - ...Object.fromEntries( - Object.entries(innerRouter.typedRoutes).map(([path, routes]) => [toOpenAPIPath(`${this.base}${path}`), routes]), - ), - }; + for (const [path, routes] of Object.entries(innerRouter.typedRoutes)) { + const documentedPath = toOpenAPIPath(`${this.base}${path}`); + + // merged per path: normalizing the keys can bring two distinct paths onto the same one, and + // replacing the map would drop the methods the other router documented + this.typedRoutes[documentedPath] = { ...this.typedRoutes[documentedPath], ...routes }; + } this.innerRouter.route(innerRouter.base, innerRouter.innerRouter); } diff --git a/packages/http-router/src/openapi.spec.ts b/packages/http-router/src/openapi.spec.ts index abcb8884d1bb4..51122aa41ddc8 100644 --- a/packages/http-router/src/openapi.spec.ts +++ b/packages/http-router/src/openapi.spec.ts @@ -310,6 +310,26 @@ describe('buildOperation', () => { expect(properties.plain).toEqual({ type: 'string' }); expect(JSON.stringify(operation)).not.toContain('nullable'); }); + it('should require every path parameter, even one the router declares optional', () => { + const operation = build('GET', '/api/v1/settings/:_id?'); + + expect(operation.parameters).toEqual([{ name: '_id', in: 'path', required: true, schema: { type: 'string' } }]); + }); + + it('should not invent a success response for a route that answers a redirect', () => { + const operation = buildOperation('GET', '/api/v1/shield.svg', { response: { 302: okSchema } }); + + expect(Object.keys(operation.responses)).not.toContain('200'); + expect(operation.responses[302]).toBeDefined(); + }); + + it('should read the permissions of the method over the wildcard ones', () => { + const operation = build('POST', '/api/v1/rooms.create', { + permissionsRequired: { 'POST': ['create-c'], '*': ['view-c-room'] }, + }); + + expect(operation['x-permissions']).toEqual(['create-c']); + }); }); describe('withOperationIds', () => { diff --git a/packages/http-router/src/openapi.ts b/packages/http-router/src/openapi.ts index 9707f487ab143..cdac8369753c7 100644 --- a/packages/http-router/src/openapi.ts +++ b/packages/http-router/src/openapi.ts @@ -215,8 +215,12 @@ const toOpenAPI31 = (schema: T): T => { return { ...rest, type: [rest.type, 'null'] } as T; } - // AJV refuses `nullable` without `type`, so there is always one to extend - return { ...rest, type: Array.isArray(rest.type) && rest.type.includes('null') ? rest.type : [...(rest.type as string[]), 'null'] } as T; + if (Array.isArray(rest.type)) { + return { ...rest, type: rest.type.includes('null') ? rest.type : [...rest.type, 'null'] } as T; + } + + // nothing to extend - AJV refuses `nullable` without a `type`, so this is a schema it never saw + return rest as T; }; const getSchema = (carrier: unknown): AnySchema => { @@ -281,19 +285,19 @@ const collectProperties = (schema: unknown): CollectedProperties | undefined => */ export const toOpenAPIPath = (path: string): string => path.replace(/:([A-Za-z0-9_]+)\??/g, '{$1}').replace(/\/{2,}/g, '/'); -const extractPathParameterNames = (path: string): { name: string; required: boolean }[] => - [...path.matchAll(/:([A-Za-z0-9_]+)(\?)?/g)].map(([, name, optional]) => ({ name, required: !optional })); +/** Every path parameter is required: an optional express segment is a different path, not an optional one. */ +const extractPathParameterNames = (path: string): string[] => [...path.matchAll(/:([A-Za-z0-9_]+)\??/g)].map(([, name]) => name); const buildPathParameters = (path: string, options: OpenAPIDocsOptions): OpenAPIParameter[] => { const documented = collectProperties(getSchema(options.params)); - return extractPathParameterNames(path).map(({ name, required }) => { + return extractPathParameterNames(path).map((name) => { const schema = documented?.properties[name]; return { name, in: 'path', - required, + required: true, schema: (schema as AnySchema) ?? { type: 'string' }, ...(typeof schema?.description === 'string' && { description: schema.description }), ...(options.examples?.params?.[name] !== undefined @@ -340,9 +344,14 @@ const normalizePermissions = (method: string, permissionsRequired: PermissionsRe return permissionsRequired; } - const forMethod = [permissionsRequired[method.toUpperCase()], permissionsRequired['*']].filter(Boolean); + // the method's own entry wins over the wildcard, the way `checkPermissionsForInvocation` reads it + const entry = permissionsRequired[method.toUpperCase()] ?? permissionsRequired['*']; + + if (!entry) { + return []; + } - return forMethod.flatMap((entry) => (Array.isArray(entry) ? entry : (entry?.permissions ?? []))); + return Array.isArray(entry) ? entry : entry.permissions; }; const buildDescription = (method: string, options: OpenAPIDocsOptions): string | undefined => { @@ -462,8 +471,9 @@ export const buildOperation = (method: string, path: string, options: OpenAPIDoc }; } - // `responses` is required by the spec, and undocumented (legacy) routes declare none. - if (!Object.keys(responses).some((status) => Number(status) < 300)) { + // `responses` is required by the spec, and undocumented (legacy) routes declare none. A declared + // redirect counts as documented: the route answers it and never answers a 200. + if (!Object.keys(responses).some((status) => Number(status) < 400)) { responses[200] = { description: describeStatus(200, options), content: { 'application/json': { schema: SUCCESS_RESPONSE_REF } }, From f5a13bbbccfc3003d6b6ede900011856b0fe2229 Mon Sep 17 00:00:00 2001 From: Guilherme Gazzo Date: Mon, 3 Aug 2026 17:48:27 -0300 Subject: [PATCH 16/16] fix(api): drop the reference to a module this branch does not have The review fixes were copied across from the branch stacked on top of this one, and brought its tag map with them: this branch has no operationTags module, so the typecheck could not resolve the import. --- apps/meteor/server/api/default/openApi.ts | 19 ++----------------- 1 file changed, 2 insertions(+), 17 deletions(-) diff --git a/apps/meteor/server/api/default/openApi.ts b/apps/meteor/server/api/default/openApi.ts index 317384108a517..dcf1630e49611 100644 --- a/apps/meteor/server/api/default/openApi.ts +++ b/apps/meteor/server/api/default/openApi.ts @@ -8,7 +8,6 @@ import swaggerUi from 'swagger-ui-express'; import { settings } from '../../settings'; import { API } from '../api'; -import { operationTags } from './operationTags'; import type { SuccessResult } from '../definition'; import { getTrimmedServerVersion } from '../lib/getTrimmedServerVersion'; @@ -61,20 +60,6 @@ const getTags = (paths: Record>) => { })); }; -/** Attaches the group each operation belongs to, which is described outside the route options. */ -const withTags = (paths: Record>): Record> => - Object.fromEntries( - Object.entries(paths).map(([path, methods]) => [ - path, - Object.fromEntries( - Object.entries(methods).map(([method, operation]) => [ - method, - { ...operation, tags: operationTags[`${method} ${path}`] ?? operation.tags }, - ]), - ), - ]), - ); - const siteUrl = () => settings.get('Site_Url')?.replace(/\/$/, ''); const makeOpenAPIResponse = (paths: Record>) => ({ @@ -92,8 +77,8 @@ const makeOpenAPIResponse = (paths: Record>) => ({ // trailing slash would make every path in the document resolve with a double one, and a Server // Object without a url is invalid, so an unset `Site_Url` means no `servers` at all ...(siteUrl() && { servers: [{ url: siteUrl() }] }), - tags: getTags(withTags(paths)), - paths: withOperationIds(withTags(paths)), + tags: getTags(paths), + paths: withOperationIds(paths), components: { securitySchemes: { userId: {