Testing infrastructure, fix truncating of dataURLs (#26)
* feat: implement comprehensive testing infrastructure - Fix image dataURL truncation bug in security.ts with configurable size limits - Add backend integration tests (22 tests) with Vitest for API validation - Add frontend unit tests (11 tests) for JSON serialization - Implement browser-based E2E tests (8 tests) with Playwright - Create Docker setup for repeatable E2E testing environment - Add GitHub Actions CI workflow for automated testing - Update .gitignore for test artifacts and temporary files Testing Infrastructure: - Backend: Vitest + Supertest for API integration tests - Frontend: Vitest + Testing Library for component tests - E2E: Playwright with Chromium for full browser automation - CI/CD: GitHub Actions with parallel test execution Security Improvements: - Make dataURL size limit configurable (default: 10MB) - Enhanced validation for image dataURLs - Block malicious content (javascript:, script tags) All tests pass: 41 total (22 backend + 11 frontend + 8 E2E) * feat(tests): add comprehensive E2E tests for dashboard workflows and image persistence chore(env): update environment variables for consistent API URL usage fix(api): centralize API request helpers for drawing and collection management style(DrawingCard): enhance accessibility with ARIA attributes and data-testid for testing * cleanup/revise documentation * cleanup/revise documentation * Add end-to-end tests for drawing CRUD, export/import, search/sort, and theme toggle functionalities - Implemented E2E tests for drawing creation, editing, and deletion in `drawing-crud.spec.ts`. - Added tests for export and import features, including JSON and SQLite formats in `export-import.spec.ts`. - Created tests for searching and sorting drawings by name and date in `search-and-sort.spec.ts`. - Developed tests for theme toggle functionality to ensure persistence across sessions in `theme-toggle.spec.ts`. * fix: exclude test files from production build to fix Docker build * feat: implement comprehensive testing infrastructure (#19) * bump version 0.1.7 * feat: implement comprehensive testing infrastructure - Fix image dataURL truncation bug in security.ts with configurable size limits - Add backend integration tests (22 tests) with Vitest for API validation - Add frontend unit tests (11 tests) for JSON serialization - Implement browser-based E2E tests (8 tests) with Playwright - Create Docker setup for repeatable E2E testing environment - Add GitHub Actions CI workflow for automated testing - Update .gitignore for test artifacts and temporary files Testing Infrastructure: - Backend: Vitest + Supertest for API integration tests - Frontend: Vitest + Testing Library for component tests - E2E: Playwright with Chromium for full browser automation - CI/CD: GitHub Actions with parallel test execution Security Improvements: - Make dataURL size limit configurable (default: 10MB) - Enhanced validation for image dataURLs - Block malicious content (javascript:, script tags) All tests pass: 41 total (22 backend + 11 frontend + 8 E2E) * feat(tests): add comprehensive E2E tests for dashboard workflows and image persistence chore(env): update environment variables for consistent API URL usage fix(api): centralize API request helpers for drawing and collection management style(DrawingCard): enhance accessibility with ARIA attributes and data-testid for testing * Add end-to-end tests for drawing CRUD, export/import, search/sort, and theme toggle functionalities - Implemented E2E tests for drawing creation, editing, and deletion in `drawing-crud.spec.ts`. - Added tests for export and import features, including JSON and SQLite formats in `export-import.spec.ts`. - Created tests for searching and sorting drawings by name and date in `search-and-sort.spec.ts`. - Developed tests for theme toggle functionality to ensure persistence across sessions in `theme-toggle.spec.ts`. * Update backend/src/__tests__/testUtils.ts --------- Co-authored-by: Zimeng Xiong <zxzimeng@gmail.com> * version bump 0.1.8 * fix(ci): consolidate E2E server startup to prevent shell isolation issues Background processes started with & in separate GitHub Actions run steps can terminate when those steps complete because each step creates a new shell. This caused the backend and frontend servers to die before the E2E tests could run. Fixed by consolidating server startup and test execution into a single shell step with: - Proper PID tracking for cleanup - Health check loops instead of fixed sleep times - All processes run in the same shell session * fix(ci): use absolute database path for E2E tests * fix(backend): use resolved DATABASE_URL path for export/import endpoints --------- Co-authored-by: Adrian Acala <adrianacala017@gmail.com>
This commit is contained in:
Zimeng Xiong
+119
-89
@@ -1,7 +1,3 @@
|
||||
/**
|
||||
* Security utilities for XSS prevention and data sanitization
|
||||
*/
|
||||
|
||||
import { z } from "zod";
|
||||
import DOMPurify from "dompurify";
|
||||
import { JSDOM } from "jsdom";
|
||||
@@ -10,6 +6,44 @@ import { JSDOM } from "jsdom";
|
||||
const window = new JSDOM("").window;
|
||||
const purify = DOMPurify(window);
|
||||
|
||||
/**
|
||||
* Configuration for security limits
|
||||
*/
|
||||
export interface SecurityConfig {
|
||||
/** Maximum size for dataURL in bytes (default: 10MB) */
|
||||
maxDataUrlSize: number;
|
||||
}
|
||||
|
||||
// Default configuration
|
||||
const defaultConfig: SecurityConfig = {
|
||||
maxDataUrlSize: 10 * 1024 * 1024, // 10MB
|
||||
};
|
||||
|
||||
// Current active configuration
|
||||
let activeConfig: SecurityConfig = { ...defaultConfig };
|
||||
|
||||
/**
|
||||
* Configure security settings
|
||||
* @param config Partial configuration to merge with defaults
|
||||
*/
|
||||
export const configureSecuritySettings = (config: Partial<SecurityConfig>): void => {
|
||||
activeConfig = { ...activeConfig, ...config };
|
||||
};
|
||||
|
||||
/**
|
||||
* Reset security settings to defaults
|
||||
*/
|
||||
export const resetSecuritySettings = (): void => {
|
||||
activeConfig = { ...defaultConfig };
|
||||
};
|
||||
|
||||
/**
|
||||
* Get current security configuration
|
||||
*/
|
||||
export const getSecurityConfig = (): SecurityConfig => {
|
||||
return { ...activeConfig };
|
||||
};
|
||||
|
||||
/**
|
||||
* Sanitize HTML/JS content using DOMPurify (battle-tested library)
|
||||
*/
|
||||
@@ -18,21 +52,9 @@ export const sanitizeHtml = (input: string): string => {
|
||||
|
||||
return purify
|
||||
.sanitize(input, {
|
||||
ALLOWED_TAGS: [
|
||||
// Allow basic text formatting that might be in drawings
|
||||
"b",
|
||||
"i",
|
||||
"u",
|
||||
"em",
|
||||
"strong",
|
||||
"p",
|
||||
"br",
|
||||
"span",
|
||||
"div",
|
||||
],
|
||||
ALLOWED_ATTR: [], // No attributes allowed by default for security
|
||||
ALLOWED_TAGS: ["b", "i", "u", "em", "strong", "p", "br", "span", "div"],
|
||||
ALLOWED_ATTR: [],
|
||||
FORBID_TAGS: [
|
||||
// Explicitly forbid dangerous tags
|
||||
"script",
|
||||
"iframe",
|
||||
"object",
|
||||
@@ -48,7 +70,6 @@ export const sanitizeHtml = (input: string): string => {
|
||||
"foreignObject",
|
||||
],
|
||||
FORBID_ATTR: [
|
||||
// Explicitly forbid dangerous attributes
|
||||
"onload",
|
||||
"onclick",
|
||||
"onerror",
|
||||
@@ -66,23 +87,17 @@ export const sanitizeHtml = (input: string): string => {
|
||||
"action",
|
||||
"formaction",
|
||||
],
|
||||
KEEP_CONTENT: true, // Keep content even if tags are removed
|
||||
KEEP_CONTENT: true,
|
||||
})
|
||||
.trim();
|
||||
};
|
||||
|
||||
/**
|
||||
* Sanitize SVG content using DOMPurify with strict SVG restrictions
|
||||
*/
|
||||
export const sanitizeSvg = (svgContent: string): string => {
|
||||
if (typeof svgContent !== "string") return "";
|
||||
|
||||
// For SVG content, we'll be very restrictive since SVG can execute JavaScript
|
||||
// We only allow basic geometric shapes without any scripts or external references
|
||||
return purify
|
||||
.sanitize(svgContent, {
|
||||
ALLOWED_TAGS: [
|
||||
// Allow only safe SVG geometric elements
|
||||
"svg",
|
||||
"g",
|
||||
"rect",
|
||||
@@ -96,7 +111,6 @@ export const sanitizeSvg = (svgContent: string): string => {
|
||||
"tspan",
|
||||
],
|
||||
ALLOWED_ATTR: [
|
||||
// Allow only safe geometric attributes
|
||||
"x",
|
||||
"y",
|
||||
"width",
|
||||
@@ -123,7 +137,6 @@ export const sanitizeSvg = (svgContent: string): string => {
|
||||
"dominant-baseline",
|
||||
],
|
||||
FORBID_TAGS: [
|
||||
// Completely forbid any script-related or external content
|
||||
"script",
|
||||
"foreignObject",
|
||||
"iframe",
|
||||
@@ -141,7 +154,6 @@ export const sanitizeSvg = (svgContent: string): string => {
|
||||
"filter",
|
||||
],
|
||||
FORBID_ATTR: [
|
||||
// Forbid any attributes that could execute code or load external content
|
||||
"onload",
|
||||
"onclick",
|
||||
"onerror",
|
||||
@@ -161,37 +173,20 @@ export const sanitizeSvg = (svgContent: string): string => {
|
||||
.trim();
|
||||
};
|
||||
|
||||
/**
|
||||
* Validate and sanitize text content using DOMPurify
|
||||
*/
|
||||
export const sanitizeText = (
|
||||
input: unknown,
|
||||
maxLength: number = 1000
|
||||
): string => {
|
||||
if (typeof input !== "string") return "";
|
||||
|
||||
// Remove null bytes and control characters except newlines and tabs
|
||||
const cleaned = input.replace(/[\x00-\x08\x0B\x0C\x0E-\x1F\x7F]/g, "");
|
||||
|
||||
// Truncate if too long
|
||||
const truncated = cleaned.slice(0, maxLength);
|
||||
|
||||
// Use DOMPurify for text content - more permissive than HTML but still safe
|
||||
return purify
|
||||
.sanitize(truncated, {
|
||||
ALLOWED_TAGS: [
|
||||
// Allow basic text formatting that might be in drawing text
|
||||
"b",
|
||||
"i",
|
||||
"u",
|
||||
"em",
|
||||
"strong",
|
||||
"br",
|
||||
"span",
|
||||
],
|
||||
ALLOWED_ATTR: [], // No attributes allowed for text content
|
||||
ALLOWED_TAGS: ["b", "i", "u", "em", "strong", "br", "span"],
|
||||
ALLOWED_ATTR: [],
|
||||
FORBID_TAGS: [
|
||||
// Block potentially dangerous tags
|
||||
"script",
|
||||
"iframe",
|
||||
"object",
|
||||
@@ -207,7 +202,6 @@ export const sanitizeText = (
|
||||
"foreignObject",
|
||||
],
|
||||
FORBID_ATTR: [
|
||||
// Block all event handlers and dangerous attributes
|
||||
"onload",
|
||||
"onclick",
|
||||
"onerror",
|
||||
@@ -231,22 +225,16 @@ export const sanitizeText = (
|
||||
.trim();
|
||||
};
|
||||
|
||||
/**
|
||||
* Sanitize URL to prevent javascript: and data: attacks
|
||||
*/
|
||||
export const sanitizeUrl = (url: unknown): string => {
|
||||
if (typeof url !== "string") return "";
|
||||
|
||||
const trimmed = url.trim();
|
||||
|
||||
// Block javascript:, data:, vbscript: URLs
|
||||
if (/^(javascript|data|vbscript):/i.test(trimmed)) {
|
||||
return "";
|
||||
}
|
||||
|
||||
// Basic URL validation
|
||||
try {
|
||||
// Allow http, https, mailto, and relative URLs
|
||||
if (/^(https?:\/\/|mailto:|\/|\.\/|\.\.\/)/i.test(trimmed)) {
|
||||
return trimmed;
|
||||
}
|
||||
@@ -256,9 +244,6 @@ export const sanitizeUrl = (url: unknown): string => {
|
||||
}
|
||||
};
|
||||
|
||||
/**
|
||||
* Very flexible Zod schema for Excalidraw elements
|
||||
*/
|
||||
export const elementSchema = z
|
||||
.object({
|
||||
id: z.string().min(1).max(200).optional().nullable(),
|
||||
@@ -293,7 +278,6 @@ export const elementSchema = z
|
||||
})
|
||||
.passthrough()
|
||||
.transform((element) => {
|
||||
// Apply basic sanitization to string values only
|
||||
const sanitized = { ...element };
|
||||
|
||||
if (typeof sanitized.text === "string") {
|
||||
@@ -307,9 +291,6 @@ export const elementSchema = z
|
||||
return sanitized;
|
||||
});
|
||||
|
||||
/**
|
||||
* Flexible Zod schema for Excalidraw app state with validation
|
||||
*/
|
||||
export const appStateSchema = z
|
||||
.object({
|
||||
gridSize: z.number().finite().min(0).max(1000).optional().nullable(),
|
||||
@@ -400,24 +381,17 @@ export const appStateSchema = z
|
||||
.nullable(),
|
||||
cursorX: z.number().finite().optional().nullable(),
|
||||
cursorY: z.number().finite().optional().nullable(),
|
||||
// Add common Excalidraw app state properties
|
||||
collaborators: z.record(z.string(), z.any()).optional().nullable(),
|
||||
})
|
||||
// Allow any additional properties
|
||||
.catchall(
|
||||
z.any().refine((val) => {
|
||||
// Sanitize string values, but be more permissive for other types
|
||||
if (typeof val === "string") {
|
||||
return sanitizeText(val, 1000);
|
||||
}
|
||||
// Allow numbers, booleans, objects, arrays, null, undefined
|
||||
return true;
|
||||
})
|
||||
);
|
||||
|
||||
/**
|
||||
* Sanitize drawing data before persistence
|
||||
*/
|
||||
export const sanitizeDrawingData = (data: {
|
||||
elements: any[];
|
||||
appState: any;
|
||||
@@ -425,30 +399,93 @@ export const sanitizeDrawingData = (data: {
|
||||
preview?: string | null;
|
||||
}) => {
|
||||
try {
|
||||
// Validate and sanitize elements
|
||||
const sanitizedElements = elementSchema.array().parse(data.elements);
|
||||
|
||||
// Validate and sanitize app state
|
||||
const sanitizedAppState = appStateSchema.parse(data.appState);
|
||||
|
||||
// Sanitize preview SVG if present
|
||||
let sanitizedPreview = data.preview;
|
||||
if (typeof sanitizedPreview === "string") {
|
||||
sanitizedPreview = sanitizeSvg(sanitizedPreview);
|
||||
}
|
||||
|
||||
// Sanitize files object
|
||||
// Sanitize files object with special handling for dataURL
|
||||
let sanitizedFiles = data.files;
|
||||
if (typeof sanitizedFiles === "object" && sanitizedFiles !== null) {
|
||||
// Recursively sanitize any string values in files
|
||||
sanitizedFiles = JSON.parse(
|
||||
JSON.stringify(sanitizedFiles, (key, value) => {
|
||||
if (typeof value === "string") {
|
||||
return sanitizeText(value, 10000);
|
||||
// Create a deep copy to avoid mutating the original input
|
||||
sanitizedFiles = structuredClone(sanitizedFiles);
|
||||
|
||||
// Safe image MIME types that we allow for dataURL (case-insensitive)
|
||||
const safeImageTypes = [
|
||||
"data:image/png",
|
||||
"data:image/jpeg",
|
||||
"data:image/jpg",
|
||||
"data:image/gif",
|
||||
"data:image/webp",
|
||||
];
|
||||
|
||||
// Dangerous URL protocols to block entirely
|
||||
const dangerousProtocols = [/^javascript:/i, /^vbscript:/i, /^data:text\/html/i];
|
||||
|
||||
// Suspicious patterns for security validation within data URLs
|
||||
const suspiciousPatterns = [/<script/i, /javascript:/i, /on\w+\s*=/i, /<iframe/i];
|
||||
|
||||
// Maximum size for dataURL (configurable, default 10MB to prevent DoS)
|
||||
const MAX_DATAURL_SIZE = activeConfig.maxDataUrlSize;
|
||||
|
||||
// Iterate over each file in the dictionary
|
||||
for (const fileId in sanitizedFiles) {
|
||||
const file = sanitizedFiles[fileId];
|
||||
if (typeof file === "object" && file !== null) {
|
||||
// Sanitize each property of the file object
|
||||
for (const key in file) {
|
||||
const value = file[key];
|
||||
if (typeof value === "string") {
|
||||
// Special handling for dataURL: allow it to be long if it's a valid image data URL
|
||||
if (key === "dataURL") {
|
||||
const normalizedValue = value.toLowerCase();
|
||||
|
||||
// First, check for dangerous protocols - block these entirely
|
||||
const hasDangerousProtocol = dangerousProtocols.some((pattern) =>
|
||||
pattern.test(value)
|
||||
);
|
||||
|
||||
if (hasDangerousProtocol) {
|
||||
// Block dangerous URLs entirely
|
||||
file[key] = "";
|
||||
continue;
|
||||
}
|
||||
|
||||
// Check if it's a safe image type
|
||||
const isSafeImageType = safeImageTypes.some((type) =>
|
||||
normalizedValue.startsWith(type)
|
||||
);
|
||||
|
||||
if (isSafeImageType) {
|
||||
// Check for suspicious content and size limits
|
||||
const hasSuspiciousContent = suspiciousPatterns.some((pattern) =>
|
||||
pattern.test(value)
|
||||
);
|
||||
const isTooLarge = value.length > MAX_DATAURL_SIZE;
|
||||
|
||||
if (hasSuspiciousContent || isTooLarge) {
|
||||
// Clear suspicious or oversized data URLs
|
||||
file[key] = "";
|
||||
} else {
|
||||
// Keep the base64-encoded image data URL as-is
|
||||
file[key] = value;
|
||||
}
|
||||
} else {
|
||||
// Not a safe image type and not a dangerous protocol
|
||||
// Sanitize as text but this likely means it's invalid anyway
|
||||
file[key] = sanitizeText(value, 1000);
|
||||
}
|
||||
} else {
|
||||
// For all other string fields (id, mimeType, etc.), apply strict sanitization
|
||||
file[key] = sanitizeText(value, 1000);
|
||||
}
|
||||
}
|
||||
}
|
||||
return value;
|
||||
})
|
||||
);
|
||||
}
|
||||
}
|
||||
}
|
||||
|
||||
return {
|
||||
@@ -463,26 +500,19 @@ export const sanitizeDrawingData = (data: {
|
||||
}
|
||||
};
|
||||
|
||||
/**
|
||||
* Validate imported .excalidraw file structure
|
||||
*/
|
||||
export const validateImportedDrawing = (data: any): boolean => {
|
||||
try {
|
||||
// Basic structure validation
|
||||
if (!data || typeof data !== "object") return false;
|
||||
|
||||
if (!Array.isArray(data.elements)) return false;
|
||||
if (typeof data.appState !== "object") return false;
|
||||
|
||||
// Check element count to prevent DoS
|
||||
if (data.elements.length > 10000) {
|
||||
throw new Error("Drawing contains too many elements (max 10,000)");
|
||||
}
|
||||
|
||||
// Sanitize and validate the data
|
||||
const sanitized = sanitizeDrawingData(data);
|
||||
|
||||
// Additional structural validation
|
||||
if (sanitized.elements.length !== data.elements.length) {
|
||||
throw new Error("Element count mismatch after sanitization");
|
||||
}
|
||||
|
||||
Reference in New Issue
Block a user