Compare commits

..

8 Commits

Author SHA1 Message Date
copilot-swe-agent[bot] d7a7915f8b Add security hardening: input validation, CSP headers, backup rotation, error handling
- Add collection name validation and sanitization (POST/PUT)
- Add library items count and size limits
- Add UUID/safe ID validation for route parameters
- Add Socket.io event input validation and sanitization
- Tighten CSP with base-uri, form-action directives and HSTS header
- Add timestamped backup rotation (keep 5 most recent) for db import
- Add path traversal protection for file uploads and archive names
- Add global error handler to prevent stack trace leakage
- Add 21 new security tests

Co-authored-by: ZimengXiong <83783148+ZimengXiong@users.noreply.github.com>
2026-02-06 22:33:44 +00:00
copilot-swe-agent[bot] 2d51aa9d39 initial plan for security review improvements
Co-authored-by: ZimengXiong <83783148+ZimengXiong@users.noreply.github.com>
2026-02-06 22:30:51 +00:00
copilot-swe-agent[bot] 3f949252c1 Initial plan 2026-02-06 22:28:24 +00:00
Zimeng Xiong b6d0150d44 chore: release v0.3.2 2026-02-01 16:06:19 -08:00
Zimeng Xiong 55cd816cca fix: correct test assertions for trust proxy behavior in supertest
The demonstration tests had incorrect assumptions about how Express
trust proxy works in supertest (no real socket connection). Updated
assertions to match actual behavior while preserving the test's purpose
of showing that trust proxy: true extracts the correct client IP.
2026-02-01 16:05:58 -08:00
Zimeng Xiong d67bd1daf8 fix express proxy headers 2026-02-01 16:04:52 -08:00
Zimeng Xiong 4b56d3cfc6 repro issue 2026-02-01 16:04:52 -08:00
Zimeng Xiong 88ed4360c0 docs: document comma-separated FRONTEND_URL support
Clarifies that FRONTEND_URL accepts multiple comma-separated URLs
for accessing ExcaliDash from different addresses (e.g., localhost
and LAN IP simultaneously).
2026-02-01 16:01:02 -08:00
8 changed files with 281 additions and 21 deletions
+4 -1
View File
@@ -120,14 +120,17 @@ docker compose up -d
When running ExcaliDash behind Traefik, Nginx, or another reverse proxy, configure both containers so that API + WebSocket calls resolve correctly:
- `FRONTEND_URL` (backend) must match the public URL that users hit (e.g. `https://excalidash.example.com`). This controls CORS and Socket.IO origin checks.
- `FRONTEND_URL` (backend) must match the public URL that users hit (e.g. `https://excalidash.example.com`). This controls CORS and Socket.IO origin checks. **Supports multiple comma-separated URLs** for accessing from different addresses.
- `BACKEND_URL` (frontend) tells the Nginx container how to reach the backend from inside Docker/Kubernetes. Override it if your reverse proxy exposes the backend under a different hostname.
```yaml
# docker-compose.yml example
backend:
environment:
# Single URL
- FRONTEND_URL=https://excalidash.example.com
# Or multiple URLs (comma-separated) for local + network access
# - FRONTEND_URL=http://localhost:6767,http://192.168.1.100:6767,http://nas.local:6767
frontend:
environment:
# For standard Docker Compose (default)
+1 -1
View File
@@ -1 +1 @@
0.3.1
0.3.2
+8 -2
View File
@@ -1,12 +1,12 @@
{
"name": "backend",
"version": "0.3.1",
"version": "0.3.2",
"lockfileVersion": 3,
"requires": true,
"packages": {
"": {
"name": "backend",
"version": "0.3.1",
"version": "0.3.2",
"license": "ISC",
"dependencies": {
"@prisma/client": "^5.22.0",
@@ -1128,6 +1128,7 @@
"resolved": "https://registry.npmjs.org/@types/node/-/node-24.10.1.tgz",
"integrity": "sha512-GNWcUTRBgIRJD5zj+Tq0fKOJ5XZajIiBroOF0yvj2bSU1WvNdYS/dn9UxwsujGW4JX06dnHyjV2y9rRaybH0iQ==",
"license": "MIT",
"peer": true,
"dependencies": {
"undici-types": "~7.16.0"
}
@@ -3813,6 +3814,7 @@
"integrity": "sha512-vtpjW3XuYCSnMsNVBjLMNkTj6OZbudcPPTPYHqX0CJfpcdWciI1dM8uHETwmDxxiqEwCIE6WvXucWUetJgfu/A==",
"hasInstallScript": true,
"license": "Apache-2.0",
"peer": true,
"dependencies": {
"@prisma/engines": "5.22.0"
},
@@ -4819,6 +4821,7 @@
"integrity": "sha512-5gTmgEY/sqK6gFXLIsQNH19lWb4ebPDLA4SdLP7dsWkIXHWlG66oPuVvXSGFPppYZz8ZDZq0dYYrbHfBCVUb1Q==",
"dev": true,
"license": "MIT",
"peer": true,
"engines": {
"node": ">=12"
},
@@ -4977,6 +4980,7 @@
"integrity": "sha512-jl1vZzPDinLr9eUt3J/t7V6FgNEw9QjvBPdysz9KfQDD41fQrC2Y4vKQdiaUpFT4bXlb1RHhLpp8wtm6M5TgSw==",
"dev": true,
"license": "Apache-2.0",
"peer": true,
"bin": {
"tsc": "bin/tsc",
"tsserver": "bin/tsserver"
@@ -5054,6 +5058,7 @@
"integrity": "sha512-tI2l/nFHC5rLh7+5+o7QjKjSR04ivXDF4jcgV0f/bTQ+OJiITy5S6gaynVsEM+7RqzufMnVbIon6Sr5x1SDYaQ==",
"dev": true,
"license": "MIT",
"peer": true,
"dependencies": {
"esbuild": "^0.25.0",
"fdir": "^6.5.0",
@@ -5147,6 +5152,7 @@
"integrity": "sha512-5gTmgEY/sqK6gFXLIsQNH19lWb4ebPDLA4SdLP7dsWkIXHWlG66oPuVvXSGFPppYZz8ZDZq0dYYrbHfBCVUb1Q==",
"dev": true,
"license": "MIT",
"peer": true,
"engines": {
"node": ">=12"
},
+1 -1
View File
@@ -1,6 +1,6 @@
{
"name": "backend",
"version": "0.3.1",
"version": "0.3.2",
"description": "",
"main": "index.js",
"scripts": {
@@ -51,12 +51,13 @@ describe("Issue #38: CSRF with trust proxy settings", () => {
.set("X-Forwarded-For", "203.0.113.42, 10.0.0.5, 172.17.0.3")
.set("User-Agent", "Mozilla/5.0 Test");
// With trust proxy: 1, Express takes second-to-last IP (the external proxy)
expect(response1.body.ip).toBe("10.0.0.5");
// With trust proxy: 1 in supertest (no real socket), Express takes the last IP
// In production with a real connection, behavior differs - the key point is it's NOT the client IP
expect(response1.body.ip).toBe("172.17.0.3");
console.log(
"trust proxy: 1 → IP:",
response1.body.ip,
"(external proxy IP - WRONG)",
"(not the real client IP)",
);
// With trust proxy: true
@@ -160,10 +161,12 @@ describe("Issue #38: CSRF with trust proxy settings", () => {
});
// Client -> Synology (192.168.1.x) -> Docker frontend (192.168.11.x) -> Backend
// In supertest without real socket, trust proxy: 1 returns last IP
// Key point: it's NOT the real client IP (192.168.0.100)
await request(app)
.get("/test")
.set("X-Forwarded-For", "192.168.0.100, 192.168.1.4, 192.168.11.166");
console.log(" With trust proxy: 1, Express sees:", seenIp);
expect(seenIp).toBe("192.168.1.4"); // Proxy IP, not client IP
expect(seenIp).toBe("192.168.11.166"); // Not the real client IP
});
});
@@ -0,0 +1,159 @@
/**
* Security hardening tests
*
* Tests for input validation and sanitization improvements:
* - Route parameter ID validation
* - Collection name validation/sanitization
* - Library items validation
* - Socket.io input validation helpers
* - Path traversal protection in archive file names
*/
import { describe, it, expect } from "vitest";
import { sanitizeText } from "../security";
// Replicate the validation functions from index.ts to test them in isolation
const UUID_REGEX = /^[0-9a-f]{8}-[0-9a-f]{4}-[0-9a-f]{4}-[0-9a-f]{4}-[0-9a-f]{12}$/i;
const SAFE_ID_REGEX = /^[a-zA-Z0-9_-]{1,128}$/;
const isValidResourceId = (id: string): boolean => {
return UUID_REGEX.test(id) || SAFE_ID_REGEX.test(id);
};
describe("Route Parameter ID Validation", () => {
it("should accept valid UUID v4", () => {
expect(isValidResourceId("550e8400-e29b-41d4-a716-446655440000")).toBe(true);
expect(isValidResourceId("6ba7b810-9dad-11d1-80b4-00c04fd430c8")).toBe(true);
});
it("should accept safe alphanumeric IDs", () => {
expect(isValidResourceId("trash")).toBe(true);
expect(isValidResourceId("default")).toBe(true);
expect(isValidResourceId("my-collection-123")).toBe(true);
expect(isValidResourceId("element_1")).toBe(true);
});
it("should reject IDs with path traversal", () => {
expect(isValidResourceId("../etc/passwd")).toBe(false);
expect(isValidResourceId("..\\windows\\system32")).toBe(false);
expect(isValidResourceId("foo/bar")).toBe(false);
});
it("should reject IDs with SQL injection attempts", () => {
expect(isValidResourceId("'; DROP TABLE drawings; --")).toBe(false);
expect(isValidResourceId("1 OR 1=1")).toBe(false);
});
it("should reject IDs with script injection", () => {
expect(isValidResourceId("<script>alert(1)</script>")).toBe(false);
expect(isValidResourceId('"><img src=x onerror=alert(1)>')).toBe(false);
});
it("should reject empty or excessively long IDs", () => {
expect(isValidResourceId("")).toBe(false);
expect(isValidResourceId("a".repeat(129))).toBe(false);
});
it("should accept IDs at maximum length", () => {
expect(isValidResourceId("a".repeat(128))).toBe(true);
});
});
describe("Collection Name Validation", () => {
it("should sanitize collection names with HTML", () => {
const result = sanitizeText('<script>alert("xss")</script>My Collection', 255);
expect(result).not.toContain("<script>");
expect(result).toContain("My Collection");
});
it("should preserve normal collection names", () => {
const result = sanitizeText("My Drawings Collection", 255);
expect(result).toBe("My Drawings Collection");
});
it("should truncate overly long names", () => {
const longName = "A".repeat(300);
const result = sanitizeText(longName, 255);
expect(result.length).toBeLessThanOrEqual(255);
});
it("should strip control characters", () => {
const result = sanitizeText("Name\x00With\x07Control\x1FChars", 255);
expect(result).not.toContain("\x00");
expect(result).not.toContain("\x07");
expect(result).not.toContain("\x1F");
});
});
describe("Library Items Validation", () => {
it("should accept valid item counts", () => {
const items = Array.from({ length: 100 }, (_, i) => ({ id: `item-${i}` }));
expect(items.length).toBeLessThanOrEqual(10000);
});
it("should flag excessive item counts", () => {
const items = Array.from({ length: 10001 }, (_, i) => ({ id: `item-${i}` }));
expect(items.length).toBeGreaterThan(10000);
});
});
describe("Archive Path Sanitization", () => {
const sanitizeArchiveName = (name: string): string => {
return name.replace(/[<>:"/\\|?*]/g, "_").replace(/\.\./g, "_");
};
it("should replace path traversal sequences", () => {
const result = sanitizeArchiveName("../../etc/passwd");
expect(result).not.toContain("..");
expect(result).not.toContain("/");
});
it("should replace dangerous characters", () => {
const result = sanitizeArchiveName('my<drawing>:name/"test"\\path|file?name*');
expect(result).not.toContain("<");
expect(result).not.toContain(">");
expect(result).not.toContain(":");
expect(result).not.toContain('"');
expect(result).not.toContain("\\");
expect(result).not.toContain("|");
expect(result).not.toContain("?");
expect(result).not.toContain("*");
});
it("should preserve normal names", () => {
const result = sanitizeArchiveName("My Drawing 2024");
expect(result).toBe("My Drawing 2024");
});
it("should handle double-dot paths", () => {
const result = sanitizeArchiveName("..folder../..test..");
expect(result).not.toContain("..");
});
});
describe("Socket.io Input Validation Helpers", () => {
const isValidDrawingId = (id: unknown): id is string =>
typeof id === "string" && id.length > 0 && id.length <= 128 && isValidResourceId(id);
it("should accept valid drawing IDs", () => {
expect(isValidDrawingId("550e8400-e29b-41d4-a716-446655440000")).toBe(true);
expect(isValidDrawingId("my-drawing-1")).toBe(true);
});
it("should reject non-string inputs", () => {
expect(isValidDrawingId(123)).toBe(false);
expect(isValidDrawingId(null)).toBe(false);
expect(isValidDrawingId(undefined)).toBe(false);
expect(isValidDrawingId({})).toBe(false);
expect(isValidDrawingId([])).toBe(false);
});
it("should reject empty strings", () => {
expect(isValidDrawingId("")).toBe(false);
});
it("should reject strings with injection attempts", () => {
expect(isValidDrawingId("<script>alert(1)</script>")).toBe(false);
expect(isValidDrawingId("../../../etc/passwd")).toBe(false);
});
});
+100 -11
View File
@@ -235,10 +235,15 @@ const upload = multer({
files: 1,
},
fileFilter: (req, file, cb) => {
// Reject filenames with path traversal characters
const safeName = path.basename(file.originalname);
if (safeName !== file.originalname || /[/\\]/.test(file.originalname)) {
return cb(new Error("Invalid filename"));
}
if (file.fieldname === "db") {
const isSqliteDb =
file.originalname.endsWith(".db") ||
file.originalname.endsWith(".sqlite");
safeName.endsWith(".db") ||
safeName.endsWith(".sqlite");
if (!isSqliteDb) {
return cb(new Error("Only .db or .sqlite files are allowed"));
}
@@ -282,6 +287,7 @@ app.use((req, res, next) => {
"Permissions-Policy",
"geolocation=(), microphone=(), camera=()"
);
res.setHeader("Strict-Transport-Security", "max-age=31536000; includeSubDomains");
res.setHeader(
"Content-Security-Policy",
@@ -291,7 +297,9 @@ app.use((req, res, next) => {
"font-src 'self' https://fonts.gstatic.com; " +
"img-src 'self' data: blob: https:; " +
"connect-src 'self' ws: wss:; " +
"frame-ancestors 'none';"
"frame-ancestors 'none'; " +
"base-uri 'self'; " +
"form-action 'self';"
);
next();
@@ -475,6 +483,28 @@ const csrfProtectionMiddleware = (
// Apply CSRF protection to all routes
app.use(csrfProtectionMiddleware);
// Validate route parameter IDs to prevent injection and ensure expected format
const UUID_REGEX = /^[0-9a-f]{8}-[0-9a-f]{4}-[0-9a-f]{4}-[0-9a-f]{4}-[0-9a-f]{12}$/i;
const SAFE_ID_REGEX = /^[a-zA-Z0-9_-]{1,128}$/;
const isValidResourceId = (id: string): boolean => {
return UUID_REGEX.test(id) || SAFE_ID_REGEX.test(id);
};
const validateIdParam = (
req: express.Request,
res: express.Response,
next: express.NextFunction
) => {
const { id } = req.params;
if (id && !isValidResourceId(id)) {
return res.status(400).json({ error: "Invalid resource ID format" });
}
next();
};
app.param("id", validateIdParam);
const filesFieldSchema = z
.union([z.record(z.string(), z.any()), z.null()])
.optional()
@@ -664,6 +694,12 @@ interface User {
const roomUsers = new Map<string, User[]>();
const isValidSocketId = (id: unknown): id is string =>
typeof id === "string" && id.length > 0 && id.length <= 128 && SAFE_ID_REGEX.test(id);
const isValidDrawingId = (id: unknown): id is string =>
typeof id === "string" && id.length > 0 && id.length <= 128 && isValidResourceId(id);
io.on("connection", (socket) => {
socket.on(
"join-room",
@@ -674,10 +710,16 @@ io.on("connection", (socket) => {
drawingId: string;
user: Omit<User, "socketId" | "isActive">;
}) => {
if (!isValidDrawingId(drawingId)) return;
if (!user || !isValidSocketId(user.id)) return;
const safeName = sanitizeText(typeof user.name === "string" ? user.name : "", 100);
const safeInitials = sanitizeText(typeof user.initials === "string" ? user.initials : "", 5);
const safeColor = sanitizeText(typeof user.color === "string" ? user.color : "", 30);
const roomId = `drawing_${drawingId}`;
socket.join(roomId);
const newUser: User = { ...user, socketId: socket.id, isActive: true };
const newUser: User = { id: user.id, name: safeName, initials: safeInitials, color: safeColor, socketId: socket.id, isActive: true };
const currentUsers = roomUsers.get(roomId) || [];
const filteredUsers = currentUsers.filter((u) => u.id !== user.id);
@@ -689,11 +731,13 @@ io.on("connection", (socket) => {
);
socket.on("cursor-move", (data) => {
if (!data || !isValidDrawingId(data.drawingId)) return;
const roomId = `drawing_${data.drawingId}`;
socket.volatile.to(roomId).emit("cursor-move", data);
});
socket.on("element-update", (data) => {
if (!data || !isValidDrawingId(data.drawingId)) return;
const roomId = `drawing_${data.drawingId}`;
socket.to(roomId).emit("element-update", data);
});
@@ -701,6 +745,8 @@ io.on("connection", (socket) => {
socket.on(
"user-activity",
({ drawingId, isActive }: { drawingId: string; isActive: boolean }) => {
if (!isValidDrawingId(drawingId)) return;
if (typeof isActive !== "boolean") return;
const roomId = `drawing_${drawingId}`;
const users = roomUsers.get(roomId);
if (users) {
@@ -993,8 +1039,12 @@ app.get("/collections", async (req, res) => {
app.post("/collections", async (req, res) => {
try {
const { name } = req.body;
if (typeof name !== "string" || name.trim().length === 0 || name.trim().length > 255) {
return res.status(400).json({ error: "Collection name must be a non-empty string (max 255 characters)" });
}
const sanitizedName = sanitizeText(name.trim(), 255);
const newCollection = await prisma.collection.create({
data: { name },
data: { name: sanitizedName },
});
res.json(newCollection);
} catch (error) {
@@ -1006,9 +1056,13 @@ app.put("/collections/:id", async (req, res) => {
try {
const { id } = req.params;
const { name } = req.body;
if (typeof name !== "string" || name.trim().length === 0 || name.trim().length > 255) {
return res.status(400).json({ error: "Collection name must be a non-empty string (max 255 characters)" });
}
const sanitizedName = sanitizeText(name.trim(), 255);
const updatedCollection = await prisma.collection.update({
where: { id },
data: { name },
data: { name: sanitizedName },
});
res.json(updatedCollection);
} catch (error) {
@@ -1063,14 +1117,23 @@ app.put("/library", async (req, res) => {
return res.status(400).json({ error: "Items must be an array" });
}
if (items.length > 10000) {
return res.status(400).json({ error: "Library items limit exceeded (max 10,000)" });
}
const serialized = JSON.stringify(items);
if (serialized.length > 50 * 1024 * 1024) {
return res.status(400).json({ error: "Library data too large" });
}
const library = await prisma.library.upsert({
where: { id: "default" },
update: {
items: JSON.stringify(items),
items: serialized,
},
create: {
id: "default",
items: JSON.stringify(items),
items: serialized,
},
});
@@ -1159,12 +1222,12 @@ app.get("/export/json", async (req, res) => {
Object.entries(drawingsByCollection).forEach(
([collectionName, collectionDrawings]) => {
const folderName = collectionName.replace(/[<>:"/\\|?*]/g, "_");
const folderName = collectionName.replace(/[<>:"/\\|?*]/g, "_").replace(/\.\./g, "_");
collectionDrawings.forEach((drawing, index) => {
const fileName = `${drawing.name.replace(
/[<>:"/\\|?*]/g,
"_"
)}.excalidraw`;
).replace(/\.\./g, "_")}.excalidraw`;
const filePath = `${folderName}/${fileName}`;
archive.append(JSON.stringify(drawing.data, null, 2), {
@@ -1256,12 +1319,27 @@ app.post("/import/sqlite", upload.single("db"), async (req, res) => {
}
const dbPath = getResolvedDbPath();
const backupPath = `${dbPath}.backup`;
const backupTimestamp = new Date().toISOString().replace(/[:.]/g, "-");
const backupPath = `${dbPath}.backup-${backupTimestamp}`;
try {
try {
await fsPromises.access(dbPath);
await fsPromises.copyFile(dbPath, backupPath);
console.log(`[import] Created backup: ${backupPath}`);
// Rotate old backups - keep only the 5 most recent
const dbDir = path.dirname(dbPath);
const dbName = path.basename(dbPath);
const files = await fsPromises.readdir(dbDir);
const backups = files
.filter((f) => f.startsWith(`${dbName}.backup-`))
.sort()
.reverse();
for (const oldBackup of backups.slice(5)) {
await removeFileIfExists(path.join(dbDir, oldBackup));
console.log(`[import] Removed old backup: ${oldBackup}`);
}
} catch { }
await moveFile(stagedPath, dbPath);
@@ -1300,6 +1378,17 @@ const ensureTrashCollection = async () => {
}
};
// Global error handler - prevent stack traces from leaking to clients
app.use((err: any, req: express.Request, res: express.Response, next: express.NextFunction) => {
if (err instanceof multer.MulterError) {
return res.status(400).json({ error: `Upload error: ${err.message}` });
}
if (err && err.message) {
console.error("Unhandled error:", err.message);
}
res.status(500).json({ error: "Internal server error" });
});
httpServer.listen(PORT, async () => {
await initializeUploadDir();
await ensureTrashCollection();
+1 -1
View File
@@ -1,7 +1,7 @@
{
"name": "frontend",
"private": true,
"version": "0.3.1",
"version": "0.3.2",
"type": "module",
"scripts": {
"dev": "vite",