diff --git a/packages/drizzle/src/find/traverseFields.ts b/packages/drizzle/src/find/traverseFields.ts index 9d67f11384b..b7d444d9e87 100644 --- a/packages/drizzle/src/find/traverseFields.ts +++ b/packages/drizzle/src/find/traverseFields.ts @@ -27,6 +27,7 @@ import { getArrayRelationName } from '../utilities/getArrayRelationName.js' import { getNameFromDrizzleTable } from '../utilities/getNameFromDrizzleTable.js' import { jsonAggBuildObject } from '../utilities/json.js' import { rawConstraint } from '../utilities/rawConstraint.js' +import { sanitizePathSegment } from '../utilities/sanitizePathSegment.js' import { InternalBlockTableNameIndex, resolveBlockTableName, @@ -63,6 +64,9 @@ const buildSQLWhere = (where: Where, alias: string) => { const value = where[k][payloadOperator] if (payloadOperator === '$raw') { + if (typeof value !== 'string') { + return undefined + } return sql.raw(value) } @@ -75,7 +79,16 @@ const buildSQLWhere = (where: Where, alias: string) => { payloadOperator = 'isNull' } - return operatorMap[payloadOperator](sql.raw(`"${alias}"."${k.split('.').join('_')}"`), value) + if (!(payloadOperator in operatorMap)) { + return undefined + } + + const sanitizedColumnName = k + .split('.') + .map((s) => sanitizePathSegment(s)) + .join('_') + + return operatorMap[payloadOperator](sql.raw(`"${alias}"."${sanitizedColumnName}"`), value) } } } diff --git a/packages/drizzle/src/postgres/createJSONQuery/index.ts b/packages/drizzle/src/postgres/createJSONQuery/index.ts index cd7940d2d92..b9db2971a86 100644 --- a/packages/drizzle/src/postgres/createJSONQuery/index.ts +++ b/packages/drizzle/src/postgres/createJSONQuery/index.ts @@ -3,6 +3,7 @@ import { APIError } from 'payload' import type { CreateJSONQueryArgs } from '../../types.js' import { SAFE_STRING_REGEX } from '../../utilities/escapeSQLValue.js' +import { sanitizePathSegment } from '../../utilities/sanitizePathSegment.js' const operatorMap: Record = { contains: '~', @@ -43,7 +44,7 @@ export const createJSONQuery = ({ column, operator, pathSegments, value }: Creat const jsonPaths = pathSegments .slice(1) .map((key) => { - return `${key}[*]` + return `${sanitizePathSegment(key)}[*]` }) .join('.') diff --git a/packages/drizzle/src/queries/parseParams.ts b/packages/drizzle/src/queries/parseParams.ts index 01ecb51630e..20824a579c2 100644 --- a/packages/drizzle/src/queries/parseParams.ts +++ b/packages/drizzle/src/queries/parseParams.ts @@ -185,7 +185,11 @@ export function parseParams({ } let formattedValue = val - if (adapter.name === 'sqlite' && operator === 'equals' && !isNaN(val)) { + if ( + adapter.name === 'sqlite' && + operator === 'equals' && + (typeof val === 'number' || typeof val === 'boolean') + ) { formattedValue = val } else if (['in', 'not_in'].includes(operator) && Array.isArray(val)) { formattedValue = `(${val.map((v) => `${escapeSQLValue(v)}`).join(',')})` diff --git a/packages/drizzle/src/sqlite/createJSONQuery/convertPathToJSONTraversal.ts b/packages/drizzle/src/sqlite/createJSONQuery/convertPathToJSONTraversal.ts index d7ec458a314..99ac25cb249 100644 --- a/packages/drizzle/src/sqlite/createJSONQuery/convertPathToJSONTraversal.ts +++ b/packages/drizzle/src/sqlite/createJSONQuery/convertPathToJSONTraversal.ts @@ -1,9 +1,13 @@ +import { sanitizePathSegment } from '../../utilities/sanitizePathSegment.js' + export const convertPathToJSONTraversal = (incomingSegments: string[]): string => { const segments = [...incomingSegments] segments.shift() return segments.reduce((res, segment) => { - const formattedSegment = Number.isNaN(parseInt(segment)) ? `'${segment}'` : segment + const formattedSegment = Number.isNaN(parseInt(segment)) + ? `'${sanitizePathSegment(segment)}'` + : segment return `${res}->>${formattedSegment}` }, '') } diff --git a/packages/drizzle/src/sqlite/createJSONQuery/index.ts b/packages/drizzle/src/sqlite/createJSONQuery/index.ts index a3830b6d7da..e08dbcd4df6 100644 --- a/packages/drizzle/src/sqlite/createJSONQuery/index.ts +++ b/packages/drizzle/src/sqlite/createJSONQuery/index.ts @@ -1,6 +1,7 @@ import type { CreateJSONQueryArgs } from '../../types.js' import { escapeSQLValue } from '../../utilities/escapeSQLValue.js' +import { sanitizePathSegment } from '../../utilities/sanitizePathSegment.js' type FromArrayArgs = { isRoot?: true @@ -20,11 +21,12 @@ const fromArray = ({ value, }: FromArrayArgs) => { const newPathSegments = pathSegments.slice(1) - const alias = `${pathSegments[isRoot ? 0 : 1]}_alias_${newPathSegments.length}` + const sanitizedSegment = sanitizePathSegment(pathSegments[isRoot ? 0 : 1]) + const alias = `${sanitizedSegment}_alias_${newPathSegments.length}` return `EXISTS ( SELECT 1 - FROM json_each(${table}.${pathSegments[0]}) AS ${alias} + FROM json_each(${table}.${sanitizePathSegment(pathSegments[0])}) AS ${alias} WHERE ${createJSONQuery({ operator, pathSegments: newPathSegments, @@ -61,25 +63,25 @@ const createConstraint = ({ if (operator === 'exists') { if (pathSegments.length === 1) { - return `EXISTS (SELECT 1 FROM json_each("${pathSegments[0]}") AS ${newAlias})` + return `EXISTS (SELECT 1 FROM json_each("${sanitizePathSegment(pathSegments[0])}") AS ${newAlias})` } return `EXISTS ( SELECT 1 - FROM json_each(${alias}.value -> '${pathSegments[0]}') AS ${newAlias} - WHERE ${newAlias}.key = '${pathSegments[1]}' + FROM json_each(${alias}.value -> '${sanitizePathSegment(pathSegments[0])}') AS ${newAlias} + WHERE ${newAlias}.key = '${sanitizePathSegment(pathSegments[1])}' )` } if (operator === 'not_exists') { if (pathSegments.length === 1) { - return `NOT EXISTS (SELECT 1 FROM json_each("${pathSegments[0]}") AS ${newAlias})` + return `NOT EXISTS (SELECT 1 FROM json_each("${sanitizePathSegment(pathSegments[0])}") AS ${newAlias})` } return `NOT EXISTS ( SELECT 1 - FROM json_each(${alias}.value -> '${pathSegments[0]}') AS ${newAlias} - WHERE ${newAlias}.key = '${pathSegments[1]}' + FROM json_each(${alias}.value -> '${sanitizePathSegment(pathSegments[0])}') AS ${newAlias} + WHERE ${newAlias}.key = '${sanitizePathSegment(pathSegments[1])}' )` } @@ -96,13 +98,13 @@ const createConstraint = ({ } if (pathSegments.length === 1) { - return `EXISTS (SELECT 1 FROM json_each("${pathSegments[0]}") AS ${newAlias} WHERE ${newAlias}.value ${formattedOperator} '${formattedValue}')` + return `EXISTS (SELECT 1 FROM json_each("${sanitizePathSegment(pathSegments[0])}") AS ${newAlias} WHERE ${newAlias}.value ${formattedOperator} '${formattedValue}')` } return `EXISTS ( SELECT 1 - FROM json_each(${alias}.value -> '${pathSegments[0]}') AS ${newAlias} - WHERE COALESCE(${newAlias}.value ->> '${pathSegments[1]}', '') ${formattedOperator} '${formattedValue}' + FROM json_each(${alias}.value -> '${sanitizePathSegment(pathSegments[0])}') AS ${newAlias} + WHERE COALESCE(${newAlias}.value ->> '${sanitizePathSegment(pathSegments[1])}', '') ${formattedOperator} '${formattedValue}' )` } diff --git a/packages/drizzle/src/utilities/json.ts b/packages/drizzle/src/utilities/json.ts index 3864e601366..fe0e1931042 100644 --- a/packages/drizzle/src/utilities/json.ts +++ b/packages/drizzle/src/utilities/json.ts @@ -1,9 +1,12 @@ import type { Column, SQL } from 'drizzle-orm' import { sql } from 'drizzle-orm' +import { APIError } from 'payload' import type { DrizzleAdapter } from '../types.js' +const SAFE_KEY_REGEX = /^\w+$/ + export function jsonAgg(adapter: DrizzleAdapter, expression: SQL) { if (adapter.name === 'sqlite') { return sql`coalesce(json_group_array(${expression}), '[]')` @@ -13,7 +16,7 @@ export function jsonAgg(adapter: DrizzleAdapter, expression: SQL) { } /** - * @param shape Potential for SQL injections, so you shouldn't allow user-specified key names + * @param shape Keys are interpolated into raw SQL — only use trusted, internally-generated key names */ export function jsonBuildObject>( adapter: DrizzleAdapter, @@ -22,6 +25,12 @@ export function jsonBuildObject>( const chunks: SQL[] = [] Object.entries(shape).forEach(([key, value]) => { + if (!SAFE_KEY_REGEX.test(key)) { + throw new APIError( + 'Unsafe key passed to jsonBuildObject. Only alphanumeric characters and underscores are allowed.', + 500, + ) + } if (chunks.length > 0) { chunks.push(sql.raw(',')) } diff --git a/packages/drizzle/src/utilities/sanitizePathSegment.ts b/packages/drizzle/src/utilities/sanitizePathSegment.ts new file mode 100644 index 00000000000..94940e1c0c5 --- /dev/null +++ b/packages/drizzle/src/utilities/sanitizePathSegment.ts @@ -0,0 +1,18 @@ +import { APIError } from 'payload' + +/** + * Validates that a path segment contains only allowed characters (word characters: [a-zA-Z0-9_]). + * + * @throws {APIError} if the segment contains characters outside /^[\w]+$/ + */ +const SAFE_PATH_SEGMENT_REGEX = /^\w+$/ + +export const sanitizePathSegment = (segment: string): string => { + if (!SAFE_PATH_SEGMENT_REGEX.test(segment)) { + throw new APIError( + 'Invalid path segment. Only alphanumeric characters and underscores are permitted.', + 400, + ) + } + return segment +} diff --git a/packages/next/src/views/Account/ResetPreferences/index.tsx b/packages/next/src/views/Account/ResetPreferences/index.tsx index aad2bf03049..dd427bd2f77 100644 --- a/packages/next/src/views/Account/ResetPreferences/index.tsx +++ b/packages/next/src/views/Account/ResetPreferences/index.tsx @@ -35,10 +35,8 @@ export const ResetPreferences: React.FC<{ { depth: 0, where: { - user: { - id: { - equals: user.id, - }, + 'user.value': { + equals: user.id, }, }, }, diff --git a/packages/payload/src/database/getLocalizedPaths.ts b/packages/payload/src/database/getLocalizedPaths.ts index f519bda11b8..c486fcc2422 100644 --- a/packages/payload/src/database/getLocalizedPaths.ts +++ b/packages/payload/src/database/getLocalizedPaths.ts @@ -7,6 +7,7 @@ import { type FlattenedField, } from '../fields/config/types.js' import { APIError, type Payload, type SanitizedCollectionConfig } from '../index.js' +import { SAFE_FIELD_PATH_REGEX } from '../types/constants.js' export function getLocalizedPaths({ collectionSlug, @@ -209,7 +210,7 @@ export function getLocalizedPaths({ case 'richText': { const upcomingSegments = pathSegments.slice(i + 1).join('.') pathSegments.forEach((path) => { - if (!/^\w+(?:\.\w+)*$/.test(path)) { + if (!SAFE_FIELD_PATH_REGEX.test(path)) { lastIncompletePath.invalid = true } }) diff --git a/packages/payload/src/database/queryValidation/validateQueryPaths.ts b/packages/payload/src/database/queryValidation/validateQueryPaths.ts index 0441108ab19..389f53ebae9 100644 --- a/packages/payload/src/database/queryValidation/validateQueryPaths.ts +++ b/packages/payload/src/database/queryValidation/validateQueryPaths.ts @@ -101,7 +101,7 @@ export async function validateQueryPaths({ versionFields, }), ) - } else if (typeof val !== 'object' || val === null) { + } else { errors.push({ path: `${path}.${operator}` }) } } diff --git a/packages/payload/src/database/queryValidation/validateSearchParams.ts b/packages/payload/src/database/queryValidation/validateSearchParams.ts index 9d1075b1a6f..f75683b1df7 100644 --- a/packages/payload/src/database/queryValidation/validateSearchParams.ts +++ b/packages/payload/src/database/queryValidation/validateSearchParams.ts @@ -5,6 +5,7 @@ import type { PayloadRequest, WhereField } from '../../types/index.js' import type { EntityPolicies, PathToQuery } from './types.js' import { fieldAffectsData } from '../../fields/config/types.js' +import { SAFE_FIELD_PATH_REGEX } from '../../types/constants.js' import { getEntityPermissions } from '../../utilities/getEntityPermissions/getEntityPermissions.js' import { isolateObjectProperty } from '../../utilities/isolateObjectProperty.js' import { getLocalizedPaths } from '../getLocalizedPaths.js' @@ -99,7 +100,7 @@ export async function validateSearchParam({ promises.push( ...paths.map(async ({ collectionSlug, field, invalid, path }, i) => { if (invalid) { - if (!polymorphicJoin) { + if (!polymorphicJoin || !SAFE_FIELD_PATH_REGEX.test(incomingPath)) { errors.push({ path }) } diff --git a/packages/payload/src/exports/shared.ts b/packages/payload/src/exports/shared.ts index ff52a5adda6..c536591fc37 100644 --- a/packages/payload/src/exports/shared.ts +++ b/packages/payload/src/exports/shared.ts @@ -136,6 +136,8 @@ export { reduceFieldsToValues } from '../utilities/reduceFieldsToValues.js' export { sanitizeFilename } from '../utilities/sanitizeFilename.js' +export { sanitizeUrl } from '../utilities/sanitizeUrl.js' + export { sanitizeUserDataForEmail } from '../utilities/sanitizeUserDataForEmail.js' export { setsAreEqual } from '../utilities/setsAreEqual.js' diff --git a/packages/payload/src/types/constants.ts b/packages/payload/src/types/constants.ts index 7cbbaeb634b..019a2f110df 100644 --- a/packages/payload/src/types/constants.ts +++ b/packages/payload/src/types/constants.ts @@ -20,3 +20,9 @@ export const validOperators = [ export type Operator = (typeof validOperators)[number] export const validOperatorSet = new Set(validOperators) + +/** + * Matches a dot-separated path where each segment is a word character (a-zA-Z0-9_). + * Used to validate field paths before they are processed by query builders. + */ +export const SAFE_FIELD_PATH_REGEX = /^\w+(?:\.\w+)*$/ diff --git a/packages/payload/src/uploads/endpoints/getFile.ts b/packages/payload/src/uploads/endpoints/getFile.ts index 2d8eef829e8..6e4281f546d 100644 --- a/packages/payload/src/uploads/endpoints/getFile.ts +++ b/packages/payload/src/uploads/endpoints/getFile.ts @@ -60,8 +60,16 @@ export const getFileHandler: PayloadHandler = async (req) => { } } + // Local filesystem fallback — cloud storage handlers return a Response above + // and have their own filename validation via sanitizeFilename. const fileDir = collection.config.upload?.staticDir || collection.config.slug - const filePath = path.resolve(`${fileDir}/${filename}`) + const resolvedDir = path.resolve(fileDir) + const filePath = path.resolve(resolvedDir, filename) + + if (!filePath.startsWith(resolvedDir + path.sep)) { + throw new APIError('Invalid filename.', httpStatus.BAD_REQUEST) + } + let stats: Stats try { diff --git a/packages/payload/src/uploads/endpoints/getFileFromURL.ts b/packages/payload/src/uploads/endpoints/getFileFromURL.ts index 9efd2bb1acb..04cad6ce788 100644 --- a/packages/payload/src/uploads/endpoints/getFileFromURL.ts +++ b/packages/payload/src/uploads/endpoints/getFileFromURL.ts @@ -5,6 +5,8 @@ import { APIError } from '../../errors/APIError.js' import { Forbidden } from '../../errors/Forbidden.js' import { getRequestCollectionWithID } from '../../utilities/getRequestEntity.js' import { isURLAllowed } from '../../utilities/isURLAllowed.js' +import { sanitizeFilename } from '../../utilities/sanitizeFilename.js' +import { safeFetch } from '../safeFetch.js' // If doc id is provided, it means we are updating the doc // /:collectionSlug/paste-url/:doc-id?src=:fileUrl @@ -38,48 +40,89 @@ export const getFileFromURLHandler: PayloadHandler = async (req) => { throw new Forbidden(req.t) } } - try { - if (!req.url) { - throw new APIError('Request URL is missing.', 400) - } - const { searchParams } = new URL(req.url) - const src = searchParams.get('src') + if (!req.url) { + throw new APIError('Request URL is missing.', 400) + } - if (!src || typeof src !== 'string') { - throw new APIError('A valid URL string is required.', 400) - } + const { searchParams } = new URL(req.url) + const src = searchParams.get('src') - const validatedUrl = new URL(src) + if (!src || typeof src !== 'string') { + throw new APIError('A valid URL string is required.', 400) + } - if (!isURLAllowed(validatedUrl.href, config.upload.pasteURL.allowList)) { - throw new APIError(`The provided URL (${validatedUrl.href}) is not allowed.`, 400) - } + const hasAllowList = + typeof config.upload.pasteURL === 'object' && Array.isArray(config.upload.pasteURL.allowList) - // Fetch the file with no compression - const response = await fetch(validatedUrl.href, { - headers: { - 'Accept-Encoding': 'identity', - }, - }) + let fileURL: string + try { + fileURL = new URL(src).href + } catch { + throw new APIError('A valid URL string is required.', 400) + } - if (!response.ok) { - throw new APIError(`Failed to fetch file from ${validatedUrl.href}`, response.status) + if (hasAllowList && !isURLAllowed(fileURL, config.upload.pasteURL.allowList)) { + throw new APIError('The provided URL is not allowed.', 400) + } + + let redirectCount = 0 + const maxRedirects = 3 + let response!: Response + + while (true) { + if (hasAllowList && isURLAllowed(fileURL, config.upload.pasteURL.allowList)) { + // Allow-listed URLs bypass SSRF filtering (e.g. internal/localhost CDNs) + response = await fetch(fileURL, { + headers: { 'Accept-Encoding': 'identity' }, + redirect: 'manual', + signal: AbortSignal.timeout(30_000), + }) + } else { + response = await safeFetch(fileURL, { + headers: { + 'Accept-Encoding': 'identity', + }, + signal: AbortSignal.timeout(30_000), + }) + } + + if (response.status >= 300 && response.status < 400) { + redirectCount++ + if (redirectCount > maxRedirects) { + throw new APIError('Too many redirects.', 403) + } + const location = response.headers.get('location') + if (location) { + fileURL = new URL(location, fileURL).href + if (hasAllowList && !isURLAllowed(fileURL, config.upload.pasteURL.allowList)) { + throw new APIError('The provided URL is not allowed.', 400) + } + continue + } } - const decodedFileName = decodeURIComponent(validatedUrl.pathname.split('/').pop() || '') - - return new Response(response.body, { - headers: { - 'Content-Disposition': `attachment; filename="${decodedFileName}"`, - 'Content-Length': response.headers.get('content-length') || '', - 'Content-Type': response.headers.get('content-type') || 'application/octet-stream', - }, - }) - } catch (err) { - throw new APIError( - `Error fetching file: ${err instanceof Error ? err.message : 'Unknown error'}`, - 500, - ) + break } + + if (!response.ok) { + throw new APIError('Failed to fetch the file from the provided URL.', response.status) + } + + const rawFileName = decodeURIComponent(new URL(fileURL).pathname.split('/').pop() || '') + const safeFileName = sanitizeFilename(rawFileName) + const encodedFileName = encodeURIComponent(safeFileName).replace( + /['()]/g, + (c) => `%${c.charCodeAt(0).toString(16).toUpperCase()}`, + ) + // Strip quotes, backslashes, and control chars from the ASCII fallback + const asciiFileName = safeFileName.replace(/["\\\r\n]/g, '_') + + return new Response(response.body, { + headers: { + 'Content-Disposition': `attachment; filename="${asciiFileName}"; filename*=UTF-8''${encodedFileName}`, + 'Content-Length': response.headers.get('content-length') || '', + 'Content-Type': response.headers.get('content-type') || 'application/octet-stream', + }, + }) } diff --git a/packages/payload/src/uploads/getExternalFile.ts b/packages/payload/src/uploads/getExternalFile.ts index 1d9172db437..b518b249bbb 100644 --- a/packages/payload/src/uploads/getExternalFile.ts +++ b/packages/payload/src/uploads/getExternalFile.ts @@ -77,6 +77,13 @@ export const getExternalFile = async ({ data, req, uploadConfig }: Args): Promis const location = res.headers.get('location') if (location) { fileURL = new URL(location, fileURL).toString() + if ( + uploadConfig.pasteURL && + uploadConfig.pasteURL.allowList && + !isURLAllowed(fileURL, uploadConfig.pasteURL.allowList) + ) { + throw new APIError('Redirect target is not allowed.', 400) + } continue } } diff --git a/packages/payload/src/uploads/safeFetch.ts b/packages/payload/src/uploads/safeFetch.ts index e2c2b35a18a..1d60a479d9c 100644 --- a/packages/payload/src/uploads/safeFetch.ts +++ b/packages/payload/src/uploads/safeFetch.ts @@ -64,7 +64,7 @@ const safeDispatcher = new Agent({ * - Validates domain names by resolving them to IP addresses and checking if they're safe. * - Undici was used because it supported interceptors as well as "credentials: include". Native fetch */ -export const safeFetch = async (...args: Parameters) => { +export const safeFetch = async (...args: Parameters): Promise => { const [unverifiedUrl, options] = args try { @@ -82,11 +82,11 @@ export const safeFetch = async (...args: Parameters) => { throw new Error(`Blocked unsafe attempt to ${hostname}`) } } - return await undiciFetch(url, { + return (await undiciFetch(url, { ...options, dispatcher: safeDispatcher, redirect: 'manual', // Prevent automatic redirects - }) + })) as unknown as Response } catch (error) { if (error instanceof Error) { if (error.cause instanceof Error && error.cause.message.includes('unsafe')) { diff --git a/packages/payload/src/utilities/sanitizeUrl.ts b/packages/payload/src/utilities/sanitizeUrl.ts new file mode 100644 index 00000000000..cbe82beb4f1 --- /dev/null +++ b/packages/payload/src/utilities/sanitizeUrl.ts @@ -0,0 +1,38 @@ +/** + * Sanitizes a URL to ensure only allowed protocols are used. + * Allows: http, https, mailto, tel, relative paths, and fragment (#) URLs. + * Returns '#' for any URL with a disallowed protocol (e.g. javascript:, data:). + */ +export function sanitizeUrl(url: string): string { + if (!url) { + return '' + } + + const trimmed = url.trim() + + // eslint-disable-next-line no-control-regex + const cleaned = trimmed.replace(/[\x00-\x1f\x7f]/g, '') + + if (cleaned.startsWith('#')) { + return cleaned + } + + if (cleaned.startsWith('/') || cleaned.startsWith('./') || cleaned.startsWith('../')) { + return cleaned + } + + const protocolMatch = cleaned.match(/^([a-z][a-z0-9+\-.]*):(?=.)/i) + if (protocolMatch) { + const protocol = protocolMatch[1]!.toLowerCase() + if ( + protocol !== 'http' && + protocol !== 'https' && + protocol !== 'mailto' && + protocol !== 'tel' + ) { + return '#' + } + } + + return cleaned +} diff --git a/packages/plugin-cloud-storage/src/utilities/getFilePrefix.ts b/packages/plugin-cloud-storage/src/utilities/getFilePrefix.ts index e823a6d8614..e7fe87f56f3 100644 --- a/packages/plugin-cloud-storage/src/utilities/getFilePrefix.ts +++ b/packages/plugin-cloud-storage/src/utilities/getFilePrefix.ts @@ -1,5 +1,21 @@ import type { CollectionConfig, PayloadRequest, UploadConfig } from 'payload' +/** + * Normalizes a storage prefix to ensure only valid path segments are included. + */ +function sanitizePrefix(prefix: string): string { + return ( + prefix + .replace(/\\/g, '/') + .split('/') + .filter((segment) => segment !== '..' && segment !== '.') + .join('/') + .replace(/^\/+/, '') + // eslint-disable-next-line no-control-regex + .replace(/[\x00-\x1f\x80-\x9f]/g, '') + ) +} + export async function getFilePrefix({ clientUploadContext, collection, @@ -18,7 +34,7 @@ export async function getFilePrefix({ 'prefix' in clientUploadContext && typeof clientUploadContext.prefix === 'string' ) { - return clientUploadContext.prefix + return sanitizePrefix(clientUploadContext.prefix) } const imageSizes = (collection?.upload as UploadConfig)?.imageSizes || [] @@ -41,5 +57,5 @@ export async function getFilePrefix({ }, }) const prefix = files?.docs?.[0]?.prefix - return prefix ? (prefix as string) : '' + return prefix ? sanitizePrefix(prefix as string) : '' } diff --git a/packages/plugin-form-builder/src/utilities/keyValuePairToHtmlTable.ts b/packages/plugin-form-builder/src/utilities/keyValuePairToHtmlTable.ts index 940df29c5cc..e9ce49b1f1b 100644 --- a/packages/plugin-form-builder/src/utilities/keyValuePairToHtmlTable.ts +++ b/packages/plugin-form-builder/src/utilities/keyValuePairToHtmlTable.ts @@ -1,8 +1,10 @@ +import escapeHTML from 'escape-html' + export function keyValuePairToHtmlTable(obj: { [key: string]: string }): string { let htmlTable = '' for (const [key, value] of Object.entries(obj)) { - htmlTable += `` + htmlTable += `` } htmlTable += '
${key}${value}
${escapeHTML(key)}${escapeHTML(value)}
' diff --git a/packages/plugin-form-builder/src/utilities/lexical/converters/heading.ts b/packages/plugin-form-builder/src/utilities/lexical/converters/heading.ts index c7b1a8a6e85..9c493f44290 100644 --- a/packages/plugin-form-builder/src/utilities/lexical/converters/heading.ts +++ b/packages/plugin-form-builder/src/utilities/lexical/converters/heading.ts @@ -2,6 +2,9 @@ import type { HTMLConverter } from '../types.js' import { convertLexicalNodesToHTML } from '../serializeLexical.js' +const ALLOWED_HEADING_TAGS = new Set(['h1', 'h2', 'h3', 'h4', 'h5', 'h6']) +const ALLOWED_TEXT_ALIGN = new Set(['center', 'end', 'justify', 'left', 'right', 'start']) + export const HeadingHTMLConverter: HTMLConverter = { async converter({ converters, node, parent, submissionData }) { const childrenText = await convertLexicalNodesToHTML({ @@ -13,13 +16,14 @@ export const HeadingHTMLConverter: HTMLConverter = { }, submissionData, }) + const tag = ALLOWED_HEADING_TAGS.has(node?.tag) ? node.tag : 'h1' const style = [ - node.format ? `text-align: ${node.format};` : '', + node.format && ALLOWED_TEXT_ALIGN.has(node.format) ? `text-align: ${node.format};` : '', node.indent > 0 ? `padding-inline-start: ${node.indent * 40}px;` : '', ] .filter(Boolean) .join(' ') - return `<${node?.tag}${style ? ` style='${style}'` : ''}>${childrenText}` + return `<${tag}${style ? ` style='${style}'` : ''}>${childrenText}` }, nodeTypes: ['heading'], } diff --git a/packages/plugin-form-builder/src/utilities/lexical/converters/link.ts b/packages/plugin-form-builder/src/utilities/lexical/converters/link.ts index 829f1a99adb..94005a42fc5 100644 --- a/packages/plugin-form-builder/src/utilities/lexical/converters/link.ts +++ b/packages/plugin-form-builder/src/utilities/lexical/converters/link.ts @@ -1,3 +1,6 @@ +import escapeHTML from 'escape-html' +import { sanitizeUrl } from 'payload/shared' + import type { HTMLConverter } from '../types.js' import { replaceDoubleCurlys } from '../../replaceDoubleCurlys.js' @@ -16,12 +19,17 @@ export const LinkHTMLConverter: HTMLConverter = { }) let href: string = - node.fields.linkType === 'custom' ? node.fields.url : node.fields.doc?.value?.id + node.fields.linkType === 'custom' + ? (node.fields.url ?? '') + : (node.fields.doc?.value?.id ?? '') if (submissionData) { href = replaceDoubleCurlys(href, submissionData) } - return `${childrenText}` + + const safeHref = escapeHTML(sanitizeUrl(href)) + + return `${childrenText}` }, nodeTypes: ['link'], } diff --git a/packages/plugin-form-builder/src/utilities/lexical/converters/list.ts b/packages/plugin-form-builder/src/utilities/lexical/converters/list.ts index 78458ff0f12..103ffd2779e 100644 --- a/packages/plugin-form-builder/src/utilities/lexical/converters/list.ts +++ b/packages/plugin-form-builder/src/utilities/lexical/converters/list.ts @@ -2,6 +2,9 @@ import type { HTMLConverter } from '../types.js' import { convertLexicalNodesToHTML } from '../serializeLexical.js' +const ALLOWED_LIST_TAGS = new Set(['ol', 'ul']) +const ALLOWED_LIST_TYPES = new Set(['bullet', 'check', 'number']) + export const ListHTMLConverter: HTMLConverter = { converter: async ({ converters, node, parent, submissionData }) => { const childrenText = await convertLexicalNodesToHTML({ @@ -14,7 +17,10 @@ export const ListHTMLConverter: HTMLConverter = { submissionData, }) - return `<${node?.tag} class="${node?.listType}">${childrenText}` + const tag = ALLOWED_LIST_TAGS.has(node?.tag) ? node.tag : 'ul' + const listType = ALLOWED_LIST_TYPES.has(node?.listType) ? node.listType : 'bullet' + + return `<${tag} class="${listType}">${childrenText}` }, nodeTypes: ['list'], } @@ -30,20 +36,22 @@ export const ListItemHTMLConverter: HTMLConverter = { }, }) + const safeValue = Number.isFinite(Number(node?.value)) ? Number(node.value) : 1 + if ('listType' in parent && parent?.listType === 'check') { - return `` } else { - return `
  • ${childrenText}
    • ` + return `
  • ${childrenText}
    • ` } }, nodeTypes: ['listitem'], diff --git a/packages/plugin-form-builder/src/utilities/lexical/converters/quote.ts b/packages/plugin-form-builder/src/utilities/lexical/converters/quote.ts index af63bfaddb1..c136a434baf 100644 --- a/packages/plugin-form-builder/src/utilities/lexical/converters/quote.ts +++ b/packages/plugin-form-builder/src/utilities/lexical/converters/quote.ts @@ -2,6 +2,8 @@ import type { HTMLConverter } from '../types.js' import { convertLexicalNodesToHTML } from '../serializeLexical.js' +const ALLOWED_TEXT_ALIGN = new Set(['center', 'end', 'justify', 'left', 'right', 'start']) + export const QuoteHTMLConverter: HTMLConverter = { async converter({ converters, node, parent, submissionData }) { const childrenText = await convertLexicalNodesToHTML({ @@ -14,7 +16,7 @@ export const QuoteHTMLConverter: HTMLConverter = { submissionData, }) const style = [ - node.format ? `text-align: ${node.format};` : '', + node.format && ALLOWED_TEXT_ALIGN.has(node.format) ? `text-align: ${node.format};` : '', node.indent > 0 ? `padding-inline-start: ${node.indent * 40}px;` : '', ] .filter(Boolean) diff --git a/packages/plugin-form-builder/src/utilities/lexical/converters/text.ts b/packages/plugin-form-builder/src/utilities/lexical/converters/text.ts index 2c8b13518c3..c9f3affb869 100644 --- a/packages/plugin-form-builder/src/utilities/lexical/converters/text.ts +++ b/packages/plugin-form-builder/src/utilities/lexical/converters/text.ts @@ -1,3 +1,5 @@ +import escapeHTML from 'escape-html' + import type { HTMLConverter } from '../types.js' import { replaceDoubleCurlys } from '../../replaceDoubleCurlys.js' @@ -5,7 +7,7 @@ import { NodeFormat } from '../nodeFormat.js' export const TextHTMLConverter: HTMLConverter = { converter({ node, submissionData }) { - let text = node.text + let text = escapeHTML(node.text) if (submissionData) { text = replaceDoubleCurlys(text, submissionData) diff --git a/packages/plugin-form-builder/src/utilities/replaceDoubleCurlys.ts b/packages/plugin-form-builder/src/utilities/replaceDoubleCurlys.ts index 2d6d6691c8b..aad9282086d 100644 --- a/packages/plugin-form-builder/src/utilities/replaceDoubleCurlys.ts +++ b/packages/plugin-form-builder/src/utilities/replaceDoubleCurlys.ts @@ -1,3 +1,5 @@ +import escapeHTML from 'escape-html' + import { keyValuePairToHtmlTable } from './keyValuePairToHtmlTable.js' interface EmailVariable { @@ -13,7 +15,9 @@ export const replaceDoubleCurlys = (str: string, variables?: EmailVariables): st return str.replace(regex, (_, variable: string) => { if (variable.includes('*')) { if (variable === '*') { - return variables.map(({ field, value }) => `${field} : ${value}`).join('
    ') + return variables + .map(({ field, value }) => `${escapeHTML(field)} : ${escapeHTML(value)}`) + .join('
    ') } else if (variable === '*:table') { return keyValuePairToHtmlTable( variables.reduce>((acc, { field, value }) => { @@ -27,7 +31,7 @@ export const replaceDoubleCurlys = (str: string, variables?: EmailVariables): st return variable === fieldName }) if (foundVariable) { - return foundVariable.value + return escapeHTML(foundVariable.value) } } diff --git a/packages/plugin-form-builder/src/utilities/slate/serializeSlate.ts b/packages/plugin-form-builder/src/utilities/slate/serializeSlate.ts index ea5c16bb4e1..3a8d5264589 100644 --- a/packages/plugin-form-builder/src/utilities/slate/serializeSlate.ts +++ b/packages/plugin-form-builder/src/utilities/slate/serializeSlate.ts @@ -1,4 +1,5 @@ import escapeHTML from 'escape-html' +import { sanitizeUrl } from 'payload/shared' import { replaceDoubleCurlys } from '../replaceDoubleCurlys.js' @@ -20,9 +21,11 @@ export const serializeSlate = (children?: Node[], submissionData?: any): string children ?.map((node: Node) => { if (isTextNode(node)) { - let text = node.text.includes('{{*') - ? replaceDoubleCurlys(node.text, submissionData) - : `${escapeHTML(replaceDoubleCurlys(node.text, submissionData))}` + let text = escapeHTML(node.text) + if (submissionData) { + text = replaceDoubleCurlys(text, submissionData) + } + text = `${text}` if (node.bold) { text = ` @@ -104,12 +107,17 @@ export const serializeSlate = (children?: Node[], submissionData?: any): string ${serializeSlate(node.children, submissionData)} ` - case 'link': + case 'link': { + let href = escapeHTML(sanitizeUrl(node.url ?? '')) + if (submissionData) { + href = replaceDoubleCurlys(href, submissionData) + } return ` - + ${serializeSlate(node.children, submissionData)} ` + } case 'ol': return `
      diff --git a/packages/richtext-lexical/src/features/converters/lexicalToHtml/async/converters/heading.ts b/packages/richtext-lexical/src/features/converters/lexicalToHtml/async/converters/heading.ts index 26465d1baca..f0fcc8d0b49 100644 --- a/packages/richtext-lexical/src/features/converters/lexicalToHtml/async/converters/heading.ts +++ b/packages/richtext-lexical/src/features/converters/lexicalToHtml/async/converters/heading.ts @@ -1,6 +1,8 @@ import type { SerializedHeadingNode } from '../../../../../nodeTypes.js' import type { HTMLConvertersAsync } from '../types.js' +const ALLOWED_HEADING_TAGS = new Set(['h1', 'h2', 'h3', 'h4', 'h5', 'h6']) + export const HeadingHTMLConverterAsync: HTMLConvertersAsync = { heading: async ({ node, nodesToHTML, providedStyleTag }) => { const children = ( @@ -9,6 +11,8 @@ export const HeadingHTMLConverterAsync: HTMLConvertersAsync${children}` + const tag = ALLOWED_HEADING_TAGS.has(node.tag) ? node.tag : 'h1' + + return `<${tag}${providedStyleTag}>${children}` }, } diff --git a/packages/richtext-lexical/src/features/converters/lexicalToHtml/async/converters/link.ts b/packages/richtext-lexical/src/features/converters/lexicalToHtml/async/converters/link.ts index c2d3851fc49..584795d2caa 100644 --- a/packages/richtext-lexical/src/features/converters/lexicalToHtml/async/converters/link.ts +++ b/packages/richtext-lexical/src/features/converters/lexicalToHtml/async/converters/link.ts @@ -1,3 +1,6 @@ +import escapeHTML from 'escape-html' +import { sanitizeUrl } from 'payload/shared' + import type { SerializedAutoLinkNode, SerializedLinkNode } from '../../../../../nodeTypes.js' import type { HTMLConvertersAsync, HTMLPopulateFn } from '../types.js' @@ -16,7 +19,9 @@ export const LinkHTMLConverterAsync: (args: { }) ).join('') - return `${children}` + const href = escapeHTML(sanitizeUrl(node.fields.url ?? '')) + + return `${children}` }, link: async ({ node, nodesToHTML, populate, providedStyleTag }) => { const children = ( @@ -30,6 +35,7 @@ export const LinkHTMLConverterAsync: (args: { if (internalDocToHref) { href = await internalDocToHref({ linkNode: node, populate }) } else { + // eslint-disable-next-line no-console console.error( 'Lexical => HTML converter: Link converter: found internal link, but internalDocToHref is not provided', ) @@ -37,6 +43,8 @@ export const LinkHTMLConverterAsync: (args: { } } - return `${children}` + const safeHref = escapeHTML(sanitizeUrl(href)) + + return `${children}` }, }) diff --git a/packages/richtext-lexical/src/features/converters/lexicalToHtml/async/converters/list.ts b/packages/richtext-lexical/src/features/converters/lexicalToHtml/async/converters/list.ts index 57119d4540e..b441dd5d3eb 100644 --- a/packages/richtext-lexical/src/features/converters/lexicalToHtml/async/converters/list.ts +++ b/packages/richtext-lexical/src/features/converters/lexicalToHtml/async/converters/list.ts @@ -3,6 +3,9 @@ import { v4 as uuidv4 } from 'uuid' import type { SerializedListItemNode, SerializedListNode } from '../../../../../nodeTypes.js' import type { HTMLConvertersAsync } from '../types.js' +const ALLOWED_LIST_TAGS = new Set(['ol', 'ul']) +const ALLOWED_LIST_TYPES = new Set(['bullet', 'check', 'number']) + export const ListHTMLConverterAsync: HTMLConvertersAsync< SerializedListItemNode | SerializedListNode > = { @@ -13,7 +16,10 @@ export const ListHTMLConverterAsync: HTMLConvertersAsync< }) ).join('') - return `<${node.tag}${providedStyleTag} class="list-${node.listType}">${children}` + const tag = ALLOWED_LIST_TAGS.has(node.tag) ? node.tag : 'ul' + const listType = ALLOWED_LIST_TYPES.has(node.listType) ? node.listType : 'bullet' + + return `<${tag}${providedStyleTag} class="list-${listType}">${children}` }, listitem: async ({ node, nodesToHTML, parent, providedCSSString }) => { const hasSubLists = node.children.some((child) => child.type === 'list') diff --git a/packages/richtext-lexical/src/features/converters/lexicalToHtml/async/converters/table.ts b/packages/richtext-lexical/src/features/converters/lexicalToHtml/async/converters/table.ts index 5a412e44d12..b7210b17314 100644 --- a/packages/richtext-lexical/src/features/converters/lexicalToHtml/async/converters/table.ts +++ b/packages/richtext-lexical/src/features/converters/lexicalToHtml/async/converters/table.ts @@ -5,6 +5,8 @@ import type { } from '../../../../../nodeTypes.js' import type { HTMLConvertersAsync } from '../types.js' +import { isSafeCssColor } from '../../shared/cssColors.js' + export const TableHTMLConverterAsync: HTMLConvertersAsync< SerializedTableCellNode | SerializedTableNode | SerializedTableRowNode > = { @@ -33,7 +35,7 @@ export const TableHTMLConverterAsync: HTMLConvertersAsync< const headerStateClass = `lexical-table-cell-header-${node.headerState}` let style = 'border: 1px solid #ccc; padding: 8px;' + providedCSSString - if (node.backgroundColor) { + if (node.backgroundColor && isSafeCssColor(node.backgroundColor)) { style += ` background-color: ${node.backgroundColor};` } diff --git a/packages/richtext-lexical/src/features/converters/lexicalToHtml/async/converters/text.ts b/packages/richtext-lexical/src/features/converters/lexicalToHtml/async/converters/text.ts index 1ecfd5a9002..1f60da4a496 100644 --- a/packages/richtext-lexical/src/features/converters/lexicalToHtml/async/converters/text.ts +++ b/packages/richtext-lexical/src/features/converters/lexicalToHtml/async/converters/text.ts @@ -1,3 +1,5 @@ +import escapeHTML from 'escape-html' + import type { SerializedTextNode } from '../../../../../nodeTypes.js' import type { HTMLConvertersAsync } from '../types.js' @@ -5,7 +7,7 @@ import { NodeFormat } from '../../../../../lexical/utils/nodeFormat.js' export const TextHTMLConverterAsync: HTMLConvertersAsync = { text: ({ node }) => { - let text = node.text + let text = escapeHTML(node.text) if (node.format & NodeFormat.IS_BOLD) { text = `${text}` diff --git a/packages/richtext-lexical/src/features/converters/lexicalToHtml/async/converters/upload.ts b/packages/richtext-lexical/src/features/converters/lexicalToHtml/async/converters/upload.ts index f7aec617011..288efdda437 100644 --- a/packages/richtext-lexical/src/features/converters/lexicalToHtml/async/converters/upload.ts +++ b/packages/richtext-lexical/src/features/converters/lexicalToHtml/async/converters/upload.ts @@ -1,5 +1,7 @@ import type { FileData, FileSizeImproved, TypeWithID } from 'payload' +import escapeHTML from 'escape-html' + import type { SerializedUploadNode } from '../../../../../nodeTypes.js' import type { UploadDataImproved } from '../../../../upload/server/nodes/UploadNode.js' import type { HTMLConvertersAsync } from '../types.js' @@ -27,13 +29,15 @@ export const UploadHTMLConverterAsync: HTMLConvertersAsync return '' } - const alt = (node.fields?.alt as string) || (uploadDoc as { alt?: string })?.alt || '' + const alt = escapeHTML( + (node.fields?.alt as string) || (uploadDoc as { alt?: string })?.alt || '', + ) - const url = uploadDoc.url + const url = escapeHTML(uploadDoc.url ?? '') // 1) If upload is NOT an image, return a link if (!uploadDoc.mimeType.startsWith('image')) { - return `${uploadDoc.filename}` + return `${escapeHTML(uploadDoc.filename ?? '')}` } // 2) If image has no different sizes, return a simple @@ -41,9 +45,9 @@ export const UploadHTMLConverterAsync: HTMLConvertersAsync return ` ` } @@ -68,9 +72,9 @@ export const UploadHTMLConverterAsync: HTMLConvertersAsync pictureHTML += ` ` } @@ -78,9 +82,9 @@ export const UploadHTMLConverterAsync: HTMLConvertersAsync pictureHTML += ` ${alt} ` diff --git a/packages/richtext-lexical/src/features/converters/lexicalToHtml/shared/cssColors.ts b/packages/richtext-lexical/src/features/converters/lexicalToHtml/shared/cssColors.ts new file mode 100644 index 00000000000..a3d4d9c84d6 --- /dev/null +++ b/packages/richtext-lexical/src/features/converters/lexicalToHtml/shared/cssColors.ts @@ -0,0 +1,164 @@ +/** Complete set of CSS named colors (CSS Color Level 4) plus common CSS-wide keywords */ +export const CSS_NAMED_COLORS = new Set([ + 'aliceblue', + 'antiquewhite', + 'aqua', + 'aquamarine', + 'azure', + 'beige', + 'bisque', + 'black', + 'blanchedalmond', + 'blue', + 'blueviolet', + 'brown', + 'burlywood', + 'cadetblue', + 'chartreuse', + 'chocolate', + 'coral', + 'cornflowerblue', + 'cornsilk', + 'crimson', + 'currentcolor', + 'cyan', + 'darkblue', + 'darkcyan', + 'darkgoldenrod', + 'darkgray', + 'darkgreen', + 'darkgrey', + 'darkkhaki', + 'darkmagenta', + 'darkolivegreen', + 'darkorange', + 'darkorchid', + 'darkred', + 'darksalmon', + 'darkseagreen', + 'darkslateblue', + 'darkslategray', + 'darkslategrey', + 'darkturquoise', + 'darkviolet', + 'deeppink', + 'deepskyblue', + 'dimgray', + 'dimgrey', + 'dodgerblue', + 'firebrick', + 'floralwhite', + 'forestgreen', + 'fuchsia', + 'gainsboro', + 'ghostwhite', + 'gold', + 'goldenrod', + 'gray', + 'green', + 'greenyellow', + 'grey', + 'honeydew', + 'hotpink', + 'indianred', + 'indigo', + 'inherit', + 'initial', + 'ivory', + 'khaki', + 'lavender', + 'lavenderblush', + 'lawngreen', + 'lemonchiffon', + 'lightblue', + 'lightcoral', + 'lightcyan', + 'lightgoldenrodyellow', + 'lightgray', + 'lightgreen', + 'lightgrey', + 'lightpink', + 'lightsalmon', + 'lightseagreen', + 'lightskyblue', + 'lightslategray', + 'lightslategrey', + 'lightsteelblue', + 'lightyellow', + 'lime', + 'limegreen', + 'linen', + 'magenta', + 'maroon', + 'mediumaquamarine', + 'mediumblue', + 'mediumorchid', + 'mediumpurple', + 'mediumseagreen', + 'mediumslateblue', + 'mediumspringgreen', + 'mediumturquoise', + 'mediumvioletred', + 'midnightblue', + 'mintcream', + 'mistyrose', + 'moccasin', + 'navajowhite', + 'navy', + 'oldlace', + 'olive', + 'olivedrab', + 'orange', + 'orangered', + 'orchid', + 'palegoldenrod', + 'palegreen', + 'paleturquoise', + 'palevioletred', + 'papayawhip', + 'peachpuff', + 'peru', + 'pink', + 'plum', + 'powderblue', + 'purple', + 'rebeccapurple', + 'red', + 'rosybrown', + 'royalblue', + 'saddlebrown', + 'salmon', + 'sandybrown', + 'seagreen', + 'seashell', + 'sienna', + 'silver', + 'skyblue', + 'slateblue', + 'slategray', + 'slategrey', + 'snow', + 'springgreen', + 'steelblue', + 'tan', + 'teal', + 'thistle', + 'tomato', + 'transparent', + 'turquoise', + 'unset', + 'violet', + 'wheat', + 'white', + 'whitesmoke', + 'yellow', + 'yellowgreen', +]) + +/** Matches hex, rgb/rgba, hsl/hsla functional notation */ +const SAFE_CSS_COLOR_FN = /^(?:#[0-9a-fA-F]{3,8}|rgba?\([\d,.\s/%]+\)|hsla?\([\d,.\s/%deg]+\))$/ + +/** Returns true when `value` is a safe CSS color (named color, hex, rgb/a, hsl/a) */ +export const isSafeCssColor = (value: string): boolean => { + return SAFE_CSS_COLOR_FN.test(value) || CSS_NAMED_COLORS.has(value.toLowerCase()) +} diff --git a/packages/richtext-lexical/src/features/converters/lexicalToHtml/sync/converters/heading.ts b/packages/richtext-lexical/src/features/converters/lexicalToHtml/sync/converters/heading.ts index 11ccd154e5e..edd42cd4654 100644 --- a/packages/richtext-lexical/src/features/converters/lexicalToHtml/sync/converters/heading.ts +++ b/packages/richtext-lexical/src/features/converters/lexicalToHtml/sync/converters/heading.ts @@ -1,12 +1,16 @@ import type { SerializedHeadingNode } from '../../../../../nodeTypes.js' import type { HTMLConverters } from '../types.js' +const ALLOWED_HEADING_TAGS = new Set(['h1', 'h2', 'h3', 'h4', 'h5', 'h6']) + export const HeadingHTMLConverter: HTMLConverters = { heading: ({ node, nodesToHTML, providedStyleTag }) => { const children = nodesToHTML({ nodes: node.children, }).join('') - return `<${node.tag}${providedStyleTag}>${children}` + const tag = ALLOWED_HEADING_TAGS.has(node.tag) ? node.tag : 'h1' + + return `<${tag}${providedStyleTag}>${children}` }, } diff --git a/packages/richtext-lexical/src/features/converters/lexicalToHtml/sync/converters/link.ts b/packages/richtext-lexical/src/features/converters/lexicalToHtml/sync/converters/link.ts index cc686fedcd7..b7c323f400a 100644 --- a/packages/richtext-lexical/src/features/converters/lexicalToHtml/sync/converters/link.ts +++ b/packages/richtext-lexical/src/features/converters/lexicalToHtml/sync/converters/link.ts @@ -1,3 +1,6 @@ +import escapeHTML from 'escape-html' +import { sanitizeUrl } from 'payload/shared' + import type { SerializedAutoLinkNode, SerializedLinkNode } from '../../../../../nodeTypes.js' import type { HTMLConverters } from '../types.js' @@ -9,7 +12,9 @@ export const LinkHTMLConverter: (args: { nodes: node.children, }).join('') - return `${children}` + const href = escapeHTML(sanitizeUrl(node.fields.url ?? '')) + + return `${children}` }, link: ({ node, nodesToHTML, providedStyleTag }) => { const children = nodesToHTML({ @@ -21,6 +26,7 @@ export const LinkHTMLConverter: (args: { if (internalDocToHref) { href = internalDocToHref({ linkNode: node }) } else { + // eslint-disable-next-line no-console console.error( 'Lexical => HTML converter: Link converter: found internal link, but internalDocToHref is not provided', ) @@ -28,6 +34,8 @@ export const LinkHTMLConverter: (args: { } } - return `${children}` + const safeHref = escapeHTML(sanitizeUrl(href)) + + return `${children}` }, }) diff --git a/packages/richtext-lexical/src/features/converters/lexicalToHtml/sync/converters/list.ts b/packages/richtext-lexical/src/features/converters/lexicalToHtml/sync/converters/list.ts index 0bd715a3f5d..bfb1077dacb 100644 --- a/packages/richtext-lexical/src/features/converters/lexicalToHtml/sync/converters/list.ts +++ b/packages/richtext-lexical/src/features/converters/lexicalToHtml/sync/converters/list.ts @@ -3,13 +3,19 @@ import { v4 as uuidv4 } from 'uuid' import type { SerializedListItemNode, SerializedListNode } from '../../../../../nodeTypes.js' import type { HTMLConverters } from '../types.js' +const ALLOWED_LIST_TAGS = new Set(['ol', 'ul']) +const ALLOWED_LIST_TYPES = new Set(['bullet', 'check', 'number']) + export const ListHTMLConverter: HTMLConverters = { list: ({ node, nodesToHTML, providedStyleTag }) => { const children = nodesToHTML({ nodes: node.children, }).join('') - return `<${node.tag}${providedStyleTag} class="list-${node.listType}">${children}` + const tag = ALLOWED_LIST_TAGS.has(node.tag) ? node.tag : 'ul' + const listType = ALLOWED_LIST_TYPES.has(node.listType) ? node.listType : 'bullet' + + return `<${tag}${providedStyleTag} class="list-${listType}">${children}` }, listitem: ({ node, nodesToHTML, parent, providedCSSString }) => { const hasSubLists = node.children.some((child) => child.type === 'list') diff --git a/packages/richtext-lexical/src/features/converters/lexicalToHtml/sync/converters/table.ts b/packages/richtext-lexical/src/features/converters/lexicalToHtml/sync/converters/table.ts index 40bfdfdc7cd..7c4518b9785 100644 --- a/packages/richtext-lexical/src/features/converters/lexicalToHtml/sync/converters/table.ts +++ b/packages/richtext-lexical/src/features/converters/lexicalToHtml/sync/converters/table.ts @@ -5,6 +5,8 @@ import type { } from '../../../../../nodeTypes.js' import type { HTMLConverters } from '../types.js' +import { isSafeCssColor } from '../../shared/cssColors.js' + export const TableHTMLConverter: HTMLConverters< SerializedTableCellNode | SerializedTableNode | SerializedTableRowNode > = { @@ -29,7 +31,7 @@ export const TableHTMLConverter: HTMLConverters< const headerStateClass = `lexical-table-cell-header-${node.headerState}` let style = 'border: 1px solid #ccc; padding: 8px;' + providedCSSString - if (node.backgroundColor) { + if (node.backgroundColor && isSafeCssColor(node.backgroundColor)) { style += ` background-color: ${node.backgroundColor};` } diff --git a/packages/richtext-lexical/src/features/converters/lexicalToHtml/sync/converters/text.ts b/packages/richtext-lexical/src/features/converters/lexicalToHtml/sync/converters/text.ts index b78955e7341..3c40c608820 100644 --- a/packages/richtext-lexical/src/features/converters/lexicalToHtml/sync/converters/text.ts +++ b/packages/richtext-lexical/src/features/converters/lexicalToHtml/sync/converters/text.ts @@ -1,3 +1,5 @@ +import escapeHTML from 'escape-html' + import type { SerializedTextNode } from '../../../../../nodeTypes.js' import type { HTMLConverters } from '../types.js' @@ -5,7 +7,7 @@ import { NodeFormat } from '../../../../../lexical/utils/nodeFormat.js' export const TextHTMLConverter: HTMLConverters = { text: ({ node }) => { - let text = node.text + let text = escapeHTML(node.text) if (node.format & NodeFormat.IS_BOLD) { text = `${text}` diff --git a/packages/richtext-lexical/src/features/converters/lexicalToHtml/sync/converters/upload.ts b/packages/richtext-lexical/src/features/converters/lexicalToHtml/sync/converters/upload.ts index 903a9dc5ea5..417cc622ae0 100644 --- a/packages/richtext-lexical/src/features/converters/lexicalToHtml/sync/converters/upload.ts +++ b/packages/richtext-lexical/src/features/converters/lexicalToHtml/sync/converters/upload.ts @@ -1,5 +1,7 @@ import type { FileData, FileSizeImproved, TypeWithID } from 'payload' +import escapeHTML from 'escape-html' + import type { SerializedUploadNode } from '../../../../../nodeTypes.js' import type { UploadDataImproved } from '../../../../upload/server/nodes/UploadNode.js' import type { HTMLConverters } from '../types.js' @@ -21,13 +23,15 @@ export const UploadHTMLConverter: HTMLConverters = { return '' } - const alt = (node.fields?.alt as string) || (uploadDoc as { alt?: string })?.alt || '' + const alt = escapeHTML( + (node.fields?.alt as string) || (uploadDoc as { alt?: string })?.alt || '', + ) - const url = uploadDoc.url + const url = escapeHTML(uploadDoc.url ?? '') // 1) If upload is NOT an image, return a link if (!uploadDoc.mimeType.startsWith('image')) { - return `${uploadDoc.filename}` + return `${escapeHTML(uploadDoc.filename ?? '')}` } // 2) If image has no different sizes, return a simple @@ -35,9 +39,9 @@ export const UploadHTMLConverter: HTMLConverters = { return ` ` } @@ -62,9 +66,9 @@ export const UploadHTMLConverter: HTMLConverters = { pictureHTML += ` ` } @@ -72,9 +76,9 @@ export const UploadHTMLConverter: HTMLConverters = { pictureHTML += ` ${alt} ` diff --git a/packages/richtext-lexical/src/field/Diff/converters/link.ts b/packages/richtext-lexical/src/field/Diff/converters/link.ts index e45feefc5bb..d46ed3285bc 100644 --- a/packages/richtext-lexical/src/field/Diff/converters/link.ts +++ b/packages/richtext-lexical/src/field/Diff/converters/link.ts @@ -1,4 +1,6 @@ import { createHash } from 'crypto' +import escapeHTML from 'escape-html' +import { sanitizeUrl } from 'payload/shared' import type { HTMLConvertersAsync, @@ -24,7 +26,9 @@ export const LinkDiffHTMLConverterAsync: (args: { // hash fields to ensure they are diffed if they change const nodeFieldsHash = createHash('sha256').update(JSON.stringify(node.fields)).digest('hex') - return ` + const href = escapeHTML(sanitizeUrl(node.fields.url ?? '')) + + return ` ${children} ` }, @@ -40,6 +44,7 @@ export const LinkDiffHTMLConverterAsync: (args: { if (internalDocToHref) { href = await internalDocToHref({ linkNode: node, populate }) } else { + // eslint-disable-next-line no-console console.error( 'Lexical => HTML converter: Link converter: found internal link, but internalDocToHref is not provided', ) @@ -52,7 +57,9 @@ export const LinkDiffHTMLConverterAsync: (args: { .update(JSON.stringify(node.fields ?? {})) .digest('hex') - return ` + const safeHref = escapeHTML(sanitizeUrl(href)) + + return ` ${children} ` }, diff --git a/packages/storage-azure/src/staticHandler.ts b/packages/storage-azure/src/staticHandler.ts index 2f9dfd482e3..0e4eee99e0b 100644 --- a/packages/storage-azure/src/staticHandler.ts +++ b/packages/storage-azure/src/staticHandler.ts @@ -7,6 +7,7 @@ import { RestError } from '@azure/storage-blob' import { getFilePrefix } from '@payloadcms/plugin-cloud-storage/utilities' import path from 'path' import { getRangeRequestInfo } from 'payload/internal' +import { sanitizeFilename } from 'payload/shared' const isNodeReadableStream = ( body: BlobDownloadResponseParsed['readableStreamBody'], @@ -58,7 +59,7 @@ export const getHandler = ({ collection, getStorageClient }: Args): StaticHandle try { const prefix = await getFilePrefix({ clientUploadContext, collection, filename, req }) const blockBlobClient = getStorageClient().getBlockBlobClient( - path.posix.join(prefix, filename), + path.posix.join(prefix, sanitizeFilename(filename)), ) // Get file size for range validation diff --git a/packages/storage-gcs/src/staticHandler.ts b/packages/storage-gcs/src/staticHandler.ts index 8b78cc3b697..1129e4b2744 100644 --- a/packages/storage-gcs/src/staticHandler.ts +++ b/packages/storage-gcs/src/staticHandler.ts @@ -5,6 +5,7 @@ import { ApiError, type Storage } from '@google-cloud/storage' import { getFilePrefix } from '@payloadcms/plugin-cloud-storage/utilities' import path from 'path' import { getRangeRequestInfo } from 'payload/internal' +import { sanitizeFilename } from 'payload/shared' interface Args { bucket: string @@ -16,7 +17,9 @@ export const getHandler = ({ bucket, collection, getStorageClient }: Args): Stat return async (req, { headers: incomingHeaders, params: { clientUploadContext, filename } }) => { try { const prefix = await getFilePrefix({ clientUploadContext, collection, filename, req }) - const file = getStorageClient().bucket(bucket).file(path.posix.join(prefix, filename)) + const file = getStorageClient() + .bucket(bucket) + .file(path.posix.join(prefix, sanitizeFilename(filename))) const [metadata] = await file.getMetadata() diff --git a/packages/storage-r2/src/staticHandler.ts b/packages/storage-r2/src/staticHandler.ts index 83f1289eea0..d76a6b9048b 100644 --- a/packages/storage-r2/src/staticHandler.ts +++ b/packages/storage-r2/src/staticHandler.ts @@ -3,6 +3,7 @@ import type { CollectionConfig } from 'payload' import path from 'path' import { getRangeRequestInfo } from 'payload/internal' +import { sanitizeFilename } from 'payload/shared' import type { R2Bucket, R2ObjectBody } from './types.js' @@ -17,7 +18,7 @@ const isMiniflare = process.env.NODE_ENV === 'development' export const getHandler = ({ bucket, collection, prefix = '' }: Args): StaticHandler => { return async (req, { headers: incomingHeaders, params: { clientUploadContext, filename } }) => { try { - const key = path.posix.join(prefix, filename) + const key = path.posix.join(prefix, sanitizeFilename(filename)) // Get file size for range validation const headObj = await bucket?.head(key) diff --git a/packages/storage-s3/src/staticHandler.ts b/packages/storage-s3/src/staticHandler.ts index 7597a1aa46f..fe7bfdec0dd 100644 --- a/packages/storage-s3/src/staticHandler.ts +++ b/packages/storage-s3/src/staticHandler.ts @@ -8,6 +8,7 @@ import { getSignedUrl } from '@aws-sdk/s3-request-presigner' import { getFilePrefix } from '@payloadcms/plugin-cloud-storage/utilities' import path from 'path' import { getRangeRequestInfo } from 'payload/internal' +import { sanitizeFilename } from 'payload/shared' export type SignedDownloadsConfig = | { @@ -76,7 +77,7 @@ export const getHandler = ({ try { const prefix = await getFilePrefix({ clientUploadContext, collection, filename, req }) - const key = path.posix.join(prefix, filename) + const key = path.posix.join(prefix, sanitizeFilename(filename)) if (signedDownloads && !clientUploadContext) { let useSignedURL = true diff --git a/packages/storage-vercel-blob/src/staticHandler.ts b/packages/storage-vercel-blob/src/staticHandler.ts index cd2e8134d8d..3e48d222a6e 100644 --- a/packages/storage-vercel-blob/src/staticHandler.ts +++ b/packages/storage-vercel-blob/src/staticHandler.ts @@ -5,6 +5,7 @@ import { getFilePrefix } from '@payloadcms/plugin-cloud-storage/utilities' import { BlobNotFoundError, head } from '@vercel/blob' import path from 'path' import { getRangeRequestInfo } from 'payload/internal' +import { sanitizeFilename } from 'payload/shared' type StaticHandlerArgs = { baseUrl: string @@ -19,7 +20,7 @@ export const getStaticHandler = ( return async (req, { headers: incomingHeaders, params: { clientUploadContext, filename } }) => { try { const prefix = await getFilePrefix({ clientUploadContext, collection, filename, req }) - const fileKey = path.posix.join(prefix, encodeURIComponent(filename)) + const fileKey = path.posix.join(prefix, encodeURIComponent(sanitizeFilename(filename))) const fileUrl = `${baseUrl}/${fileKey}` const etagFromHeaders = req.headers.get('etag') || req.headers.get('if-none-match') const blobMetadata = await head(fileUrl, { token }) diff --git a/test/fields/int.spec.ts b/test/fields/int.spec.ts index 846aa7fb122..cbdde58af20 100644 --- a/test/fields/int.spec.ts +++ b/test/fields/int.spec.ts @@ -3863,6 +3863,39 @@ describe('Fields', () => { }), ).rejects.toBeTruthy() }) + + it('should reject disallowed characters in JSON field path segments', async () => { + // Path segments in JSON queries must only contain word characters. + const badPaths = [ + "json.key'bad", + 'json.key"bad', + 'json.key;bad', + 'json.key(bad', + 'json.key)bad', + ] + + for (const path of badPaths) { + await expect( + payload.find({ + collection: 'json-fields', + where: { + [path]: { equals: 'test' }, + }, + }), + ).rejects.toBeTruthy() + } + }) + + it('should accept valid path segments in JSON field queries', async () => { + const result = await payload.find({ + collection: 'json-fields', + where: { + 'json.valid_key': { equals: 'test' }, + }, + }) + + expect(result.docs).toBeDefined() + }) }) }) diff --git a/test/joins/int.spec.ts b/test/joins/int.spec.ts index a8365b7d216..0df9b491934 100644 --- a/test/joins/int.spec.ts +++ b/test/joins/int.spec.ts @@ -927,8 +927,8 @@ describe('Joins Field', () => { }) it('should respect access control for join request `where` queries', async () => { - await expect(async () => { - await payload.findByID({ + await expect( + payload.findByID({ id: category.id, collection: categoriesSlug, overrideAccess: false, @@ -940,8 +940,8 @@ describe('Joins Field', () => { }, }, }, - }) - }).rejects.toThrow('The following path cannot be queried: restrictedField') + }), + ).rejects.toThrow('The following path cannot be queried: restrictedField') }) it('should respect access control of join field configured `where` queries', async () => { @@ -958,14 +958,14 @@ describe('Joins Field', () => { category: restrictedCategory.id, }, }) - await expect(async () => { - await payload.findByID({ + await expect( + payload.findByID({ id: category.id, collection: restrictedCategoriesSlug, overrideAccess: false, user, - }) - }).rejects.toThrow('The following path cannot be queried: restrictedField') + }), + ).rejects.toThrow('The following path cannot be queried: restrictedField') }) it('should sort joins', async () => { @@ -1949,6 +1949,179 @@ describe('Joins Field', () => { expect(response.status).toBe(200) }) + + it('should reject unknown operators regardless of value type', async () => { + const payloads = [ + { title: { bogus_operator: 'primitive' } }, + { title: { not_a_real_op: { nested: 'object' } } }, + { title: { fake: { deeply: { nested: true } } } }, + ] + + for (const where of payloads) { + const response = await restClient.GET('/categories', { + query: { + limit: 1, + joins: { + polymorphicJoin: { + limit: 1, + where, + }, + }, + }, + }) + + expect(response.status).toBe(400) + } + }) + + it('should reject unknown operators when value is an array', async () => { + // When qs parses [$raw][0]=value, the value becomes an array. + // Arrays must be rejected the same as other value types. + const response = await restClient.GET('/categories', { + query: { + limit: 1, + joins: { + polymorphicJoin: { + limit: 1, + where: { x: { $raw: ['true'] } }, + }, + }, + }, + }) + + expect(response.status).toBe(400) + }) + + it('should reject disallowed characters in polymorphic join where paths', async () => { + // Polymorphic joins suppress "field not found" errors so that + // fields unique to one collection are accepted. But paths with + // disallowed characters must always be rejected. + const badPaths = [ + "title'", + "title'; DROP TABLE posts; --", + 'title"bad', + 'title;bad', + 'title(bad', + 'title)bad', + ] + + for (const path of badPaths) { + const response = await restClient.GET('/categories', { + query: { + limit: 1, + joins: { + polymorphicJoin: { + limit: 1, + where: { [path]: { equals: 'test' } }, + }, + }, + }, + }) + + expect(response.status).toBe(400) + } + }) + + it('should reject $raw in non-polymorphic join where clause', async () => { + // Non-polymorphic joins use the full parseParams pipeline, not + // buildSQLWhere. The $raw operator must still be rejected. + const response = await restClient.GET('/categories', { + query: { + limit: 1, + joins: { + relatedPosts: { + limit: 1, + where: { title: { $raw: 'true' } }, + }, + }, + }, + }) + + expect(response.status).toBe(400) + }) + }) + + describe('Top-level query validation', () => { + it('should reject $raw operator on top-level collection queries via REST', async () => { + // $raw is an internal-only operator and must be rejected + // at the top-level collection query as well as in joins. + const response = await restClient.GET(`/${postsSlug}`, { + query: { + limit: 1, + where: { title: { $raw: 'true' } }, + }, + }) + + expect(response.status).toBe(400) + }) + + it('should reject $raw operator via Local API', async () => { + // The Local API goes through the same validateQueryPaths pipeline. + await expect( + payload.find({ + collection: postsSlug, + limit: 1, + where: { title: { $raw: 'true' } } as any, + }), + ).rejects.toBeTruthy() + }) + + it('should reject unknown operators on top-level collection queries', async () => { + const response = await restClient.GET(`/${postsSlug}`, { + query: { + limit: 1, + where: { title: { bogus_operator: 'test' } }, + }, + }) + + expect(response.status).toBe(400) + }) + + it('should reject $raw nested inside AND combinator', async () => { + // Unrecognized operators inside boolean combinators should + // also be caught by recursive validation. + const response = await restClient.GET(`/${postsSlug}`, { + query: { + limit: 1, + where: { + and: [{ title: { $raw: 'true' } }], + }, + }, + }) + + expect(response.status).toBe(400) + }) + + it('should reject $raw nested inside OR combinator', async () => { + const response = await restClient.GET(`/${postsSlug}`, { + query: { + limit: 1, + where: { + or: [{ title: { $raw: 'true' } }], + }, + }, + }) + + expect(response.status).toBe(400) + }) + + it('should reject $raw nested inside AND combinator in join where', async () => { + const response = await restClient.GET('/categories', { + query: { + limit: 1, + joins: { + polymorphicJoin: { + limit: 1, + where: { + and: [{ title: { $raw: 'true' } }], + }, + }, + }, + }, + }) + + expect(response.status).toBe(400) + }) }) }) diff --git a/test/lexical/lexical.int.spec.ts b/test/lexical/lexical.int.spec.ts index 0ad14c00417..c5bfbca7d0d 100644 --- a/test/lexical/lexical.int.spec.ts +++ b/test/lexical/lexical.int.spec.ts @@ -13,11 +13,30 @@ import { type SerializedUploadNode, } from '@payloadcms/richtext-lexical' import path from 'path' +import { sanitizeUrl } from 'payload/shared' import { fileURLToPath } from 'url' -import { beforeAll, beforeEach, describe, expect } from 'vitest' +import { beforeAll, beforeEach, describe, expect, it as vitestIt } from 'vitest' import type { LexicalField, LexicalMigrateField, RichTextField } from './payload-types.js' +// Sync converters +import { HeadingHTMLConverter } from '../../packages/richtext-lexical/src/features/converters/lexicalToHtml/sync/converters/heading.js' +import { LinkHTMLConverter } from '../../packages/richtext-lexical/src/features/converters/lexicalToHtml/sync/converters/link.js' +import { ListHTMLConverter } from '../../packages/richtext-lexical/src/features/converters/lexicalToHtml/sync/converters/list.js' +import { TableHTMLConverter } from '../../packages/richtext-lexical/src/features/converters/lexicalToHtml/sync/converters/table.js' +import { TextHTMLConverter } from '../../packages/richtext-lexical/src/features/converters/lexicalToHtml/sync/converters/text.js' +import { UploadHTMLConverter } from '../../packages/richtext-lexical/src/features/converters/lexicalToHtml/sync/converters/upload.js' + +// Async converters +import { HeadingHTMLConverterAsync } from '../../packages/richtext-lexical/src/features/converters/lexicalToHtml/async/converters/heading.js' +import { LinkHTMLConverterAsync } from '../../packages/richtext-lexical/src/features/converters/lexicalToHtml/async/converters/link.js' +import { ListHTMLConverterAsync } from '../../packages/richtext-lexical/src/features/converters/lexicalToHtml/async/converters/list.js' +import { TableHTMLConverterAsync } from '../../packages/richtext-lexical/src/features/converters/lexicalToHtml/async/converters/table.js' +import { TextHTMLConverterAsync } from '../../packages/richtext-lexical/src/features/converters/lexicalToHtml/async/converters/text.js' +import { UploadHTMLConverterAsync } from '../../packages/richtext-lexical/src/features/converters/lexicalToHtml/async/converters/upload.js' + +// Diff converter +import { LinkDiffHTMLConverterAsync } from '../../packages/richtext-lexical/src/field/Diff/converters/link.js' import { it } from '../__helpers/int/vitest.js' import { initPayloadInt } from '../__helpers/shared/initPayloadInt.js' import { NextRESTClient } from '../__helpers/shared/NextRESTClient.js' @@ -970,4 +989,486 @@ describe('Lexical', () => { expect(autosaveHookLog.relationshipField?.value).toBe('Updated block title') }) }) + + describe('sanitizeUrl', () => { + vitestIt.each([ + ['http://example.com', 'http://example.com'], + ['https://example.com/page', 'https://example.com/page'], + ['mailto:user@example.com', 'mailto:user@example.com'], + ['tel:+1234567890', 'tel:+1234567890'], + ['#section', '#section'], + ['/path/to/page', '/path/to/page'], + ['./relative/path', './relative/path'], + ['../parent/path', '../parent/path'], + ['', ''], + ['example.com', 'example.com'], + ])('allows safe URL: %s', (input, expected) => { + expect(sanitizeUrl(input)).toBe(expected) + }) + + vitestIt.each([ + ['javascript:alert(1)', '#'], + ['JavaScript:alert(document.cookie)', '#'], + ['JAVASCRIPT:void(0)', '#'], + ['data:text/html,', '#'], + ['vbscript:MsgBox("test")', '#'], + ['blob:http://example.com/uuid', '#'], + ])('blocks disallowed protocol: %s', (input, expected) => { + expect(sanitizeUrl(input)).toBe(expected) + }) + + vitestIt('trims whitespace', () => { + expect(sanitizeUrl(' https://example.com ')).toBe('https://example.com') + }) + }) + + const noopNodesToHTML = ({ nodes }: { nodes: any[] }) => nodes.map(() => '') + const noopNodesToHTMLAsync = ({ nodes }: { nodes: any[] }) => Promise.resolve(nodes.map(() => '')) + + const converterBaseArgs = { + parent: {} as any, + providedCSSString: '', + providedStyleTag: '', + submissionData: undefined, + textContent: '', + } + + const converterVariants = [ + { + label: 'Sync', + heading: HeadingHTMLConverter, + link: LinkHTMLConverter({}), + list: ListHTMLConverter, + table: TableHTMLConverter, + text: TextHTMLConverter, + upload: UploadHTMLConverter, + noop: noopNodesToHTML, + }, + { + label: 'Async', + heading: HeadingHTMLConverterAsync, + link: LinkHTMLConverterAsync({}), + list: ListHTMLConverterAsync, + table: TableHTMLConverterAsync, + text: TextHTMLConverterAsync, + upload: UploadHTMLConverterAsync, + noop: noopNodesToHTMLAsync, + }, + ] as const + + for (const variant of converterVariants) { + describe(`HTML Converters (${variant.label})`, () => { + // ── Text ── + describe('TextHTMLConverter', () => { + vitestIt('escapes script tags in text content', async () => { + const result = await variant.text.text!({ + ...converterBaseArgs, + node: { format: 0, text: '' } as any, + nodesToHTML: variant.noop as any, + }) + expect(result).not.toContain('', + }, + } as any, + nodesToHTML: variant.noop as any, + }) + expect(result).not.toContain('data:') + expect(result).toContain('href="#"') + }) + + vitestIt('escapes HTML entities in href attribute', async () => { + const result = await variant.link.autolink!({ + ...converterBaseArgs, + node: { + children: [], + fields: { + newTab: false, + url: 'https://example.com/path?a=1&b=2"onmouseover="alert(1)', + }, + } as any, + nodesToHTML: variant.noop as any, + }) + expect(result).not.toContain('"onmouseover') + expect(result).toContain('&') + }) + + vitestIt('allows safe https URLs', async () => { + const result = await variant.link.autolink!({ + ...converterBaseArgs, + node: { + children: [], + fields: { newTab: false, url: 'https://example.com/safe-page' }, + } as any, + nodesToHTML: variant.noop as any, + }) + expect(result).toContain('href="https://example.com/safe-page"') + }) + + vitestIt('preserves query params with proper encoding', async () => { + const result = await variant.link.autolink!({ + ...converterBaseArgs, + node: { + children: [], + fields: { newTab: false, url: 'https://example.com/search?q=hello&lang=en' }, + } as any, + nodesToHTML: variant.noop as any, + }) + expect(result).toContain('href="https://example.com/search?q=hello&lang=en"') + }) + }) + + // ── Upload ── + describe('UploadHTMLConverter', () => { + const baseUploadNode = { + fields: {}, + relationTo: 'uploads', + value: { + filename: 'test.pdf', + height: 100, + id: '1', + mimeType: 'application/pdf', + sizes: {}, + url: '/uploads/test.pdf', + width: 100, + }, + } + + vitestIt('escapes HTML in non-image filename', async () => { + const result = await variant.upload.upload!({ + ...converterBaseArgs, + node: { + ...baseUploadNode, + value: { + ...baseUploadNode.value, + filename: '', + }, + } as any, + nodesToHTML: variant.noop as any, + }) + expect(result).not.toContain('') + }) + + vitestIt('escapes HTML in alt attribute', async () => { + const result = await variant.upload.upload!({ + ...converterBaseArgs, + node: { + ...baseUploadNode, + fields: { alt: '">' }, + value: { ...baseUploadNode.value, mimeType: 'image/png' }, + } as any, + nodesToHTML: variant.noop as any, + }) + expect(result).not.toContain('', + }, + } as any, + nodesToHTML: variant.noop as any, + }) + expect(result).not.toContain('', + children: [], + colSpan: 1, + headerState: 0, + rowSpan: 1, + } as any, + nodesToHTML: variant.noop as any, + }) + expect(result).not.toContain('', + url: '">', + width: 100, + }, + }, + url: '/uploads/photo.jpg', + width: 800, + }, + } as any, + nodesToHTML: noopNodesToHTML as any, + }) + expect(result).not.toContain('', + }, + } as any, + nodesToHTML: noopNodesToHTMLAsync as any, + }) + expect(result).not.toContain('data:') + expect(result).toContain('href="#"') + }) + + vitestIt('properly encodes special characters in href', async () => { + const result = await diffLinkConverter.autolink!({ + ...converterBaseArgs, + node: { + children: [], + fields: { newTab: false, url: 'https://x.com/"onmouseover="alert(1)' }, + } as any, + nodesToHTML: noopNodesToHTMLAsync as any, + }) + expect(result).not.toContain('"onmouseover') + expect(result).toContain('"') + }) + + vitestIt('allows safe URLs and includes fields hash', async () => { + const result = await diffLinkConverter.autolink!({ + ...converterBaseArgs, + node: { + children: [], + fields: { newTab: false, url: 'https://example.com/page' }, + } as any, + nodesToHTML: noopNodesToHTMLAsync as any, + }) + expect(result).toContain('href="https://example.com/page"') + expect(result).toContain('data-fields-hash=') + }) + }) }) diff --git a/test/plugin-cloud-storage/int.spec.ts b/test/plugin-cloud-storage/int.spec.ts index 3f63611e929..5f9153bd342 100644 --- a/test/plugin-cloud-storage/int.spec.ts +++ b/test/plugin-cloud-storage/int.spec.ts @@ -2,7 +2,10 @@ import type { Payload } from 'payload' import type { SuiteAPI } from 'vitest' import * as AWS from '@aws-sdk/client-s3' +import { getFilePrefix } from '@payloadcms/plugin-cloud-storage/utilities' import path from 'path' +import { APIError } from 'payload' +import { sanitizeFilename } from 'payload/shared' import shelljs from 'shelljs' import { fileURLToPath } from 'url' import { afterAll, afterEach, beforeAll, describe, expect, it } from 'vitest' @@ -57,6 +60,170 @@ describe('@payloadcms/plugin-cloud-storage', () => { await payload.destroy() }) + describe('getFilePrefix', () => { + const mockReq = { + payload: { + find: () => ({ docs: [] }), + }, + } as any + + const mockCollection = { + slug: 'media', + upload: {}, + } as any + + describe('clientUploadContext prefix sanitization', () => { + it('should return a valid prefix unchanged', async () => { + const result = await getFilePrefix({ + clientUploadContext: { prefix: 'media/images' }, + collection: mockCollection, + filename: 'test.png', + req: mockReq, + }) + expect(result).toBe('media/images') + }) + + it('should strip invalid segments from the prefix', async () => { + const result = await getFilePrefix({ + clientUploadContext: { prefix: '../other-collection/private' }, + collection: mockCollection, + filename: 'test.png', + req: mockReq, + }) + expect(result).toBe('other-collection/private') + expect(result).not.toContain('..') + }) + + it('should handle deeply nested invalid segments', async () => { + const result = await getFilePrefix({ + clientUploadContext: { prefix: 'a/../../outside' }, + collection: mockCollection, + filename: 'test.png', + req: mockReq, + }) + expect(result).toBe('a/outside') + expect(result).not.toContain('..') + }) + + it('should strip leading slashes from the prefix', async () => { + const result = await getFilePrefix({ + clientUploadContext: { prefix: '/absolute/path' }, + collection: mockCollection, + filename: 'test.png', + req: mockReq, + }) + expect(result).toBe('absolute/path') + expect(result).not.toMatch(/^\//) + }) + + it('should strip dot segments from the prefix', async () => { + const result = await getFilePrefix({ + clientUploadContext: { prefix: './relative/./path' }, + collection: mockCollection, + filename: 'test.png', + req: mockReq, + }) + expect(result).toBe('relative/path') + }) + + it('should normalize backslash separators', async () => { + const result = await getFilePrefix({ + clientUploadContext: { prefix: '..\\..\\outside' }, + collection: mockCollection, + filename: 'test.png', + req: mockReq, + }) + expect(result).not.toContain('..') + }) + + it('should strip control characters from the prefix', async () => { + const result = await getFilePrefix({ + clientUploadContext: { prefix: 'media\x00/images' }, + collection: mockCollection, + filename: 'test.png', + req: mockReq, + }) + expect(result).toBe('media/images') + }) + + it('should return empty string for a prefix of only invalid segments', async () => { + const result = await getFilePrefix({ + clientUploadContext: { prefix: '../../..' }, + collection: mockCollection, + filename: 'test.png', + req: mockReq, + }) + expect(result).toBe('') + }) + }) + + describe('fallback to DB lookup', () => { + it('should return empty string when no clientUploadContext and no DB match', async () => { + const result = await getFilePrefix({ + collection: mockCollection, + filename: 'test.png', + req: mockReq, + }) + expect(result).toBe('') + }) + + it('should return prefix from DB when doc has a prefix', async () => { + const reqWithDoc = { + payload: { + find: () => ({ docs: [{ prefix: 'db-prefix' }] }), + }, + } as any + + const result = await getFilePrefix({ + collection: mockCollection, + filename: 'test.png', + req: reqWithDoc, + }) + expect(result).toBe('db-prefix') + }) + }) + }) + + describe('sanitizeFilename', () => { + it('should return a simple filename unchanged', () => { + expect(sanitizeFilename('image.png')).toBe('image.png') + }) + + it('should return only the base name from a path', () => { + expect(sanitizeFilename('some/path/to/file.jpg')).toBe('file.jpg') + expect(sanitizeFilename('a/b/../../c/d/../file.txt')).toBe('file.txt') + }) + + it('should normalize backslash separators', () => { + expect(sanitizeFilename('..\\..\\windows\\system32\\config')).toBe('config') + }) + + it('should reject standalone dot filenames', () => { + expect(() => sanitizeFilename('.')).toThrow(APIError) + expect(() => sanitizeFilename('..')).toThrow(APIError) + }) + + it('should reject empty filenames', () => { + expect(() => sanitizeFilename('')).toThrow(APIError) + expect(() => sanitizeFilename('/')).toThrow(APIError) + expect(() => sanitizeFilename('//')).toThrow(APIError) + }) + + it('should strip control characters', () => { + expect(sanitizeFilename('file\x00name\x1f.txt')).toBe('filename.txt') + expect(sanitizeFilename('file\x80name\x9f.txt')).toBe('filename.txt') + }) + + it('should preserve unicode filenames', () => { + expect(sanitizeFilename('résumé.pdf')).toBe('résumé.pdf') + expect(sanitizeFilename('日本語.txt')).toBe('日本語.txt') + }) + + it('should treat percent-encoded strings as literal filenames', () => { + expect(sanitizeFilename('%2e%2e%2f')).toBe('%2e%2e%2f') + }) + }) + let client: AWS.S3Client describeIfInCIOrHasLocalstack()('plugin-cloud-storage', () => { describe('S3', () => { diff --git a/test/plugin-form-builder/int.spec.ts b/test/plugin-form-builder/int.spec.ts index ea2c765982c..268bf9eb6c5 100644 --- a/test/plugin-form-builder/int.spec.ts +++ b/test/plugin-form-builder/int.spec.ts @@ -1,13 +1,15 @@ import type { Payload } from 'payload' -import { describe, beforeAll, afterAll, it, expect } from 'vitest' import path from 'path' import { ValidationError } from 'payload' import { fileURLToPath } from 'url' +import { afterAll, beforeAll, describe, expect, it } from 'vitest' import type { Form } from './payload-types.js' +import { keyValuePairToHtmlTable } from '../../packages/plugin-form-builder/src/utilities/keyValuePairToHtmlTable.js' import { serializeLexical } from '../../packages/plugin-form-builder/src/utilities/lexical/serializeLexical.js' +import { replaceDoubleCurlys } from '../../packages/plugin-form-builder/src/utilities/replaceDoubleCurlys.js' import { serializeSlate } from '../../packages/plugin-form-builder/src/utilities/slate/serializeSlate.js' import { initPayloadInt } from '../__helpers/shared/initPayloadInt.js' import { formsSlug, formSubmissionsSlug } from './shared.js' @@ -441,4 +443,273 @@ describe('@payloadcms/plugin-form-builder', () => { }) }) }) + + describe('replaceDoubleCurlys', () => { + const testVariables = [ + { field: 'name', value: '' }, + { field: '', value: 'normal' }, + ] + + it('escapes HTML in named variable replacement', () => { + const result = replaceDoubleCurlys('Hello {{name}}', testVariables) + expect(result).not.toContain('': '', + name: 'safe value', + }) + expect(result).toContain('') + expect(result).not.toContain('' }] + const result = serializeSlate(nodes) + expect(result).not.toContain('', + }, + ] + const result = serializeSlate(nodes) + expect(result).not.toContain('' }, + submissionData: undefined, + } as any) + expect(result).not.toContain('', + }, + }, + parent: {}, + submissionData: undefined, + } as any) + expect(result).not.toContain('data:') + expect(result).toContain('href="#"') + }) + + it('properly encodes special characters in href', async () => { + const result = await FormBuilderLinkConverter.converter({ + converters: [], + node: { + children: [], + fields: { linkType: 'custom', newTab: false, url: '">' }, + }, + parent: {}, + submissionData: undefined, + } as any) + expect(result).not.toContain(' { + const result = await FormBuilderLinkConverter.converter({ + converters: [], + node: { + children: [], + fields: { linkType: 'custom', newTab: false, url: 'https://example.com/page' }, + }, + parent: {}, + submissionData: undefined, + } as any) + expect(result).toContain('href="https://example.com/page"') + }) + + it('allows mailto: URLs', async () => { + const result = await FormBuilderLinkConverter.converter({ + converters: [], + node: { + children: [], + fields: { linkType: 'custom', newTab: false, url: 'mailto:test@example.com' }, + }, + parent: {}, + submissionData: undefined, + } as any) + expect(result).toContain('href="mailto:test@example.com"') + }) + + it('allows relative URLs', async () => { + const result = await FormBuilderLinkConverter.converter({ + converters: [], + node: { + children: [], + fields: { linkType: 'custom', newTab: false, url: '/contact' }, + }, + parent: {}, + submissionData: undefined, + } as any) + expect(result).toContain('href="/contact"') + }) + }) }) diff --git a/test/uploads/int.spec.ts b/test/uploads/int.spec.ts index 7d2fe206b13..006d0698a27 100644 --- a/test/uploads/int.spec.ts +++ b/test/uploads/int.spec.ts @@ -1863,7 +1863,32 @@ describe('Collections - Uploads', () => { } }) + it('should enforce allowList on redirect targets', async () => { + const redirectServer = createServer((req, res) => { + // Redirect to a host that is NOT on the allowList + res.writeHead(302, { Location: 'http://192.168.99.99/file.png' }) + res.end() + }) + + const redirectServerPort = await startServer(redirectServer) + + try { + await expect( + payload.create({ + collection: allowListMediaSlug, + data: { + filename: 'redirect-test.png', + url: `http://127.0.0.1:${redirectServerPort}/image.png`, + }, + }), + ).rejects.toThrow() + } finally { + redirectServer.close() + } + }) + it('should not allow infinite redirect loops', async () => { + // eslint-disable-next-line prefer-const let redirectServerPort: number const redirectServer = createServer((req, res) => { @@ -1889,6 +1914,67 @@ describe('Collections - Uploads', () => { }) }) + describe('paste-url endpoint', () => { + it('should return 400 when pasteURL is not configured', async () => { + const response = await restClient.GET(`/${mediaSlug}/paste-url`, { + query: { src: 'http://example.com/file.png' }, + }) + expect(response.status).toBe(400) + }) + + it('should return 400 when pasteURL is disabled', async () => { + const response = await restClient.GET(`/${focalNoSizesSlug}/paste-url`, { + query: { src: 'http://example.com/file.png' }, + }) + expect(response.status).toBe(400) + }) + + it('should reject requests to non-public addresses', async () => { + const response = await restClient.GET(`/${allowListMediaSlug}/paste-url`, { + query: { src: 'http://127.0.0.1/file.png' }, + }) + expect(response.status).toBe(500) + }) + + it('should validate resolved addresses', async () => { + const globalCachedFn = _internal_safeFetchGlobal.lookup + + // @ts-expect-error mock lookup + _internal_safeFetchGlobal.lookup = (_hostname, _options, callback) => { + callback(null, '127.0.0.1' as any, 4) + } + + try { + const response = await restClient.GET(`/${allowListMediaSlug}/paste-url`, { + query: { src: 'http://localhost/file.png' }, + }) + expect(response.status).toBe(500) + } finally { + _internal_safeFetchGlobal.lookup = globalCachedFn + } + }) + + it('should reject URLs not matching the allowList', async () => { + const response = await restClient.GET(`/${allowListMediaSlug}/paste-url`, { + query: { src: 'http://other.example.com/file.png' }, + }) + expect(response.status).toBe(400) + }) + + it('should require authentication', async () => { + const response = await restClient.GET(`/${allowListMediaSlug}/paste-url`, { + query: { src: 'http://127.0.0.1/file.png' }, + auth: false, + }) + expect(response.status).toBe(403) + }) + + it('should require a src query parameter', async () => { + const response = await restClient.GET(`/${allowListMediaSlug}/paste-url`) + expect(response.status).toBeGreaterThanOrEqual(400) + }) + }) + describe('tempFileDir', () => { it.each([ { dir: '/tmp', expectedPrefix: '/tmp', description: 'absolute path like /tmp' },