diff --git a/.changeset/famous-poets-reply.md b/.changeset/famous-poets-reply.md new file mode 100644 index 00000000..ae5659ad --- /dev/null +++ b/.changeset/famous-poets-reply.md @@ -0,0 +1,5 @@ +--- +"swagger-typescript-api": patch +--- + +Fix JSDoc comment security vulnerability: escape only necessary `*/` sequences to prevent code injection while reducing noise in generated documentation diff --git a/src/code-gen-process.ts b/src/code-gen-process.ts index 320273f8..1814c3e4 100644 --- a/src/code-gen-process.ts +++ b/src/code-gen-process.ts @@ -223,6 +223,8 @@ export class CodeGenProcess { Ts: this.config.Ts, formatDescription: this.schemaParserFabric.schemaFormatters.formatDescription, + escapeJSDocContent: + this.schemaParserFabric.schemaFormatters.escapeJSDocContent, internalCase: internalCase, classNameCase: pascalCase, pascalCase: pascalCase, diff --git a/src/schema-parser/schema-formatters.ts b/src/schema-parser/schema-formatters.ts index ccd57d7d..6dc4e559 100644 --- a/src/schema-parser/schema-formatters.ts +++ b/src/schema-parser/schema-formatters.ts @@ -102,18 +102,28 @@ export class SchemaFormatters { return formatterFn?.(parsedSchema) || parsedSchema; }; + escapeJSDocContent = (content) => { + if (!content) return ""; + // Escape */ sequences to prevent breaking out of JSDoc comments + // Note: /* sequences inside JSDoc comments are harmless and don't need escaping + return content.replace(/\*\//g, "*\\/"); + }; + formatDescription = (description, inline) => { if (!description) return ""; - const hasMultipleLines = description.includes("\n"); + // Always escape JSDoc comment characters (function is idempotent) + const escapedDescription = this.escapeJSDocContent(description); + + const hasMultipleLines = escapedDescription.includes("\n"); - if (!hasMultipleLines) return description; + if (!hasMultipleLines) return escapedDescription; if (inline) { return ( lodash // @ts-expect-error TS(2339) FIXME: Property '_' does not exist on type 'LoDashStatic'... Remove this comment to see the full error message - ._(description) + ._(escapedDescription) .split(/\n/g) .map((part) => part.trim()) .compact() @@ -122,7 +132,7 @@ export class SchemaFormatters { ); } - return description.replace(/\n$/g, ""); + return escapedDescription.replace(/\n$/g, ""); }; formatObjectContent = (content) => { diff --git a/templates/base/object-field-jsdoc.ejs b/templates/base/object-field-jsdoc.ejs index ec39314b..9d2bfdda 100644 --- a/templates/base/object-field-jsdoc.ejs +++ b/templates/base/object-field-jsdoc.ejs @@ -1,18 +1,19 @@ + <% const { field, utils } = it; -const { formatDescription, require, _ } = utils; +const { formatDescription, escapeJSDocContent, require, _ } = utils; const comments = _.uniq( _.compact([ - field.title, - field.description, + field.title && escapeJSDocContent(field.title), + field.description && escapeJSDocContent(field.description), field.deprecated && ` * @deprecated`, !_.isUndefined(field.format) && `@format ${field.format}`, !_.isUndefined(field.minimum) && `@min ${field.minimum}`, !_.isUndefined(field.maximum) && `@max ${field.maximum}`, - !_.isUndefined(field.pattern) && `@pattern ${field.pattern}`, + !_.isUndefined(field.pattern) && `@pattern ${escapeJSDocContent(field.pattern)}`, !_.isUndefined(field.example) && - `@example ${_.isObject(field.example) ? JSON.stringify(field.example) : field.example}`, + `@example ${_.isObject(field.example) ? JSON.stringify(field.example) : escapeJSDocContent(field.example)}`, ]).reduce((acc, comment) => [...acc, ...comment.split(/\n/g)], []), ); %> diff --git a/templates/base/route-docs.ejs b/templates/base/route-docs.ejs index 3de625ad..4b67fb9c 100644 --- a/templates/base/route-docs.ejs +++ b/templates/base/route-docs.ejs @@ -1,6 +1,6 @@ <% const { config, route, utils } = it; -const { _, formatDescription, fmtToJSDocLine, pascalCase, require } = utils; +const { _, formatDescription, escapeJSDocContent, fmtToJSDocLine, pascalCase, require } = utils; const { raw, request, routeName } = route; const jsDocDescription = raw.description ? @@ -9,7 +9,7 @@ const jsDocDescription = raw.description ? const jsDocLines = _.compact([ _.size(raw.tags) && ` * @tags ${raw.tags.join(", ")}`, ` * @name ${pascalCase(routeName.usage)}`, - raw.summary && ` * @summary ${raw.summary}`, + raw.summary && ` * @summary ${formatDescription(raw.summary, true)}`, ` * @request ${_.upperCase(request.method)}:${raw.route}`, raw.deprecated && ` * @deprecated`, routeName.duplicate && ` * @originalName ${routeName.original}`, @@ -18,7 +18,7 @@ const jsDocLines = _.compact([ ...(config.generateResponses && raw.responsesTypes.length ? raw.responsesTypes.map( ({ type, status, description, isSuccess }) => - ` * @response \`${status}\` \`${_.replace(_.replace(type, /\/\*/g, "\\*"), /\*\//g, "*\\")}\` ${description}`, + ` * @response \`${status}\` \`${_.replace(_.replace(type, /\/\*/g, "\\*"), /\*\//g, "*\\")}\` ${formatDescription(description, true)}`, ) : []), ]).map(str => str.trimEnd()).join("\n"); diff --git a/tests/spec/jsdoc-escaping/__snapshots__/basic.test.ts.snap b/tests/spec/jsdoc-escaping/__snapshots__/basic.test.ts.snap new file mode 100644 index 00000000..ba9cc41b --- /dev/null +++ b/tests/spec/jsdoc-escaping/__snapshots__/basic.test.ts.snap @@ -0,0 +1,307 @@ +// Vitest Snapshot v1, https://vitest.dev/guide/snapshot.html + +exports[`jsdoc-escaping > should escape JSDoc comment characters in descriptions 1`] = ` +"/* eslint-disable */ +/* tslint:disable */ +// @ts-nocheck +/* + * --------------------------------------------------------------- + * ## THIS FILE WAS GENERATED VIA SWAGGER-TYPESCRIPT-API ## + * ## ## + * ## AUTHOR: acacode ## + * ## SOURCE: https://github.com/acacode/swagger-typescript-api ## + * --------------------------------------------------------------- + */ + +/** Information schema with malicious **\\/ window.location='http://evil.com' /** content */ +export interface Information { + /** The ID of the information record. Contains **\\/ dangerous content /** here. */ + id?: number; + /** Title field with *\\/ and /* characters that could break comments */ + title?: string; + /** + * Multi-line description + * with *\\/ characters + * and /* other markers + * that could break JSDoc comments + */ + content?: string; +} + +export type QueryParamsType = Record; +export type ResponseFormat = keyof Omit; + +export interface FullRequestParams extends Omit { + /** set parameter to \`true\` for call \`securityWorker\` for this request */ + secure?: boolean; + /** request path */ + path: string; + /** content type of request body */ + type?: ContentType; + /** query params */ + query?: QueryParamsType; + /** format of response (i.e. response.json() -> format: "json") */ + format?: ResponseFormat; + /** request body */ + body?: unknown; + /** base url */ + baseUrl?: string; + /** request cancellation token */ + cancelToken?: CancelToken; +} + +export type RequestParams = Omit< + FullRequestParams, + "body" | "method" | "query" | "path" +>; + +export interface ApiConfig { + baseUrl?: string; + baseApiParams?: Omit; + securityWorker?: ( + securityData: SecurityDataType | null, + ) => Promise | RequestParams | void; + customFetch?: typeof fetch; +} + +export interface HttpResponse + extends Response { + data: D; + error: E; +} + +type CancelToken = Symbol | string | number; + +export enum ContentType { + Json = "application/json", + JsonApi = "application/vnd.api+json", + FormData = "multipart/form-data", + UrlEncoded = "application/x-www-form-urlencoded", + Text = "text/plain", +} + +export class HttpClient { + public baseUrl: string = ""; + private securityData: SecurityDataType | null = null; + private securityWorker?: ApiConfig["securityWorker"]; + private abortControllers = new Map(); + private customFetch = (...fetchParams: Parameters) => + fetch(...fetchParams); + + private baseApiParams: RequestParams = { + credentials: "same-origin", + headers: {}, + redirect: "follow", + referrerPolicy: "no-referrer", + }; + + constructor(apiConfig: ApiConfig = {}) { + Object.assign(this, apiConfig); + } + + public setSecurityData = (data: SecurityDataType | null) => { + this.securityData = data; + }; + + protected encodeQueryParam(key: string, value: any) { + const encodedKey = encodeURIComponent(key); + return \`\${encodedKey}=\${encodeURIComponent(typeof value === "number" ? value : \`\${value}\`)}\`; + } + + protected addQueryParam(query: QueryParamsType, key: string) { + return this.encodeQueryParam(key, query[key]); + } + + protected addArrayQueryParam(query: QueryParamsType, key: string) { + const value = query[key]; + return value.map((v: any) => this.encodeQueryParam(key, v)).join("&"); + } + + protected toQueryString(rawQuery?: QueryParamsType): string { + const query = rawQuery || {}; + const keys = Object.keys(query).filter( + (key) => "undefined" !== typeof query[key], + ); + return keys + .map((key) => + Array.isArray(query[key]) + ? this.addArrayQueryParam(query, key) + : this.addQueryParam(query, key), + ) + .join("&"); + } + + protected addQueryParams(rawQuery?: QueryParamsType): string { + const queryString = this.toQueryString(rawQuery); + return queryString ? \`?\${queryString}\` : ""; + } + + private contentFormatters: Record any> = { + [ContentType.Json]: (input: any) => + input !== null && (typeof input === "object" || typeof input === "string") + ? JSON.stringify(input) + : input, + [ContentType.JsonApi]: (input: any) => + input !== null && (typeof input === "object" || typeof input === "string") + ? JSON.stringify(input) + : input, + [ContentType.Text]: (input: any) => + input !== null && typeof input !== "string" + ? JSON.stringify(input) + : input, + [ContentType.FormData]: (input: any) => + Object.keys(input || {}).reduce((formData, key) => { + const property = input[key]; + formData.append( + key, + property instanceof Blob + ? property + : typeof property === "object" && property !== null + ? JSON.stringify(property) + : \`\${property}\`, + ); + return formData; + }, new FormData()), + [ContentType.UrlEncoded]: (input: any) => this.toQueryString(input), + }; + + protected mergeRequestParams( + params1: RequestParams, + params2?: RequestParams, + ): RequestParams { + return { + ...this.baseApiParams, + ...params1, + ...(params2 || {}), + headers: { + ...(this.baseApiParams.headers || {}), + ...(params1.headers || {}), + ...((params2 && params2.headers) || {}), + }, + }; + } + + protected createAbortSignal = ( + cancelToken: CancelToken, + ): AbortSignal | undefined => { + if (this.abortControllers.has(cancelToken)) { + const abortController = this.abortControllers.get(cancelToken); + if (abortController) { + return abortController.signal; + } + return void 0; + } + + const abortController = new AbortController(); + this.abortControllers.set(cancelToken, abortController); + return abortController.signal; + }; + + public abortRequest = (cancelToken: CancelToken) => { + const abortController = this.abortControllers.get(cancelToken); + + if (abortController) { + abortController.abort(); + this.abortControllers.delete(cancelToken); + } + }; + + public request = async ({ + body, + secure, + path, + type, + query, + format, + baseUrl, + cancelToken, + ...params + }: FullRequestParams): Promise> => { + const secureParams = + ((typeof secure === "boolean" ? secure : this.baseApiParams.secure) && + this.securityWorker && + (await this.securityWorker(this.securityData))) || + {}; + const requestParams = this.mergeRequestParams(params, secureParams); + const queryString = query && this.toQueryString(query); + const payloadFormatter = this.contentFormatters[type || ContentType.Json]; + const responseFormat = format || requestParams.format; + + return this.customFetch( + \`\${baseUrl || this.baseUrl || ""}\${path}\${queryString ? \`?\${queryString}\` : ""}\`, + { + ...requestParams, + headers: { + ...(requestParams.headers || {}), + ...(type && type !== ContentType.FormData + ? { "Content-Type": type } + : {}), + }, + signal: + (cancelToken + ? this.createAbortSignal(cancelToken) + : requestParams.signal) || null, + body: + typeof body === "undefined" || body === null + ? null + : payloadFormatter(body), + }, + ).then(async (response) => { + const r = response.clone() as HttpResponse; + r.data = null as unknown as T; + r.error = null as unknown as E; + + const data = !responseFormat + ? r + : await response[responseFormat]() + .then((data) => { + if (r.ok) { + r.data = data; + } else { + r.error = data; + } + return r; + }) + .catch((e) => { + r.error = e; + return r; + }); + + if (cancelToken) { + this.abortControllers.delete(cancelToken); + } + + if (!response.ok) throw data; + return data; + }); + }; +} + +/** + * @title JSDoc Escaping Test API + * @version 1.0.0 + * + * Test API for verifying proper escaping of JSDoc comment characters + */ +export class Api< + SecurityDataType extends unknown, +> extends HttpClient { + information = { + /** + * @description Get service point file of all Nordic countries (SE,FI,DK,NO) from S3 storage. You can download previous service point file upto 7 days from current date. This is equivalent to **\\/information** endpoint with parameters \`countryCode:SE,FI,DK,NO\` and \`context:ALL\` and header \`Accept-Encoding:gzip\`. + * + * @name InformationList + * @summary Get service point file with **\\/ alert('XSS') /** injection attempt + * @request GET:/information + */ + informationList: (params: RequestParams = {}) => + this.request({ + path: \`/information\`, + method: "GET", + format: "json", + ...params, + }), + }; +} +" +`; diff --git a/tests/spec/jsdoc-escaping/basic.test.ts b/tests/spec/jsdoc-escaping/basic.test.ts new file mode 100644 index 00000000..ed070e7d --- /dev/null +++ b/tests/spec/jsdoc-escaping/basic.test.ts @@ -0,0 +1,45 @@ +import * as fs from "node:fs/promises"; +import * as os from "node:os"; +import * as path from "node:path"; +import { afterAll, beforeAll, describe, expect, test } from "vitest"; +import { generateApi } from "../../../src/index.js"; + +describe("jsdoc-escaping", async () => { + let tmpdir = ""; + + beforeAll(async () => { + tmpdir = await fs.mkdtemp(path.join(os.tmpdir(), "swagger-typescript-api")); + }); + + afterAll(async () => { + await fs.rm(tmpdir, { recursive: true }); + }); + + test("should escape JSDoc comment characters in descriptions", async () => { + await generateApi({ + fileName: "schema", + input: path.resolve(import.meta.dirname, "schema.json"), + output: tmpdir, + silent: true, + }); + + const content = await fs.readFile(path.join(tmpdir, "schema.ts"), { + encoding: "utf8", + }); + + // Check that the dangerous patterns are escaped and not executable + expect(content).not.toMatch(/\*\/ alert\(/); // No unescaped code injection + expect(content).not.toMatch(/\*\/ window\./); // No unescaped window manipulation + expect(content).not.toMatch(/\*\/ dangerous content \*\//); // No unescaped dangerous content + + // Check that only necessary escaping is applied + expect(content).toMatch(/\*\\\//); // Should contain escaped */ + expect(content).not.toMatch(/\\\*\//); // Should NOT contain escaped /* sequences + + // Check that alert and window are escaped + expect(content).toMatch(/alert\('XSS'\)/); // Should still contain the content but safely escaped + expect(content).toMatch(/window\.location/); // Should still contain but safely escaped + + expect(content).toMatchSnapshot(); + }); +}); diff --git a/tests/spec/jsdoc-escaping/schema.json b/tests/spec/jsdoc-escaping/schema.json new file mode 100644 index 00000000..ebeb797e --- /dev/null +++ b/tests/spec/jsdoc-escaping/schema.json @@ -0,0 +1,50 @@ +{ + "openapi": "3.0.0", + "info": { + "title": "JSDoc Escaping Test API", + "version": "1.0.0", + "description": "Test API for verifying proper escaping of JSDoc comment characters" + }, + "paths": { + "/information": { + "get": { + "summary": "Get service point file with **/ alert('XSS') /** injection attempt", + "description": "Get service point file of all Nordic countries (SE,FI,DK,NO) from S3 storage. You can download previous service point file upto 7 days from current date. This is equivalent to **/information** endpoint with parameters `countryCode:SE,FI,DK,NO` and `context:ALL` and header `Accept-Encoding:gzip`.", + "responses": { + "200": { + "description": "Success response with **/ console.log('Code injection') /** attempt", + "content": { + "application/json": { + "schema": { + "$ref": "#/components/schemas/Information" + } + } + } + } + } + } + } + }, + "components": { + "schemas": { + "Information": { + "type": "object", + "description": "Information schema with malicious **/ window.location='http://evil.com' /** content", + "properties": { + "id": { + "type": "integer", + "description": "The ID of the information record. Contains **/ dangerous content /** here." + }, + "title": { + "type": "string", + "description": "Title field with */ and /* characters that could break comments" + }, + "content": { + "type": "string", + "description": "Multi-line description\nwith */ characters\nand /* other markers\nthat could break JSDoc comments" + } + } + } + } + } +}