From ef1db35f80499280f3631653ce40219256ccd484 Mon Sep 17 00:00:00 2001 From: Novice Date: Tue, 4 Nov 2025 15:45:22 +0800 Subject: [PATCH 01/18] feat: implement file extension blacklist for upload security (#27540) --- api/.env.example | 6 + api/configs/feature/__init__.py | 25 +++ api/controllers/common/errors.py | 6 + api/controllers/console/files.py | 3 + api/services/errors/file.py | 4 + api/services/file_service.py | 6 +- .../services/test_file_service.py | 149 +++++++++++++++++- docker/.env.example | 6 + docker/docker-compose.yaml | 1 + .../components/base/file-uploader/hooks.ts | 11 +- .../components/base/file-uploader/utils.ts | 25 ++- .../components/base/image-uploader/hooks.ts | 12 +- .../components/base/image-uploader/utils.ts | 25 ++- .../custom/custom-web-app-brand/index.tsx | 7 +- .../datasets/create/file-uploader/index.tsx | 4 +- .../data-source/local-file/index.tsx | 4 +- .../detail/batch-modal/csv-uploader.tsx | 4 +- web/i18n/en-US/common.ts | 1 + web/i18n/zh-Hans/common.ts | 1 + 19 files changed, 277 insertions(+), 23 deletions(-) diff --git a/api/.env.example b/api/.env.example index 22dd7600ed..f5bfa72254 100644 --- a/api/.env.example +++ b/api/.env.example @@ -371,6 +371,12 @@ UPLOAD_IMAGE_FILE_SIZE_LIMIT=10 UPLOAD_VIDEO_FILE_SIZE_LIMIT=100 UPLOAD_AUDIO_FILE_SIZE_LIMIT=50 +# Comma-separated list of file extensions blocked from upload for security reasons. +# Extensions should be lowercase without dots (e.g., exe,bat,sh,dll). +# Empty by default to allow all file types. +# Recommended: exe,bat,cmd,com,scr,vbs,ps1,msi,dll +UPLOAD_FILE_EXTENSION_BLACKLIST= + # Model configuration MULTIMODAL_SEND_FORMAT=base64 PROMPT_GENERATION_MAX_TOKENS=512 diff --git a/api/configs/feature/__init__.py b/api/configs/feature/__init__.py index 59fc0b9661..843e6b6f70 100644 --- a/api/configs/feature/__init__.py +++ b/api/configs/feature/__init__.py @@ -331,6 +331,31 @@ class FileUploadConfig(BaseSettings): default=10, ) + inner_UPLOAD_FILE_EXTENSION_BLACKLIST: str = Field( + description=( + "Comma-separated list of file extensions that are blocked from upload. " + "Extensions should be lowercase without dots (e.g., 'exe,bat,sh,dll'). " + "Empty by default to allow all file types." + ), + validation_alias=AliasChoices("UPLOAD_FILE_EXTENSION_BLACKLIST"), + default="", + ) + + @computed_field # type: ignore[misc] + @property + def UPLOAD_FILE_EXTENSION_BLACKLIST(self) -> set[str]: + """ + Parse and return the blacklist as a set of lowercase extensions. + Returns an empty set if no blacklist is configured. + """ + if not self.inner_UPLOAD_FILE_EXTENSION_BLACKLIST: + return set() + return { + ext.strip().lower().strip(".") + for ext in self.inner_UPLOAD_FILE_EXTENSION_BLACKLIST.split(",") + if ext.strip() + } + class HttpConfig(BaseSettings): """ diff --git a/api/controllers/common/errors.py b/api/controllers/common/errors.py index 6e2ea952fc..252cf3549a 100644 --- a/api/controllers/common/errors.py +++ b/api/controllers/common/errors.py @@ -25,6 +25,12 @@ class UnsupportedFileTypeError(BaseHTTPException): code = 415 +class BlockedFileExtensionError(BaseHTTPException): + error_code = "file_extension_blocked" + description = "The file extension is blocked for security reasons." + code = 400 + + class TooManyFilesError(BaseHTTPException): error_code = "too_many_files" description = "Only one file is allowed." diff --git a/api/controllers/console/files.py b/api/controllers/console/files.py index 36fcd460bb..fdd7c2f479 100644 --- a/api/controllers/console/files.py +++ b/api/controllers/console/files.py @@ -8,6 +8,7 @@ import services from configs import dify_config from constants import DOCUMENT_EXTENSIONS from controllers.common.errors import ( + BlockedFileExtensionError, FilenameNotExistsError, FileTooLargeError, NoFileUploadedError, @@ -83,6 +84,8 @@ class FileApi(Resource): raise FileTooLargeError(file_too_large_error.description) except services.errors.file.UnsupportedFileTypeError: raise UnsupportedFileTypeError() + except services.errors.file.BlockedFileExtensionError as blocked_extension_error: + raise BlockedFileExtensionError(blocked_extension_error.description) return upload_file, 201 diff --git a/api/services/errors/file.py b/api/services/errors/file.py index 29f3f44eec..bf9d65a25b 100644 --- a/api/services/errors/file.py +++ b/api/services/errors/file.py @@ -11,3 +11,7 @@ class FileTooLargeError(BaseServiceError): class UnsupportedFileTypeError(BaseServiceError): pass + + +class BlockedFileExtensionError(BaseServiceError): + description = "File extension '{extension}' is not allowed for security reasons" diff --git a/api/services/file_service.py b/api/services/file_service.py index dd6a829ea2..b0c5a32c9f 100644 --- a/api/services/file_service.py +++ b/api/services/file_service.py @@ -23,7 +23,7 @@ from models import Account from models.enums import CreatorUserRole from models.model import EndUser, UploadFile -from .errors.file import FileTooLargeError, UnsupportedFileTypeError +from .errors.file import BlockedFileExtensionError, FileTooLargeError, UnsupportedFileTypeError PREVIEW_WORDS_LIMIT = 3000 @@ -59,6 +59,10 @@ class FileService: if len(filename) > 200: filename = filename.split(".")[0][:200] + "." + extension + # check if extension is in blacklist + if extension and extension in dify_config.UPLOAD_FILE_EXTENSION_BLACKLIST: + raise BlockedFileExtensionError(f"File extension '.{extension}' is not allowed for security reasons") + if source == "datasets" and extension not in DOCUMENT_EXTENSIONS: raise UnsupportedFileTypeError() diff --git a/api/tests/test_containers_integration_tests/services/test_file_service.py b/api/tests/test_containers_integration_tests/services/test_file_service.py index 4c94e42f3e..93516a0030 100644 --- a/api/tests/test_containers_integration_tests/services/test_file_service.py +++ b/api/tests/test_containers_integration_tests/services/test_file_service.py @@ -11,7 +11,7 @@ from configs import dify_config from models import Account, Tenant from models.enums import CreatorUserRole from models.model import EndUser, UploadFile -from services.errors.file import FileTooLargeError, UnsupportedFileTypeError +from services.errors.file import BlockedFileExtensionError, FileTooLargeError, UnsupportedFileTypeError from services.file_service import FileService @@ -943,3 +943,150 @@ class TestFileService: # Should have the signed URL when source_url is empty assert upload_file2.source_url == "https://example.com/signed-url" + + # Test file extension blacklist + def test_upload_file_blocked_extension( + self, db_session_with_containers, engine, mock_external_service_dependencies + ): + """ + Test file upload with blocked extension. + """ + fake = Faker() + account = self._create_test_account(db_session_with_containers, mock_external_service_dependencies) + + # Mock blacklist configuration by patching the inner field + with patch.object(dify_config, "inner_UPLOAD_FILE_EXTENSION_BLACKLIST", "exe,bat,sh"): + filename = "malware.exe" + content = b"test content" + mimetype = "application/x-msdownload" + + with pytest.raises(BlockedFileExtensionError): + FileService(engine).upload_file( + filename=filename, + content=content, + mimetype=mimetype, + user=account, + ) + + def test_upload_file_blocked_extension_case_insensitive( + self, db_session_with_containers, engine, mock_external_service_dependencies + ): + """ + Test file upload with blocked extension (case insensitive). + """ + fake = Faker() + account = self._create_test_account(db_session_with_containers, mock_external_service_dependencies) + + # Mock blacklist configuration by patching the inner field + with patch.object(dify_config, "inner_UPLOAD_FILE_EXTENSION_BLACKLIST", "exe,bat"): + # Test with uppercase extension + filename = "malware.EXE" + content = b"test content" + mimetype = "application/x-msdownload" + + with pytest.raises(BlockedFileExtensionError): + FileService(engine).upload_file( + filename=filename, + content=content, + mimetype=mimetype, + user=account, + ) + + def test_upload_file_not_in_blacklist(self, db_session_with_containers, engine, mock_external_service_dependencies): + """ + Test file upload with extension not in blacklist. + """ + fake = Faker() + account = self._create_test_account(db_session_with_containers, mock_external_service_dependencies) + + # Mock blacklist configuration by patching the inner field + with patch.object(dify_config, "inner_UPLOAD_FILE_EXTENSION_BLACKLIST", "exe,bat,sh"): + filename = "document.pdf" + content = b"test content" + mimetype = "application/pdf" + + upload_file = FileService(engine).upload_file( + filename=filename, + content=content, + mimetype=mimetype, + user=account, + ) + + assert upload_file is not None + assert upload_file.name == filename + assert upload_file.extension == "pdf" + + def test_upload_file_empty_blacklist(self, db_session_with_containers, engine, mock_external_service_dependencies): + """ + Test file upload with empty blacklist (default behavior). + """ + fake = Faker() + account = self._create_test_account(db_session_with_containers, mock_external_service_dependencies) + + # Mock empty blacklist configuration by patching the inner field + with patch.object(dify_config, "inner_UPLOAD_FILE_EXTENSION_BLACKLIST", ""): + # Should allow all file types when blacklist is empty + filename = "script.sh" + content = b"#!/bin/bash\necho test" + mimetype = "application/x-sh" + + upload_file = FileService(engine).upload_file( + filename=filename, + content=content, + mimetype=mimetype, + user=account, + ) + + assert upload_file is not None + assert upload_file.extension == "sh" + + def test_upload_file_multiple_blocked_extensions( + self, db_session_with_containers, engine, mock_external_service_dependencies + ): + """ + Test file upload with multiple blocked extensions. + """ + fake = Faker() + account = self._create_test_account(db_session_with_containers, mock_external_service_dependencies) + + # Mock blacklist with multiple extensions by patching the inner field + blacklist_str = "exe,bat,cmd,com,scr,vbs,ps1,msi,dll" + with patch.object(dify_config, "inner_UPLOAD_FILE_EXTENSION_BLACKLIST", blacklist_str): + for ext in blacklist_str.split(","): + filename = f"malware.{ext}" + content = b"test content" + mimetype = "application/octet-stream" + + with pytest.raises(BlockedFileExtensionError): + FileService(engine).upload_file( + filename=filename, + content=content, + mimetype=mimetype, + user=account, + ) + + def test_upload_file_no_extension_with_blacklist( + self, db_session_with_containers, engine, mock_external_service_dependencies + ): + """ + Test file upload with no extension when blacklist is configured. + """ + fake = Faker() + account = self._create_test_account(db_session_with_containers, mock_external_service_dependencies) + + # Mock blacklist configuration by patching the inner field + with patch.object(dify_config, "inner_UPLOAD_FILE_EXTENSION_BLACKLIST", "exe,bat"): + # Files with no extension should not be blocked + filename = "README" + content = b"test content" + mimetype = "text/plain" + + upload_file = FileService(engine).upload_file( + filename=filename, + content=content, + mimetype=mimetype, + user=account, + ) + + assert upload_file is not None + assert upload_file.extension == "" diff --git a/docker/.env.example b/docker/.env.example index 386e328d99..7c7a938fd9 100644 --- a/docker/.env.example +++ b/docker/.env.example @@ -762,6 +762,12 @@ UPLOAD_FILE_SIZE_LIMIT=15 # The maximum number of files that can be uploaded at a time, default 5. UPLOAD_FILE_BATCH_LIMIT=5 +# Comma-separated list of file extensions blocked from upload for security reasons. +# Extensions should be lowercase without dots (e.g., exe,bat,sh,dll). +# Empty by default to allow all file types. +# Recommended: exe,bat,cmd,com,scr,vbs,ps1,msi,dll +UPLOAD_FILE_EXTENSION_BLACKLIST= + # ETL type, support: `dify`, `Unstructured` # `dify` Dify's proprietary file extraction scheme # `Unstructured` Unstructured.io file extraction scheme diff --git a/docker/docker-compose.yaml b/docker/docker-compose.yaml index cc69c13ce2..fda53f8d93 100644 --- a/docker/docker-compose.yaml +++ b/docker/docker-compose.yaml @@ -353,6 +353,7 @@ x-shared-env: &shared-api-worker-env CLICKZETTA_VECTOR_DISTANCE_FUNCTION: ${CLICKZETTA_VECTOR_DISTANCE_FUNCTION:-cosine_distance} UPLOAD_FILE_SIZE_LIMIT: ${UPLOAD_FILE_SIZE_LIMIT:-15} UPLOAD_FILE_BATCH_LIMIT: ${UPLOAD_FILE_BATCH_LIMIT:-5} + UPLOAD_FILE_EXTENSION_BLACKLIST: ${UPLOAD_FILE_EXTENSION_BLACKLIST:-} ETL_TYPE: ${ETL_TYPE:-dify} UNSTRUCTURED_API_URL: ${UNSTRUCTURED_API_URL:-} UNSTRUCTURED_API_KEY: ${UNSTRUCTURED_API_KEY:-} diff --git a/web/app/components/base/file-uploader/hooks.ts b/web/app/components/base/file-uploader/hooks.ts index 3f4d4a6b06..9675123fe7 100644 --- a/web/app/components/base/file-uploader/hooks.ts +++ b/web/app/components/base/file-uploader/hooks.ts @@ -11,6 +11,7 @@ import type { FileEntity } from './types' import { useFileStore } from './store' import { fileUpload, + getFileUploadErrorMessage, getSupportFileType, isAllowedFileExtension, } from './utils' @@ -172,8 +173,9 @@ export const useFile = (fileConfig: FileUpload) => { onSuccessCallback: (res) => { handleUpdateFile({ ...uploadingFile, uploadedId: res.id, progress: 100 }) }, - onErrorCallback: () => { - notify({ type: 'error', message: t('common.fileUploader.uploadFromComputerUploadError') }) + onErrorCallback: (error?: any) => { + const errorMessage = getFileUploadErrorMessage(error, t('common.fileUploader.uploadFromComputerUploadError'), t) + notify({ type: 'error', message: errorMessage }) handleUpdateFile({ ...uploadingFile, progress: -1 }) }, }, !!params.token) @@ -279,8 +281,9 @@ export const useFile = (fileConfig: FileUpload) => { onSuccessCallback: (res) => { handleUpdateFile({ ...uploadingFile, uploadedId: res.id, progress: 100 }) }, - onErrorCallback: () => { - notify({ type: 'error', message: t('common.fileUploader.uploadFromComputerUploadError') }) + onErrorCallback: (error?: any) => { + const errorMessage = getFileUploadErrorMessage(error, t('common.fileUploader.uploadFromComputerUploadError'), t) + notify({ type: 'error', message: errorMessage }) handleUpdateFile({ ...uploadingFile, progress: -1 }) }, }, !!params.token) diff --git a/web/app/components/base/file-uploader/utils.ts b/web/app/components/base/file-uploader/utils.ts index 9c217646ca..e0a1a0250f 100644 --- a/web/app/components/base/file-uploader/utils.ts +++ b/web/app/components/base/file-uploader/utils.ts @@ -7,11 +7,30 @@ import { SupportUploadFileTypes } from '@/app/components/workflow/types' import type { FileResponse } from '@/types/workflow' import { TransferMethod } from '@/types/app' +/** + * Get appropriate error message for file upload errors + * @param error - The error object from upload failure + * @param defaultMessage - Default error message to use if no specific error is matched + * @param t - Translation function + * @returns Localized error message + */ +export const getFileUploadErrorMessage = (error: any, defaultMessage: string, t: (key: string) => string): string => { + const errorCode = error?.response?.code + + if (errorCode === 'forbidden') + return error?.response?.message + + if (errorCode === 'file_extension_blocked') + return t('common.fileUploader.fileExtensionBlocked') + + return defaultMessage +} + type FileUploadParams = { file: File onProgressCallback: (progress: number) => void onSuccessCallback: (res: { id: string }) => void - onErrorCallback: () => void + onErrorCallback: (error?: any) => void } type FileUpload = (v: FileUploadParams, isPublic?: boolean, url?: string) => void export const fileUpload: FileUpload = ({ @@ -37,8 +56,8 @@ export const fileUpload: FileUpload = ({ .then((res: { id: string }) => { onSuccessCallback(res) }) - .catch(() => { - onErrorCallback() + .catch((error) => { + onErrorCallback(error) }) } diff --git a/web/app/components/base/image-uploader/hooks.ts b/web/app/components/base/image-uploader/hooks.ts index 41074000a2..524e86cc1b 100644 --- a/web/app/components/base/image-uploader/hooks.ts +++ b/web/app/components/base/image-uploader/hooks.ts @@ -2,7 +2,7 @@ import { useCallback, useMemo, useRef, useState } from 'react' import type { ClipboardEvent } from 'react' import { useParams } from 'next/navigation' import { useTranslation } from 'react-i18next' -import { imageUpload } from './utils' +import { getImageUploadErrorMessage, imageUpload } from './utils' import { useToastContext } from '@/app/components/base/toast' import { ALLOW_FILE_EXTENSIONS, TransferMethod } from '@/types/app' import type { ImageFile, VisionSettings } from '@/types/app' @@ -81,8 +81,9 @@ export const useImageFiles = () => { filesRef.current = newFiles setFiles(newFiles) }, - onErrorCallback: () => { - notify({ type: 'error', message: t('common.imageUploader.uploadFromComputerUploadError') }) + onErrorCallback: (error?: any) => { + const errorMessage = getImageUploadErrorMessage(error, t('common.imageUploader.uploadFromComputerUploadError'), t) + notify({ type: 'error', message: errorMessage }) const newFiles = [...files.slice(0, index), { ...currentImageFile, progress: -1 }, ...files.slice(index + 1)] filesRef.current = newFiles setFiles(newFiles) @@ -158,8 +159,9 @@ export const useLocalFileUploader = ({ limit, disabled = false, onUpload }: useL onSuccessCallback: (res) => { onUpload({ ...imageFile, fileId: res.id, progress: 100 }) }, - onErrorCallback: () => { - notify({ type: 'error', message: t('common.imageUploader.uploadFromComputerUploadError') }) + onErrorCallback: (error?: any) => { + const errorMessage = getImageUploadErrorMessage(error, t('common.imageUploader.uploadFromComputerUploadError'), t) + notify({ type: 'error', message: errorMessage }) onUpload({ ...imageFile, progress: -1 }) }, }, !!params.token) diff --git a/web/app/components/base/image-uploader/utils.ts b/web/app/components/base/image-uploader/utils.ts index 0c1ada747d..3579d0541e 100644 --- a/web/app/components/base/image-uploader/utils.ts +++ b/web/app/components/base/image-uploader/utils.ts @@ -1,10 +1,29 @@ import { upload } from '@/service/base' +/** + * Get appropriate error message for image upload errors + * @param error - The error object from upload failure + * @param defaultMessage - Default error message to use if no specific error is matched + * @param t - Translation function + * @returns Localized error message + */ +export const getImageUploadErrorMessage = (error: any, defaultMessage: string, t: (key: string) => string): string => { + const errorCode = error?.response?.code + + if (errorCode === 'forbidden') + return error?.response?.message + + if (errorCode === 'file_extension_blocked') + return t('common.fileUploader.fileExtensionBlocked') + + return defaultMessage +} + type ImageUploadParams = { file: File onProgressCallback: (progress: number) => void onSuccessCallback: (res: { id: string }) => void - onErrorCallback: () => void + onErrorCallback: (error?: any) => void } type ImageUpload = (v: ImageUploadParams, isPublic?: boolean, url?: string) => void export const imageUpload: ImageUpload = ({ @@ -30,7 +49,7 @@ export const imageUpload: ImageUpload = ({ .then((res: { id: string }) => { onSuccessCallback(res) }) - .catch(() => { - onErrorCallback() + .catch((error) => { + onErrorCallback(error) }) } diff --git a/web/app/components/custom/custom-web-app-brand/index.tsx b/web/app/components/custom/custom-web-app-brand/index.tsx index eb06265042..fee0bf75f7 100644 --- a/web/app/components/custom/custom-web-app-brand/index.tsx +++ b/web/app/components/custom/custom-web-app-brand/index.tsx @@ -16,7 +16,7 @@ import Button from '@/app/components/base/button' import Divider from '@/app/components/base/divider' import { useProviderContext } from '@/context/provider-context' import { Plan } from '@/app/components/billing/type' -import { imageUpload } from '@/app/components/base/image-uploader/utils' +import { getImageUploadErrorMessage, imageUpload } from '@/app/components/base/image-uploader/utils' import { useToastContext } from '@/app/components/base/toast' import { BubbleTextMod } from '@/app/components/base/icons/src/vender/solid/communication' import { @@ -67,8 +67,9 @@ const CustomWebAppBrand = () => { setUploadProgress(100) setFileId(res.id) }, - onErrorCallback: () => { - notify({ type: 'error', message: t('common.imageUploader.uploadFromComputerUploadError') }) + onErrorCallback: (error?: any) => { + const errorMessage = getImageUploadErrorMessage(error, t('common.imageUploader.uploadFromComputerUploadError'), t) + notify({ type: 'error', message: errorMessage }) setUploadProgress(-1) }, }, false, '/workspaces/custom-config/webapp-logo/upload') diff --git a/web/app/components/datasets/create/file-uploader/index.tsx b/web/app/components/datasets/create/file-uploader/index.tsx index 75557b37c9..aee2192b6c 100644 --- a/web/app/components/datasets/create/file-uploader/index.tsx +++ b/web/app/components/datasets/create/file-uploader/index.tsx @@ -18,6 +18,7 @@ import { LanguagesSupported } from '@/i18n-config/language' import { IS_CE_EDITION } from '@/config' import { Theme } from '@/types/app' import useTheme from '@/hooks/use-theme' +import { getFileUploadErrorMessage } from '@/app/components/base/file-uploader/utils' type IFileUploaderProps = { fileList: FileItem[] @@ -134,7 +135,8 @@ const FileUploader = ({ return Promise.resolve({ ...completeFile }) }) .catch((e) => { - notify({ type: 'error', message: e?.response?.code === 'forbidden' ? e?.response?.message : t('datasetCreation.stepOne.uploader.failed') }) + const errorMessage = getFileUploadErrorMessage(e, t('datasetCreation.stepOne.uploader.failed'), t) + notify({ type: 'error', message: errorMessage }) onFileUpdate(fileItem, -2, fileListRef.current) return Promise.resolve({ ...fileItem }) }) diff --git a/web/app/components/datasets/documents/create-from-pipeline/data-source/local-file/index.tsx b/web/app/components/datasets/documents/create-from-pipeline/data-source/local-file/index.tsx index 361378362e..868621e1a3 100644 --- a/web/app/components/datasets/documents/create-from-pipeline/data-source/local-file/index.tsx +++ b/web/app/components/datasets/documents/create-from-pipeline/data-source/local-file/index.tsx @@ -8,6 +8,7 @@ import cn from '@/utils/classnames' import type { CustomFile as File, FileItem } from '@/models/datasets' import { ToastContext } from '@/app/components/base/toast' import { upload } from '@/service/base' +import { getFileUploadErrorMessage } from '@/app/components/base/file-uploader/utils' import I18n from '@/context/i18n' import { LanguagesSupported } from '@/i18n-config/language' import { IS_CE_EDITION } from '@/config' @@ -154,7 +155,8 @@ const LocalFile = ({ return Promise.resolve({ ...completeFile }) }) .catch((e) => { - notify({ type: 'error', message: e?.response?.code === 'forbidden' ? e?.response?.message : t('datasetCreation.stepOne.uploader.failed') }) + const errorMessage = getFileUploadErrorMessage(e, t('datasetCreation.stepOne.uploader.failed'), t) + notify({ type: 'error', message: errorMessage }) updateFile(fileItem, -2, fileListRef.current) return Promise.resolve({ ...fileItem }) }) diff --git a/web/app/components/datasets/documents/detail/batch-modal/csv-uploader.tsx b/web/app/components/datasets/documents/detail/batch-modal/csv-uploader.tsx index 96cab11c9c..317db84c43 100644 --- a/web/app/components/datasets/documents/detail/batch-modal/csv-uploader.tsx +++ b/web/app/components/datasets/documents/detail/batch-modal/csv-uploader.tsx @@ -12,6 +12,7 @@ import { ToastContext } from '@/app/components/base/toast' import Button from '@/app/components/base/button' import type { FileItem } from '@/models/datasets' import { upload } from '@/service/base' +import { getFileUploadErrorMessage } from '@/app/components/base/file-uploader/utils' import useSWR from 'swr' import { fetchFileUploadConfig } from '@/service/common' import SimplePieChart from '@/app/components/base/simple-pie-chart' @@ -74,7 +75,8 @@ const CSVUploader: FC = ({ return Promise.resolve({ ...completeFile }) }) .catch((e) => { - notify({ type: 'error', message: e?.response?.code === 'forbidden' ? e?.response?.message : t('datasetCreation.stepOne.uploader.failed') }) + const errorMessage = getFileUploadErrorMessage(e, t('datasetCreation.stepOne.uploader.failed'), t) + notify({ type: 'error', message: errorMessage }) const errorFile = { ...fileItem, progress: -2, diff --git a/web/i18n/en-US/common.ts b/web/i18n/en-US/common.ts index d01fe0f642..a18b3dd4a6 100644 --- a/web/i18n/en-US/common.ts +++ b/web/i18n/en-US/common.ts @@ -734,6 +734,7 @@ const translation = { uploadFromComputerLimit: 'Upload {{type}} cannot exceed {{size}}', pasteFileLinkInvalid: 'Invalid file link', fileExtensionNotSupport: 'File extension not supported', + fileExtensionBlocked: 'This file type is blocked for security reasons', }, tag: { placeholder: 'All Tags', diff --git a/web/i18n/zh-Hans/common.ts b/web/i18n/zh-Hans/common.ts index 89ab567fed..bae15d71a9 100644 --- a/web/i18n/zh-Hans/common.ts +++ b/web/i18n/zh-Hans/common.ts @@ -728,6 +728,7 @@ const translation = { uploadFromComputerLimit: '上传 {{type}} 不能超过 {{size}}', pasteFileLinkInvalid: '文件链接无效', fileExtensionNotSupport: '文件类型不支持', + fileExtensionBlocked: '出于安全考虑，该文件类型已被禁止上传', }, tag: { placeholder: '全部标签', From 829796514a102af997b3e0bd8f8d1e2bb6d522dc Mon Sep 17 00:00:00 2001 From: zhengchangchun <31502830+zhengchangchun@users.noreply.github.com> Date: Tue, 4 Nov 2025 16:40:44 +0800 Subject: [PATCH 02/18] =?UTF-8?q?fix:knowledge=20base=20reference=20inform?= =?UTF-8?q?ation=20is=20overwritten=20when=20using=20mu=E2=80=A6=20(#27799?= =?UTF-8?q?)?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Co-authored-by: zhengchangchun Co-authored-by: autofix-ci[bot] <114827586+autofix-ci[bot]@users.noreply.github.com> --- .../task_pipeline/message_cycle_manager.py | 22 ++++++++++++++++++- 1 file changed, 21 insertions(+), 1 deletion(-) diff --git a/api/core/app/task_pipeline/message_cycle_manager.py b/api/core/app/task_pipeline/message_cycle_manager.py index 7a384e5c92..e7daeb4a32 100644 --- a/api/core/app/task_pipeline/message_cycle_manager.py +++ b/api/core/app/task_pipeline/message_cycle_manager.py @@ -140,7 +140,27 @@ class MessageCycleManager: if not self._application_generate_entity.app_config.additional_features: raise ValueError("Additional features not found") if self._application_generate_entity.app_config.additional_features.show_retrieve_source: - self._task_state.metadata.retriever_resources = event.retriever_resources + merged_resources = [r for r in self._task_state.metadata.retriever_resources or [] if r] + existing_ids = {(r.dataset_id, r.document_id) for r in merged_resources if r.dataset_id and r.document_id} + + # Add new unique resources from the event + for resource in event.retriever_resources or []: + if not resource: + continue + + is_duplicate = ( + resource.dataset_id + and resource.document_id + and (resource.dataset_id, resource.document_id) in existing_ids + ) + + if not is_duplicate: + merged_resources.append(resource) + + for i, resource in enumerate(merged_resources, 1): + resource.position = i + + self._task_state.metadata.retriever_resources = merged_resources def message_file_to_stream_response(self, event: QueueMessageFileEvent) -> MessageFileStreamResponse | None: """ From e9738b891fe316b2b44abefbe5642aad412a8db1 Mon Sep 17 00:00:00 2001 From: aka James4u Date: Tue, 4 Nov 2025 05:06:44 -0800 Subject: [PATCH 03/18] test: adding some web tests (#27792) --- web/__tests__/check-i18n.test.ts | 100 ++++++++ web/__tests__/navigation-utils.test.ts | 112 +++++++++ web/service/_tools_util.spec.ts | 36 +++ web/utils/clipboard.spec.ts | 109 +++++++++ web/utils/emoji.spec.ts | 77 +++++++ web/utils/format.spec.ts | 94 +++++++- web/utils/index.spec.ts | 305 +++++++++++++++++++++++++ web/utils/urlValidation.spec.ts | 49 ++++ web/utils/validators.spec.ts | 139 +++++++++++ web/utils/var.spec.ts | 236 +++++++++++++++++++ 10 files changed, 1256 insertions(+), 1 deletion(-) create mode 100644 web/utils/clipboard.spec.ts create mode 100644 web/utils/emoji.spec.ts create mode 100644 web/utils/urlValidation.spec.ts create mode 100644 web/utils/validators.spec.ts create mode 100644 web/utils/var.spec.ts diff --git a/web/__tests__/check-i18n.test.ts b/web/__tests__/check-i18n.test.ts index b579f22d4b..7773edcdbb 100644 --- a/web/__tests__/check-i18n.test.ts +++ b/web/__tests__/check-i18n.test.ts @@ -759,4 +759,104 @@ export default translation` expect(result).not.toContain('Zbuduj inteligentnego agenta') }) }) + + describe('Performance and Scalability', () => { + it('should handle large translation files efficiently', async () => { + // Create a large translation file with 1000 keys + const largeContent = `const translation = { +${Array.from({ length: 1000 }, (_, i) => ` key${i}: 'value${i}',`).join('\n')} +} + +export default translation` + + fs.writeFileSync(path.join(testEnDir, 'large.ts'), largeContent) + + const startTime = Date.now() + const keys = await getKeysFromLanguage('en-US') + const endTime = Date.now() + + expect(keys.length).toBe(1000) + expect(endTime - startTime).toBeLessThan(1000) // Should complete in under 1 second + }) + + it('should handle multiple translation files concurrently', async () => { + // Create multiple files + for (let i = 0; i < 10; i++) { + const content = `const translation = { + key${i}: 'value${i}', + nested${i}: { + subkey: 'subvalue' + } +} + +export default translation` + fs.writeFileSync(path.join(testEnDir, `file${i}.ts`), content) + } + + const startTime = Date.now() + const keys = await getKeysFromLanguage('en-US') + const endTime = Date.now() + + expect(keys.length).toBe(20) // 10 files * 2 keys each + expect(endTime - startTime).toBeLessThan(500) + }) + }) + + describe('Unicode and Internationalization', () => { + it('should handle Unicode characters in keys and values', async () => { + const unicodeContent = `const translation = { + '中文键': '中文值', + 'العربية': 'قيمة', + 'emoji_😀': 'value with emoji 🎉', + 'mixed_中文_English': 'mixed value' +} + +export default translation` + + fs.writeFileSync(path.join(testEnDir, 'unicode.ts'), unicodeContent) + + const keys = await getKeysFromLanguage('en-US') + + expect(keys).toContain('unicode.中文键') + expect(keys).toContain('unicode.العربية') + expect(keys).toContain('unicode.emoji_😀') + expect(keys).toContain('unicode.mixed_中文_English') + }) + + it('should handle RTL language files', async () => { + const rtlContent = `const translation = { + مرحبا: 'Hello', + العالم: 'World', + nested: { + مفتاح: 'key' + } +} + +export default translation` + + fs.writeFileSync(path.join(testEnDir, 'rtl.ts'), rtlContent) + + const keys = await getKeysFromLanguage('en-US') + + expect(keys).toContain('rtl.مرحبا') + expect(keys).toContain('rtl.العالم') + expect(keys).toContain('rtl.nested.مفتاح') + }) + }) + + describe('Error Recovery', () => { + it('should handle syntax errors in translation files gracefully', async () => { + const invalidContent = `const translation = { + validKey: 'valid value', + invalidKey: 'missing quote, + anotherKey: 'another value' +} + +export default translation` + + fs.writeFileSync(path.join(testEnDir, 'invalid.ts'), invalidContent) + + await expect(getKeysFromLanguage('en-US')).rejects.toThrow() + }) + }) }) diff --git a/web/__tests__/navigation-utils.test.ts b/web/__tests__/navigation-utils.test.ts index fa4986e63d..3eeba52943 100644 --- a/web/__tests__/navigation-utils.test.ts +++ b/web/__tests__/navigation-utils.test.ts @@ -286,4 +286,116 @@ describe('Navigation Utilities', () => { expect(mockPush).toHaveBeenCalledWith('/datasets/filtered-set/documents?page=1&limit=50&status=active&type=pdf&sort=created_at&order=desc') }) }) + + describe('Edge Cases and Error Handling', () => { + test('handles special characters in query parameters', () => { + Object.defineProperty(window, 'location', { + value: { search: '?keyword=hello%20world&filter=type%3Apdf&tag=%E4%B8%AD%E6%96%87' }, + writable: true, + }) + + const path = createNavigationPath('/datasets/123/documents') + expect(path).toContain('hello+world') + expect(path).toContain('type%3Apdf') + expect(path).toContain('%E4%B8%AD%E6%96%87') + }) + + test('handles duplicate query parameters', () => { + Object.defineProperty(window, 'location', { + value: { search: '?tag=tag1&tag=tag2&tag=tag3' }, + writable: true, + }) + + const params = extractQueryParams(['tag']) + // URLSearchParams.get() returns the first value + expect(params.tag).toBe('tag1') + }) + + test('handles very long query strings', () => { + const longValue = 'a'.repeat(1000) + Object.defineProperty(window, 'location', { + value: { search: `?data=${longValue}` }, + writable: true, + }) + + const path = createNavigationPath('/datasets/123/documents') + expect(path).toContain(longValue) + expect(path.length).toBeGreaterThan(1000) + }) + + test('handles empty string values in query parameters', () => { + const path = createNavigationPathWithParams('/datasets/123/documents', { + page: 1, + keyword: '', + filter: '', + sort: 'name', + }) + + expect(path).toBe('/datasets/123/documents?page=1&sort=name') + expect(path).not.toContain('keyword=') + expect(path).not.toContain('filter=') + }) + + test('handles null and undefined values in mergeQueryParams', () => { + Object.defineProperty(window, 'location', { + value: { search: '?page=1&limit=10&keyword=test' }, + writable: true, + }) + + const merged = mergeQueryParams({ + keyword: null, + filter: undefined, + sort: 'name', + }) + const result = merged.toString() + + expect(result).toContain('page=1') + expect(result).toContain('limit=10') + expect(result).not.toContain('keyword') + expect(result).toContain('sort=name') + }) + + test('handles navigation with hash fragments', () => { + Object.defineProperty(window, 'location', { + value: { search: '?page=1', hash: '#section-2' }, + writable: true, + }) + + const path = createNavigationPath('/datasets/123/documents') + // Should preserve query params but not hash + expect(path).toBe('/datasets/123/documents?page=1') + }) + + test('handles malformed query strings gracefully', () => { + Object.defineProperty(window, 'location', { + value: { search: '?page=1&invalid&limit=10&=value&key=' }, + writable: true, + }) + + const params = extractQueryParams(['page', 'limit', 'invalid', 'key']) + expect(params.page).toBe('1') + expect(params.limit).toBe('10') + // Malformed params should be handled by URLSearchParams + expect(params.invalid).toBe('') // for `&invalid` + expect(params.key).toBe('') // for `&key=` + }) + }) + + describe('Performance Tests', () => { + test('handles large number of query parameters efficiently', () => { + const manyParams = Array.from({ length: 50 }, (_, i) => `param${i}=value${i}`).join('&') + Object.defineProperty(window, 'location', { + value: { search: `?${manyParams}` }, + writable: true, + }) + + const startTime = Date.now() + const path = createNavigationPath('/datasets/123/documents') + const endTime = Date.now() + + expect(endTime - startTime).toBeLessThan(50) // Should be fast + expect(path).toContain('param0=value0') + expect(path).toContain('param49=value49') + }) + }) }) diff --git a/web/service/_tools_util.spec.ts b/web/service/_tools_util.spec.ts index f06e5a1e34..658c276df1 100644 --- a/web/service/_tools_util.spec.ts +++ b/web/service/_tools_util.spec.ts @@ -14,3 +14,39 @@ describe('makeProviderQuery', () => { expect(buildProviderQuery('ABC?DEF')).toBe('provider=ABC%3FDEF') }) }) + +describe('Tools Utilities', () => { + describe('buildProviderQuery', () => { + it('should build query string with provider parameter', () => { + const result = buildProviderQuery('openai') + expect(result).toBe('provider=openai') + }) + + it('should handle provider names with special characters', () => { + const result = buildProviderQuery('provider-name') + expect(result).toBe('provider=provider-name') + }) + + it('should handle empty string', () => { + const result = buildProviderQuery('') + expect(result).toBe('provider=') + }) + + it('should URL encode special characters', () => { + const result = buildProviderQuery('provider name') + expect(result).toBe('provider=provider+name') + }) + + it('should handle Unicode characters', () => { + const result = buildProviderQuery('提供者') + expect(result).toContain('provider=') + expect(decodeURIComponent(result)).toBe('provider=提供者') + }) + + it('should handle provider names with slashes', () => { + const result = buildProviderQuery('langgenius/openai/gpt-4') + expect(result).toContain('provider=') + expect(decodeURIComponent(result)).toBe('provider=langgenius/openai/gpt-4') + }) + }) +}) diff --git a/web/utils/clipboard.spec.ts b/web/utils/clipboard.spec.ts new file mode 100644 index 0000000000..ccdafe83f4 --- /dev/null +++ b/web/utils/clipboard.spec.ts @@ -0,0 +1,109 @@ +import { writeTextToClipboard } from './clipboard' + +describe('Clipboard Utilities', () => { + describe('writeTextToClipboard', () => { + afterEach(() => { + jest.restoreAllMocks() + }) + + it('should use navigator.clipboard.writeText when available', async () => { + const mockWriteText = jest.fn().mockResolvedValue(undefined) + Object.defineProperty(navigator, 'clipboard', { + value: { writeText: mockWriteText }, + writable: true, + configurable: true, + }) + + await writeTextToClipboard('test text') + expect(mockWriteText).toHaveBeenCalledWith('test text') + }) + + it('should fallback to execCommand when clipboard API not available', async () => { + Object.defineProperty(navigator, 'clipboard', { + value: undefined, + writable: true, + configurable: true, + }) + + const mockExecCommand = jest.fn().mockReturnValue(true) + document.execCommand = mockExecCommand + + const appendChildSpy = jest.spyOn(document.body, 'appendChild') + const removeChildSpy = jest.spyOn(document.body, 'removeChild') + + await writeTextToClipboard('fallback text') + + expect(appendChildSpy).toHaveBeenCalled() + expect(mockExecCommand).toHaveBeenCalledWith('copy') + expect(removeChildSpy).toHaveBeenCalled() + }) + + it('should handle execCommand failure', async () => { + Object.defineProperty(navigator, 'clipboard', { + value: undefined, + writable: true, + configurable: true, + }) + + const mockExecCommand = jest.fn().mockReturnValue(false) + document.execCommand = mockExecCommand + + await expect(writeTextToClipboard('fail text')).rejects.toThrow() + }) + + it('should handle execCommand exception', async () => { + Object.defineProperty(navigator, 'clipboard', { + value: undefined, + writable: true, + configurable: true, + }) + + const mockExecCommand = jest.fn().mockImplementation(() => { + throw new Error('execCommand error') + }) + document.execCommand = mockExecCommand + + await expect(writeTextToClipboard('error text')).rejects.toThrow('execCommand error') + }) + + it('should clean up textarea after fallback', async () => { + Object.defineProperty(navigator, 'clipboard', { + value: undefined, + writable: true, + configurable: true, + }) + + document.execCommand = jest.fn().mockReturnValue(true) + const removeChildSpy = jest.spyOn(document.body, 'removeChild') + + await writeTextToClipboard('cleanup test') + + expect(removeChildSpy).toHaveBeenCalled() + }) + + it('should handle empty string', async () => { + const mockWriteText = jest.fn().mockResolvedValue(undefined) + Object.defineProperty(navigator, 'clipboard', { + value: { writeText: mockWriteText }, + writable: true, + configurable: true, + }) + + await writeTextToClipboard('') + expect(mockWriteText).toHaveBeenCalledWith('') + }) + + it('should handle special characters', async () => { + const mockWriteText = jest.fn().mockResolvedValue(undefined) + Object.defineProperty(navigator, 'clipboard', { + value: { writeText: mockWriteText }, + writable: true, + configurable: true, + }) + + const specialText = 'Test\n\t"quotes"\n中文\n😀' + await writeTextToClipboard(specialText) + expect(mockWriteText).toHaveBeenCalledWith(specialText) + }) + }) +}) diff --git a/web/utils/emoji.spec.ts b/web/utils/emoji.spec.ts new file mode 100644 index 0000000000..df9520234a --- /dev/null +++ b/web/utils/emoji.spec.ts @@ -0,0 +1,77 @@ +import { searchEmoji } from './emoji' +import { SearchIndex } from 'emoji-mart' + +jest.mock('emoji-mart', () => ({ + SearchIndex: { + search: jest.fn(), + }, +})) + +describe('Emoji Utilities', () => { + describe('searchEmoji', () => { + beforeEach(() => { + jest.clearAllMocks() + }) + + it('should return emoji natives for search results', async () => { + const mockEmojis = [ + { skins: [{ native: '😀' }] }, + { skins: [{ native: '😃' }] }, + { skins: [{ native: '😄' }] }, + ] + ;(SearchIndex.search as jest.Mock).mockResolvedValue(mockEmojis) + + const result = await searchEmoji('smile') + expect(result).toEqual(['😀', '😃', '😄']) + }) + + it('should return empty array when no results', async () => { + ;(SearchIndex.search as jest.Mock).mockResolvedValue([]) + + const result = await searchEmoji('nonexistent') + expect(result).toEqual([]) + }) + + it('should return empty array when search returns null', async () => { + ;(SearchIndex.search as jest.Mock).mockResolvedValue(null) + + const result = await searchEmoji('test') + expect(result).toEqual([]) + }) + + it('should handle search with empty string', async () => { + ;(SearchIndex.search as jest.Mock).mockResolvedValue([]) + + const result = await searchEmoji('') + expect(result).toEqual([]) + expect(SearchIndex.search).toHaveBeenCalledWith('') + }) + + it('should extract native from first skin', async () => { + const mockEmojis = [ + { + skins: [ + { native: '👍' }, + { native: '👍🏻' }, + { native: '👍🏼' }, + ], + }, + ] + ;(SearchIndex.search as jest.Mock).mockResolvedValue(mockEmojis) + + const result = await searchEmoji('thumbs') + expect(result).toEqual(['👍']) + }) + + it('should handle multiple search terms', async () => { + const mockEmojis = [ + { skins: [{ native: '❤️' }] }, + { skins: [{ native: '💙' }] }, + ] + ;(SearchIndex.search as jest.Mock).mockResolvedValue(mockEmojis) + + const result = await searchEmoji('heart love') + expect(result).toEqual(['❤️', '💙']) + }) + }) +}) diff --git a/web/utils/format.spec.ts b/web/utils/format.spec.ts index c94495d597..20e54fe1a4 100644 --- a/web/utils/format.spec.ts +++ b/web/utils/format.spec.ts @@ -1,4 +1,4 @@ -import { downloadFile, formatFileSize, formatNumber, formatTime } from './format' +import { downloadFile, formatFileSize, formatNumber, formatNumberAbbreviated, formatTime } from './format' describe('formatNumber', () => { test('should correctly format integers', () => { @@ -102,3 +102,95 @@ describe('downloadFile', () => { jest.restoreAllMocks() }) }) + +describe('formatNumberAbbreviated', () => { + it('should return number as string when less than 1000', () => { + expect(formatNumberAbbreviated(0)).toBe('0') + expect(formatNumberAbbreviated(1)).toBe('1') + expect(formatNumberAbbreviated(999)).toBe('999') + }) + + it('should format thousands with k suffix', () => { + expect(formatNumberAbbreviated(1000)).toBe('1k') + expect(formatNumberAbbreviated(1200)).toBe('1.2k') + expect(formatNumberAbbreviated(1500)).toBe('1.5k') + expect(formatNumberAbbreviated(9999)).toBe('10k') + }) + + it('should format millions with M suffix', () => { + expect(formatNumberAbbreviated(1000000)).toBe('1M') + expect(formatNumberAbbreviated(1500000)).toBe('1.5M') + expect(formatNumberAbbreviated(2300000)).toBe('2.3M') + expect(formatNumberAbbreviated(999999999)).toBe('1000M') + }) + + it('should format billions with B suffix', () => { + expect(formatNumberAbbreviated(1000000000)).toBe('1B') + expect(formatNumberAbbreviated(1500000000)).toBe('1.5B') + expect(formatNumberAbbreviated(2300000000)).toBe('2.3B') + }) + + it('should remove .0 from whole numbers', () => { + expect(formatNumberAbbreviated(1000)).toBe('1k') + expect(formatNumberAbbreviated(2000000)).toBe('2M') + expect(formatNumberAbbreviated(3000000000)).toBe('3B') + }) + + it('should keep decimal for non-whole numbers', () => { + expect(formatNumberAbbreviated(1100)).toBe('1.1k') + expect(formatNumberAbbreviated(1500000)).toBe('1.5M') + expect(formatNumberAbbreviated(2700000000)).toBe('2.7B') + }) + + it('should handle edge cases', () => { + expect(formatNumberAbbreviated(950)).toBe('950') + expect(formatNumberAbbreviated(1001)).toBe('1k') + expect(formatNumberAbbreviated(999999)).toBe('1000k') + }) +}) + +describe('formatNumber edge cases', () => { + it('should handle very large numbers', () => { + expect(formatNumber(1234567890123)).toBe('1,234,567,890,123') + }) + + it('should handle numbers with many decimal places', () => { + expect(formatNumber(1234.56789)).toBe('1,234.56789') + }) + + it('should handle negative decimals', () => { + expect(formatNumber(-1234.56)).toBe('-1,234.56') + }) + + it('should handle string with decimals', () => { + expect(formatNumber('9876543.21')).toBe('9,876,543.21') + }) +}) + +describe('formatFileSize edge cases', () => { + it('should handle exactly 1024 bytes', () => { + expect(formatFileSize(1024)).toBe('1.00 KB') + }) + + it('should handle fractional bytes', () => { + expect(formatFileSize(512.5)).toBe('512.50 bytes') + }) +}) + +describe('formatTime edge cases', () => { + it('should handle exactly 60 seconds', () => { + expect(formatTime(60)).toBe('1.00 min') + }) + + it('should handle exactly 3600 seconds', () => { + expect(formatTime(3600)).toBe('1.00 h') + }) + + it('should handle fractional seconds', () => { + expect(formatTime(45.5)).toBe('45.50 sec') + }) + + it('should handle very large durations', () => { + expect(formatTime(86400)).toBe('24.00 h') // 24 hours + }) +}) diff --git a/web/utils/index.spec.ts b/web/utils/index.spec.ts index 21a0d80dd0..beda974e5c 100644 --- a/web/utils/index.spec.ts +++ b/web/utils/index.spec.ts @@ -293,3 +293,308 @@ describe('removeSpecificQueryParam', () => { expect(replaceStateCall[2]).toMatch(/param3=value3/) }) }) + +describe('sleep', () => { + it('should resolve after specified milliseconds', async () => { + const start = Date.now() + await sleep(100) + const end = Date.now() + expect(end - start).toBeGreaterThanOrEqual(90) // Allow some tolerance + }) + + it('should handle zero milliseconds', async () => { + await expect(sleep(0)).resolves.toBeUndefined() + }) +}) + +describe('asyncRunSafe extended', () => { + it('should handle promise that resolves with null', async () => { + const [error, result] = await asyncRunSafe(Promise.resolve(null)) + expect(error).toBeNull() + expect(result).toBeNull() + }) + + it('should handle promise that resolves with undefined', async () => { + const [error, result] = await asyncRunSafe(Promise.resolve(undefined)) + expect(error).toBeNull() + expect(result).toBeUndefined() + }) + + it('should handle promise that resolves with false', async () => { + const [error, result] = await asyncRunSafe(Promise.resolve(false)) + expect(error).toBeNull() + expect(result).toBe(false) + }) + + it('should handle promise that resolves with 0', async () => { + const [error, result] = await asyncRunSafe(Promise.resolve(0)) + expect(error).toBeNull() + expect(result).toBe(0) + }) + + // TODO: pre-commit blocks this test case + // Error msg: "Expected the Promise rejection reason to be an Error" + + // it('should handle promise that rejects with null', async () => { + // const [error] = await asyncRunSafe(Promise.reject(null)) + // expect(error).toBeInstanceOf(Error) + // expect(error?.message).toBe('unknown error') + // }) +}) + +describe('getTextWidthWithCanvas', () => { + it('should return 0 when canvas context is not available', () => { + const mockGetContext = jest.fn().mockReturnValue(null) + jest.spyOn(document, 'createElement').mockReturnValue({ + getContext: mockGetContext, + } as any) + + const width = getTextWidthWithCanvas('test') + expect(width).toBe(0) + + jest.restoreAllMocks() + }) + + it('should measure text width with custom font', () => { + const mockMeasureText = jest.fn().mockReturnValue({ width: 123.456 }) + const mockContext = { + font: '', + measureText: mockMeasureText, + } + jest.spyOn(document, 'createElement').mockReturnValue({ + getContext: jest.fn().mockReturnValue(mockContext), + } as any) + + const width = getTextWidthWithCanvas('test', '16px Arial') + expect(mockContext.font).toBe('16px Arial') + expect(width).toBe(123.46) + + jest.restoreAllMocks() + }) + + it('should handle empty string', () => { + const mockMeasureText = jest.fn().mockReturnValue({ width: 0 }) + jest.spyOn(document, 'createElement').mockReturnValue({ + getContext: jest.fn().mockReturnValue({ + font: '', + measureText: mockMeasureText, + }), + } as any) + + const width = getTextWidthWithCanvas('') + expect(width).toBe(0) + + jest.restoreAllMocks() + }) +}) + +describe('randomString extended', () => { + it('should generate string of exact length', () => { + expect(randomString(10).length).toBe(10) + expect(randomString(50).length).toBe(50) + expect(randomString(100).length).toBe(100) + }) + + it('should generate different strings on multiple calls', () => { + const str1 = randomString(20) + const str2 = randomString(20) + const str3 = randomString(20) + expect(str1).not.toBe(str2) + expect(str2).not.toBe(str3) + expect(str1).not.toBe(str3) + }) + + it('should only contain valid characters', () => { + const validChars = /^[0-9a-zA-Z_-]+$/ + const str = randomString(100) + expect(validChars.test(str)).toBe(true) + }) + + it('should handle length of 1', () => { + const str = randomString(1) + expect(str.length).toBe(1) + }) + + it('should handle length of 0', () => { + const str = randomString(0) + expect(str).toBe('') + }) +}) + +describe('getPurifyHref extended', () => { + it('should escape HTML entities', () => { + expect(getPurifyHref('')).not.toContain('')).toThrow('Authorization URL must be HTTP or HTTPS') + }) + + it('should reject file: protocol', () => { + expect(() => validateRedirectUrl('file:///etc/passwd')).toThrow('Authorization URL must be HTTP or HTTPS') + }) + + it('should reject ftp: protocol', () => { + expect(() => validateRedirectUrl('ftp://example.com')).toThrow('Authorization URL must be HTTP or HTTPS') + }) + + it('should reject vbscript: protocol', () => { + expect(() => validateRedirectUrl('vbscript:msgbox(1)')).toThrow('Authorization URL must be HTTP or HTTPS') + }) + + it('should reject malformed URLs', () => { + expect(() => validateRedirectUrl('not a url')).toThrow('Invalid URL') + expect(() => validateRedirectUrl('://example.com')).toThrow('Invalid URL') + expect(() => validateRedirectUrl('')).toThrow('Invalid URL') + }) + + it('should handle URLs with query parameters', () => { + expect(() => validateRedirectUrl('https://example.com?param=value')).not.toThrow() + expect(() => validateRedirectUrl('https://example.com?redirect=http://evil.com')).not.toThrow() + }) + + it('should handle URLs with fragments', () => { + expect(() => validateRedirectUrl('https://example.com#section')).not.toThrow() + expect(() => validateRedirectUrl('https://example.com/path#fragment')).not.toThrow() + }) + + it('should handle URLs with authentication', () => { + expect(() => validateRedirectUrl('https://user:pass@example.com')).not.toThrow() + }) + + it('should handle international domain names', () => { + expect(() => validateRedirectUrl('https://例え.jp')).not.toThrow() + }) + + it('should reject protocol-relative URLs', () => { + expect(() => validateRedirectUrl('//example.com')).toThrow('Invalid URL') + }) + }) +}) diff --git a/web/utils/validators.spec.ts b/web/utils/validators.spec.ts new file mode 100644 index 0000000000..b09955d12e --- /dev/null +++ b/web/utils/validators.spec.ts @@ -0,0 +1,139 @@ +import { draft07Validator, forbidBooleanProperties } from './validators' + +describe('Validators', () => { + describe('draft07Validator', () => { + it('should validate a valid JSON schema', () => { + const validSchema = { + type: 'object', + properties: { + name: { type: 'string' }, + age: { type: 'number' }, + }, + } + const result = draft07Validator(validSchema) + expect(result.valid).toBe(true) + expect(result.errors).toHaveLength(0) + }) + + it('should invalidate schema with unknown type', () => { + const invalidSchema = { + type: 'invalid_type', + } + const result = draft07Validator(invalidSchema) + expect(result.valid).toBe(false) + expect(result.errors.length).toBeGreaterThan(0) + }) + + it('should validate nested schemas', () => { + const nestedSchema = { + type: 'object', + properties: { + user: { + type: 'object', + properties: { + name: { type: 'string' }, + address: { + type: 'object', + properties: { + street: { type: 'string' }, + city: { type: 'string' }, + }, + }, + }, + }, + }, + } + const result = draft07Validator(nestedSchema) + expect(result.valid).toBe(true) + }) + + it('should validate array schemas', () => { + const arraySchema = { + type: 'array', + items: { type: 'string' }, + } + const result = draft07Validator(arraySchema) + expect(result.valid).toBe(true) + }) + }) + + describe('forbidBooleanProperties', () => { + it('should return empty array for schema without boolean properties', () => { + const schema = { + properties: { + name: { type: 'string' }, + age: { type: 'number' }, + }, + } + const errors = forbidBooleanProperties(schema) + expect(errors).toHaveLength(0) + }) + + it('should detect boolean property at root level', () => { + const schema = { + properties: { + name: true, + age: { type: 'number' }, + }, + } + const errors = forbidBooleanProperties(schema) + expect(errors).toHaveLength(1) + expect(errors[0]).toContain('name') + }) + + it('should detect boolean properties in nested objects', () => { + const schema = { + properties: { + user: { + properties: { + name: true, + profile: { + properties: { + bio: false, + }, + }, + }, + }, + }, + } + const errors = forbidBooleanProperties(schema) + expect(errors).toHaveLength(2) + expect(errors.some(e => e.includes('user.name'))).toBe(true) + expect(errors.some(e => e.includes('user.profile.bio'))).toBe(true) + }) + + it('should handle schema without properties', () => { + const schema = { type: 'string' } + const errors = forbidBooleanProperties(schema) + expect(errors).toHaveLength(0) + }) + + it('should handle null schema', () => { + const errors = forbidBooleanProperties(null) + expect(errors).toHaveLength(0) + }) + + it('should handle empty schema', () => { + const errors = forbidBooleanProperties({}) + expect(errors).toHaveLength(0) + }) + + it('should provide correct path in error messages', () => { + const schema = { + properties: { + level1: { + properties: { + level2: { + properties: { + level3: true, + }, + }, + }, + }, + }, + } + const errors = forbidBooleanProperties(schema) + expect(errors[0]).toContain('level1.level2.level3') + }) + }) +}) diff --git a/web/utils/var.spec.ts b/web/utils/var.spec.ts new file mode 100644 index 0000000000..6f55df0d34 --- /dev/null +++ b/web/utils/var.spec.ts @@ -0,0 +1,236 @@ +import { + checkKey, + checkKeys, + getMarketplaceUrl, + getNewVar, + getNewVarInWorkflow, + getVars, + hasDuplicateStr, + replaceSpaceWithUnderscoreInVarNameInput, +} from './var' +import { InputVarType } from '@/app/components/workflow/types' + +describe('Variable Utilities', () => { + describe('checkKey', () => { + it('should return error for empty key when canBeEmpty is false', () => { + expect(checkKey('', false)).toBe('canNoBeEmpty') + }) + + it('should return true for empty key when canBeEmpty is true', () => { + expect(checkKey('', true)).toBe(true) + }) + + it('should return error for key that is too long', () => { + const longKey = 'a'.repeat(101) // Assuming MAX_VAR_KEY_LENGTH is 100 + expect(checkKey(longKey)).toBe('tooLong') + }) + + it('should return error for key starting with number', () => { + expect(checkKey('1variable')).toBe('notStartWithNumber') + }) + + it('should return true for valid key', () => { + expect(checkKey('valid_variable_name')).toBe(true) + expect(checkKey('validVariableName')).toBe(true) + expect(checkKey('valid123')).toBe(true) + }) + + it('should return error for invalid characters', () => { + expect(checkKey('invalid-key')).toBe('notValid') + expect(checkKey('invalid key')).toBe('notValid') + expect(checkKey('invalid.key')).toBe('notValid') + expect(checkKey('invalid@key')).toBe('notValid') + }) + + it('should handle underscore correctly', () => { + expect(checkKey('_valid')).toBe(true) + expect(checkKey('valid_name')).toBe(true) + expect(checkKey('valid_name_123')).toBe(true) + }) + }) + + describe('checkKeys', () => { + it('should return valid for all valid keys', () => { + const result = checkKeys(['key1', 'key2', 'validKey']) + expect(result.isValid).toBe(true) + expect(result.errorKey).toBe('') + expect(result.errorMessageKey).toBe('') + }) + + it('should return error for first invalid key', () => { + const result = checkKeys(['validKey', '1invalid', 'anotherValid']) + expect(result.isValid).toBe(false) + expect(result.errorKey).toBe('1invalid') + expect(result.errorMessageKey).toBe('notStartWithNumber') + }) + + it('should handle empty array', () => { + const result = checkKeys([]) + expect(result.isValid).toBe(true) + }) + + it('should stop checking after first error', () => { + const result = checkKeys(['valid', 'invalid-key', '1invalid']) + expect(result.isValid).toBe(false) + expect(result.errorKey).toBe('invalid-key') + expect(result.errorMessageKey).toBe('notValid') + }) + }) + + describe('hasDuplicateStr', () => { + it('should return false for unique strings', () => { + expect(hasDuplicateStr(['a', 'b', 'c'])).toBe(false) + }) + + it('should return true for duplicate strings', () => { + expect(hasDuplicateStr(['a', 'b', 'a'])).toBe(true) + expect(hasDuplicateStr(['test', 'test'])).toBe(true) + }) + + it('should handle empty array', () => { + expect(hasDuplicateStr([])).toBe(false) + }) + + it('should handle single element', () => { + expect(hasDuplicateStr(['single'])).toBe(false) + }) + + it('should handle multiple duplicates', () => { + expect(hasDuplicateStr(['a', 'b', 'a', 'b', 'c'])).toBe(true) + }) + }) + + describe('getVars', () => { + it('should extract variables from template string', () => { + const result = getVars('Hello {{name}}, your age is {{age}}') + expect(result).toEqual(['name', 'age']) + }) + + it('should handle empty string', () => { + expect(getVars('')).toEqual([]) + }) + + it('should handle string without variables', () => { + expect(getVars('Hello world')).toEqual([]) + }) + + it('should remove duplicate variables', () => { + const result = getVars('{{name}} and {{name}} again') + expect(result).toEqual(['name']) + }) + + it('should filter out placeholder variables', () => { + const result = getVars('{{#context#}} {{name}} {{#histories#}}') + expect(result).toEqual(['name']) + }) + + it('should handle variables with underscores', () => { + const result = getVars('{{user_name}} {{user_age}}') + expect(result).toEqual(['user_name', 'user_age']) + }) + + it('should handle variables with numbers', () => { + const result = getVars('{{var1}} {{var2}} {{var123}}') + expect(result).toEqual(['var1', 'var2', 'var123']) + }) + + it('should ignore invalid variable names', () => { + const result = getVars('{{1invalid}} {{valid}} {{-invalid}}') + expect(result).toEqual(['valid']) + }) + + it('should filter out variables that are too long', () => { + const longVar = 'a'.repeat(101) + const result = getVars(`{{${longVar}}} {{valid}}`) + expect(result).toEqual(['valid']) + }) + }) + + describe('getNewVar', () => { + it('should create new string variable', () => { + const result = getNewVar('testKey', 'string') + expect(result.key).toBe('testKey') + expect(result.type).toBe('string') + expect(result.name).toBe('testKey') + }) + + it('should create new number variable', () => { + const result = getNewVar('numKey', 'number') + expect(result.key).toBe('numKey') + expect(result.type).toBe('number') + }) + + it('should truncate long names', () => { + const longKey = 'a'.repeat(100) + const result = getNewVar(longKey, 'string') + expect(result.name.length).toBeLessThanOrEqual(result.key.length) + }) + }) + + describe('getNewVarInWorkflow', () => { + it('should create text input variable by default', () => { + const result = getNewVarInWorkflow('testVar') + expect(result.variable).toBe('testVar') + expect(result.type).toBe(InputVarType.textInput) + expect(result.label).toBe('testVar') + }) + + it('should create select variable', () => { + const result = getNewVarInWorkflow('selectVar', InputVarType.select) + expect(result.variable).toBe('selectVar') + expect(result.type).toBe(InputVarType.select) + }) + + it('should create number variable', () => { + const result = getNewVarInWorkflow('numVar', InputVarType.number) + expect(result.variable).toBe('numVar') + expect(result.type).toBe(InputVarType.number) + }) + }) + + describe('getMarketplaceUrl', () => { + beforeEach(() => { + Object.defineProperty(window, 'location', { + value: { origin: 'https://example.com' }, + writable: true, + }) + }) + + it('should add additional parameters', () => { + const url = getMarketplaceUrl('/plugins', { category: 'ai', version: '1.0' }) + expect(url).toContain('category=ai') + expect(url).toContain('version=1.0') + }) + + it('should skip undefined parameters', () => { + const url = getMarketplaceUrl('/plugins', { category: 'ai', version: undefined }) + expect(url).toContain('category=ai') + expect(url).not.toContain('version=') + }) + }) + + describe('replaceSpaceWithUnderscoreInVarNameInput', () => { + it('should replace spaces with underscores', () => { + const input = document.createElement('input') + input.value = 'test variable name' + replaceSpaceWithUnderscoreInVarNameInput(input) + expect(input.value).toBe('test_variable_name') + }) + + it('should preserve cursor position', () => { + const input = document.createElement('input') + input.value = 'test name' + input.setSelectionRange(5, 5) + replaceSpaceWithUnderscoreInVarNameInput(input) + expect(input.selectionStart).toBe(5) + expect(input.selectionEnd).toBe(5) + }) + + it('should handle multiple spaces', () => { + const input = document.createElement('input') + input.value = 'test multiple spaces' + replaceSpaceWithUnderscoreInVarNameInput(input) + expect(input.value).toBe('test__multiple___spaces') + }) + }) +}) From 34be16874f528f2a6591f5b497d5f20b6127895f Mon Sep 17 00:00:00 2001 From: Novice Date: Wed, 5 Nov 2025 09:28:49 +0800 Subject: [PATCH 04/18] feat: add validation to prevent saving empty opening statement in conversation opener modal (#27843) --- .../new-feature-panel/conversation-opener/modal.tsx | 11 +++++++++-- 1 file changed, 9 insertions(+), 2 deletions(-) diff --git a/web/app/components/base/features/new-feature-panel/conversation-opener/modal.tsx b/web/app/components/base/features/new-feature-panel/conversation-opener/modal.tsx index f0af893f0d..8ab007e66b 100644 --- a/web/app/components/base/features/new-feature-panel/conversation-opener/modal.tsx +++ b/web/app/components/base/features/new-feature-panel/conversation-opener/modal.tsx @@ -1,4 +1,4 @@ -import React, { useCallback, useEffect, useState } from 'react' +import React, { useCallback, useEffect, useMemo, useState } from 'react' import { useTranslation } from 'react-i18next' import { useBoolean } from 'ahooks' import { produce } from 'immer' @@ -45,7 +45,13 @@ const OpeningSettingModal = ({ const [isShowConfirmAddVar, { setTrue: showConfirmAddVar, setFalse: hideConfirmAddVar }] = useBoolean(false) const [notIncludeKeys, setNotIncludeKeys] = useState([]) + const isSaveDisabled = useMemo(() => !tempValue.trim(), [tempValue]) + const handleSave = useCallback((ignoreVariablesCheck?: boolean) => { + // Prevent saving if opening statement is empty + if (isSaveDisabled) + return + if (!ignoreVariablesCheck) { const keys = getInputKeys(tempValue) const promptKeys = promptVariables.map(item => item.key) @@ -75,7 +81,7 @@ const OpeningSettingModal = ({ } }) onSave(newOpening) - }, [data, onSave, promptVariables, workflowVariables, showConfirmAddVar, tempSuggestedQuestions, tempValue]) + }, [data, onSave, promptVariables, workflowVariables, showConfirmAddVar, tempSuggestedQuestions, tempValue, isSaveDisabled]) const cancelAutoAddVar = useCallback(() => { hideConfirmAddVar() @@ -217,6 +223,7 @@ const OpeningSettingModal = ({ From f31b821cc082f8324f36a25285d7c2a4600f10cc Mon Sep 17 00:00:00 2001 From: yangzheli <43645580+yangzheli@users.noreply.github.com> Date: Wed, 5 Nov 2025 09:29:13 +0800 Subject: [PATCH 05/18] fix(web): improve the consistency of the inputs-form UI (#27837) --- .../chat/chat-with-history/inputs-form/content.tsx | 2 +- .../chat/embedded-chatbot/inputs-form/content.tsx | 2 +- .../share/text-generation/run-once/index.tsx | 11 +++++++---- .../_base/components/before-run-form/form-item.tsx | 6 +++--- web/i18n/de-DE/app-debug.ts | 1 - web/i18n/en-US/app-debug.ts | 1 - web/i18n/es-ES/app-debug.ts | 1 - web/i18n/fa-IR/app-debug.ts | 1 - web/i18n/fr-FR/app-debug.ts | 1 - web/i18n/hi-IN/app-debug.ts | 1 - web/i18n/id-ID/app-debug.ts | 1 - web/i18n/it-IT/app-debug.ts | 1 - web/i18n/ja-JP/app-debug.ts | 1 - web/i18n/ko-KR/app-debug.ts | 1 - web/i18n/pl-PL/app-debug.ts | 1 - web/i18n/pt-BR/app-debug.ts | 1 - web/i18n/ro-RO/app-debug.ts | 1 - web/i18n/ru-RU/app-debug.ts | 1 - web/i18n/sl-SI/app-debug.ts | 1 - web/i18n/th-TH/app-debug.ts | 1 - web/i18n/tr-TR/app-debug.ts | 1 - web/i18n/uk-UA/app-debug.ts | 1 - web/i18n/vi-VN/app-debug.ts | 1 - web/i18n/zh-Hans/app-debug.ts | 1 - web/i18n/zh-Hant/app-debug.ts | 1 - 25 files changed, 12 insertions(+), 30 deletions(-) diff --git a/web/app/components/base/chat/chat-with-history/inputs-form/content.tsx b/web/app/components/base/chat/chat-with-history/inputs-form/content.tsx index 392bdf2b77..c7785ebd89 100644 --- a/web/app/components/base/chat/chat-with-history/inputs-form/content.tsx +++ b/web/app/components/base/chat/chat-with-history/inputs-form/content.tsx @@ -49,7 +49,7 @@ const InputsFormContent = ({ showTip }: Props) => {
{form.label}
{!form.required && ( -
{t('appDebug.variableTable.optional')}
+
{t('workflow.panel.optional')}
)}
)} diff --git a/web/app/components/base/chat/embedded-chatbot/inputs-form/content.tsx b/web/app/components/base/chat/embedded-chatbot/inputs-form/content.tsx index dd65f0ce72..caf4e363ff 100644 --- a/web/app/components/base/chat/embedded-chatbot/inputs-form/content.tsx +++ b/web/app/components/base/chat/embedded-chatbot/inputs-form/content.tsx @@ -49,7 +49,7 @@ const InputsFormContent = ({ showTip }: Props) => {
{form.label}
{!form.required && ( -
{t('appDebug.variableTable.optional')}
+
{t('workflow.panel.optional')}
)}
)} diff --git a/web/app/components/share/text-generation/run-once/index.tsx b/web/app/components/share/text-generation/run-once/index.tsx index d24428f32a..112f08a1d7 100644 --- a/web/app/components/share/text-generation/run-once/index.tsx +++ b/web/app/components/share/text-generation/run-once/index.tsx @@ -100,7 +100,10 @@ const RunOnce: FC = ({ : promptConfig.prompt_variables.map(item => (
{item.type !== 'checkbox' && ( - +
+
{item.name}
+ {!item.required && {t('workflow.panel.optional')}} +
)}
{item.type === 'select' && ( @@ -115,7 +118,7 @@ const RunOnce: FC = ({ {item.type === 'string' && ( ) => { handleInputsChange({ ...inputsRef.current, [item.key]: e.target.value }) }} maxLength={item.max_length || DEFAULT_VALUE_MAX_LEN} @@ -124,7 +127,7 @@ const RunOnce: FC = ({ {item.type === 'paragraph' && (